Re: [RFC v1 23/26] x86/tdx: Make pages shared in ioremap()

2021-04-06 Thread Dave Hansen
On 4/6/21 9:00 AM, Kirill A. Shutemov wrote:
>>> --- a/arch/x86/mm/ioremap.c
>>> +++ b/arch/x86/mm/ioremap.c
>>> @@ -87,12 +87,12 @@ static unsigned int __ioremap_check_ram(struct resource 
>>> *res)
>>>  }
>>>  
>>>  /*
>>> - * In a SEV guest, NONE and RESERVED should not be mapped encrypted because
>>> - * there the whole memory is already encrypted.
>>> + * In a SEV or TDX guest, NONE and RESERVED should not be mapped encrypted 
>>> (or
>>> + * private in TDX case) because there the whole memory is already 
>>> encrypted.
>>>   */
>> But doesn't this mean that we can't ioremap() normal memory?
> It's not allowed anyway: see (io_desc.flags & IORES_MAP_SYSTEM_RAM) in the
> __ioremap_caller().
> 
>> I was somehow expecting that we would need to do this for some
>> host<->guest communication pages.
> It goes though DMA API, not ioremap().

Ahh, got it.  Thanks for the clarification.

It would help to make mention of that stuff in the changelog to make it
more obvious going forward.


Re: [RFC v1 23/26] x86/tdx: Make pages shared in ioremap()

2021-04-06 Thread Kirill A. Shutemov
On Thu, Apr 01, 2021 at 01:26:23PM -0700, Dave Hansen wrote:
> On 2/5/21 3:38 PM, Kuppuswamy Sathyanarayanan wrote:
> > From: "Kirill A. Shutemov" 
> > 
> > All ioremap()ed paged that are not backed by normal memory (NONE or
> > RESERVED) have to be mapped as shared.
> 
> s/paged/pages/
> 
> 
> > +/* Make the page accesable by VMM */
> > +#define pgprot_tdx_shared(prot) __pgprot(pgprot_val(prot) | 
> > tdx_shared_mask())
> > +
> >  #ifndef __ASSEMBLY__
> >  #include 
> >  #include 
> > diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
> > index 9e5ccc56f8e0..a0ba760866d4 100644
> > --- a/arch/x86/mm/ioremap.c
> > +++ b/arch/x86/mm/ioremap.c
> > @@ -87,12 +87,12 @@ static unsigned int __ioremap_check_ram(struct resource 
> > *res)
> >  }
> >  
> >  /*
> > - * In a SEV guest, NONE and RESERVED should not be mapped encrypted because
> > - * there the whole memory is already encrypted.
> > + * In a SEV or TDX guest, NONE and RESERVED should not be mapped encrypted 
> > (or
> > + * private in TDX case) because there the whole memory is already 
> > encrypted.
> >   */
> 
> But doesn't this mean that we can't ioremap() normal memory?

It's not allowed anyway: see (io_desc.flags & IORES_MAP_SYSTEM_RAM) in the
__ioremap_caller().


> I was somehow expecting that we would need to do this for some
> host<->guest communication pages.

It goes though DMA API, not ioremap().

-- 
 Kirill A. Shutemov


Re: [RFC v1 23/26] x86/tdx: Make pages shared in ioremap()

2021-04-01 Thread Dave Hansen
On 2/5/21 3:38 PM, Kuppuswamy Sathyanarayanan wrote:
> From: "Kirill A. Shutemov" 
> 
> All ioremap()ed paged that are not backed by normal memory (NONE or
> RESERVED) have to be mapped as shared.

s/paged/pages/


> +/* Make the page accesable by VMM */
> +#define pgprot_tdx_shared(prot) __pgprot(pgprot_val(prot) | 
> tdx_shared_mask())
> +
>  #ifndef __ASSEMBLY__
>  #include 
>  #include 
> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
> index 9e5ccc56f8e0..a0ba760866d4 100644
> --- a/arch/x86/mm/ioremap.c
> +++ b/arch/x86/mm/ioremap.c
> @@ -87,12 +87,12 @@ static unsigned int __ioremap_check_ram(struct resource 
> *res)
>  }
>  
>  /*
> - * In a SEV guest, NONE and RESERVED should not be mapped encrypted because
> - * there the whole memory is already encrypted.
> + * In a SEV or TDX guest, NONE and RESERVED should not be mapped encrypted 
> (or
> + * private in TDX case) because there the whole memory is already encrypted.
>   */

But doesn't this mean that we can't ioremap() normal memory?  I was
somehow expecting that we would need to do this for some host<->guest
communication pages.

>  static unsigned int __ioremap_check_encrypted(struct resource *res)
>  {
> - if (!sev_active())
> + if (!sev_active() && !is_tdx_guest())
>   return 0;
>  
>   switch (res->desc) {
> @@ -244,6 +244,8 @@ __ioremap_caller(resource_size_t phys_addr, unsigned long 
> size,
>   prot = PAGE_KERNEL_IO;
>   if ((io_desc.flags & IORES_MAP_ENCRYPTED) || encrypted)
>   prot = pgprot_encrypted(prot);
> + else if (is_tdx_guest())
> + prot = pgprot_tdx_shared(prot);