In preparation for adding stage-2 support, add Stage-2 PTW code.
Only Aarch64 format is supported as stage-1.
Nesting stage-1 and stage-2 is not supported right now.
HTTU is not supported, SW is expected to maintain the Access flag.
This is described in the SMMUv3 manual "5.2. Stream Table Entry" in
"[181] S2AFFD".
This flag determines the behavior on access of a stage-2 page whose
descriptor has AF == 0:
- 0b0: An Access flag fault occurs (stall not supported).
- 0b1: An Access flag fault never occurs.
An Access fault takes priority over a Permission fault.
Checks for IPA and output PA are done according to the user manual
"3.4 Address sizes".
Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
Changes in v2:
- Squash S2AFF PTW code.
- Use common functions between stage-1 and stage-2.
- Add checks for IPA and out PA.
---
hw/arm/smmu-common.c | 132 ++++++++++++++++++++++++++++++++++++++++-
hw/arm/smmu-internal.h | 39 ++++++++++++
2 files changed, 170 insertions(+), 1 deletion(-)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index b49c1affdb..3f448bc82e 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -363,6 +363,130 @@ error:
return -EINVAL;
}
+/**
+ * smmu_ptw_64_s2 - VMSAv8-64 Walk of the page tables for a given IOVA
+ * for stage-2.
+ * @cfg: translation config
+ * @iova: iova to translate
+ * @perm: access type
+ * @tlbe: SMMUTLBEntry (out)
+ * @info: handle to an error info
+ *
+ * Return 0 on success, < 0 on error. In case of error, @info is filled
+ * and tlbe->perm is set to IOMMU_NONE.
+ * Upon success, @tlbe is filled with translated_addr and entry
+ * permission rights.
+ */
+static int smmu_ptw_64_s2(SMMUTransCfg *cfg,
+ dma_addr_t iova, IOMMUAccessFlags perm,
+ SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
+{
+ const int stage = 2;
+ int granule_sz = cfg->s2cfg.granule_sz;
+ /* ARM ARM: Table D8-7. */
+ int inputsize = 64 - cfg->s2cfg.tsz;
+ int level = get_start_level(cfg->s2cfg.sl0, granule_sz);
+ int stride = SMMU_STRIDE(granule_sz);
+ int idx = pgd_idx(level, granule_sz, iova);
+ /*
+ * Get the ttb from concatenated structure.
+ * The offset is the idx * size of each ttb(number of ptes * (sizeof(pte))
+ */
+ uint64_t baseaddr = extract64(cfg->s2cfg.vttb, 0, 48) + (1 << stride) *
+ idx * sizeof(uint64_t);
+ dma_addr_t indexmask = SMMU_IDXMSK(inputsize, stride, level);
+
+ baseaddr &= ~indexmask;
+
+ /*
+ * If the input address of a transaction exceeds the size of the IAS, a
+ * stage 1 Address Size fault occurs.
+ * For AA64, IAS = OAS
+ */
+ if (iova >= (1ULL << cfg->s2cfg.oas)) {
+ info->type = SMMU_PTW_ERR_ADDR_SIZE;
+ info->stage = 1;
+ goto error_no_stage;
+ }
+
+ while (level < SMMU_LEVELS) {
+ uint64_t subpage_size = 1ULL << level_shift(level, granule_sz);
+ uint64_t mask = subpage_size - 1;
+ uint32_t offset = iova_level_offset(iova, inputsize, level, granule_sz);
+ uint64_t pte, gpa;
+ dma_addr_t pte_addr = baseaddr + offset * sizeof(pte);
+ uint8_t ap;
+
+ if (get_pte(baseaddr, offset, &pte, info)) {
+ goto error;
+ }
+ trace_smmu_ptw_level(stage, level, iova, subpage_size,
+ baseaddr, offset, pte);
+ if (is_invalid_pte(pte) || is_reserved_pte(pte, level)) {
+ trace_smmu_ptw_invalid_pte(stage, level, baseaddr,
+ pte_addr, offset, pte);
+ break;
+ }
+
+ if (is_table_pte(pte, level)) {
+ baseaddr = get_table_pte_address(pte, granule_sz);
+ level++;
+ continue;
+ } else if (is_page_pte(pte, level)) {
+ gpa = get_page_pte_address(pte, granule_sz);
+ trace_smmu_ptw_page_pte(stage, level, iova,
+ baseaddr, pte_addr, pte, gpa);
+ } else {
+ uint64_t block_size;
+
+ gpa = get_block_pte_address(pte, level, granule_sz,
+ &block_size);
+ trace_smmu_ptw_block_pte(stage, level, baseaddr,
+ pte_addr, pte, iova, gpa,
+ block_size >> 20);
+ }
+
+ /*
+ * If S2AFFD and PTE.AF are 0 => fault. (5.2. Stream Table Entry)
+ * An Access fault takes priority over a Permission fault.
+ */
+ if (!PTE_AF(pte) && !cfg->s2cfg.affd) {
+ info->type = SMMU_PTW_ERR_ACCESS;
+ goto error;
+ }
+
+ ap = PTE_AP(pte);
+ if (is_permission_fault_s2(ap, perm)) {
+ info->type = SMMU_PTW_ERR_PERMISSION;
+ goto error;
+ }
+
+ /*
+ * The address output from the translation causes a stage 2 Address
+ * Size fault if it exceeds the effective PA output range.
+ */
+ if (gpa >= (1ULL << cfg->s2cfg.oas)) {
+ info->type = SMMU_PTW_ERR_ADDR_SIZE;
+ goto error;
+ }
+
+ tlbe->entry.translated_addr = gpa;
+ tlbe->entry.iova = iova & ~mask;
+ tlbe->entry.addr_mask = mask;
+ tlbe->entry.perm = ap;
+ tlbe->level = level;
+ tlbe->granule = granule_sz;
+ return 0;
+ }
+ info->type = SMMU_PTW_ERR_TRANSLATION;
+
+error:
+ info->stage = 2;
+error_no_stage:
+ tlbe->entry.perm = IOMMU_NONE;
+ return -EINVAL;
+}
+
/**
* smmu_ptw - Walk the page tables for an IOVA, according to @cfg
*
@@ -377,7 +501,13 @@ error:
int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm,
SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
{
- return smmu_ptw_64_s1(cfg, iova, perm, tlbe, info);
+ if (cfg->stage == 1) {
+ return smmu_ptw_64_s1(cfg, iova, perm, tlbe, info);
+ } else if (cfg->stage == 2) {
+ return smmu_ptw_64_s2(cfg, iova, perm, tlbe, info);
+ }
+
+ g_assert_not_reached();
}
/**
diff --git a/hw/arm/smmu-internal.h b/hw/arm/smmu-internal.h
index 2d75b31953..f79c389cd3 100644
--- a/hw/arm/smmu-internal.h
+++ b/hw/arm/smmu-internal.h
@@ -66,6 +66,8 @@
#define PTE_APTABLE(pte) \
(extract64(pte, 61, 2))
+#define PTE_AF(pte) \
+ (extract64(pte, 10, 1))
/*
* TODO: At the moment all transactions are considered as privileged (EL1)
* as IOMMU translation callback does not pass user/priv attributes.
@@ -73,6 +75,9 @@
#define is_permission_fault(ap, perm) \
(((perm) & IOMMU_WO) && ((ap) & 0x2))
+#define is_permission_fault_s2(ap, perm) \
+ (!((ap & perm) == perm))
+
#define PTE_AP_TO_PERM(ap) \
(IOMMU_ACCESS_FLAG(true, !((ap) & 0x2)))
@@ -96,6 +101,40 @@ uint64_t iova_level_offset(uint64_t iova, int inputsize,
MAKE_64BIT_MASK(0, gsz - 3);
}
+#define SMMU_MAX_S2_CONCAT 16
+
+/*
+ * Relies on correctness of gran and sl0 from caller.
+ * FEAT_LPA2 and FEAT_TTST are not implemented.
+ */
+static inline int get_start_level(int sl0 , int gran)
+{
+ /* ARM ARM: Table D8-12. */
+ if (gran == 12) {
+ return 2 - sl0;
+ }
+ /* ARM ARM: Table D8-22 and Table D8-31. */
+ return 3 - sl0;
+}
+
+/*
+ * Index in a concatenated first level stage-2 page table.
+ * ARM ARM: D8.2.2 Concatenated translation tables.
+ */
+static inline int pgd_idx(int start_level, int granule, dma_addr_t iova)
+{
+ uint64_t ret;
+ /*
+ * Get the number of bits handled by next levels, then any extra bits in
+ * the address should index the concatenated tables. This relation can
+ * deduced from tables in ARM ARM: D8.2.7-9
+ */
+ int shift = (SMMU_LEVELS - start_level) * (granule - 3) + granule;
+
+ ret = iova >> shift;
+ return ret;
+}
+
#define SMMU_IOTLB_ASID(key) ((key).asid)
typedef struct SMMUIOTLBPageInvInfo {
--
2.39.2.637.g21b0678d19-googHi Mostafa,
On 2/26/23 23:06, Mostafa Saleh wrote:
> In preparation for adding stage-2 support, add Stage-2 PTW code.
> Only Aarch64 format is supported as stage-1.
>
> Nesting stage-1 and stage-2 is not supported right now.
>
> HTTU is not supported, SW is expected to maintain the Access flag.
> This is described in the SMMUv3 manual "5.2. Stream Table Entry" in
> "[181] S2AFFD".
> This flag determines the behavior on access of a stage-2 page whose
> descriptor has AF == 0:
> - 0b0: An Access flag fault occurs (stall not supported).
> - 0b1: An Access flag fault never occurs.
> An Access fault takes priority over a Permission fault.
>
> Checks for IPA and output PA are done according to the user manual
> "3.4 Address sizes".
replace user manual by the exact ref of the doc. I guess this is IHI0070E
>
> Signed-off-by: Mostafa Saleh <smostafa@google.com>
> ---
> Changes in v2:
> - Squash S2AFF PTW code.
> - Use common functions between stage-1 and stage-2.
> - Add checks for IPA and out PA.
> ---
> hw/arm/smmu-common.c | 132 ++++++++++++++++++++++++++++++++++++++++-
> hw/arm/smmu-internal.h | 39 ++++++++++++
> 2 files changed, 170 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
> index b49c1affdb..3f448bc82e 100644
> --- a/hw/arm/smmu-common.c
> +++ b/hw/arm/smmu-common.c
> @@ -363,6 +363,130 @@ error:
> return -EINVAL;
> }
>
> +/**
> + * smmu_ptw_64_s2 - VMSAv8-64 Walk of the page tables for a given IOVA
> + * for stage-2.
> + * @cfg: translation config
> + * @iova: iova to translate
> + * @perm: access type
> + * @tlbe: SMMUTLBEntry (out)
> + * @info: handle to an error info
> + *
> + * Return 0 on success, < 0 on error. In case of error, @info is filled
> + * and tlbe->perm is set to IOMMU_NONE.
> + * Upon success, @tlbe is filled with translated_addr and entry
> + * permission rights.
> + */
> +static int smmu_ptw_64_s2(SMMUTransCfg *cfg,
> + dma_addr_t iova, IOMMUAccessFlags perm,
Rename iova into ipa?
> + SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
> +{
> + const int stage = 2;
> + int granule_sz = cfg->s2cfg.granule_sz;
> + /* ARM ARM: Table D8-7. */
You may refer the full reference
in DDI0487I.a as the chapter/table may vary. For instance in
ARM DDI 0487F.c D8 corresponds to the activity monitor extensions
> + int inputsize = 64 - cfg->s2cfg.tsz;
> + int level = get_start_level(cfg->s2cfg.sl0, granule_sz);
> + int stride = SMMU_STRIDE(granule_sz);
> + int idx = pgd_idx(level, granule_sz, iova);
> + /*
> + * Get the ttb from concatenated structure.
> + * The offset is the idx * size of each ttb(number of ptes * (sizeof(pte))
> + */
what I don't get is the spec that concatenated tables are used if the
initial lookup level would require less or equal than 16 entries. I
don't see such kind of check or is done implicitly somewhere else?
> + uint64_t baseaddr = extract64(cfg->s2cfg.vttb, 0, 48) + (1 << stride) *
> + idx * sizeof(uint64_t);
> + dma_addr_t indexmask = SMMU_IDXMSK(inputsize, stride, level);
> +
> + baseaddr &= ~indexmask;
> +
> + /*
> + * If the input address of a transaction exceeds the size of the IAS, a
> + * stage 1 Address Size fault occurs.
> + * For AA64, IAS = OAS
then you may use a local variable max_as = cfg->s2cfg.oas used below and
in
if (gpa >= (1ULL << cfg->s2cfg.oas)) {
this is not obvious though that the ias = oas. Where does it come from?
In implementations of SMMUv3.1 and later, this value is Reserved and S2PS behaves as 0b110 (52 bits).
Effective_S2PS = MIN(STE.S2PS, SMMU_IDR5.OAS);
OAS is a maximum of 52 bits in SMMUv3.1 and later
> + */
> + if (iova >= (1ULL << cfg->s2cfg.oas)) {
> + info->type = SMMU_PTW_ERR_ADDR_SIZE;
> + info->stage = 1;
> + goto error_no_stage;
> + }
> +
> + while (level < SMMU_LEVELS) {
I would rename SMMU_LEVELs
> + uint64_t subpage_size = 1ULL << level_shift(level, granule_sz);
> + uint64_t mask = subpage_size - 1;
> + uint32_t offset = iova_level_offset(iova, inputsize, level, granule_sz);
> + uint64_t pte, gpa;
> + dma_addr_t pte_addr = baseaddr + offset * sizeof(pte);
> + uint8_t ap;
> +
> + if (get_pte(baseaddr, offset, &pte, info)) {
> + goto error;
> + }
> + trace_smmu_ptw_level(stage, level, iova, subpage_size,
> + baseaddr, offset, pte);
> + if (is_invalid_pte(pte) || is_reserved_pte(pte, level)) {
> + trace_smmu_ptw_invalid_pte(stage, level, baseaddr,
> + pte_addr, offset, pte);
> + break;
> + }
> +
> + if (is_table_pte(pte, level)) {
> + baseaddr = get_table_pte_address(pte, granule_sz);
> + level++;
> + continue;
> + } else if (is_page_pte(pte, level)) {
> + gpa = get_page_pte_address(pte, granule_sz);
> + trace_smmu_ptw_page_pte(stage, level, iova,
> + baseaddr, pte_addr, pte, gpa);
> + } else {
> + uint64_t block_size;
> +
> + gpa = get_block_pte_address(pte, level, granule_sz,
> + &block_size);
> + trace_smmu_ptw_block_pte(stage, level, baseaddr,
> + pte_addr, pte, iova, gpa,
> + block_size >> 20);
> + }
> +
> + /*
> + * If S2AFFD and PTE.AF are 0 => fault. (5.2. Stream Table Entry)
> + * An Access fault takes priority over a Permission fault.
> + */
> + if (!PTE_AF(pte) && !cfg->s2cfg.affd) {
> + info->type = SMMU_PTW_ERR_ACCESS;
> + goto error;
> + }
> +
> + ap = PTE_AP(pte);
> + if (is_permission_fault_s2(ap, perm)) {
> + info->type = SMMU_PTW_ERR_PERMISSION;
> + goto error;
> + }
> +
> + /*
> + * The address output from the translation causes a stage 2 Address
> + * Size fault if it exceeds the effective PA output range.
> + */
> + if (gpa >= (1ULL << cfg->s2cfg.oas)) {
> + info->type = SMMU_PTW_ERR_ADDR_SIZE;
> + goto error;
> + }
> +
> + tlbe->entry.translated_addr = gpa;
> + tlbe->entry.iova = iova & ~mask;
> + tlbe->entry.addr_mask = mask;
> + tlbe->entry.perm = ap;
> + tlbe->level = level;
> + tlbe->granule = granule_sz;
> + return 0;
> + }
> + info->type = SMMU_PTW_ERR_TRANSLATION;
> +
> +error:
> + info->stage = 2;
> +error_no_stage:
> + tlbe->entry.perm = IOMMU_NONE;
> + return -EINVAL;
> +}
> +
> /**
> * smmu_ptw - Walk the page tables for an IOVA, according to @cfg
> *
> @@ -377,7 +501,13 @@ error:
> int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm,
> SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
> {
> - return smmu_ptw_64_s1(cfg, iova, perm, tlbe, info);
> + if (cfg->stage == 1) {
> + return smmu_ptw_64_s1(cfg, iova, perm, tlbe, info);
> + } else if (cfg->stage == 2) {
> + return smmu_ptw_64_s2(cfg, iova, perm, tlbe, info);
> + }
> +
> + g_assert_not_reached();
> }
>
> /**
> diff --git a/hw/arm/smmu-internal.h b/hw/arm/smmu-internal.h
> index 2d75b31953..f79c389cd3 100644
> --- a/hw/arm/smmu-internal.h
> +++ b/hw/arm/smmu-internal.h
> @@ -66,6 +66,8 @@
> #define PTE_APTABLE(pte) \
> (extract64(pte, 61, 2))
>
> +#define PTE_AF(pte) \
> + (extract64(pte, 10, 1))
> /*
> * TODO: At the moment all transactions are considered as privileged (EL1)
> * as IOMMU translation callback does not pass user/priv attributes.
> @@ -73,6 +75,9 @@
> #define is_permission_fault(ap, perm) \
> (((perm) & IOMMU_WO) && ((ap) & 0x2))
>
> +#define is_permission_fault_s2(ap, perm) \
I would rename ap param into s2ap. This is the name of the field in the spec
> + (!((ap & perm) == perm))
> +
> #define PTE_AP_TO_PERM(ap) \
> (IOMMU_ACCESS_FLAG(true, !((ap) & 0x2)))
>
> @@ -96,6 +101,40 @@ uint64_t iova_level_offset(uint64_t iova, int inputsize,
> MAKE_64BIT_MASK(0, gsz - 3);
> }
>
> +#define SMMU_MAX_S2_CONCAT 16
it is not really an SMMU property (same as SMMU_LEVELS by the way). This
is rather something related to VMSA spec, no?.
> +
> +/*
> + * Relies on correctness of gran and sl0 from caller.
I would remove the above line as we generally expect the caller to
behave properly or do you mean we do not handle any sanity check?
I refer to some of them in target/arm/ptw.c:check_s2_mmu_setup()
> + * FEAT_LPA2 and FEAT_TTST are not implemented.
> + */
> +static inline int get_start_level(int sl0 , int gran)
> +{
> + /* ARM ARM: Table D8-12. */
> + if (gran == 12) {
> + return 2 - sl0;
> + }
> + /* ARM ARM: Table D8-22 and Table D8-31. */
> + return 3 - sl0;
> +}
> +
> +/*
> + * Index in a concatenated first level stage-2 page table.
> + * ARM ARM: D8.2.2 Concatenated translation tables.
> + */
> +static inline int pgd_idx(int start_level, int granule, dma_addr_t iova)
then the name of the function may better reflect what is does?
> +{
> + uint64_t ret;
> + /*
> + * Get the number of bits handled by next levels, then any extra bits in
> + * the address should index the concatenated tables. This relation can
> + * deduced from tables in ARM ARM: D8.2.7-9
> + */
> + int shift = (SMMU_LEVELS - start_level) * (granule - 3) + granule;
this looks like level_shift() with level = start_level - 1, no.
s/granule_sz/gsz or granule_sz to match the rest of the code
> +
> + ret = iova >> shift;
> + return ret;
> +}
> +
> #define SMMU_IOTLB_ASID(key) ((key).asid)
>
> typedef struct SMMUIOTLBPageInvInfo {
Thanks
Eric
Hi Eric,
On Mon, Mar 20, 2023 at 03:56:26PM +0100, Eric Auger wrote:
> Hi Mostafa,
>
> On 2/26/23 23:06, Mostafa Saleh wrote:
> > In preparation for adding stage-2 support, add Stage-2 PTW code.
> > Only Aarch64 format is supported as stage-1.
> >
> > Nesting stage-1 and stage-2 is not supported right now.
> >
> > HTTU is not supported, SW is expected to maintain the Access flag.
> > This is described in the SMMUv3 manual "5.2. Stream Table Entry" in
> > "[181] S2AFFD".
> > This flag determines the behavior on access of a stage-2 page whose
> > descriptor has AF == 0:
> > - 0b0: An Access flag fault occurs (stall not supported).
> > - 0b1: An Access flag fault never occurs.
> > An Access fault takes priority over a Permission fault.
> >
> > Checks for IPA and output PA are done according to the user manual
> > "3.4 Address sizes".
> replace user manual by the exact ref of the doc. I guess this is IHI0070E
Will do.
> > + * Return 0 on success, < 0 on error. In case of error, @info is filled
> > + * and tlbe->perm is set to IOMMU_NONE.
> > + * Upon success, @tlbe is filled with translated_addr and entry
> > + * permission rights.
> > + */
> > +static int smmu_ptw_64_s2(SMMUTransCfg *cfg,
> > + dma_addr_t iova, IOMMUAccessFlags perm,
> Rename iova into ipa?
Will do.
> > + SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
> > +{
> > + const int stage = 2;
> > + int granule_sz = cfg->s2cfg.granule_sz;
> > + /* ARM ARM: Table D8-7. */
> You may refer the full reference
> in DDI0487I.a as the chapter/table may vary. For instance in
> ARM DDI 0487F.c D8 corresponds to the activity monitor extensions
Will do.
> > + int inputsize = 64 - cfg->s2cfg.tsz;
> > + int level = get_start_level(cfg->s2cfg.sl0, granule_sz);
> > + int stride = SMMU_STRIDE(granule_sz);
> > + int idx = pgd_idx(level, granule_sz, iova);
> > + /*
> > + * Get the ttb from concatenated structure.
> > + * The offset is the idx * size of each ttb(number of ptes * (sizeof(pte))
> > + */
> what I don't get is the spec that concatenated tables are used if the
> initial lookup level would require less or equal than 16 entries. I
> don't see such kind of check or is done implicitly somewhere else?
Yes, this is checked in the STE patch in function
s2_pgtable_config_valid, where it checks that the max input will not
need more than 16 tables which means that the pagetable config is
inconsistent, which means the STE is ILLEGAL.
> > + uint64_t baseaddr = extract64(cfg->s2cfg.vttb, 0, 48) + (1 << stride) *
> > + idx * sizeof(uint64_t);
> > + dma_addr_t indexmask = SMMU_IDXMSK(inputsize, stride, level);
> > +
> > + baseaddr &= ~indexmask;
> > +
> > + /*
> > + * If the input address of a transaction exceeds the size of the IAS, a
> > + * stage 1 Address Size fault occurs.
> > + * For AA64, IAS = OAS
> then you may use a local variable max_as = cfg->s2cfg.oas used below and
> in
>
> if (gpa >= (1ULL << cfg->s2cfg.oas)) {
> this is not obvious though that the ias = oas. Where does it come from?
>
> In implementations of SMMUv3.1 and later, this value is Reserved and S2PS behaves as 0b110 (52 bits).
> Effective_S2PS = MIN(STE.S2PS, SMMU_IDR5.OAS);
> OAS is a maximum of 52 bits in SMMUv3.1 and later
IAS = OAS for AA64. Described in "3.4 Address sizes".
The first check is actually not correct, as input address should be
compared to IAS(or OAS) while s2cfg.oas is effective PS.
I will rename s2cfg.oas to s2cfg.eff_ps to avoid confusion.
I will change the check here to be against s2t0sz and in this case it
causes stage-2 addr size fault.
I think the check for the IAS shouldn't be done here.
> > + */
> > + if (iova >= (1ULL << cfg->s2cfg.oas)) {
> > + info->type = SMMU_PTW_ERR_ADDR_SIZE;
> > + info->stage = 1;
> > + goto error_no_stage;
> > + }
> > +
> > + while (level < SMMU_LEVELS) {
> I would rename SMMU_LEVELs
You mean replace SMMU_LEVELS with SMMU_LEVELs?
> >
> > +#define PTE_AF(pte) \
> > + (extract64(pte, 10, 1))
> > /*
> > * TODO: At the moment all transactions are considered as privileged (EL1)
> > * as IOMMU translation callback does not pass user/priv attributes.
> > @@ -73,6 +75,9 @@
> > #define is_permission_fault(ap, perm) \
> > (((perm) & IOMMU_WO) && ((ap) & 0x2))
> >
> > +#define is_permission_fault_s2(ap, perm) \
> I would rename ap param into s2ap. This is the name of the field in the spec
Will do.
> > + (!((ap & perm) == perm))
> > +
> > #define PTE_AP_TO_PERM(ap) \
> > (IOMMU_ACCESS_FLAG(true, !((ap) & 0x2)))
> >
> > @@ -96,6 +101,40 @@ uint64_t iova_level_offset(uint64_t iova, int inputsize,
> > MAKE_64BIT_MASK(0, gsz - 3);
> > }
> >
> > +#define SMMU_MAX_S2_CONCAT 16
> it is not really an SMMU property (same as SMMU_LEVELS by the way). This
> is rather something related to VMSA spec, no?.
Yes, they are part of VMSA, however they are named this way as they are
part of SMMU headers, should we rename them to something else?
> > +
> > +/*
> > + * Relies on correctness of gran and sl0 from caller.
> I would remove the above line as we generally expect the caller to
> behave properly or do you mean we do not handle any sanity check?
> I refer to some of them in target/arm/ptw.c:check_s2_mmu_setup()
We do sanity check in STE parsing, I added this line as this function
is in a header file and anyone can use it, so to make it clear.
However, it is very trivial, I will remove the comment.
> > + * FEAT_LPA2 and FEAT_TTST are not implemented.
> > + */
> > +static inline int get_start_level(int sl0 , int gran)
> > +{
> > + /* ARM ARM: Table D8-12. */
> > + if (gran == 12) {
> > + return 2 - sl0;
> > + }
> > + /* ARM ARM: Table D8-22 and Table D8-31. */
> > + return 3 - sl0;
> > +}
> > +
> > +/*
> > + * Index in a concatenated first level stage-2 page table.
> > + * ARM ARM: D8.2.2 Concatenated translation tables.
> > + */
> > +static inline int pgd_idx(int start_level, int granule, dma_addr_t iova)
> then the name of the function may better reflect what is does?
This returns the index of the page table descriptor in a concatenated
structure, this is actually close to what kvm calls it
“kvm_pgd_page_idx()”, however, I can call it something more clear as
pgd_concatenated_idx()?
> > +{
> > + uint64_t ret;
> > + /*
> > + * Get the number of bits handled by next levels, then any extra bits in
> > + * the address should index the concatenated tables. This relation can
> > + * deduced from tables in ARM ARM: D8.2.7-9
> > + */
> > + int shift = (SMMU_LEVELS - start_level) * (granule - 3) + granule;
> this looks like level_shift() with level = start_level - 1, no.
Yes, I will use level_shift() instead.
> s/granule_sz/gsz or granule_sz to match the rest of the code
Will do.
Thanks,
Mostafa
© 2016 - 2025 Red Hat, Inc.