| Message ID | 20240701110241.2005222-4-smostafa@google.com |
|---|---|
| State | New |
| Headers | show |
| Series | SMMUv3 nested translation support | expand |
On Mon, Jul 01, 2024 at 11:02:25AM +0000, Mostafa Saleh wrote: > The SMMUv3 spec (ARM IHI 0070 F.b - 7.3 Event records) defines the > class of events faults as: > > CLASS: The class of the operation that caused the fault: > - 0b00: CD, CD fetch. > - 0b01: TTD, Stage 1 translation table fetch. > - 0b10: IN, Input address > > However, this value was not set and left as 0 which means CD and not > IN (0b10). > While at it, add an enum for class as it would be used for nesting. > However, at the moment stage-1 and stage-2 use the same class values. > > Fixes: 9bde7f0674 “hw/arm/smmuv3: Implement translate callback” > Reviewed-by: Eric Auger <eric.auger@redhat.com> > Signed-off-by: Mostafa Saleh <smostafa@google.com> > --- > hw/arm/smmuv3-internal.h | 6 ++++++ > hw/arm/smmuv3.c | 6 +++++- > 2 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h > index e4dd11e1e6..0f3ecec804 100644 > --- a/hw/arm/smmuv3-internal.h > +++ b/hw/arm/smmuv3-internal.h > @@ -32,6 +32,12 @@ typedef enum SMMUTranslationStatus { > SMMU_TRANS_SUCCESS, > } SMMUTranslationStatus; > > +typedef enum SMMUTranslationClass { > + SMMU_CLASS_CD, > + SMMU_CLASS_TT, > + SMMU_CLASS_IN, > +} SMMUTranslationClass; > + > /* MMIO Registers */ > > REG32(IDR0, 0x0) > diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c > index 9dd3ea48e4..1eb5b160d2 100644 > --- a/hw/arm/smmuv3.c > +++ b/hw/arm/smmuv3.c > @@ -942,7 +942,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr, > event.type = SMMU_EVT_F_WALK_EABT; > event.u.f_walk_eabt.addr = addr; > event.u.f_walk_eabt.rnw = flag & 0x1; > - event.u.f_walk_eabt.class = 0x1; > + event.u.f_walk_eabt.class = SMMU_CLASS_TT; For EABT I think we have to differentiate S1/S2: * s1-only walk that encounters EABT on S1 descriptor access is reported as class=TT, * s2 walk that encounters EABT on S2 descriptor while translating non-descriptor IPA is reported as class=IN, even when doing s2-only. Maybe it can be done in the later patch where you propagate the different classes, because it's a minor detail. Thanks, Jean > event.u.f_walk_eabt.addr2 = ptw_info.addr; > break; > case SMMU_PTW_ERR_TRANSLATION: > @@ -950,6 +950,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr, > event.type = SMMU_EVT_F_TRANSLATION; > event.u.f_translation.addr = addr; > event.u.f_translation.addr2 = ptw_info.addr; > + event.u.f_translation.class = SMMU_CLASS_IN; > event.u.f_translation.rnw = flag & 0x1; > } > break; > @@ -958,6 +959,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr, > event.type = SMMU_EVT_F_ADDR_SIZE; > event.u.f_addr_size.addr = addr; > event.u.f_addr_size.addr2 = ptw_info.addr; > + event.u.f_translation.class = SMMU_CLASS_IN; > event.u.f_addr_size.rnw = flag & 0x1; > } > break; > @@ -966,6 +968,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr, > event.type = SMMU_EVT_F_ACCESS; > event.u.f_access.addr = addr; > event.u.f_access.addr2 = ptw_info.addr; > + event.u.f_translation.class = SMMU_CLASS_IN; > event.u.f_access.rnw = flag & 0x1; > } > break; > @@ -974,6 +977,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr, > event.type = SMMU_EVT_F_PERMISSION; > event.u.f_permission.addr = addr; > event.u.f_permission.addr2 = ptw_info.addr; > + event.u.f_translation.class = SMMU_CLASS_IN; > event.u.f_permission.rnw = flag & 0x1; > } > break; > -- > 2.45.2.803.g4e1b14247a-goog >
Hi Jean, On Thu, Jul 04, 2024 at 07:02:00PM +0100, Jean-Philippe Brucker wrote: > On Mon, Jul 01, 2024 at 11:02:25AM +0000, Mostafa Saleh wrote: > > The SMMUv3 spec (ARM IHI 0070 F.b - 7.3 Event records) defines the > > class of events faults as: > > > > CLASS: The class of the operation that caused the fault: > > - 0b00: CD, CD fetch. > > - 0b01: TTD, Stage 1 translation table fetch. > > - 0b10: IN, Input address > > > > However, this value was not set and left as 0 which means CD and not > > IN (0b10). > > While at it, add an enum for class as it would be used for nesting. > > However, at the moment stage-1 and stage-2 use the same class values. > > > > Fixes: 9bde7f0674 “hw/arm/smmuv3: Implement translate callback” > > Reviewed-by: Eric Auger <eric.auger@redhat.com> > > Signed-off-by: Mostafa Saleh <smostafa@google.com> > > --- > > hw/arm/smmuv3-internal.h | 6 ++++++ > > hw/arm/smmuv3.c | 6 +++++- > > 2 files changed, 11 insertions(+), 1 deletion(-) > > > > diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h > > index e4dd11e1e6..0f3ecec804 100644 > > --- a/hw/arm/smmuv3-internal.h > > +++ b/hw/arm/smmuv3-internal.h > > @@ -32,6 +32,12 @@ typedef enum SMMUTranslationStatus { > > SMMU_TRANS_SUCCESS, > > } SMMUTranslationStatus; > > > > +typedef enum SMMUTranslationClass { > > + SMMU_CLASS_CD, > > + SMMU_CLASS_TT, > > + SMMU_CLASS_IN, > > +} SMMUTranslationClass; > > + > > /* MMIO Registers */ > > > > REG32(IDR0, 0x0) > > diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c > > index 9dd3ea48e4..1eb5b160d2 100644 > > --- a/hw/arm/smmuv3.c > > +++ b/hw/arm/smmuv3.c > > @@ -942,7 +942,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr, > > event.type = SMMU_EVT_F_WALK_EABT; > > event.u.f_walk_eabt.addr = addr; > > event.u.f_walk_eabt.rnw = flag & 0x1; > > - event.u.f_walk_eabt.class = 0x1; > > + event.u.f_walk_eabt.class = SMMU_CLASS_TT; > > For EABT I think we have to differentiate S1/S2: > > * s1-only walk that encounters EABT on S1 descriptor access is reported as > class=TT, > * s2 walk that encounters EABT on S2 descriptor while translating > non-descriptor IPA is reported as class=IN, even when doing s2-only. > > Maybe it can be done in the later patch where you propagate the different > classes, because it's a minor detail. Thanks, for pointing it out. It is a bit tricky, for s2 only we need class = IN which is easy. But for nested, if we get a fault with s2 == 1 and EABT, we need to know if it’s from: - Translation of a s1 table IPA (translate_table_addr_ipa) as class => TT - Access for s2 table as class = IN We might need a new member in SMMUPTWEventInfo to distinguish this as bool is_ipa_descriptor Thanks, Mostafa > > Thanks, > Jean > > > event.u.f_walk_eabt.addr2 = ptw_info.addr; > > break; > > case SMMU_PTW_ERR_TRANSLATION: > > @@ -950,6 +950,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr, > > event.type = SMMU_EVT_F_TRANSLATION; > > event.u.f_translation.addr = addr; > > event.u.f_translation.addr2 = ptw_info.addr; > > + event.u.f_translation.class = SMMU_CLASS_IN; > > event.u.f_translation.rnw = flag & 0x1; > > } > > break; > > @@ -958,6 +959,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr, > > event.type = SMMU_EVT_F_ADDR_SIZE; > > event.u.f_addr_size.addr = addr; > > event.u.f_addr_size.addr2 = ptw_info.addr; > > + event.u.f_translation.class = SMMU_CLASS_IN; > > event.u.f_addr_size.rnw = flag & 0x1; > > } > > break; > > @@ -966,6 +968,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr, > > event.type = SMMU_EVT_F_ACCESS; > > event.u.f_access.addr = addr; > > event.u.f_access.addr2 = ptw_info.addr; > > + event.u.f_translation.class = SMMU_CLASS_IN; > > event.u.f_access.rnw = flag & 0x1; > > } > > break; > > @@ -974,6 +977,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr, > > event.type = SMMU_EVT_F_PERMISSION; > > event.u.f_permission.addr = addr; > > event.u.f_permission.addr2 = ptw_info.addr; > > + event.u.f_translation.class = SMMU_CLASS_IN; > > event.u.f_permission.rnw = flag & 0x1; > > } > > break; > > -- > > 2.45.2.803.g4e1b14247a-goog > >
diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h index e4dd11e1e6..0f3ecec804 100644 --- a/hw/arm/smmuv3-internal.h +++ b/hw/arm/smmuv3-internal.h @@ -32,6 +32,12 @@ typedef enum SMMUTranslationStatus { SMMU_TRANS_SUCCESS, } SMMUTranslationStatus; +typedef enum SMMUTranslationClass { + SMMU_CLASS_CD, + SMMU_CLASS_TT, + SMMU_CLASS_IN, +} SMMUTranslationClass; + /* MMIO Registers */ REG32(IDR0, 0x0) diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c index 9dd3ea48e4..1eb5b160d2 100644 --- a/hw/arm/smmuv3.c +++ b/hw/arm/smmuv3.c @@ -942,7 +942,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr, event.type = SMMU_EVT_F_WALK_EABT; event.u.f_walk_eabt.addr = addr; event.u.f_walk_eabt.rnw = flag & 0x1; - event.u.f_walk_eabt.class = 0x1; + event.u.f_walk_eabt.class = SMMU_CLASS_TT; event.u.f_walk_eabt.addr2 = ptw_info.addr; break; case SMMU_PTW_ERR_TRANSLATION: @@ -950,6 +950,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr, event.type = SMMU_EVT_F_TRANSLATION; event.u.f_translation.addr = addr; event.u.f_translation.addr2 = ptw_info.addr; + event.u.f_translation.class = SMMU_CLASS_IN; event.u.f_translation.rnw = flag & 0x1; } break; @@ -958,6 +959,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr, event.type = SMMU_EVT_F_ADDR_SIZE; event.u.f_addr_size.addr = addr; event.u.f_addr_size.addr2 = ptw_info.addr; + event.u.f_translation.class = SMMU_CLASS_IN; event.u.f_addr_size.rnw = flag & 0x1; } break; @@ -966,6 +968,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr, event.type = SMMU_EVT_F_ACCESS; event.u.f_access.addr = addr; event.u.f_access.addr2 = ptw_info.addr; + event.u.f_translation.class = SMMU_CLASS_IN; event.u.f_access.rnw = flag & 0x1; } break; @@ -974,6 +977,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr, event.type = SMMU_EVT_F_PERMISSION; event.u.f_permission.addr = addr; event.u.f_permission.addr2 = ptw_info.addr; + event.u.f_translation.class = SMMU_CLASS_IN; event.u.f_permission.rnw = flag & 0x1; } break;