On Fri, Sep 06, 2024 at 10:34:44AM -0300, Jason Gunthorpe wrote:
> On Fri, Sep 06, 2024 at 11:07:47AM +0000, Mostafa Saleh wrote:
>
> > However, I believe the UAPI can be more clear and solid in terms of
> > what is supported (maybe a typical struct with the CD, and some
> > extra configs?) I will give it a think.
>
> I don't think breaking up the STE into fields in another struct is
> going to be a big improvement, it adds more code and corner cases to
> break up and reassemble it.
>
> #define STRTAB_STE_0_NESTING_ALLOWED \
> cpu_to_le64(STRTAB_STE_0_V | STRTAB_STE_0_CFG | STRTAB_STE_0_S1FMT | \
> STRTAB_STE_0_S1CTXPTR_MASK | STRTAB_STE_0_S1CDMAX)
> #define STRTAB_STE_1_NESTING_ALLOWED \
> cpu_to_le64(STRTAB_STE_1_S1DSS | STRTAB_STE_1_S1CIR | \
> STRTAB_STE_1_S1COR | STRTAB_STE_1_S1CSH | \
> STRTAB_STE_1_S1STALLD | STRTAB_STE_1_EATS)
>
> It is 11 fields that would need to be recoded, that's alot.. Even if
> you say the 3 cache ones are not needed it is still alot.
I was thinking of providing a higher level semantics
(no need for caching, valid...), something like:
struct smmu_user_table {
u64 cd_table;
u32 smmu_cd_cfg; /* linear or 2lvl,.... */
u32 smmu_trans_cfg; /* Translate, bypass, abort */
u32 dev_feat; /*ATS, STALL, …*/
};
I feel that is a bit more clear for user space? Instead of
partially setting the STE, and it should be easier to extend than
masking the STE.
I’am not opposed to the vSTE, I just feel it's loosely defined,
that's why I was asking for the docs.
>
> > > Reporting a static kernel capability through GET_INFO output is
> > > easier/saner than providing some kind of policy flags in the GET_INFO
> > > input to specify how the sanitization should work.
> >
> > I don’t think it’s “policy”, it’s just giving userspace the minimum
> > knowledge it needs to create the vSMMU, but again no really strong
> > opinion about that.
>
> There is no single "minimum knowledge" though, it depends on what the
> VMM is able to support. IMHO once you go over to the "VMM has to
> ignore bits it doesn't understand" you may as well just show
> everything. Then the kernel side can't be wrong.
>
> If the kernel side can be wrong, then you are back to handshaking
> policy because the kernel can't assume that all existing VMMs wil not
> rely on the kernel to do the masking.
>
I agree it’s tricky, again no strong opinion on that, although I doubt
that a VMM would care about all the SMMU features.
> > > > But this is a UAPI. How can userspace implement that if it has no
> > > > documentation, and how can it be maintained if there is no clear
> > > > interface with userspace with what is expected/returned...
> > >
> > > I'm not sure what you are looking for here? I don't think an entire
> > > tutorial on how to build a paravirtualized vSMMU is appropriate to
> > > put in comments?
> >
> > Sorry, I don’t think I was clear, I meant actual documentation for
> > the UAPI, as in RST files for example. If I want to support that
> > in kvmtool how can I implement it?
>
> Well, you need thousands of lines of code in kvtool to build a vIOMMU :)
>
> Nicolin is looking at writing something, lets see.
>
> I think for here we should focus on the comments being succinct but
> sufficient to understand what the uAPI does itself.
>
Actually I think the opposite, I think UAPI docs is more important
here, especially for the vSTE, that's how we can compare the code to
what is expected from user-space.
> > > I would *really* like everyone to sit down and figure out how to
> > > manage virtual device lifecycle in a single language!
> >
> > Yes, just like the guest_memfd work. There has been also
> > some work to unify some of the guest HVC bits:
> > https://lore.kernel.org/all/20240830130150.8568-1-will@xxxxxxxxxx/
>
> I think Dan Williams is being ringleader for the PCI side effort on CC
Thanks, I will try to spend some time on the secure VFIO work.
Thanks,
Mostafa
>
> Jason
