Re: [PATCH v7 3/5] x86/mm: Make tdx_enc_status_changed() vmalloc address compatible

[Sathyanarayanan Kuppuswamy]

On 5/30/22 3:47 AM, Kai Huang wrote:
> On Mon, 2022-05-23 at 21:05 -0700, Kuppuswamy Sathyanarayanan wrote:
> > set_memory_*crypted() APIs are used to change encryption or decryption
> > page attributes for the given address. It also by default support the
> > conversion for the vmalloc'ed memory address.
> >
> > In TDX Guest, tdx_enc_status_changed() function is triggered by
> > set_memory_*crypted() APIs when converting memory from/to shared or
> > private. Internally this function uses __pa() for physical address
> > conversion, which breaks the vmalloc address compatibility of the
> > set_memory_*crypted() APIs.
> >
> > So add support to fix the vmalloc'ed address compatibility issue.
> >
> > Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>
> > ---
> >   arch/x86/coco/tdx/tdx.c | 11 +++++++++--
> >   1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
> > index b49211994864..37d58675ccf1 100644
> > --- a/arch/x86/coco/tdx/tdx.c
> > +++ b/arch/x86/coco/tdx/tdx.c
> > @@ -15,6 +15,7 @@
> >   #include <asm/idtentry.h>
> >   #include <asm/irq_regs.h>
> >   #include <asm/desc.h>
> > +#include <asm/io.h>
> >   
> >   /* TDX module Call Leaf IDs */
> >   #define TDX_GET_INFO			1
> > @@ -680,8 +681,14 @@ static bool try_accept_one(phys_addr_t *start, unsigned long len,
> >    */
> >   static bool tdx_enc_status_changed(unsigned long vaddr, int numpages, bool enc)
> >   {
> > -	phys_addr_t start = __pa(vaddr);
> > -	phys_addr_t end   = __pa(vaddr + numpages * PAGE_SIZE);
> > +	phys_addr_t start, end;
> > +
> > +	if (is_vmalloc_addr((void *)vaddr))
> > +		start = vmalloc_to_pfn((void *) vaddr) << PAGE_SHIFT;
> > +	else
> > +		start = __pa(vaddr);
> > +
> > +	end = start + numpages * PAGE_SIZE;
> >   
> >   	if (!enc) {
> >   		/* Set the shared (decrypted) bits: */
>
> AMD uses lookup_address() which doesn't require the vaddr being vmap() address.
> Shouldn't TDX use the same way?

AMD uses it to add some additional checks for address validation and
missing PTE entry. But in our case, reaching here means address is valid
and there is a valid PTE entry. So there is no need to lookup again and
convert it. It is easier to use above method.

--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer