Hi Eric,

On 2021/1/29 5:30, Auger Eric wrote:
Hi Zenghui,

On 1/28/21 9:25 AM, Auger Eric wrote:
Hi Zenghui,

On 12/25/20 10:50 AM, Zenghui Yu wrote:
When performing range-based IOTLB invalidation, we should decode the TG
field into the corresponding translation granule size so that we can pass
the correct invalidation range to backend. Set @granule to (tg * 2 + 10) to
properly emulate the architecture.

Fixes: d52915616c05 ("hw/arm/smmuv3: Get prepared for range invalidation")
Signed-off-by: Zenghui Yu <yuzeng...@huawei.com>

Good catch! I tested with older guest kernels though. I wonder how I did
not face the bug?
Please ignore this wrong comment as this corresponds to recent kernels
instead. Still puzzled anyway ;-)

I noticed this when looking through your nested SMMU series and I didn't
have much clue about the impact on the real setups.

I guess we may receive some unexpected fault events with this bug. But I
think we may miss it for some reasons:

 - the stale TLB entries happen to be evicted due to heavy traffic
 - some form of over-invalidation is performed by your implementation
 - ...

---
  hw/arm/smmuv3.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index bbca0e9f20..65231c7d52 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -801,7 +801,7 @@ static void smmuv3_notify_iova(IOMMUMemoryRegion *mr,
  {
      SMMUDevice *sdev = container_of(mr, SMMUDevice, iommu);
      IOMMUTLBEvent event;
-    uint8_t granule = tg;
+    uint8_t granule;
if (!tg) {
          SMMUEventInfo event = {.inval_ste_allowed = true};
@@ -821,6 +821,8 @@ static void smmuv3_notify_iova(IOMMUMemoryRegion *mr,
              return;
          }
          granule = tt->granule_sz;
+    } else {
+        guanule = tg * 2 + 10;
maybe just init granule to this value above while fixing the typo.

My intention is to initialize @granule to this value explicitly for the
range-based invalidation case. But I'm okay with either way.


Thanks,
Zenghui

Reply via email to