Re: [PATCH v2 08/25] x86/virt/tdx: Add SEAMCALL wrappers for TDX page cache management

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2024-11-12 at 16:20 -0800, Dave Hansen wrote:
> If this were, instead:
> 
> u64 tdh_phymem_page_reclaim(u64 page, u64 *type, u64 *owner, u64 *size)
> {
> 	...
> 	*type = args.rcx;
> 	*owner = args.rdx;
> 	*size = args.r8;
> 
> Then you wouldn't need the comment in the first place.  Then you could
> also be thinking about adding _some_ kind of type safety to the
> arguments.  The 'size' or the 'type' could totally be enums.

Yes, *rcx and *rdx stand out.

> 
> There's really zero value in having wrappers like these.  They don't
> have any type safety or add any readability or make the seamcall easier
> to use.  There's almost no value in having these versus just exporting
> seamcall_ret() itself.

Hoping to solicit some more thoughts on the value question...

I thought the main thing was to not export *all* SEAMCALLs. Future TDX modules
could add new leafs that do who-knows-what.

For this SEAMCALL wrapper, the only use of the out args is printing them in an
error message (based on other logic). So turning them into enums would just add
a layer of translation to be decoded. A developer would have to translate them
back into the registers they came from to try to extract meaning from the TDX
docs.

However, some future user of TDH.PHYMEM.PAGE.RECLAIM might want to do something
else where the enums could add code clarity. But this goes down the road of
building things that are not needed today.

Is there value in maintaining a sensible looking API to be exported, even if it
is not needed today?




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux