Re: [PATCH v3 08/23] xen/arm: vsmmuv3: Add support for registers emulation

[Milan Djokic <milan_djokic@xxxxxxxx>]

Hi Julien,

On 6/16/26 22:04, Julien Grall wrote:
> Hi Milan,
>
> On 09/06/2026 00:06, Milan Djokic wrote:
> > > > > > +#define DWORDS_BYTES        8
> > > > > > +#define ARM_SMMU_IIDR_VAL   0x12
> > > > > I am not sure which implementer this is referring to. But how do you
> > > > > plan to handle errata? Are we sure they can always be handled by Xen?
> > > > This is currently a dummy value used to avoid triggering guest driver
> > > > errata/quirk paths. I will replace it with a more meaningful value.
> > > > Using the Arm implementer ID with the remaining fields cleared should be
> > > > sufficient.
> > > I am not sure to understand why would that value be unused. Do you have
> > > more details?
> > I think that the IIDR is always used by the guest driver during
> > initialization to identify the implementer/product revision and enable
> > any required workarounds.
> >
> > If that is the usage you are referring to, then using a generic IIDR
> > value would prevent the guest driver from activating any implementer-
> > specific workaround paths.
> >
> > > > My expectation is that errata handling should remain in Xen rather than
> > > > the guest.
> > > I am not fully convinced you will be able to apply all the errata in the
> > > hypervisor. At least with close to no cost.
> > Yes, this is potentially problematic. However, at the moment I am not
> > sure what the alternative would be, as I think that guest-side errata
> > handling could be applied incorrectly due to the emulation layer.
> I think the risk is limited. But we could always check whether Xen is
> running in the errata handler.
>
> Anyway, I guess we could leave this for now. But this would want to a be
> a TODO as I think we want to address it before the stage-1 SMMU is
> (security) supported.
I'll keep this as a TODO and document it as a known limitation for now.
There are still future updates needed for this, as well as for other 
existing limitations, before the vIOMMU can be considered security 
supported.
> [...]
>
> > > > > > +    }
> > > > > > +
> > > > > >          return IO_HANDLED;
> > > > > >      }
> > > > > >      static int vsmmuv3_mmio_read(struct vcpu *v, mmio_info_t *info,
> > > > > >                                   register_t *r, void *priv)
> > > > > >      {
> > > > > > +    struct virt_smmu *smmu = priv;
> > > > > > +    uint64_t reg;
> > > > > > +
> > > > > > +    switch ( info->gpa & 0xffff )
> > > > > > +    {
> > > > > > +    case VREG32(ARM_SMMU_IDR0):
> > > > > > +        reg  = FIELD_PREP(IDR0_S1P, 1) | FIELD_PREP(IDR0_TTF, 2) |
> > > > > As the page-table will be used by the HW, shouldn't TTF reflect what
> > > > > the
> > > > > HW supports? This would allow the vIOMMU to work for 32-bit domains.
> > > > If my understanding is correct, Xen SMMU driver only supports AArch64
> > > > table format, so I think that we can't advertise 32-bit table format in
> > > > the emulation layer even if the hardware supports it.
> > > Do you mind pointing me to the code? The page-tables are shared between
> > > the SMMU and the CPU. So we ought to support both.
> > The reason I assumed only the AArch64 format should be advertised is
> > that the Xen SMMU driver currently appears to require AArch64 table
> > format support during device probe.
> >
> > In arm_smmu_device_hw_probe() (xen/drivers/passthrough/arm/smmu-v3.c),
> > the handling is:
> >
> > /* We only support the AArch64 table format at present */
> > switch (FIELD_GET(IDR0_TTF, reg)) {
> > case IDR0_TTF_AARCH32_64:
> >       smmu->ias = 40;
> >       fallthrough;
> Wouldn't this allow 32-bit format? Even if we decide to disallow it what
> would prevent the guest to use it (we can't rely on the guest to follow
> the IDR)? Are we preventing configure the stage-1 SMMU for 32-bit domain?
Yes, it will allow it if the underlying hardware supports it. I 
initially misinterpreted this part and incorrectly concluded that the 
32-bit table format should not be advertised by the vIOMMU. I will 
update the vIOMMU to reflect the capabilities of the underlying hardware.
BR,
Milan