[v3,11/11] hw/arm/smmuv3: Advertise SMMUv3.2 range invalidation
diff mbox series

Message ID 20200708141856.15776-12-eric.auger@redhat.com
State New
Headers show
Series
  • SMMUv3.2 Range-based TLB Invalidation Support
Related show

Commit Message

Auger Eric July 8, 2020, 2:18 p.m. UTC
Expose the RIL bit so that the guest driver uses range
invalidation. Range invalidation being an SMMU3.2 feature,
let AIDR advertise SMMUv3.2 support.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 hw/arm/smmuv3-internal.h | 1 +
 hw/arm/smmuv3.c          | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

Comments

Peter Maydell July 10, 2020, 9:47 a.m. UTC | #1
On Wed, 8 Jul 2020 at 15:20, Eric Auger <eric.auger@redhat.com> wrote:
>
> Expose the RIL bit so that the guest driver uses range
> invalidation. Range invalidation being an SMMU3.2 feature,
> let AIDR advertise SMMUv3.2 support.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>

I think that to advertise SMMUv3.2 we would also need to
set the IDR3.BBML field to something non-zero. That means
we need to analyze our implementation of the caching of page
table structures to see if we need to do anything different
(per the behaviours and guarantees described in section 3.21.1
of the spec).

Alternatively, we could take advantage of the language
in section 2.5 that says that a v3.x implementation is
allowed to implement features from v3.(x+1), and just
set the RIL bit while leaving AIDR advertising us as v3.1.

thanks
-- PMM
Auger Eric July 10, 2020, 10:05 a.m. UTC | #2
Hi Peter,

On 7/10/20 11:47 AM, Peter Maydell wrote:
> On Wed, 8 Jul 2020 at 15:20, Eric Auger <eric.auger@redhat.com> wrote:
>>
>> Expose the RIL bit so that the guest driver uses range
>> invalidation. Range invalidation being an SMMU3.2 feature,
>> let AIDR advertise SMMUv3.2 support.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> 
> I think that to advertise SMMUv3.2 we would also need to
> set the IDR3.BBML field to something non-zero. That means
> we need to analyze our implementation of the caching of page
> table structures to see if we need to do anything different
> (per the behaviours and guarantees described in section 3.21.1
> of the spec).
you're right. I need to further study this feature.

I felt difficult to find out which features are mandatory for a given
revision number.
> 
> Alternatively, we could take advantage of the language
> in section 2.5 that says that a v3.x implementation is
> allowed to implement features from v3.(x+1), and just
> set the RIL bit while leaving AIDR advertising us as v3.1.
Indeed :-)

Thank you for the review!

Eric
> 
> thanks
> -- PMM
>
Peter Maydell July 10, 2020, 10:39 a.m. UTC | #3
On Fri, 10 Jul 2020 at 11:05, Auger Eric <eric.auger@redhat.com> wrote:
> On 7/10/20 11:47 AM, Peter Maydell wrote:
> > I think that to advertise SMMUv3.2 we would also need to
> > set the IDR3.BBML field to something non-zero. That means
> > we need to analyze our implementation of the caching of page
> > table structures to see if we need to do anything different
> > (per the behaviours and guarantees described in section 3.21.1
> > of the spec).
>
> you're right. I need to further study this feature.
>
> I felt difficult to find out which features are mandatory for a given
> revision number.

Mmm. I ended up just working through all the ID register
fields looking for references to 3.1 or 3.2 in them,
but it's easy to overlook one (I failed to catch
the IDR3.HAD requirement in my initial scan,
for instance :-)).

-- PMM
Auger Eric July 28, 2020, 3:09 p.m. UTC | #4
Hi Peter,

On 7/10/20 11:47 AM, Peter Maydell wrote:
> On Wed, 8 Jul 2020 at 15:20, Eric Auger <eric.auger@redhat.com> wrote:
>>
>> Expose the RIL bit so that the guest driver uses range
>> invalidation. Range invalidation being an SMMU3.2 feature,
>> let AIDR advertise SMMUv3.2 support.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> 
> I think that to advertise SMMUv3.2 we would also need to
> set the IDR3.BBML field to something non-zero. That means
> we need to analyze our implementation of the caching of page
> table structures to see if we need to do anything different
> (per the behaviours and guarantees described in section 3.21.1
> of the spec).
> 
> Alternatively, we could take advantage of the language
> in section 2.5 that says that a v3.x implementation is
> allowed to implement features from v3.(x+1), and just
> set the RIL bit while leaving AIDR advertising us as v3.1.
So I eventually lazily chose this solution. I will address BBML support
in a separate patch series.

Thanks

Eric
> 
> thanks
> -- PMM
>

Patch
diff mbox series

diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
index 4bc1548dff..932d5701da 100644
--- a/hw/arm/smmuv3-internal.h
+++ b/hw/arm/smmuv3-internal.h
@@ -55,6 +55,7 @@  REG32(IDR1,                0x4)
 REG32(IDR2,                0x8)
 REG32(IDR3,                0xc)
      FIELD(IDR3, HAD,         2, 1);
+     FIELD(IDR3, RIL,        10, 1);
 REG32(IDR4,                0x10)
 REG32(IDR5,                0x14)
      FIELD(IDR5, OAS,         0, 3);
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index b262f0e4a7..9f9e9877b9 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -254,6 +254,7 @@  static void smmuv3_init_regs(SMMUv3State *s)
     s->idr[1] = FIELD_DP32(s->idr[1], IDR1, EVENTQS, SMMU_EVENTQS);
     s->idr[1] = FIELD_DP32(s->idr[1], IDR1, CMDQS,   SMMU_CMDQS);
 
+    s->idr[3] = FIELD_DP32(s->idr[3], IDR3, RIL, 1);
     s->idr[3] = FIELD_DP32(s->idr[3], IDR3, HAD, 1);
 
    /* 4K and 64K granule support */
@@ -272,7 +273,7 @@  static void smmuv3_init_regs(SMMUv3State *s)
 
     s->features = 0;
     s->sid_split = 0;
-    s->aidr = 0x1;
+    s->aidr = 0x2;
 }
 
 static int smmu_get_ste(SMMUv3State *s, dma_addr_t addr, STE *buf,