RE: [PATCH 08/11] swiotlb: make the swiotlb_init interface more useful

["Michael Kelley (LINUX)" <mikelley@xxxxxxxxxxxxx>]

From: Christoph Hellwig <hch@xxxxxx> Sent: Monday, February 28, 2022 3:31 AM
> 
> On Mon, Feb 28, 2022 at 02:53:39AM +0000, Michael Kelley (LINUX) wrote:
> > From: Christoph Hellwig <hch@xxxxxx> Sent: Sunday, February 27, 2022 6:31 AM
> > >
> > > Pass a bool to pass if swiotlb needs to be enabled based on the
> > > addressing needs and replace the verbose argument with a set of
> > > flags, including one to force enable bounce buffering.
> > >
> > > Note that this patch removes the possibility to force xen-swiotlb
> > > use using swiotlb=force on the command line on x86 (arm and arm64
> > > never supported that), but this interface will be restored shortly.
> > >
> > > Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> > > ---
> > >  arch/arm/mm/init.c                     |  6 +----
> > >  arch/arm64/mm/init.c                   |  6 +----
> > >  arch/ia64/mm/init.c                    |  4 +--
> > >  arch/mips/cavium-octeon/dma-octeon.c   |  2 +-
> > >  arch/mips/loongson64/dma.c             |  2 +-
> > >  arch/mips/sibyte/common/dma.c          |  2 +-
> > >  arch/powerpc/include/asm/swiotlb.h     |  1 +
> > >  arch/powerpc/mm/mem.c                  |  3 ++-
> > >  arch/powerpc/platforms/pseries/setup.c |  3 ---
> > >  arch/riscv/mm/init.c                   |  8 +-----
> > >  arch/s390/mm/init.c                    |  3 +--
> > >  arch/x86/kernel/cpu/mshyperv.c         |  8 ------
> > >  arch/x86/kernel/pci-dma.c              | 15 ++++++-----
> > >  arch/x86/mm/mem_encrypt_amd.c          |  3 ---
> > >  drivers/xen/swiotlb-xen.c              |  4 +--
> > >  include/linux/swiotlb.h                | 15 ++++++-----
> > >  include/trace/events/swiotlb.h         | 29 ++++++++-------------
> > >  kernel/dma/swiotlb.c                   | 35 ++++++++++++++------------
> > >  18 files changed, 56 insertions(+), 93 deletions(-)
> >
> > [snip]
> >
> > >
> > > diff --git a/arch/x86/kernel/cpu/mshyperv.c 
> > > b/arch/x86/kernel/cpu/mshyperv.c
> > > index 5a99f993e6392..568274917f1cd 100644
> > > --- a/arch/x86/kernel/cpu/mshyperv.c
> > > +++ b/arch/x86/kernel/cpu/mshyperv.c
> > > @@ -336,14 +336,6 @@ static void __init ms_hyperv_init_platform(void)
> > >                   swiotlb_unencrypted_base =
> ms_hyperv.shared_gpa_boundary;
> > >  #endif
> > >           }
> > > -
> > > -#ifdef CONFIG_SWIOTLB
> > > -         /*
> > > -          * Enable swiotlb force mode in Isolation VM to
> > > -          * use swiotlb bounce buffer for dma transaction.
> > > -          */
> > > -         swiotlb_force = SWIOTLB_FORCE;
> > > -#endif
> >
> > With this code removed, it's not clear to me what forces the use of the
> > swiotlb in a Hyper-V isolated VM.  The code in pci_swiotlb_detect_4g() 
> > doesn't
> > catch this case because cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT)
> > returns "false" in a Hyper-V guest.  In the Hyper-V guest, it's only
> > cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT) that returns "true".  I'm
> > looking more closely at the meaning of the CC_ATTR_* values, and it may
> > be that Hyper-V should also return "true" for CC_ATTR_MEM_ENCRYPT,
> > but I don't think CC_ATTR_HOST_MEM_ENCRYPT should return "true".
> 
> Ok, I assumed that CC_ATTR_HOST_MEM_ENCRYPT returned true in this case.
> I guess we just need to check for CC_ATTR_GUEST_MEM_ENCRYPT as well
> there?

I'm unsure.

The comments for CC_ATTR_HOST_MEM_ENCRYPT indicates that it is for
SME.   The comments for both CC_ATTR_MEM_ENCRYPT and
CC_ATTR_GUEST_MEM_ENCRYPT mention SEV and SEV-ES (and presumably
SEV-SNP).   But I haven't looked at the details of the core SNP patches from
the AMD folks.   I'd say that they need to weigh in on the right approach
here that will work for both SME and the various SEV flavors, and then
hopefully the Hyper-V case will fit in.

Michael