Re: [PATCH v8 11/25] iommu/arm-smmu-v3-kvm: Add the kernel driver

[Mostafa Saleh <smostafa@google.com>]

On Wed, Sep 23, 2026 at 12:55:07PM -0300, Jason Gunthorpe wrote:
> On Wed, Sep 23, 2026 at 11:52:36AM +0000, Mostafa Saleh wrote:
> > I can add more info in the cover letter about the code split. Mainly:
> > - arm-smmu-v3-kvm.c: Is the kernel driver for KVM SMMUv3 (this runs in
> >   EL1 and can use all the kernel functions).
> >   The main job of this driver is for discovery, it doesn't do anything
> >   else in the runtime.
> 
> Maybe call this el1 stub or something like that?
> 
> guest_pkvm_stub.c
> pkvm/hyp.c
> 

It is not really a stub and I was following the convention of
arm-smmu-v3-(iommufd/test/sva) I think for the pkvm/ one it can be:
- pkvm/arm-smmu-v3-pkvm.c
- pkvm/arm-smmu-v3-nested.c
- pkvm/arm-smmu-v3-el2.c
- pkvm/arm-smmu-v3-hyp.c

All of those are fine and for the other one:
- arm-smmu-v3-{pkvm, nested, el2, hyp}-init.c
- arm-smmu-v3-pkvm-el1.c

Anything along this lines, otherwise it's too vague IMHO.

> ?
> 
> > > > +extern struct pkvm_iommu_ops kvm_nvhe_sym(smmu_ops);
> > > > +
> > > > +static size_t				kvm_arm_smmu_count;
> > > > +static struct hyp_arm_smmu_v3_device	*kvm_arm_smmu_array;
> > > > +static size_t				kvm_arm_smmu_cur;
> > > 
> > > That spaces/tabs in those two lines look a bit odd..
> > > 
> > 
> > These are tabs to indent the variables, similar cases exist in the
> > SMMUv3 driver, check arm_smmu_cmdq and friends for example.
> 
> Personally I hate this style, it is a huge PITA for maintenance and
> makes diffs bigger than they need to do. I remove it whenever I have
> the chance :)

No strong opinion I can change it.

> 
> > TBH, this is a bit of a hack. The hypervisor can not allocate memory
> > at the runtime. All of the hypervisor memory comes from a carveout
> > allocated at boot (see kvm_hyp_reserve())
> > 
> > So we allocate the worst case for memory mapping with leaf granule. But
> > the hypervisor also need to allocate L2 pointers and the SID space can
> > be massive making the upper limit for this too large.
> > 
> > However, smmu_hyp_pgt_pages() defines the minimum pages required,
> > actual allocation comes from the command line, so it is possible to
> > tune the system without re-compiling the kernel.
> > 
> > Thinking about it now, we can just drop the 500 as this is the lower
> > bound, earlier versions of this series would allocate the carveout
> > based on this size, but it is not needed anymore.
> 
> I see, and you can't scan the ACPI to figure out all the SIDs used at
> this point to get an exact estimation?

I thought about it, but it is complicated to scan the DT/ACPI/platform
bus and I don't think a driver should be doing that; going through each
device with IOMMUs, duplicating existing logic...

And even with that, things as PCI won't be there as they are dynamic.

I think the best way is to remove the 500 as it is hacky, and rely
on the command line option.

> 
> Not being able to manage memory after boot is rather an extreme
> limitation.. But a huge amount of RMM's API complexity comes from its
> granular memory management so I can't really fault that..
> 

Yes, that was one of the main sources of complexity with the
para-virtual approach also.

Thanks,
Mostafa

> Jason