| Message ID | 1669951831-4180-6-git-send-email-mikelley@microsoft.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Add PCI pass-thru support to Hyper-V Confidential VMs | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On 12/1/22 7:30 PM, Michael Kelley wrote: > Full Hyper-V initialization, including support for hypercalls, is done > as an apic_post_init callback via late_time_init(). mem_encrypt_init() > needs to make hypercalls when it marks swiotlb memory as decrypted. > But mem_encrypt_init() is currently called a few lines before > late_time_init(), so the hypercalls don't work. Did you consider moving hyper-v hypercall initialization before mem_encrypt_init(). Is there any dependency issue? > > Fix this by moving mem_encrypt_init() after late_time_init() and > related clock initializations. The intervening initializations don't > do any I/O that requires the swiotlb, so moving mem_encrypt_init() > slightly later has no impact. > > Signed-off-by: Michael Kelley <mikelley@microsoft.com> > Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com> > --- > init/main.c | 19 +++++++++++-------- > 1 file changed, 11 insertions(+), 8 deletions(-) > > diff --git a/init/main.c b/init/main.c > index e1c3911..5a7c466 100644 > --- a/init/main.c > +++ b/init/main.c > @@ -1088,14 +1088,6 @@ asmlinkage __visible void __init __no_sanitize_address start_kernel(void) > */ > locking_selftest(); > > - /* > - * This needs to be called before any devices perform DMA > - * operations that might use the SWIOTLB bounce buffers. It will > - * mark the bounce buffers as decrypted so that their usage will > - * not cause "plain-text" data to be decrypted when accessed. > - */ > - mem_encrypt_init(); > - > #ifdef CONFIG_BLK_DEV_INITRD > if (initrd_start && !initrd_below_start_ok && > page_to_pfn(virt_to_page((void *)initrd_start)) < min_low_pfn) { > @@ -1112,6 +1104,17 @@ asmlinkage __visible void __init __no_sanitize_address start_kernel(void) > late_time_init(); > sched_clock_init(); > calibrate_delay(); > + > + /* > + * This needs to be called before any devices perform DMA > + * operations that might use the SWIOTLB bounce buffers. It will > + * mark the bounce buffers as decrypted so that their usage will > + * not cause "plain-text" data to be decrypted when accessed. It > + * must be called after late_time_init() so that Hyper-V x86/x64 > + * hypercalls work when the SWIOTLB bounce buffers are decrypted. > + */ > + mem_encrypt_init(); > + > pid_idr_init(); > anon_vma_init(); > #ifdef CONFIG_X86
From: Sathyanarayanan Kuppuswamy <sathyanarayanan.kuppuswamy@linux.intel.com> > > > > On 12/1/22 7:30 PM, Michael Kelley wrote: > > Full Hyper-V initialization, including support for hypercalls, is done > > as an apic_post_init callback via late_time_init(). mem_encrypt_init() > > needs to make hypercalls when it marks swiotlb memory as decrypted. > > But mem_encrypt_init() is currently called a few lines before > > late_time_init(), so the hypercalls don't work. > > Did you consider moving hyper-v hypercall initialization before > mem_encrypt_init(). Is there any dependency issue? Hyper-V initialization has historically been done using the callbacks that exist in the x86 initialization paths, rather than adding explicit Hyper-V init calls. As noted above, the full Hyper-V init is done on the apic_post_init callback via late_time_init(). Conceivably we could add an explicit call to do the Hyper-V init, but I think there's still a problem in putting that Hyper-V init prior to the current location of mem_encrypt_init(). I'd have to go check the history, but I think the Hyper-V init needs to happen after the APIC is initialized. It seems like moving mem_encrypt_init() slightly later is the cleaner long-term solution. Are you aware of a likely problem arising in the future with moving mem_encrypt_init()? Michael > > > > > Fix this by moving mem_encrypt_init() after late_time_init() and > > related clock initializations. The intervening initializations don't > > do any I/O that requires the swiotlb, so moving mem_encrypt_init() > > slightly later has no impact. > >
On 12/6/22 12:13 PM, Michael Kelley (LINUX) wrote: > From: Sathyanarayanan Kuppuswamy <sathyanarayanan.kuppuswamy@linux.intel.com> >> >> >> >> On 12/1/22 7:30 PM, Michael Kelley wrote: >>> Full Hyper-V initialization, including support for hypercalls, is done >>> as an apic_post_init callback via late_time_init(). mem_encrypt_init() >>> needs to make hypercalls when it marks swiotlb memory as decrypted. >>> But mem_encrypt_init() is currently called a few lines before >>> late_time_init(), so the hypercalls don't work. >> >> Did you consider moving hyper-v hypercall initialization before >> mem_encrypt_init(). Is there any dependency issue? > > Hyper-V initialization has historically been done using the callbacks > that exist in the x86 initialization paths, rather than adding explicit > Hyper-V init calls. As noted above, the full Hyper-V init is done on > the apic_post_init callback via late_time_init(). Conceivably we could > add an explicit call to do the Hyper-V init, but I think there's still a > problem in putting that Hyper-V init prior to the current location of > mem_encrypt_init(). I'd have to go check the history, but I think the > Hyper-V init needs to happen after the APIC is initialized. Ok. If there is a dependency or complexity issue, I recommend adding that detail in the commit log. > > It seems like moving mem_encrypt_init() slightly later is the cleaner > long-term solution. Are you aware of a likely problem arising in the > future with moving mem_encrypt_init()? I did not investigate in depth, but there appears to be no problem with moving mem_encrypt_init(). But my point is, if it is possible to fix this easily by changing Hyper-v specific initialization, we should consider it first before considering moving the common mem_encrypt_init() function. > > Michael > >> >>> >>> Fix this by moving mem_encrypt_init() after late_time_init() and >>> related clock initializations. The intervening initializations don't >>> do any I/O that requires the swiotlb, so moving mem_encrypt_init() >>> slightly later has no impact. >>>
diff --git a/init/main.c b/init/main.c index e1c3911..5a7c466 100644 --- a/init/main.c +++ b/init/main.c @@ -1088,14 +1088,6 @@ asmlinkage __visible void __init __no_sanitize_address start_kernel(void) */ locking_selftest(); - /* - * This needs to be called before any devices perform DMA - * operations that might use the SWIOTLB bounce buffers. It will - * mark the bounce buffers as decrypted so that their usage will - * not cause "plain-text" data to be decrypted when accessed. - */ - mem_encrypt_init(); - #ifdef CONFIG_BLK_DEV_INITRD if (initrd_start && !initrd_below_start_ok && page_to_pfn(virt_to_page((void *)initrd_start)) < min_low_pfn) { @@ -1112,6 +1104,17 @@ asmlinkage __visible void __init __no_sanitize_address start_kernel(void) late_time_init(); sched_clock_init(); calibrate_delay(); + + /* + * This needs to be called before any devices perform DMA + * operations that might use the SWIOTLB bounce buffers. It will + * mark the bounce buffers as decrypted so that their usage will + * not cause "plain-text" data to be decrypted when accessed. It + * must be called after late_time_init() so that Hyper-V x86/x64 + * hypercalls work when the SWIOTLB bounce buffers are decrypted. + */ + mem_encrypt_init(); + pid_idr_init(); anon_vma_init(); #ifdef CONFIG_X86