Re: [PATCH v6 4/6] virt: tdx-guest: Add a helper for the Quote buffer size

[Peter Fang <peter.fang@intel.com>]

On Wed, Sep 30, 2026 at 03:14:14PM -0700, Edgecombe, Rick P wrote:
> On Wed, 2026-09-30 at 03:30 -0700, Peter Fang wrote:
> > +
> >  static void free_quote_buf(struct tdx_quote_buf *buf)
> >  {
> > -	size_t len = PAGE_ALIGN(GET_QUOTE_BUF_SIZE);
> > -	unsigned int count = len >> PAGE_SHIFT;
> > +	size_t alloc_size = PAGE_ALIGN(get_quote_buf_size());
> > +	unsigned int count;
> > +
> > +	count = alloc_size >> PAGE_SHIFT;
> 
> Why change the count to be set outside of the declarations here and below?

Ah good catch... It was previously in my tree:

        size_t alloc_size = get_quote_buf_size();
        unsigned int count = alloc_size >> PAGE_SHIFT;

But with PAGE_ALIGN() added it keeps the reverse fir tree order. I'll
fix it up. Thanks.

> 
> >  
> > @@ -266,6 +276,7 @@ static int tdx_report_new_locked(struct tsm_report *report)
> >  {
> >  	u8 *buf;
> >  	struct tsm_report_desc *desc = &report->desc;
> > +	size_t quote_buf_size = get_quote_buf_size();
> 
> This local var is a performance optimization? Or a line shortener?

Both I think. Calling get_quote_buf_size() 3 times in this function felt
a bit much. Should I drop this?

> 
> > @@ -292,7 +303,7 @@ static int tdx_report_new_locked(struct tsm_report *report)
> >  	if (ret)
> >  		return ret;
> >  
> > -	err = tdx_hcall_get_quote(quote_buf, GET_QUOTE_BUF_SIZE);
> > +	err = tdx_hcall_get_quote(quote_buf, PAGE_ALIGN(quote_buf_size));
> 
> The point of leaving page alignment to the callers was to not churn the existing
> code in this patch. But this caller is getting changed anyway for some reason. 
> 
> In patch 6, it changes to the dynamic buffer, which might not be page aligned.
> But what if len passed through the GHCI call is not page aligned? Does it cause
> a problem? Oh! GHCI docs say "R13 - Size of shared GPA. The size must be 4KB-
> aligned."

Yes, and it's documented in tdx_hcall_get_quote() as well.

> 
> So it needs new alignment only because of the dynamic buffer, and to fulfill the
> GHCI spec. Or otherwise I guess you could claim that the page alignment is added
> here because it was always required and now it's too hard to see that any
> possible size is already aligned. I think it's weak. I'd put it in patch 6 and

I think my original intent was to make sure all the PAGE_ALIGN()'s
appear in the same patch. For example in alloc_quote_buf() and
free_quote_buf() it's easier to see PAGE_ALIGN() is needed by the
'count'. I thought about adding a comment above the
tdx_hcall_get_quote() call... But that function is already pretty well
documented.

There's an existing inconsistency in the driver. In alloc_quote_buf()
and free_quote_buf() the size is PAGE_ALIGN()'d despite
GET_QUOTE_BUF_SIZE being 128K. But in tdx_report_new_locked() it skips
the PAGE_ALIGN() when calling tdx_hcall_get_quote().

I'm ok moving this to patch 6. I'm a bit concerned it would make patch 6
look like doing two things at once, because Dave asked for the feature
itself to be contained in a single patch. Do you still think putting it
here is too weak?

> explain why it is now needed at that point.
>