On Wed, Oct 12, 2022 at 10:13:50AM -0700, Sathyanarayanan Kuppuswamy wrote: > Hi, > > On 10/12/22 9:23 AM, Greg Kroah-Hartman wrote: > > On Wed, Oct 12, 2022 at 08:44:04AM -0700, Sathyanarayanan Kuppuswamy wrote: > >> > >> > >> On 10/12/22 7:27 AM, Borislav Petkov wrote: > >>> On Wed, Oct 12, 2022 at 06:35:56AM -0700, Sathyanarayanan Kuppuswamy wrote: > >>>> So we should create a new wrapper for this use case or use > >>> > >>> Yes, you got it - a new wrapper pls. > >> > >> Ok. I will add a new wrapper to get the TDREPORT. > >> > >> +/* > >> > >> + * Add a wrapper for TDG.MR.REPORT TDCALL. It is used in TDX guest > >> > >> + * driver module to get the TDREPORT. > >> > >> + */ > >> > >> +long tdx_mcall_get_report(void *reportdata, void *tdreport, u8 subtype) > > > > Why "long"? > > We used long because __tdx_module_call() call returns u64 value. Great, then use u64 please. Or if you are returning negative errors, use s64 to be specific. > Alternatively, we can also check for return value of __tdx_module_call() here > and return 0/-EIO as return values. In this case we can change return value > to int. That would make more sense, right? > > > > Why void *? Don't you have real types for these? > > We use these buffers as an intermediary to transfer data between userspace and > the TDX module. In the kernel we don't consume these datas. So we did not define > the type of the data. Then these are userspace pointers? Why are they not marked as such? thanks, greg k-h