* [Patch v3 01/14] x86/ioremap: Fix page aligned size calculation in __ioremap_caller()
2022-11-16 18:41 [Patch v3 00/14] Add PCI pass-thru support to Hyper-V Confidential VMs Michael Kelley
@ 2022-11-16 18:41 ` Michael Kelley
2022-11-21 13:32 ` Borislav Petkov
2022-11-21 18:14 ` Dave Hansen
2022-11-16 18:41 ` [Patch v3 02/14] x86/ioapic: Gate decrypted mapping on cc_platform_has() attribute Michael Kelley
` (12 subsequent siblings)
13 siblings, 2 replies; 66+ messages in thread
From: Michael Kelley @ 2022-11-16 18:41 UTC (permalink / raw)
To: hpa, kys, haiyangz, wei.liu, decui, luto, peterz, davem, edumazet,
kuba, pabeni, lpieralisi, robh, kw, bhelgaas, arnd, hch,
m.szyprowski, robin.murphy, thomas.lendacky, brijesh.singh, tglx,
mingo, bp, dave.hansen, Tianyu.Lan, kirill.shutemov,
sathyanarayanan.kuppuswamy, ak, isaku.yamahata, dan.j.williams,
jane.chu, seanjc, tony.luck, x86, linux-kernel, linux-hyperv,
netdev, linux-pci, linux-arch, iommu
Cc: mikelley
Current code re-calculates the size after aligning the starting and
ending physical addresses on a page boundary. But the re-calculation
also embeds the masking of high order bits that exceed the size of
the physical address space (via PHYSICAL_PAGE_MASK). If the masking
removes any high order bits, the size calculation results in a huge
value that is likely to immediately fail.
Fix this by re-calculating the page-aligned size first. Then mask any
high order bits using PHYSICAL_PAGE_MASK.
Signed-off-by: Michael Kelley <mikelley@microsoft.com>
---
arch/x86/mm/ioremap.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index 78c5bc6..6453fba 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -217,9 +217,15 @@ static void __ioremap_check_mem(resource_size_t addr, unsigned long size,
* Mappings have to be page-aligned
*/
offset = phys_addr & ~PAGE_MASK;
- phys_addr &= PHYSICAL_PAGE_MASK;
+ phys_addr &= PAGE_MASK;
size = PAGE_ALIGN(last_addr+1) - phys_addr;
+ /*
+ * Mask out any bits not part of the actual physical
+ * address, like memory encryption bits.
+ */
+ phys_addr &= PHYSICAL_PAGE_MASK;
+
retval = memtype_reserve(phys_addr, (u64)phys_addr + size,
pcm, &new_pcm);
if (retval) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 66+ messages in thread* Re: [Patch v3 01/14] x86/ioremap: Fix page aligned size calculation in __ioremap_caller()
2022-11-16 18:41 ` [Patch v3 01/14] x86/ioremap: Fix page aligned size calculation in __ioremap_caller() Michael Kelley
@ 2022-11-21 13:32 ` Borislav Petkov
2022-11-21 16:40 ` Michael Kelley (LINUX)
2022-11-21 18:14 ` Dave Hansen
1 sibling, 1 reply; 66+ messages in thread
From: Borislav Petkov @ 2022-11-21 13:32 UTC (permalink / raw)
To: Michael Kelley
Cc: hpa, kys, haiyangz, wei.liu, decui, luto, peterz, davem, edumazet,
kuba, pabeni, lpieralisi, robh, kw, bhelgaas, arnd, hch,
m.szyprowski, robin.murphy, thomas.lendacky, brijesh.singh, tglx,
mingo, dave.hansen, Tianyu.Lan, kirill.shutemov,
sathyanarayanan.kuppuswamy, ak, isaku.yamahata, dan.j.williams,
jane.chu, seanjc, tony.luck, x86, linux-kernel, linux-hyperv,
netdev, linux-pci, linux-arch, iommu
On Wed, Nov 16, 2022 at 10:41:24AM -0800, Michael Kelley wrote:
> Current code re-calculates the size after aligning the starting and
> ending physical addresses on a page boundary. But the re-calculation
> also embeds the masking of high order bits that exceed the size of
> the physical address space (via PHYSICAL_PAGE_MASK). If the masking
> removes any high order bits, the size calculation results in a huge
> value that is likely to immediately fail.
>
> Fix this by re-calculating the page-aligned size first. Then mask any
> high order bits using PHYSICAL_PAGE_MASK.
>
> Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> ---
> arch/x86/mm/ioremap.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
> index 78c5bc6..6453fba 100644
> --- a/arch/x86/mm/ioremap.c
> +++ b/arch/x86/mm/ioremap.c
> @@ -217,9 +217,15 @@ static void __ioremap_check_mem(resource_size_t addr, unsigned long size,
> * Mappings have to be page-aligned
> */
> offset = phys_addr & ~PAGE_MASK;
> - phys_addr &= PHYSICAL_PAGE_MASK;
> + phys_addr &= PAGE_MASK;
> size = PAGE_ALIGN(last_addr+1) - phys_addr;
>
> + /*
> + * Mask out any bits not part of the actual physical
> + * address, like memory encryption bits.
> + */
> + phys_addr &= PHYSICAL_PAGE_MASK;
> +
> retval = memtype_reserve(phys_addr, (u64)phys_addr + size,
> pcm, &new_pcm);
> if (retval) {
> --
This looks like a fix to me that needs to go to independently to stable.
And it would need a Fixes tag.
/me does some git archeology...
I guess this one:
ffa71f33a820 ("x86, ioremap: Fix incorrect physical address handling in PAE mode")
should be old enough so that it goes to all relevant stable kernels...
Hmm?
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 66+ messages in thread* RE: [Patch v3 01/14] x86/ioremap: Fix page aligned size calculation in __ioremap_caller()
2022-11-21 13:32 ` Borislav Petkov
@ 2022-11-21 16:40 ` Michael Kelley (LINUX)
2022-11-21 19:45 ` Borislav Petkov
0 siblings, 1 reply; 66+ messages in thread
From: Michael Kelley (LINUX) @ 2022-11-21 16:40 UTC (permalink / raw)
To: Borislav Petkov
Cc: hpa@zytor.com, KY Srinivasan, Haiyang Zhang, wei.liu@kernel.org,
Dexuan Cui, luto@kernel.org, peterz@infradead.org,
davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, lpieralisi@kernel.org, robh@kernel.org,
kw@linux.com, bhelgaas@google.com, arnd@arndb.de,
hch@infradead.org, m.szyprowski@samsung.com, robin.murphy@arm.com,
thomas.lendacky@amd.com, brijesh.singh@amd.com,
tglx@linutronix.de, mingo@redhat.com, dave.hansen@linux.intel.com,
Tianyu Lan, kirill.shutemov@linux.intel.com,
sathyanarayanan.kuppuswamy@linux.intel.com, ak@linux.intel.com,
isaku.yamahata@intel.com, Williams, Dan J, jane.chu@oracle.com,
seanjc@google.com, tony.luck@intel.com, x86@kernel.org,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org, linux-pci@vger.kernel.org,
linux-arch@vger.kernel.org, iommu@lists.linux.dev
From: Borislav Petkov <bp@alien8.de> Sent: Monday, November 21, 2022 5:33 AM
>
> On Wed, Nov 16, 2022 at 10:41:24AM -0800, Michael Kelley wrote:
> > Current code re-calculates the size after aligning the starting and
> > ending physical addresses on a page boundary. But the re-calculation
> > also embeds the masking of high order bits that exceed the size of
> > the physical address space (via PHYSICAL_PAGE_MASK). If the masking
> > removes any high order bits, the size calculation results in a huge
> > value that is likely to immediately fail.
> >
> > Fix this by re-calculating the page-aligned size first. Then mask any
> > high order bits using PHYSICAL_PAGE_MASK.
> >
> > Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> > ---
> > arch/x86/mm/ioremap.c | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
> > index 78c5bc6..6453fba 100644
> > --- a/arch/x86/mm/ioremap.c
> > +++ b/arch/x86/mm/ioremap.c
> > @@ -217,9 +217,15 @@ static void __ioremap_check_mem(resource_size_t addr,
> unsigned long size,
> > * Mappings have to be page-aligned
> > */
> > offset = phys_addr & ~PAGE_MASK;
> > - phys_addr &= PHYSICAL_PAGE_MASK;
> > + phys_addr &= PAGE_MASK;
> > size = PAGE_ALIGN(last_addr+1) - phys_addr;
> >
> > + /*
> > + * Mask out any bits not part of the actual physical
> > + * address, like memory encryption bits.
> > + */
> > + phys_addr &= PHYSICAL_PAGE_MASK;
> > +
> > retval = memtype_reserve(phys_addr, (u64)phys_addr + size,
> > pcm, &new_pcm);
> > if (retval) {
> > --
>
> This looks like a fix to me that needs to go to independently to stable.
> And it would need a Fixes tag.
>
> /me does some git archeology...
>
> I guess this one:
>
> ffa71f33a820 ("x86, ioremap: Fix incorrect physical address handling in PAE mode")
>
> should be old enough so that it goes to all relevant stable kernels...
>
> Hmm?
>
As discussed in a parallel thread [1], the incorrect code here doesn't have
any real impact in already released Linux kernels. It only affects the
transition that my patch series implements to change the way vTOM
is handled.
I don't know what the tradeoffs are for backporting a fix that doesn't solve
a real problem vs. just letting it be. Every backport carries some overhead
in the process and there's always a non-zero risk of breaking something.
I've leaned away from adding the "Fixes:" tag in such cases. But if it's better
to go ahead and add the "Fixes:" tag for what's only a theoretical problem,
I'm OK with doing so.
Michael
[1] https://lkml.org/lkml/2022/11/11/1348
^ permalink raw reply [flat|nested] 66+ messages in thread* Re: [Patch v3 01/14] x86/ioremap: Fix page aligned size calculation in __ioremap_caller()
2022-11-21 16:40 ` Michael Kelley (LINUX)
@ 2022-11-21 19:45 ` Borislav Petkov
2022-11-21 21:02 ` Michael Kelley (LINUX)
0 siblings, 1 reply; 66+ messages in thread
From: Borislav Petkov @ 2022-11-21 19:45 UTC (permalink / raw)
To: Michael Kelley (LINUX)
Cc: hpa@zytor.com, KY Srinivasan, Haiyang Zhang, wei.liu@kernel.org,
Dexuan Cui, luto@kernel.org, peterz@infradead.org,
davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, lpieralisi@kernel.org, robh@kernel.org,
kw@linux.com, bhelgaas@google.com, arnd@arndb.de,
hch@infradead.org, m.szyprowski@samsung.com, robin.murphy@arm.com,
thomas.lendacky@amd.com, brijesh.singh@amd.com,
tglx@linutronix.de, mingo@redhat.com, dave.hansen@linux.intel.com,
Tianyu Lan, kirill.shutemov@linux.intel.com,
sathyanarayanan.kuppuswamy@linux.intel.com, ak@linux.intel.com,
isaku.yamahata@intel.com, Williams, Dan J, jane.chu@oracle.com,
seanjc@google.com, tony.luck@intel.com, x86@kernel.org,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org, linux-pci@vger.kernel.org,
linux-arch@vger.kernel.org, iommu@lists.linux.dev
On Mon, Nov 21, 2022 at 04:40:16PM +0000, Michael Kelley (LINUX) wrote:
> As discussed in a parallel thread [1], the incorrect code here doesn't have
> any real impact in already released Linux kernels. It only affects the
> transition that my patch series implements to change the way vTOM
> is handled.
Are you sure?
PHYSICAL_PAGE_MASK is controlled by __PHYSICAL_MASK which is determined
by CONFIG_DYNAMIC_PHYSICAL_MASK and __PHYSICAL_MASK_SHIFT which all
differ depending on configurations and also dynamic.
It is probably still ok, in probably all possible cases even though I
wouldn't bet on it.
And this fix is simple and all clear so lemme ask it differently: what
would be any downsides in backporting it to stable, just in case?
> I don't know what the tradeoffs are for backporting a fix that doesn't solve
> a real problem vs. just letting it be. Every backport carries some overhead
> in the process
Have you seen the deluge of stable fixes? :-)
> and there's always a non-zero risk of breaking something.
I don't see how this one would cause any breakage...
> I've leaned away from adding the "Fixes:" tag in such cases. But if
> it's better to go ahead and add the "Fixes:" tag for what's only a
> theoretical problem, I'm OK with doing so.
I think this is a good to have fix anyway as it is Obviously
Correct(tm).
Unless you have any reservations you haven't shared yet...
> [1] https://lkml.org/lkml/2022/11/11/1348
Btw, the proper way to reference to a mail message now is simply to do:
https://lore.kernel.org/r/<Message-ID>
as long as it has been posted on some ML which lore archives. And I
think it archives all.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 66+ messages in thread* RE: [Patch v3 01/14] x86/ioremap: Fix page aligned size calculation in __ioremap_caller()
2022-11-21 19:45 ` Borislav Petkov
@ 2022-11-21 21:02 ` Michael Kelley (LINUX)
0 siblings, 0 replies; 66+ messages in thread
From: Michael Kelley (LINUX) @ 2022-11-21 21:02 UTC (permalink / raw)
To: Borislav Petkov
Cc: hpa@zytor.com, KY Srinivasan, Haiyang Zhang, wei.liu@kernel.org,
Dexuan Cui, luto@kernel.org, peterz@infradead.org,
davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, lpieralisi@kernel.org, robh@kernel.org,
kw@linux.com, bhelgaas@google.com, arnd@arndb.de,
hch@infradead.org, m.szyprowski@samsung.com, robin.murphy@arm.com,
thomas.lendacky@amd.com, brijesh.singh@amd.com,
tglx@linutronix.de, mingo@redhat.com, dave.hansen@linux.intel.com,
Tianyu Lan, kirill.shutemov@linux.intel.com,
sathyanarayanan.kuppuswamy@linux.intel.com, ak@linux.intel.com,
isaku.yamahata@intel.com, Williams, Dan J, jane.chu@oracle.com,
seanjc@google.com, tony.luck@intel.com, x86@kernel.org,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org, linux-pci@vger.kernel.org,
linux-arch@vger.kernel.org, iommu@lists.linux.dev
From: Borislav Petkov <bp@alien8.de> Sent: Monday, November 21, 2022 11:45 AM
>
> On Mon, Nov 21, 2022 at 04:40:16PM +0000, Michael Kelley (LINUX) wrote:
> > As discussed in a parallel thread [1], the incorrect code here doesn't have
> > any real impact in already released Linux kernels. It only affects the
> > transition that my patch series implements to change the way vTOM
> > is handled.
>
> Are you sure?
>
> PHYSICAL_PAGE_MASK is controlled by __PHYSICAL_MASK which is determined
> by CONFIG_DYNAMIC_PHYSICAL_MASK and __PHYSICAL_MASK_SHIFT which all
> differ depending on configurations and also dynamic.
>
> It is probably still ok, in probably all possible cases even though I
> wouldn't bet on it.
>
> And this fix is simple and all clear so lemme ask it differently: what
> would be any downsides in backporting it to stable, just in case?
None
>
> > I don't know what the tradeoffs are for backporting a fix that doesn't solve
> > a real problem vs. just letting it be. Every backport carries some overhead
> > in the process
>
> Have you seen the deluge of stable fixes? :-)
>
> > and there's always a non-zero risk of breaking something.
>
> I don't see how this one would cause any breakage...
>
> > I've leaned away from adding the "Fixes:" tag in such cases. But if
> > it's better to go ahead and add the "Fixes:" tag for what's only a
> > theoretical problem, I'm OK with doing so.
>
> I think this is a good to have fix anyway as it is Obviously
> Correct(tm).
>
> Unless you have any reservations you haven't shared yet...
>
No reservations. I'll add the "Fixes:" tag.
Michael
^ permalink raw reply [flat|nested] 66+ messages in thread
* Re: [Patch v3 01/14] x86/ioremap: Fix page aligned size calculation in __ioremap_caller()
2022-11-16 18:41 ` [Patch v3 01/14] x86/ioremap: Fix page aligned size calculation in __ioremap_caller() Michael Kelley
2022-11-21 13:32 ` Borislav Petkov
@ 2022-11-21 18:14 ` Dave Hansen
2022-11-21 21:04 ` Michael Kelley (LINUX)
1 sibling, 1 reply; 66+ messages in thread
From: Dave Hansen @ 2022-11-21 18:14 UTC (permalink / raw)
To: Michael Kelley, hpa, kys, haiyangz, wei.liu, decui, luto, peterz,
davem, edumazet, kuba, pabeni, lpieralisi, robh, kw, bhelgaas,
arnd, hch, m.szyprowski, robin.murphy, thomas.lendacky,
brijesh.singh, tglx, mingo, bp, dave.hansen, Tianyu.Lan,
kirill.shutemov, sathyanarayanan.kuppuswamy, ak, isaku.yamahata,
dan.j.williams, jane.chu, seanjc, tony.luck, x86, linux-kernel,
linux-hyperv, netdev, linux-pci, linux-arch, iommu
On 11/16/22 10:41, Michael Kelley wrote:
> Current code re-calculates the size after aligning the starting and
> ending physical addresses on a page boundary. But the re-calculation
> also embeds the masking of high order bits that exceed the size of
> the physical address space (via PHYSICAL_PAGE_MASK). If the masking
> removes any high order bits, the size calculation results in a huge
> value that is likely to immediately fail.
>
> Fix this by re-calculating the page-aligned size first. Then mask any
> high order bits using PHYSICAL_PAGE_MASK.
>
> Signed-off-by: Michael Kelley <mikelley@microsoft.com>
Looks good:
Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
Although I do agree with Boris that this superficially looks like
something that's important to backport. It would be best to either beef
up the changelog to explain why that's not the case, or to treat this as
an actual fix and submit separately.
^ permalink raw reply [flat|nested] 66+ messages in thread
* RE: [Patch v3 01/14] x86/ioremap: Fix page aligned size calculation in __ioremap_caller()
2022-11-21 18:14 ` Dave Hansen
@ 2022-11-21 21:04 ` Michael Kelley (LINUX)
2022-11-21 21:08 ` Borislav Petkov
0 siblings, 1 reply; 66+ messages in thread
From: Michael Kelley (LINUX) @ 2022-11-21 21:04 UTC (permalink / raw)
To: Dave Hansen, hpa@zytor.com, KY Srinivasan, Haiyang Zhang,
wei.liu@kernel.org, Dexuan Cui, luto@kernel.org,
peterz@infradead.org, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, pabeni@redhat.com, lpieralisi@kernel.org,
robh@kernel.org, kw@linux.com, bhelgaas@google.com, arnd@arndb.de,
hch@infradead.org, m.szyprowski@samsung.com, robin.murphy@arm.com,
thomas.lendacky@amd.com, brijesh.singh@amd.com,
tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
dave.hansen@linux.intel.com, Tianyu Lan,
kirill.shutemov@linux.intel.com,
sathyanarayanan.kuppuswamy@linux.intel.com, ak@linux.intel.com,
isaku.yamahata@intel.com, Williams, Dan J, jane.chu@oracle.com,
seanjc@google.com, tony.luck@intel.com, x86@kernel.org,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org, linux-pci@vger.kernel.org,
linux-arch@vger.kernel.org, iommu@lists.linux.dev
From: Dave Hansen <dave.hansen@intel.com> Sent: Monday, November 21, 2022 10:14 AM
>
> On 11/16/22 10:41, Michael Kelley wrote:
> > Current code re-calculates the size after aligning the starting and
> > ending physical addresses on a page boundary. But the re-calculation
> > also embeds the masking of high order bits that exceed the size of
> > the physical address space (via PHYSICAL_PAGE_MASK). If the masking
> > removes any high order bits, the size calculation results in a huge
> > value that is likely to immediately fail.
> >
> > Fix this by re-calculating the page-aligned size first. Then mask any
> > high order bits using PHYSICAL_PAGE_MASK.
> >
> > Signed-off-by: Michael Kelley <mikelley@microsoft.com>
>
> Looks good:
>
> Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
>
> Although I do agree with Boris that this superficially looks like
> something that's important to backport. It would be best to either beef
> up the changelog to explain why that's not the case, or to treat this as
> an actual fix and submit separately.
You and Boris agree and I have no objection, so I'll add the "Fixes:" tag.
I'd like to keep the patch as part of this series because it *is* needed to
make the series work.
Michael
^ permalink raw reply [flat|nested] 66+ messages in thread
* Re: [Patch v3 01/14] x86/ioremap: Fix page aligned size calculation in __ioremap_caller()
2022-11-21 21:04 ` Michael Kelley (LINUX)
@ 2022-11-21 21:08 ` Borislav Petkov
0 siblings, 0 replies; 66+ messages in thread
From: Borislav Petkov @ 2022-11-21 21:08 UTC (permalink / raw)
To: Michael Kelley (LINUX)
Cc: Dave Hansen, hpa@zytor.com, KY Srinivasan, Haiyang Zhang,
wei.liu@kernel.org, Dexuan Cui, luto@kernel.org,
peterz@infradead.org, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, pabeni@redhat.com, lpieralisi@kernel.org,
robh@kernel.org, kw@linux.com, bhelgaas@google.com, arnd@arndb.de,
hch@infradead.org, m.szyprowski@samsung.com, robin.murphy@arm.com,
thomas.lendacky@amd.com, brijesh.singh@amd.com,
tglx@linutronix.de, mingo@redhat.com, dave.hansen@linux.intel.com,
Tianyu Lan, kirill.shutemov@linux.intel.com,
sathyanarayanan.kuppuswamy@linux.intel.com, ak@linux.intel.com,
isaku.yamahata@intel.com, Williams, Dan J, jane.chu@oracle.com,
seanjc@google.com, tony.luck@intel.com, x86@kernel.org,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org, linux-pci@vger.kernel.org,
linux-arch@vger.kernel.org, iommu@lists.linux.dev
On Mon, Nov 21, 2022 at 09:04:06PM +0000, Michael Kelley (LINUX) wrote:
> You and Boris agree and I have no objection, so I'll add the "Fixes:" tag.
> I'd like to keep the patch as part of this series because it *is* needed to
> make the series work.
Yeah, no worries. I can take it tomorrow through urgent and send it to
Linus this week so whatever you rebase your tree on, it should already
have it.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 66+ messages in thread
* [Patch v3 02/14] x86/ioapic: Gate decrypted mapping on cc_platform_has() attribute
2022-11-16 18:41 [Patch v3 00/14] Add PCI pass-thru support to Hyper-V Confidential VMs Michael Kelley
2022-11-16 18:41 ` [Patch v3 01/14] x86/ioremap: Fix page aligned size calculation in __ioremap_caller() Michael Kelley
@ 2022-11-16 18:41 ` Michael Kelley
2022-11-17 21:39 ` Sathyanarayanan Kuppuswamy
2022-11-21 13:50 ` Borislav Petkov
2022-11-16 18:41 ` [Patch v3 03/14] x86/hyperv: Reorder code in prep for subsequent patch Michael Kelley
` (11 subsequent siblings)
13 siblings, 2 replies; 66+ messages in thread
From: Michael Kelley @ 2022-11-16 18:41 UTC (permalink / raw)
To: hpa, kys, haiyangz, wei.liu, decui, luto, peterz, davem, edumazet,
kuba, pabeni, lpieralisi, robh, kw, bhelgaas, arnd, hch,
m.szyprowski, robin.murphy, thomas.lendacky, brijesh.singh, tglx,
mingo, bp, dave.hansen, Tianyu.Lan, kirill.shutemov,
sathyanarayanan.kuppuswamy, ak, isaku.yamahata, dan.j.williams,
jane.chu, seanjc, tony.luck, x86, linux-kernel, linux-hyperv,
netdev, linux-pci, linux-arch, iommu
Cc: mikelley
Current code always maps the IOAPIC as shared (decrypted) in a
confidential VM. But Hyper-V guest VMs on AMD SEV-SNP with vTOM
enabled use a paravisor running in VMPL0 to emulate the IOAPIC.
In such a case, the IOAPIC must be accessed as private (encrypted).
Fix this by gating the IOAPIC decrypted mapping on a new
cc_platform_has() attribute that a subsequent patch in the series
will set only for Hyper-V guests.
Signed-off-by: Michael Kelley <mikelley@microsoft.com>
Reviewed-by: Wei Liu <wei.liu@kernel.org>
---
arch/x86/kernel/apic/io_apic.c | 3 ++-
include/linux/cc_platform.h | 12 ++++++++++++
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index a868b76..c65e0cc 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -2686,7 +2686,8 @@ static void io_apic_set_fixmap(enum fixed_addresses idx, phys_addr_t phys)
* Ensure fixmaps for IOAPIC MMIO respect memory encryption pgprot
* bits, just like normal ioremap():
*/
- flags = pgprot_decrypted(flags);
+ if (!cc_platform_has(CC_ATTR_EMULATED_IOAPIC))
+ flags = pgprot_decrypted(flags);
__set_fixmap(idx, phys, flags);
}
diff --git a/include/linux/cc_platform.h b/include/linux/cc_platform.h
index cb0d6cd..7a0da75 100644
--- a/include/linux/cc_platform.h
+++ b/include/linux/cc_platform.h
@@ -90,6 +90,18 @@ enum cc_attr {
* Examples include TDX Guest.
*/
CC_ATTR_HOTPLUG_DISABLED,
+
+ /**
+ * @CC_ATTR_EMULATED_IOAPIC: Guest VM has an emulated I/O APIC
+ *
+ * The platform/OS is running as a guest/virtual machine with
+ * an I/O APIC that is emulated by a paravisor running in the
+ * guest VM context. As such, the I/O APIC is accessed in the
+ * encrypted portion of the guest physical address space.
+ *
+ * Examples include Hyper-V SEV-SNP guests using vTOM.
+ */
+ CC_ATTR_EMULATED_IOAPIC,
};
#ifdef CONFIG_ARCH_HAS_CC_PLATFORM
--
1.8.3.1
^ permalink raw reply related [flat|nested] 66+ messages in thread* Re: [Patch v3 02/14] x86/ioapic: Gate decrypted mapping on cc_platform_has() attribute
2022-11-16 18:41 ` [Patch v3 02/14] x86/ioapic: Gate decrypted mapping on cc_platform_has() attribute Michael Kelley
@ 2022-11-17 21:39 ` Sathyanarayanan Kuppuswamy
2022-11-21 13:50 ` Borislav Petkov
1 sibling, 0 replies; 66+ messages in thread
From: Sathyanarayanan Kuppuswamy @ 2022-11-17 21:39 UTC (permalink / raw)
To: Michael Kelley, hpa, kys, haiyangz, wei.liu, decui, luto, peterz,
davem, edumazet, kuba, pabeni, lpieralisi, robh, kw, bhelgaas,
arnd, hch, m.szyprowski, robin.murphy, thomas.lendacky,
brijesh.singh, tglx, mingo, bp, dave.hansen, Tianyu.Lan,
kirill.shutemov, ak, isaku.yamahata, dan.j.williams, jane.chu,
seanjc, tony.luck, x86, linux-kernel, linux-hyperv, netdev,
linux-pci, linux-arch, iommu
On 11/16/22 10:41 AM, Michael Kelley wrote:
> Current code always maps the IOAPIC as shared (decrypted) in a
> confidential VM. But Hyper-V guest VMs on AMD SEV-SNP with vTOM
> enabled use a paravisor running in VMPL0 to emulate the IOAPIC.
> In such a case, the IOAPIC must be accessed as private (encrypted).
>
> Fix this by gating the IOAPIC decrypted mapping on a new
> cc_platform_has() attribute that a subsequent patch in the series
> will set only for Hyper-V guests.
>
> Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> Reviewed-by: Wei Liu <wei.liu@kernel.org>
> ---
Looks fine to me.
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> arch/x86/kernel/apic/io_apic.c | 3 ++-
> include/linux/cc_platform.h | 12 ++++++++++++
> 2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index a868b76..c65e0cc 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -2686,7 +2686,8 @@ static void io_apic_set_fixmap(enum fixed_addresses idx, phys_addr_t phys)
> * Ensure fixmaps for IOAPIC MMIO respect memory encryption pgprot
> * bits, just like normal ioremap():
> */
> - flags = pgprot_decrypted(flags);
> + if (!cc_platform_has(CC_ATTR_EMULATED_IOAPIC))
> + flags = pgprot_decrypted(flags);
>
> __set_fixmap(idx, phys, flags);
> }
> diff --git a/include/linux/cc_platform.h b/include/linux/cc_platform.h
> index cb0d6cd..7a0da75 100644
> --- a/include/linux/cc_platform.h
> +++ b/include/linux/cc_platform.h
> @@ -90,6 +90,18 @@ enum cc_attr {
> * Examples include TDX Guest.
> */
> CC_ATTR_HOTPLUG_DISABLED,
> +
> + /**
> + * @CC_ATTR_EMULATED_IOAPIC: Guest VM has an emulated I/O APIC
> + *
> + * The platform/OS is running as a guest/virtual machine with
> + * an I/O APIC that is emulated by a paravisor running in the
> + * guest VM context. As such, the I/O APIC is accessed in the
> + * encrypted portion of the guest physical address space.
> + *
> + * Examples include Hyper-V SEV-SNP guests using vTOM.
> + */
> + CC_ATTR_EMULATED_IOAPIC,
> };
>
> #ifdef CONFIG_ARCH_HAS_CC_PLATFORM
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
^ permalink raw reply [flat|nested] 66+ messages in thread* Re: [Patch v3 02/14] x86/ioapic: Gate decrypted mapping on cc_platform_has() attribute
2022-11-16 18:41 ` [Patch v3 02/14] x86/ioapic: Gate decrypted mapping on cc_platform_has() attribute Michael Kelley
2022-11-17 21:39 ` Sathyanarayanan Kuppuswamy
@ 2022-11-21 13:50 ` Borislav Petkov
2022-11-21 16:43 ` Michael Kelley (LINUX)
1 sibling, 1 reply; 66+ messages in thread
From: Borislav Petkov @ 2022-11-21 13:50 UTC (permalink / raw)
To: Michael Kelley
Cc: hpa, kys, haiyangz, wei.liu, decui, luto, peterz, davem, edumazet,
kuba, pabeni, lpieralisi, robh, kw, bhelgaas, arnd, hch,
m.szyprowski, robin.murphy, thomas.lendacky, brijesh.singh, tglx,
mingo, dave.hansen, Tianyu.Lan, kirill.shutemov,
sathyanarayanan.kuppuswamy, ak, isaku.yamahata, dan.j.williams,
jane.chu, seanjc, tony.luck, x86, linux-kernel, linux-hyperv,
netdev, linux-pci, linux-arch, iommu
On Wed, Nov 16, 2022 at 10:41:25AM -0800, Michael Kelley wrote:
> Current code always maps the IOAPIC as shared (decrypted) in a
> confidential VM. But Hyper-V guest VMs on AMD SEV-SNP with vTOM
> enabled use a paravisor running in VMPL0 to emulate the IOAPIC.
"IO-APIC" I guess, in all your text.
> In such a case, the IOAPIC must be accessed as private (encrypted).
So the condition for the IO-APIC is pretty specific but the naming
CC_ATTR_EMULATED_IOAPIC too generic. Other HVs emulate IO-APICs too,
right?
If you have to be precise, the proper check should be (pseudo code):
if (cc_vendor(HYPERV) &&
SNP enabled &&
SNP features has vTOM &&
paravisor in use)
so I guess you're probably better off calling it
CC_ATTR_ACCESS_IOAPIC_ENCRYPTED
which then gets set on exactly those guests and nothing else.
I'd say.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 66+ messages in thread* RE: [Patch v3 02/14] x86/ioapic: Gate decrypted mapping on cc_platform_has() attribute
2022-11-21 13:50 ` Borislav Petkov
@ 2022-11-21 16:43 ` Michael Kelley (LINUX)
2022-11-21 19:47 ` Borislav Petkov
0 siblings, 1 reply; 66+ messages in thread
From: Michael Kelley (LINUX) @ 2022-11-21 16:43 UTC (permalink / raw)
To: Borislav Petkov
Cc: hpa@zytor.com, KY Srinivasan, Haiyang Zhang, wei.liu@kernel.org,
Dexuan Cui, luto@kernel.org, peterz@infradead.org,
davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, lpieralisi@kernel.org, robh@kernel.org,
kw@linux.com, bhelgaas@google.com, arnd@arndb.de,
hch@infradead.org, m.szyprowski@samsung.com, robin.murphy@arm.com,
thomas.lendacky@amd.com, brijesh.singh@amd.com,
tglx@linutronix.de, mingo@redhat.com, dave.hansen@linux.intel.com,
Tianyu Lan, kirill.shutemov@linux.intel.com,
sathyanarayanan.kuppuswamy@linux.intel.com, ak@linux.intel.com,
isaku.yamahata@intel.com, Williams, Dan J, jane.chu@oracle.com,
seanjc@google.com, tony.luck@intel.com, x86@kernel.org,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org, linux-pci@vger.kernel.org,
linux-arch@vger.kernel.org, iommu@lists.linux.dev
From: Borislav Petkov <bp@alien8.de> Sent: Monday, November 21, 2022 5:51 AM
>
> On Wed, Nov 16, 2022 at 10:41:25AM -0800, Michael Kelley wrote:
> > Current code always maps the IOAPIC as shared (decrypted) in a
> > confidential VM. But Hyper-V guest VMs on AMD SEV-SNP with vTOM
> > enabled use a paravisor running in VMPL0 to emulate the IOAPIC.
>
> "IO-APIC" I guess, in all your text.
>
> > In such a case, the IOAPIC must be accessed as private (encrypted).
>
> So the condition for the IO-APIC is pretty specific but the naming
> CC_ATTR_EMULATED_IOAPIC too generic. Other HVs emulate IO-APICs too,
> right?
>
> If you have to be precise, the proper check should be (pseudo code):
>
> if (cc_vendor(HYPERV) &&
> SNP enabled &&
> SNP features has vTOM &&
> paravisor in use)
>
> so I guess you're probably better off calling it
>
> CC_ATTR_ACCESS_IOAPIC_ENCRYPTED
>
> which then gets set on exactly those guests and nothing else.
>
> I'd say.
>
I'm OK with naming it very narrowly. When/if there's a more general
case later, we can generalize to whatever degree is appropriate.
Michael
^ permalink raw reply [flat|nested] 66+ messages in thread
* Re: [Patch v3 02/14] x86/ioapic: Gate decrypted mapping on cc_platform_has() attribute
2022-11-21 16:43 ` Michael Kelley (LINUX)
@ 2022-11-21 19:47 ` Borislav Petkov
0 siblings, 0 replies; 66+ messages in thread
From: Borislav Petkov @ 2022-11-21 19:47 UTC (permalink / raw)
To: Michael Kelley (LINUX)
Cc: hpa@zytor.com, KY Srinivasan, Haiyang Zhang, wei.liu@kernel.org,
Dexuan Cui, luto@kernel.org, peterz@infradead.org,
davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, lpieralisi@kernel.org, robh@kernel.org,
kw@linux.com, bhelgaas@google.com, arnd@arndb.de,
hch@infradead.org, m.szyprowski@samsung.com, robin.murphy@arm.com,
thomas.lendacky@amd.com, brijesh.singh@amd.com,
tglx@linutronix.de, mingo@redhat.com, dave.hansen@linux.intel.com,
Tianyu Lan, kirill.shutemov@linux.intel.com,
sathyanarayanan.kuppuswamy@linux.intel.com, ak@linux.intel.com,
isaku.yamahata@intel.com, Williams, Dan J, jane.chu@oracle.com,
seanjc@google.com, tony.luck@intel.com, x86@kernel.org,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org, linux-pci@vger.kernel.org,
linux-arch@vger.kernel.org, iommu@lists.linux.dev
On Mon, Nov 21, 2022 at 04:43:01PM +0000, Michael Kelley (LINUX) wrote:
> I'm OK with naming it very narrowly. When/if there's a more general
> case later, we can generalize to whatever degree is appropriate.
Exactly.
Those defines are free to change when we see fit.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 66+ messages in thread
* [Patch v3 03/14] x86/hyperv: Reorder code in prep for subsequent patch
2022-11-16 18:41 [Patch v3 00/14] Add PCI pass-thru support to Hyper-V Confidential VMs Michael Kelley
2022-11-16 18:41 ` [Patch v3 01/14] x86/ioremap: Fix page aligned size calculation in __ioremap_caller() Michael Kelley
2022-11-16 18:41 ` [Patch v3 02/14] x86/ioapic: Gate decrypted mapping on cc_platform_has() attribute Michael Kelley
@ 2022-11-16 18:41 ` Michael Kelley
2022-11-16 18:41 ` [Patch v3 04/14] Drivers: hv: Explicitly request decrypted in vmap_pfn() calls Michael Kelley
` (10 subsequent siblings)
13 siblings, 0 replies; 66+ messages in thread
From: Michael Kelley @ 2022-11-16 18:41 UTC (permalink / raw)
To: hpa, kys, haiyangz, wei.liu, decui, luto, peterz, davem, edumazet,
kuba, pabeni, lpieralisi, robh, kw, bhelgaas, arnd, hch,
m.szyprowski, robin.murphy, thomas.lendacky, brijesh.singh, tglx,
mingo, bp, dave.hansen, Tianyu.Lan, kirill.shutemov,
sathyanarayanan.kuppuswamy, ak, isaku.yamahata, dan.j.williams,
jane.chu, seanjc, tony.luck, x86, linux-kernel, linux-hyperv,
netdev, linux-pci, linux-arch, iommu
Cc: mikelley
Reorder some code as preparation for a subsequent patch. No
functional change.
Signed-off-by: Michael Kelley <mikelley@microsoft.com>
Reviewed-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
arch/x86/hyperv/ivm.c | 68 +++++++++++++++++++++++++--------------------------
1 file changed, 34 insertions(+), 34 deletions(-)
diff --git a/arch/x86/hyperv/ivm.c b/arch/x86/hyperv/ivm.c
index 1dbcbd9..f33c67e 100644
--- a/arch/x86/hyperv/ivm.c
+++ b/arch/x86/hyperv/ivm.c
@@ -235,40 +235,6 @@ void hv_ghcb_msr_read(u64 msr, u64 *value)
EXPORT_SYMBOL_GPL(hv_ghcb_msr_read);
#endif
-enum hv_isolation_type hv_get_isolation_type(void)
-{
- if (!(ms_hyperv.priv_high & HV_ISOLATION))
- return HV_ISOLATION_TYPE_NONE;
- return FIELD_GET(HV_ISOLATION_TYPE, ms_hyperv.isolation_config_b);
-}
-EXPORT_SYMBOL_GPL(hv_get_isolation_type);
-
-/*
- * hv_is_isolation_supported - Check system runs in the Hyper-V
- * isolation VM.
- */
-bool hv_is_isolation_supported(void)
-{
- if (!cpu_feature_enabled(X86_FEATURE_HYPERVISOR))
- return false;
-
- if (!hypervisor_is_type(X86_HYPER_MS_HYPERV))
- return false;
-
- return hv_get_isolation_type() != HV_ISOLATION_TYPE_NONE;
-}
-
-DEFINE_STATIC_KEY_FALSE(isolation_type_snp);
-
-/*
- * hv_isolation_type_snp - Check system runs in the AMD SEV-SNP based
- * isolation VM.
- */
-bool hv_isolation_type_snp(void)
-{
- return static_branch_unlikely(&isolation_type_snp);
-}
-
/*
* hv_mark_gpa_visibility - Set pages visible to host via hvcall.
*
@@ -387,3 +353,37 @@ void hv_unmap_memory(void *addr)
{
vunmap(addr);
}
+
+enum hv_isolation_type hv_get_isolation_type(void)
+{
+ if (!(ms_hyperv.priv_high & HV_ISOLATION))
+ return HV_ISOLATION_TYPE_NONE;
+ return FIELD_GET(HV_ISOLATION_TYPE, ms_hyperv.isolation_config_b);
+}
+EXPORT_SYMBOL_GPL(hv_get_isolation_type);
+
+/*
+ * hv_is_isolation_supported - Check system runs in the Hyper-V
+ * isolation VM.
+ */
+bool hv_is_isolation_supported(void)
+{
+ if (!cpu_feature_enabled(X86_FEATURE_HYPERVISOR))
+ return false;
+
+ if (!hypervisor_is_type(X86_HYPER_MS_HYPERV))
+ return false;
+
+ return hv_get_isolation_type() != HV_ISOLATION_TYPE_NONE;
+}
+
+DEFINE_STATIC_KEY_FALSE(isolation_type_snp);
+
+/*
+ * hv_isolation_type_snp - Check system runs in the AMD SEV-SNP based
+ * isolation VM.
+ */
+bool hv_isolation_type_snp(void)
+{
+ return static_branch_unlikely(&isolation_type_snp);
+}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 66+ messages in thread* [Patch v3 04/14] Drivers: hv: Explicitly request decrypted in vmap_pfn() calls
2022-11-16 18:41 [Patch v3 00/14] Add PCI pass-thru support to Hyper-V Confidential VMs Michael Kelley
` (2 preceding siblings ...)
2022-11-16 18:41 ` [Patch v3 03/14] x86/hyperv: Reorder code in prep for subsequent patch Michael Kelley
@ 2022-11-16 18:41 ` Michael Kelley
2022-11-16 18:41 ` [Patch v3 05/14] x86/mm: Handle decryption/re-encryption of bss_decrypted consistently Michael Kelley
` (9 subsequent siblings)
13 siblings, 0 replies; 66+ messages in thread
From: Michael Kelley @ 2022-11-16 18:41 UTC (permalink / raw)
To: hpa, kys, haiyangz, wei.liu, decui, luto, peterz, davem, edumazet,
kuba, pabeni, lpieralisi, robh, kw, bhelgaas, arnd, hch,
m.szyprowski, robin.murphy, thomas.lendacky, brijesh.singh, tglx,
mingo, bp, dave.hansen, Tianyu.Lan, kirill.shutemov,
sathyanarayanan.kuppuswamy, ak, isaku.yamahata, dan.j.williams,
jane.chu, seanjc, tony.luck, x86, linux-kernel, linux-hyperv,
netdev, linux-pci, linux-arch, iommu
Cc: mikelley
In preparation for a subsequent patch, update vmap_pfn() calls to
explicitly request that the mapping be for decrypted access to
the memory. There's no change in functionality since the PFNs
passed to vmap_pfn() are above the shared_gpa_boundary, implicitly
producing a decrypted mapping. But explicitly requesting decrypted
allows the code to work before and after a subsequent patch
that will cause vmap_pfn() to mask the PFNs to being below the
shared_gpa_boundary. While another subsesquent patch removes the
vmap_pfn() calls entirely, this temporary tweak avoids the need
for a large patch that makes all the changes at once.
Signed-off-by: Michael Kelley <mikelley@microsoft.com>
Reviewed-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
arch/x86/hyperv/ivm.c | 2 +-
drivers/hv/ring_buffer.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/hyperv/ivm.c b/arch/x86/hyperv/ivm.c
index f33c67e..e8be4c2 100644
--- a/arch/x86/hyperv/ivm.c
+++ b/arch/x86/hyperv/ivm.c
@@ -343,7 +343,7 @@ void *hv_map_memory(void *addr, unsigned long size)
pfns[i] = vmalloc_to_pfn(addr + i * PAGE_SIZE) +
(ms_hyperv.shared_gpa_boundary >> PAGE_SHIFT);
- vaddr = vmap_pfn(pfns, size / PAGE_SIZE, PAGE_KERNEL_IO);
+ vaddr = vmap_pfn(pfns, size / PAGE_SIZE, pgprot_decrypted(PAGE_KERNEL_NOENC));
kfree(pfns);
return vaddr;
diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index 59a4aa8..b4a91b1 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -211,7 +211,7 @@ int hv_ringbuffer_init(struct hv_ring_buffer_info *ring_info,
ring_info->ring_buffer = (struct hv_ring_buffer *)
vmap_pfn(pfns_wraparound, page_cnt * 2 - 1,
- PAGE_KERNEL);
+ pgprot_decrypted(PAGE_KERNEL_NOENC));
kfree(pfns_wraparound);
if (!ring_info->ring_buffer)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 66+ messages in thread* [Patch v3 05/14] x86/mm: Handle decryption/re-encryption of bss_decrypted consistently
2022-11-16 18:41 [Patch v3 00/14] Add PCI pass-thru support to Hyper-V Confidential VMs Michael Kelley
` (3 preceding siblings ...)
2022-11-16 18:41 ` [Patch v3 04/14] Drivers: hv: Explicitly request decrypted in vmap_pfn() calls Michael Kelley
@ 2022-11-16 18:41 ` Michael Kelley
2022-11-16 20:35 ` Tom Lendacky
` (2 more replies)
2022-11-16 18:41 ` [Patch v3 06/14] init: Call mem_encrypt_init() after Hyper-V hypercall init is done Michael Kelley
` (8 subsequent siblings)
13 siblings, 3 replies; 66+ messages in thread
From: Michael Kelley @ 2022-11-16 18:41 UTC (permalink / raw)
To: hpa, kys, haiyangz, wei.liu, decui, luto, peterz, davem, edumazet,
kuba, pabeni, lpieralisi, robh, kw, bhelgaas, arnd, hch,
m.szyprowski, robin.murphy, thomas.lendacky, brijesh.singh, tglx,
mingo, bp, dave.hansen, Tianyu.Lan, kirill.shutemov,
sathyanarayanan.kuppuswamy, ak, isaku.yamahata, dan.j.williams,
jane.chu, seanjc, tony.luck, x86, linux-kernel, linux-hyperv,
netdev, linux-pci, linux-arch, iommu
Cc: mikelley
Current code in sme_postprocess_startup() decrypts the bss_decrypted
section when sme_me_mask is non-zero. But code in
mem_encrypt_free_decrytped_mem() re-encrypts the unused portion based
on CC_ATTR_MEM_ENCRYPT. In a Hyper-V guest VM using vTOM, these
conditions are not equivalent as sme_me_mask is always zero when
using vTOM. Consequently, mem_encrypt_free_decrypted_mem() attempts
to re-encrypt memory that was never decrypted.
Fix this in mem_encrypt_free_decrypted_mem() by conditioning the
re-encryption on the same test for non-zero sme_me_mask. Hyper-V
guests using vTOM don't need the bss_decrypted section to be
decrypted, so skipping the decryption/re-encryption doesn't cause
a problem.
Signed-off-by: Michael Kelley <mikelley@microsoft.com>
---
arch/x86/mm/mem_encrypt_amd.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/arch/x86/mm/mem_encrypt_amd.c b/arch/x86/mm/mem_encrypt_amd.c
index 9c4d8db..5a51343 100644
--- a/arch/x86/mm/mem_encrypt_amd.c
+++ b/arch/x86/mm/mem_encrypt_amd.c
@@ -513,10 +513,14 @@ void __init mem_encrypt_free_decrypted_mem(void)
npages = (vaddr_end - vaddr) >> PAGE_SHIFT;
/*
- * The unused memory range was mapped decrypted, change the encryption
- * attribute from decrypted to encrypted before freeing it.
+ * If the unused memory range was mapped decrypted, change the encryption
+ * attribute from decrypted to encrypted before freeing it. Base the
+ * re-encryption on the same condition used for the decryption in
+ * sme_postprocess_startup(). Higher level abstractions, such as
+ * CC_ATTR_MEM_ENCRYPT, aren't necessarily equivalent in a Hyper-V VM
+ * using vTOM, where sme_me_mask is always zero.
*/
- if (cc_platform_has(CC_ATTR_MEM_ENCRYPT)) {
+ if (sme_get_me_mask()) {
r = set_memory_encrypted(vaddr, npages);
if (r) {
pr_warn("failed to free unused decrypted pages\n");
--
1.8.3.1
^ permalink raw reply related [flat|nested] 66+ messages in thread* Re: [Patch v3 05/14] x86/mm: Handle decryption/re-encryption of bss_decrypted consistently
2022-11-16 18:41 ` [Patch v3 05/14] x86/mm: Handle decryption/re-encryption of bss_decrypted consistently Michael Kelley
@ 2022-11-16 20:35 ` Tom Lendacky
2022-11-16 21:15 ` Tom Lendacky
2022-11-17 21:47 ` Sathyanarayanan Kuppuswamy
2022-11-21 14:40 ` Borislav Petkov
2 siblings, 1 reply; 66+ messages in thread
From: Tom Lendacky @ 2022-11-16 20:35 UTC (permalink / raw)
To: Michael Kelley, hpa, kys, haiyangz, wei.liu, decui, luto, peterz,
davem, edumazet, kuba, pabeni, lpieralisi, robh, kw, bhelgaas,
arnd, hch, m.szyprowski, robin.murphy, brijesh.singh, tglx, mingo,
bp, dave.hansen, Tianyu.Lan, kirill.shutemov,
sathyanarayanan.kuppuswamy, ak, isaku.yamahata, dan.j.williams,
jane.chu, seanjc, tony.luck, x86, linux-kernel, linux-hyperv,
netdev, linux-pci, linux-arch, iommu
On 11/16/22 12:41, Michael Kelley wrote:
> Current code in sme_postprocess_startup() decrypts the bss_decrypted
> section when sme_me_mask is non-zero. But code in
> mem_encrypt_free_decrytped_mem() re-encrypts the unused portion based
> on CC_ATTR_MEM_ENCRYPT. In a Hyper-V guest VM using vTOM, these
> conditions are not equivalent as sme_me_mask is always zero when
> using vTOM. Consequently, mem_encrypt_free_decrypted_mem() attempts
> to re-encrypt memory that was never decrypted.
>
> Fix this in mem_encrypt_free_decrypted_mem() by conditioning the
> re-encryption on the same test for non-zero sme_me_mask. Hyper-V
> guests using vTOM don't need the bss_decrypted section to be
> decrypted, so skipping the decryption/re-encryption doesn't cause
> a problem.
>
> Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> ---
> arch/x86/mm/mem_encrypt_amd.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/mm/mem_encrypt_amd.c b/arch/x86/mm/mem_encrypt_amd.c
> index 9c4d8db..5a51343 100644
> --- a/arch/x86/mm/mem_encrypt_amd.c
> +++ b/arch/x86/mm/mem_encrypt_amd.c
> @@ -513,10 +513,14 @@ void __init mem_encrypt_free_decrypted_mem(void)
> npages = (vaddr_end - vaddr) >> PAGE_SHIFT;
>
> /*
> - * The unused memory range was mapped decrypted, change the encryption
> - * attribute from decrypted to encrypted before freeing it.
> + * If the unused memory range was mapped decrypted, change the encryption
> + * attribute from decrypted to encrypted before freeing it. Base the
> + * re-encryption on the same condition used for the decryption in
> + * sme_postprocess_startup(). Higher level abstractions, such as
> + * CC_ATTR_MEM_ENCRYPT, aren't necessarily equivalent in a Hyper-V VM
> + * using vTOM, where sme_me_mask is always zero.
> */
> - if (cc_platform_has(CC_ATTR_MEM_ENCRYPT)) {
> + if (sme_get_me_mask()) {
To be consistent within this file, you should use sme_me_mask directly.
Thanks,
Tom
> r = set_memory_encrypted(vaddr, npages);
> if (r) {
> pr_warn("failed to free unused decrypted pages\n");
^ permalink raw reply [flat|nested] 66+ messages in thread* Re: [Patch v3 05/14] x86/mm: Handle decryption/re-encryption of bss_decrypted consistently
2022-11-16 20:35 ` Tom Lendacky
@ 2022-11-16 21:15 ` Tom Lendacky
2022-11-18 2:59 ` Michael Kelley (LINUX)
0 siblings, 1 reply; 66+ messages in thread
From: Tom Lendacky @ 2022-11-16 21:15 UTC (permalink / raw)
To: Michael Kelley, hpa, kys, haiyangz, wei.liu, decui, luto, peterz,
davem, edumazet, kuba, pabeni, lpieralisi, robh, kw, bhelgaas,
arnd, hch, m.szyprowski, robin.murphy, brijesh.singh, tglx, mingo,
bp, dave.hansen, Tianyu.Lan, kirill.shutemov,
sathyanarayanan.kuppuswamy, ak, isaku.yamahata, dan.j.williams,
jane.chu, seanjc, tony.luck, x86, linux-kernel, linux-hyperv,
netdev, linux-pci, linux-arch, iommu
On 11/16/22 14:35, Tom Lendacky wrote:
> On 11/16/22 12:41, Michael Kelley wrote:
>> Current code in sme_postprocess_startup() decrypts the bss_decrypted
>> section when sme_me_mask is non-zero. But code in
>> mem_encrypt_free_decrytped_mem() re-encrypts the unused portion based
>> on CC_ATTR_MEM_ENCRYPT. In a Hyper-V guest VM using vTOM, these
>> conditions are not equivalent as sme_me_mask is always zero when
>> using vTOM. Consequently, mem_encrypt_free_decrypted_mem() attempts
>> to re-encrypt memory that was never decrypted.
>>
>> Fix this in mem_encrypt_free_decrypted_mem() by conditioning the
>> re-encryption on the same test for non-zero sme_me_mask. Hyper-V
>> guests using vTOM don't need the bss_decrypted section to be
>> decrypted, so skipping the decryption/re-encryption doesn't cause
>> a problem.
>>
>> Signed-off-by: Michael Kelley <mikelley@microsoft.com>
Meant to add this in the previous reply...
With the change to use sme_me_mask directly
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
>> ---
>> arch/x86/mm/mem_encrypt_amd.c | 10 +++++++---
>> 1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/mm/mem_encrypt_amd.c b/arch/x86/mm/mem_encrypt_amd.c
>> index 9c4d8db..5a51343 100644
>> --- a/arch/x86/mm/mem_encrypt_amd.c
>> +++ b/arch/x86/mm/mem_encrypt_amd.c
>> @@ -513,10 +513,14 @@ void __init mem_encrypt_free_decrypted_mem(void)
>> npages = (vaddr_end - vaddr) >> PAGE_SHIFT;
>> /*
>> - * The unused memory range was mapped decrypted, change the encryption
>> - * attribute from decrypted to encrypted before freeing it.
>> + * If the unused memory range was mapped decrypted, change the
>> encryption
>> + * attribute from decrypted to encrypted before freeing it. Base the
>> + * re-encryption on the same condition used for the decryption in
>> + * sme_postprocess_startup(). Higher level abstractions, such as
>> + * CC_ATTR_MEM_ENCRYPT, aren't necessarily equivalent in a Hyper-V VM
>> + * using vTOM, where sme_me_mask is always zero.
>> */
>> - if (cc_platform_has(CC_ATTR_MEM_ENCRYPT)) {
>> + if (sme_get_me_mask()) {
>
> To be consistent within this file, you should use sme_me_mask directly.
>
> Thanks,
> Tom
>
>> r = set_memory_encrypted(vaddr, npages);
>> if (r) {
>> pr_warn("failed to free unused decrypted pages\n");
^ permalink raw reply [flat|nested] 66+ messages in thread* RE: [Patch v3 05/14] x86/mm: Handle decryption/re-encryption of bss_decrypted consistently
2022-11-16 21:15 ` Tom Lendacky
@ 2022-11-18 2:59 ` Michael Kelley (LINUX)
0 siblings, 0 replies; 66+ messages in thread
From: Michael Kelley (LINUX) @ 2022-11-18 2:59 UTC (permalink / raw)
To: Tom Lendacky, hpa@zytor.com, KY Srinivasan, Haiyang Zhang,
wei.liu@kernel.org, Dexuan Cui, luto@kernel.org,
peterz@infradead.org, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, pabeni@redhat.com, lpieralisi@kernel.org,
robh@kernel.org, kw@linux.com, bhelgaas@google.com, arnd@arndb.de,
hch@infradead.org, m.szyprowski@samsung.com, robin.murphy@arm.com,
brijesh.singh@amd.com, tglx@linutronix.de, mingo@redhat.com,
bp@alien8.de, dave.hansen@linux.intel.com, Tianyu Lan,
kirill.shutemov@linux.intel.com,
sathyanarayanan.kuppuswamy@linux.intel.com, ak@linux.intel.com,
isaku.yamahata@intel.com, Williams, Dan J, jane.chu@oracle.com,
seanjc@google.com, tony.luck@intel.com, x86@kernel.org,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org, linux-pci@vger.kernel.org,
linux-arch@vger.kernel.org, iommu@lists.linux.dev
From: Tom Lendacky <thomas.lendacky@amd.com> Sent: Wednesday, November 16, 2022 1:16 PM
>
> On 11/16/22 14:35, Tom Lendacky wrote:
> > On 11/16/22 12:41, Michael Kelley wrote:
> >> Current code in sme_postprocess_startup() decrypts the bss_decrypted
> >> section when sme_me_mask is non-zero. But code in
> >> mem_encrypt_free_decrytped_mem() re-encrypts the unused portion based
> >> on CC_ATTR_MEM_ENCRYPT. In a Hyper-V guest VM using vTOM, these
> >> conditions are not equivalent as sme_me_mask is always zero when
> >> using vTOM. Consequently, mem_encrypt_free_decrypted_mem() attempts
> >> to re-encrypt memory that was never decrypted.
> >>
> >> Fix this in mem_encrypt_free_decrypted_mem() by conditioning the
> >> re-encryption on the same test for non-zero sme_me_mask. Hyper-V
> >> guests using vTOM don't need the bss_decrypted section to be
> >> decrypted, so skipping the decryption/re-encryption doesn't cause
> >> a problem.
> >>
> >> Signed-off-by: Michael Kelley <mikelley@microsoft.com>
>
> Meant to add this in the previous reply...
>
> With the change to use sme_me_mask directly
>
> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
>
Thanks for the reviews. And I see your point about sme_me_mask. I had
not previously noticed that it is defined in the module, so no need to use
a getter function.
Michael
^ permalink raw reply [flat|nested] 66+ messages in thread
* Re: [Patch v3 05/14] x86/mm: Handle decryption/re-encryption of bss_decrypted consistently
2022-11-16 18:41 ` [Patch v3 05/14] x86/mm: Handle decryption/re-encryption of bss_decrypted consistently Michael Kelley
2022-11-16 20:35 ` Tom Lendacky
@ 2022-11-17 21:47 ` Sathyanarayanan Kuppuswamy
2022-11-18 2:55 ` Michael Kelley (LINUX)
2022-11-21 14:40 ` Borislav Petkov
2 siblings, 1 reply; 66+ messages in thread
From: Sathyanarayanan Kuppuswamy @ 2022-11-17 21:47 UTC (permalink / raw)
To: Michael Kelley, hpa, kys, haiyangz, wei.liu, decui, luto, peterz,
davem, edumazet, kuba, pabeni, lpieralisi, robh, kw, bhelgaas,
arnd, hch, m.szyprowski, robin.murphy, thomas.lendacky,
brijesh.singh, tglx, mingo, bp, dave.hansen, Tianyu.Lan,
kirill.shutemov, ak, isaku.yamahata, dan.j.williams, jane.chu,
seanjc, tony.luck, x86, linux-kernel, linux-hyperv, netdev,
linux-pci, linux-arch, iommu
On 11/16/22 10:41 AM, Michael Kelley wrote:
> Current code in sme_postprocess_startup() decrypts the bss_decrypted
> section when sme_me_mask is non-zero. But code in
> mem_encrypt_free_decrytped_mem() re-encrypts the unused portion based
> on CC_ATTR_MEM_ENCRYPT. In a Hyper-V guest VM using vTOM, these
> conditions are not equivalent as sme_me_mask is always zero when
> using vTOM. Consequently, mem_encrypt_free_decrypted_mem() attempts
> to re-encrypt memory that was never decrypted.
>
> Fix this in mem_encrypt_free_decrypted_mem() by conditioning the
> re-encryption on the same test for non-zero sme_me_mask. Hyper-V
> guests using vTOM don't need the bss_decrypted section to be
> decrypted, so skipping the decryption/re-encryption doesn't cause
> a problem.
>
Do you think it needs Fixes tag?
> Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> ---
> arch/x86/mm/mem_encrypt_amd.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/mm/mem_encrypt_amd.c b/arch/x86/mm/mem_encrypt_amd.c
> index 9c4d8db..5a51343 100644
> --- a/arch/x86/mm/mem_encrypt_amd.c
> +++ b/arch/x86/mm/mem_encrypt_amd.c
> @@ -513,10 +513,14 @@ void __init mem_encrypt_free_decrypted_mem(void)
> npages = (vaddr_end - vaddr) >> PAGE_SHIFT;
>
> /*
> - * The unused memory range was mapped decrypted, change the encryption
> - * attribute from decrypted to encrypted before freeing it.
> + * If the unused memory range was mapped decrypted, change the encryption
> + * attribute from decrypted to encrypted before freeing it. Base the
> + * re-encryption on the same condition used for the decryption in
> + * sme_postprocess_startup(). Higher level abstractions, such as
> + * CC_ATTR_MEM_ENCRYPT, aren't necessarily equivalent in a Hyper-V VM
> + * using vTOM, where sme_me_mask is always zero.
> */
> - if (cc_platform_has(CC_ATTR_MEM_ENCRYPT)) {
> + if (sme_get_me_mask()) {
> r = set_memory_encrypted(vaddr, npages);
> if (r) {
> pr_warn("failed to free unused decrypted pages\n");
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
^ permalink raw reply [flat|nested] 66+ messages in thread* RE: [Patch v3 05/14] x86/mm: Handle decryption/re-encryption of bss_decrypted consistently
2022-11-17 21:47 ` Sathyanarayanan Kuppuswamy
@ 2022-11-18 2:55 ` Michael Kelley (LINUX)
2022-11-21 14:39 ` Borislav Petkov
2022-11-28 2:52 ` Dexuan Cui
0 siblings, 2 replies; 66+ messages in thread
From: Michael Kelley (LINUX) @ 2022-11-18 2:55 UTC (permalink / raw)
To: Sathyanarayanan Kuppuswamy, hpa@zytor.com, KY Srinivasan,
Haiyang Zhang, wei.liu@kernel.org, Dexuan Cui, luto@kernel.org,
peterz@infradead.org, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, pabeni@redhat.com, lpieralisi@kernel.org,
robh@kernel.org, kw@linux.com, bhelgaas@google.com, arnd@arndb.de,
hch@infradead.org, m.szyprowski@samsung.com, robin.murphy@arm.com,
thomas.lendacky@amd.com, brijesh.singh@amd.com,
tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
dave.hansen@linux.intel.com, Tianyu Lan,
kirill.shutemov@linux.intel.com, ak@linux.intel.com,
isaku.yamahata@intel.com, Williams, Dan J, jane.chu@oracle.com,
seanjc@google.com, tony.luck@intel.com, x86@kernel.org,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org, linux-pci@vger.kernel.org,
linux-arch@vger.kernel.org, iommu@lists.linux.dev
From: Sathyanarayanan Kuppuswamy <sathyanarayanan.kuppuswamy@linux.intel.com>
>
> On 11/16/22 10:41 AM, Michael Kelley wrote:
> > Current code in sme_postprocess_startup() decrypts the bss_decrypted
> > section when sme_me_mask is non-zero. But code in
> > mem_encrypt_free_decrytped_mem() re-encrypts the unused portion based
> > on CC_ATTR_MEM_ENCRYPT. In a Hyper-V guest VM using vTOM, these
> > conditions are not equivalent as sme_me_mask is always zero when
> > using vTOM. Consequently, mem_encrypt_free_decrypted_mem() attempts
> > to re-encrypt memory that was never decrypted.
> >
> > Fix this in mem_encrypt_free_decrypted_mem() by conditioning the
> > re-encryption on the same test for non-zero sme_me_mask. Hyper-V
> > guests using vTOM don't need the bss_decrypted section to be
> > decrypted, so skipping the decryption/re-encryption doesn't cause
> > a problem.
> >
>
> Do you think it needs Fixes tag?
>
At least for my purposes, it doesn't. The original assumption that non-zero
sme_me_mask and CC_ATTR_MEM_ENCRYPT are equivalent was valid until
this patch series where Hyper-V guests are reporting CC_ATTR_MEM_ENCRYPT
as "true" but sme_me_mask is zero. This patch series won't be backported,
so the old assumption remains valid for older kernels. There's no benefit in
backporting the change.
But I had not thought about TDX. In the TDX case, it appears that
sme_postprocess_startup() will not decrypt the bss_decrypted section.
The corresponding mem_encrypt_free_decrypted_mem() is a no-op unless
CONFIG_AMD_MEM_ENCRYPT is set. But maybe if someone builds a
kernel image that supports both TDX and AMD encryption, it could break
at runtime on a TDX system. I would also note that on a TDX system
without CONFIG_AMD_MEM_ENCRYPT, the unused memory in the
bss_decrypted section never gets freed.
But check my logic. :-) I'm not averse to adding the Fixes: tag if there's a
scenario for TDX where doing the backport will solve a real problem.
And thanks for reviewing the code!
Michael
^ permalink raw reply [flat|nested] 66+ messages in thread
* Re: [Patch v3 05/14] x86/mm: Handle decryption/re-encryption of bss_decrypted consistently
2022-11-18 2:55 ` Michael Kelley (LINUX)
@ 2022-11-21 14:39 ` Borislav Petkov
2022-11-21 22:06 ` Sathyanarayanan Kuppuswamy
2022-11-28 2:52 ` Dexuan Cui
1 sibling, 1 reply; 66+ messages in thread
From: Borislav Petkov @ 2022-11-21 14:39 UTC (permalink / raw)
To: Michael Kelley (LINUX)
Cc: Sathyanarayanan Kuppuswamy, hpa@zytor.com, KY Srinivasan,
Haiyang Zhang, wei.liu@kernel.org, Dexuan Cui, luto@kernel.org,
peterz@infradead.org, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, pabeni@redhat.com, lpieralisi@kernel.org,
robh@kernel.org, kw@linux.com, bhelgaas@google.com, arnd@arndb.de,
hch@infradead.org, m.szyprowski@samsung.com, robin.murphy@arm.com,
thomas.lendacky@amd.com, brijesh.singh@amd.com,
tglx@linutronix.de, mingo@redhat.com, dave.hansen@linux.intel.com,
Tianyu Lan, kirill.shutemov@linux.intel.com, ak@linux.intel.com,
isaku.yamahata@intel.com, Williams, Dan J, jane.chu@oracle.com,
seanjc@google.com, tony.luck@intel.com, x86@kernel.org,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org, linux-pci@vger.kernel.org,
linux-arch@vger.kernel.org, iommu@lists.linux.dev
On Fri, Nov 18, 2022 at 02:55:32AM +0000, Michael Kelley (LINUX) wrote:
> But I had not thought about TDX. In the TDX case, it appears that
> sme_postprocess_startup() will not decrypt the bss_decrypted section.
> The corresponding mem_encrypt_free_decrypted_mem() is a no-op unless
> CONFIG_AMD_MEM_ENCRYPT is set. But maybe if someone builds a
> kernel image that supports both TDX and AMD encryption, it could break
sme_me_mask better be 0 on a kernel with both built in and running as a
TDX guest.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 66+ messages in thread* Re: [Patch v3 05/14] x86/mm: Handle decryption/re-encryption of bss_decrypted consistently
2022-11-21 14:39 ` Borislav Petkov
@ 2022-11-21 22:06 ` Sathyanarayanan Kuppuswamy
2022-11-22 17:59 ` Michael Kelley (LINUX)
0 siblings, 1 reply; 66+ messages in thread
From: Sathyanarayanan Kuppuswamy @ 2022-11-21 22:06 UTC (permalink / raw)
To: Borislav Petkov, Michael Kelley (LINUX)
Cc: hpa@zytor.com, KY Srinivasan, Haiyang Zhang, wei.liu@kernel.org,
Dexuan Cui, luto@kernel.org, peterz@infradead.org,
davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, lpieralisi@kernel.org, robh@kernel.org,
kw@linux.com, bhelgaas@google.com, arnd@arndb.de,
hch@infradead.org, m.szyprowski@samsung.com, robin.murphy@arm.com,
thomas.lendacky@amd.com, brijesh.singh@amd.com,
tglx@linutronix.de, mingo@redhat.com, dave.hansen@linux.intel.com,
Tianyu Lan, kirill.shutemov@linux.intel.com, ak@linux.intel.com,
isaku.yamahata@intel.com, Williams, Dan J, jane.chu@oracle.com,
seanjc@google.com, tony.luck@intel.com, x86@kernel.org,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org, linux-pci@vger.kernel.org,
linux-arch@vger.kernel.org, iommu@lists.linux.dev
On 11/21/22 6:39 AM, Borislav Petkov wrote:
> On Fri, Nov 18, 2022 at 02:55:32AM +0000, Michael Kelley (LINUX) wrote:
>> But I had not thought about TDX. In the TDX case, it appears that
>> sme_postprocess_startup() will not decrypt the bss_decrypted section.
>> The corresponding mem_encrypt_free_decrypted_mem() is a no-op unless
>> CONFIG_AMD_MEM_ENCRYPT is set. But maybe if someone builds a
>> kernel image that supports both TDX and AMD encryption, it could break
>
> sme_me_mask better be 0 on a kernel with both built in and running as a
> TDX guest.
>
Yes. It will be 0 in TDX. In sme_enable(), AMD code checks for CPUID
support before updating the sme_me_mask.
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
^ permalink raw reply [flat|nested] 66+ messages in thread
* RE: [Patch v3 05/14] x86/mm: Handle decryption/re-encryption of bss_decrypted consistently
2022-11-21 22:06 ` Sathyanarayanan Kuppuswamy
@ 2022-11-22 17:59 ` Michael Kelley (LINUX)
2022-11-28 10:50 ` Borislav Petkov
0 siblings, 1 reply; 66+ messages in thread
From: Michael Kelley (LINUX) @ 2022-11-22 17:59 UTC (permalink / raw)
To: Sathyanarayanan Kuppuswamy, Borislav Petkov
Cc: hpa@zytor.com, KY Srinivasan, Haiyang Zhang, wei.liu@kernel.org,
Dexuan Cui, luto@kernel.org, peterz@infradead.org,
davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, lpieralisi@kernel.org, robh@kernel.org,
kw@linux.com, bhelgaas@google.com, arnd@arndb.de,
hch@infradead.org, m.szyprowski@samsung.com, robin.murphy@arm.com,
thomas.lendacky@amd.com, brijesh.singh@amd.com,
tglx@linutronix.de, mingo@redhat.com, dave.hansen@linux.intel.com,
Tianyu Lan, kirill.shutemov@linux.intel.com, ak@linux.intel.com,
isaku.yamahata@intel.com, Williams, Dan J, jane.chu@oracle.com,
seanjc@google.com, tony.luck@intel.com, x86@kernel.org,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org, linux-pci@vger.kernel.org,
linux-arch@vger.kernel.org, iommu@lists.linux.dev
From: Sathyanarayanan Kuppuswamy <sathyanarayanan.kuppuswamy@linux.intel.com>
>
> On 11/21/22 6:39 AM, Borislav Petkov wrote:
> > On Fri, Nov 18, 2022 at 02:55:32AM +0000, Michael Kelley (LINUX) wrote:
> >> But I had not thought about TDX. In the TDX case, it appears that
> >> sme_postprocess_startup() will not decrypt the bss_decrypted section.
> >> The corresponding mem_encrypt_free_decrypted_mem() is a no-op unless
> >> CONFIG_AMD_MEM_ENCRYPT is set. But maybe if someone builds a
> >> kernel image that supports both TDX and AMD encryption, it could break
> >
> > sme_me_mask better be 0 on a kernel with both built in and running as a
> > TDX guest.
> >
>
> Yes. It will be 0 in TDX. In sme_enable(), AMD code checks for CPUID
> support before updating the sme_me_mask.
>
Right. But here's my point: With current code and an image built with
CONFIG_AMD_MEM_ENCRYPT=y and running as a TDX guest,
sme_postprocess_startup() will not decrypt the bss_decrypted section.
Then later mem_encrypt_free_decrypted_mem() will run, see that
CC_ATTR_MEM_ENCRYPT is true, and try to re-encrypt the memory.
In other words, a TDX guest would break in the same way as a Hyper-V
vTOM guest would break. This patch fixes the problem for both cases.
The only things I see in the bss_decrypted section are two clock structures
In arch/x86/kernel/kvmclock.c, which aren't needed when Hyper-V is the
hypervisor. But with a TDX guest on KVM, will *not* decrypting the
bss_decrypted section be a problem? I don't know that kvmclock
code or why the two clock structures need to be decrypted for
AMD mem encryption.
Michael
^ permalink raw reply [flat|nested] 66+ messages in thread
* Re: [Patch v3 05/14] x86/mm: Handle decryption/re-encryption of bss_decrypted consistently
2022-11-22 17:59 ` Michael Kelley (LINUX)
@ 2022-11-28 10:50 ` Borislav Petkov
0 siblings, 0 replies; 66+ messages in thread
From: Borislav Petkov @ 2022-11-28 10:50 UTC (permalink / raw)
To: Michael Kelley (LINUX)
Cc: Sathyanarayanan Kuppuswamy, hpa@zytor.com, KY Srinivasan,
Haiyang Zhang, wei.liu@kernel.org, Dexuan Cui, luto@kernel.org,
peterz@infradead.org, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, pabeni@redhat.com, lpieralisi@kernel.org,
robh@kernel.org, kw@linux.com, bhelgaas@google.com, arnd@arndb.de,
hch@infradead.org, m.szyprowski@samsung.com, robin.murphy@arm.com,
thomas.lendacky@amd.com, brijesh.singh@amd.com,
tglx@linutronix.de, mingo@redhat.com, dave.hansen@linux.intel.com,
Tianyu Lan, kirill.shutemov@linux.intel.com, ak@linux.intel.com,
isaku.yamahata@intel.com, Williams, Dan J, jane.chu@oracle.com,
seanjc@google.com, tony.luck@intel.com, x86@kernel.org,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org, linux-pci@vger.kernel.org,
linux-arch@vger.kernel.org, iommu@lists.linux.dev
On Tue, Nov 22, 2022 at 05:59:04PM +0000, Michael Kelley (LINUX) wrote:
> Right. But here's my point: With current code and an image built with
> CONFIG_AMD_MEM_ENCRYPT=y and running as a TDX guest,
> sme_postprocess_startup() will not decrypt the bss_decrypted section.
> Then later mem_encrypt_free_decrypted_mem() will run, see that
> CC_ATTR_MEM_ENCRYPT is true, and try to re-encrypt the memory.
> In other words, a TDX guest would break in the same way as a Hyper-V
> vTOM guest would break. This patch fixes the problem for both cases.
I guess making the check more concrete by checking sme_me_mask directly
along with a comment makes sense.
We need to be very careful here not to fragment the code too much for
all the different guest types.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 66+ messages in thread
* RE: [Patch v3 05/14] x86/mm: Handle decryption/re-encryption of bss_decrypted consistently
2022-11-18 2:55 ` Michael Kelley (LINUX)
2022-11-21 14:39 ` Borislav Petkov
@ 2022-11-28 2:52 ` Dexuan Cui
2022-11-28 14:15 ` Tom Lendacky
1 sibling, 1 reply; 66+ messages in thread
From: Dexuan Cui @ 2022-11-28 2:52 UTC (permalink / raw)
To: Michael Kelley (LINUX), Sathyanarayanan Kuppuswamy, hpa@zytor.com,
KY Srinivasan, Haiyang Zhang, wei.liu@kernel.org, luto@kernel.org,
peterz@infradead.org, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, pabeni@redhat.com, lpieralisi@kernel.org,
robh@kernel.org, kw@linux.com, bhelgaas@google.com, arnd@arndb.de,
hch@infradead.org, m.szyprowski@samsung.com, robin.murphy@arm.com,
thomas.lendacky@amd.com, brijesh.singh@amd.com,
tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
dave.hansen@linux.intel.com, Tianyu Lan,
kirill.shutemov@linux.intel.com, ak@linux.intel.com,
isaku.yamahata@intel.com, Williams, Dan J, jane.chu@oracle.com,
seanjc@google.com, tony.luck@intel.com, x86@kernel.org,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org, linux-pci@vger.kernel.org,
linux-arch@vger.kernel.org, iommu@lists.linux.dev
> From: Michael Kelley (LINUX) <mikelley@microsoft.com>
> Sent: Thursday, November 17, 2022 6:56 PM
> [...]
>
> But I had not thought about TDX. In the TDX case, it appears that
> sme_postprocess_startup() will not decrypt the bss_decrypted section.
> The corresponding mem_encrypt_free_decrypted_mem() is a no-op unless
> CONFIG_AMD_MEM_ENCRYPT is set. But maybe if someone builds a
> kernel image that supports both TDX and AMD encryption, it could break
> at runtime on a TDX system. I would also note that on a TDX system
> without CONFIG_AMD_MEM_ENCRYPT, the unused memory in the
> bss_decrypted section never gets freed.
On a TDX system *with* CONFIG_AMD_MEM_ENCRYPT, the unused
memory in the bss_decrypted section also never gets freed due to the
below "return;"
I'd suggest a Fixes tag should be added to make sure the distro vendors
notice the patch and backport it :-)
BTW, I just posted a similar patch as I didn't notice this patch. I have
replied to my patch email, asking people to ignore my patch.
Fixes: b3f0907c71e0 ("x86/mm: Add .bss..decrypted section to hold shared variables")
void __init mem_encrypt_free_decrypted_mem(void)
{
unsigned long vaddr, vaddr_end, npages;
int r;
vaddr = (unsigned long)__start_bss_decrypted_unused;
vaddr_end = (unsigned long)__end_bss_decrypted;
npages = (vaddr_end - vaddr) >> PAGE_SHIFT;
/*
* The unused memory range was mapped decrypted, change the encryption
* attribute from decrypted to encrypted before freeing it.
*/
if (cc_platform_has(CC_ATTR_MEM_ENCRYPT)) {
r = set_memory_encrypted(vaddr, npages);
if (r) {
pr_warn("failed to free unused decrypted pages\n");
return;
}
}
free_init_pages("unused decrypted", vaddr, vaddr_end);
}
^ permalink raw reply [flat|nested] 66+ messages in thread* Re: [Patch v3 05/14] x86/mm: Handle decryption/re-encryption of bss_decrypted consistently
2022-11-28 2:52 ` Dexuan Cui
@ 2022-11-28 14:15 ` Tom Lendacky
2022-11-28 18:06 ` Dexuan Cui
0 siblings, 1 reply; 66+ messages in thread
From: Tom Lendacky @ 2022-11-28 14:15 UTC (permalink / raw)
To: Dexuan Cui, Michael Kelley (LINUX), Sathyanarayanan Kuppuswamy,
hpa@zytor.com, KY Srinivasan, Haiyang Zhang, wei.liu@kernel.org,
luto@kernel.org, peterz@infradead.org, davem@davemloft.net,
edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
lpieralisi@kernel.org, robh@kernel.org, kw@linux.com,
bhelgaas@google.com, arnd@arndb.de, hch@infradead.org,
m.szyprowski@samsung.com, robin.murphy@arm.com,
brijesh.singh@amd.com, tglx@linutronix.de, mingo@redhat.com,
bp@alien8.de, dave.hansen@linux.intel.com, Tianyu Lan,
kirill.shutemov@linux.intel.com, ak@linux.intel.com,
isaku.yamahata@intel.com, Williams, Dan J, jane.chu@oracle.com,
seanjc@google.com, tony.luck@intel.com, x86@kernel.org,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org, linux-pci@vger.kernel.org,
linux-arch@vger.kernel.org, iommu@lists.linux.dev
On 11/27/22 20:52, Dexuan Cui wrote:
>> From: Michael Kelley (LINUX) <mikelley@microsoft.com>
>> Sent: Thursday, November 17, 2022 6:56 PM
>> [...]
>>
>> But I had not thought about TDX. In the TDX case, it appears that
>> sme_postprocess_startup() will not decrypt the bss_decrypted section.
>> The corresponding mem_encrypt_free_decrypted_mem() is a no-op unless
>> CONFIG_AMD_MEM_ENCRYPT is set. But maybe if someone builds a
>> kernel image that supports both TDX and AMD encryption, it could break
>> at runtime on a TDX system. I would also note that on a TDX system
>> without CONFIG_AMD_MEM_ENCRYPT, the unused memory in the
>> bss_decrypted section never gets freed.
>
> On a TDX system *with* CONFIG_AMD_MEM_ENCRYPT, the unused
> memory in the bss_decrypted section also never gets freed due to the
> below "return;"
>
> I'd suggest a Fixes tag should be added to make sure the distro vendors
> notice the patch and backport it :-)
>
> BTW, I just posted a similar patch as I didn't notice this patch. I have
> replied to my patch email, asking people to ignore my patch.
>
> Fixes: b3f0907c71e0 ("x86/mm: Add .bss..decrypted section to hold shared variables")
I think the Fixes: tag should really be:
e9d1d2bb75b2 ("treewide: Replace the use of mem_encrypt_active() with cc_platform_has()")
since mem_encrypt_active() used to return sme_me_mask, so the checks were
balanced at that point.
Thanks,
Tom
>
> void __init mem_encrypt_free_decrypted_mem(void)
> {
> unsigned long vaddr, vaddr_end, npages;
> int r;
>
> vaddr = (unsigned long)__start_bss_decrypted_unused;
> vaddr_end = (unsigned long)__end_bss_decrypted;
> npages = (vaddr_end - vaddr) >> PAGE_SHIFT;
>
> /*
> * The unused memory range was mapped decrypted, change the encryption
> * attribute from decrypted to encrypted before freeing it.
> */
> if (cc_platform_has(CC_ATTR_MEM_ENCRYPT)) {
> r = set_memory_encrypted(vaddr, npages);
> if (r) {
> pr_warn("failed to free unused decrypted pages\n");
> return;
> }
> }
>
> free_init_pages("unused decrypted", vaddr, vaddr_end);
> }
>
^ permalink raw reply [flat|nested] 66+ messages in thread* RE: [Patch v3 05/14] x86/mm: Handle decryption/re-encryption of bss_decrypted consistently
2022-11-28 14:15 ` Tom Lendacky
@ 2022-11-28 18:06 ` Dexuan Cui
0 siblings, 0 replies; 66+ messages in thread
From: Dexuan Cui @ 2022-11-28 18:06 UTC (permalink / raw)
To: Tom Lendacky, Michael Kelley (LINUX), Sathyanarayanan Kuppuswamy,
hpa@zytor.com, KY Srinivasan, Haiyang Zhang, wei.liu@kernel.org,
luto@kernel.org, peterz@infradead.org, davem@davemloft.net,
edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
lpieralisi@kernel.org, robh@kernel.org, kw@linux.com,
bhelgaas@google.com, arnd@arndb.de, hch@infradead.org,
m.szyprowski@samsung.com, robin.murphy@arm.com,
brijesh.singh@amd.com, tglx@linutronix.de, mingo@redhat.com,
bp@alien8.de, dave.hansen@linux.intel.com, Tianyu Lan,
kirill.shutemov@linux.intel.com, ak@linux.intel.com,
isaku.yamahata@intel.com, Williams, Dan J, jane.chu@oracle.com,
seanjc@google.com, tony.luck@intel.com, x86@kernel.org,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org, linux-pci@vger.kernel.org,
linux-arch@vger.kernel.org, iommu@lists.linux.dev
> From: Tom Lendacky <thomas.lendacky@amd.com>
> Sent: Monday, November 28, 2022 6:16 AM
> > [...]
> > On a TDX system *with* CONFIG_AMD_MEM_ENCRYPT, the unused
> > memory in the bss_decrypted section also never gets freed due to the
> > below "return;"
> >
> > I'd suggest a Fixes tag should be added to make sure the distro vendors
> > notice the patch and backport it :-)
> > [...]
> > Fixes: b3f0907c71e0 ("x86/mm: Add .bss..decrypted section to hold shared
> variables")
>
> I think the Fixes: tag should really be:
>
> e9d1d2bb75b2 ("treewide: Replace the use of mem_encrypt_active() with
> cc_platform_has()")
Yes, you're correct.
^ permalink raw reply [flat|nested] 66+ messages in thread
* Re: [Patch v3 05/14] x86/mm: Handle decryption/re-encryption of bss_decrypted consistently
2022-11-16 18:41 ` [Patch v3 05/14] x86/mm: Handle decryption/re-encryption of bss_decrypted consistently Michael Kelley
2022-11-16 20:35 ` Tom Lendacky
2022-11-17 21:47 ` Sathyanarayanan Kuppuswamy
@ 2022-11-21 14:40 ` Borislav Petkov
2 siblings, 0 replies; 66+ messages in thread
From: Borislav Petkov @ 2022-11-21 14:40 UTC (permalink / raw)
To: Michael Kelley
Cc: hpa, kys, haiyangz, wei.liu, decui, luto, peterz, davem, edumazet,
kuba, pabeni, lpieralisi, robh, kw, bhelgaas, arnd, hch,
m.szyprowski, robin.murphy, thomas.lendacky, brijesh.singh, tglx,
mingo, dave.hansen, Tianyu.Lan, kirill.shutemov,
sathyanarayanan.kuppuswamy, ak, isaku.yamahata, dan.j.williams,
jane.chu, seanjc, tony.luck, x86, linux-kernel, linux-hyperv,
netdev, linux-pci, linux-arch, iommu
On Wed, Nov 16, 2022 at 10:41:28AM -0800, Michael Kelley wrote:
> Current code in sme_postprocess_startup() decrypts the bss_decrypted
> section when sme_me_mask is non-zero. But code in
> mem_encrypt_free_decrytped_mem() re-encrypts the unused portion based
^^
letters flipped.
> @@ -513,10 +513,14 @@ void __init mem_encrypt_free_decrypted_mem(void)
> npages = (vaddr_end - vaddr) >> PAGE_SHIFT;
>
> /*
> - * The unused memory range was mapped decrypted, change the encryption
> - * attribute from decrypted to encrypted before freeing it.
> + * If the unused memory range was mapped decrypted, change the encryption
> + * attribute from decrypted to encrypted before freeing it. Base the
> + * re-encryption on the same condition used for the decryption in
> + * sme_postprocess_startup(). Higher level abstractions, such as
> + * CC_ATTR_MEM_ENCRYPT, aren't necessarily equivalent in a Hyper-V VM
> + * using vTOM, where sme_me_mask is always zero.
Good, an example why one needs to pay attention here.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 66+ messages in thread
* [Patch v3 06/14] init: Call mem_encrypt_init() after Hyper-V hypercall init is done
2022-11-16 18:41 [Patch v3 00/14] Add PCI pass-thru support to Hyper-V Confidential VMs Michael Kelley
` (4 preceding siblings ...)
2022-11-16 18:41 ` [Patch v3 05/14] x86/mm: Handle decryption/re-encryption of bss_decrypted consistently Michael Kelley
@ 2022-11-16 18:41 ` Michael Kelley
2022-11-16 21:14 ` Tom Lendacky
2022-11-21 14:46 ` Borislav Petkov
2022-11-16 18:41 ` [Patch v3 07/14] x86/hyperv: Change vTOM handling to use standard coco mechanisms Michael Kelley
` (7 subsequent siblings)
13 siblings, 2 replies; 66+ messages in thread
From: Michael Kelley @ 2022-11-16 18:41 UTC (permalink / raw)
To: hpa, kys, haiyangz, wei.liu, decui, luto, peterz, davem, edumazet,
kuba, pabeni, lpieralisi, robh, kw, bhelgaas, arnd, hch,
m.szyprowski, robin.murphy, thomas.lendacky, brijesh.singh, tglx,
mingo, bp, dave.hansen, Tianyu.Lan, kirill.shutemov,
sathyanarayanan.kuppuswamy, ak, isaku.yamahata, dan.j.williams,
jane.chu, seanjc, tony.luck, x86, linux-kernel, linux-hyperv,
netdev, linux-pci, linux-arch, iommu
Cc: mikelley
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.
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>
---
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
--
1.8.3.1
^ permalink raw reply related [flat|nested] 66+ messages in thread* Re: [Patch v3 06/14] init: Call mem_encrypt_init() after Hyper-V hypercall init is done
2022-11-16 18:41 ` [Patch v3 06/14] init: Call mem_encrypt_init() after Hyper-V hypercall init is done Michael Kelley
@ 2022-11-16 21:14 ` Tom Lendacky
2022-11-21 14:46 ` Borislav Petkov
1 sibling, 0 replies; 66+ messages in thread
From: Tom Lendacky @ 2022-11-16 21:14 UTC (permalink / raw)
To: Michael Kelley, hpa, kys, haiyangz, wei.liu, decui, luto, peterz,
davem, edumazet, kuba, pabeni, lpieralisi, robh, kw, bhelgaas,
arnd, hch, m.szyprowski, robin.murphy, brijesh.singh, tglx, mingo,
bp, dave.hansen, Tianyu.Lan, kirill.shutemov,
sathyanarayanan.kuppuswamy, ak, isaku.yamahata, dan.j.williams,
jane.chu, seanjc, tony.luck, x86, linux-kernel, linux-hyperv,
netdev, linux-pci, linux-arch, iommu
On 11/16/22 12:41, 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.
>
> 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>
Some quick testing with mem_encrypt_init() in the new location hasn't
shown any problems under SME/SEV.
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
^ permalink raw reply [flat|nested] 66+ messages in thread* Re: [Patch v3 06/14] init: Call mem_encrypt_init() after Hyper-V hypercall init is done
2022-11-16 18:41 ` [Patch v3 06/14] init: Call mem_encrypt_init() after Hyper-V hypercall init is done Michael Kelley
2022-11-16 21:14 ` Tom Lendacky
@ 2022-11-21 14:46 ` Borislav Petkov
1 sibling, 0 replies; 66+ messages in thread
From: Borislav Petkov @ 2022-11-21 14:46 UTC (permalink / raw)
To: Michael Kelley
Cc: hpa, kys, haiyangz, wei.liu, decui, luto, peterz, davem, edumazet,
kuba, pabeni, lpieralisi, robh, kw, bhelgaas, arnd, hch,
m.szyprowski, robin.murphy, thomas.lendacky, brijesh.singh, tglx,
mingo, dave.hansen, Tianyu.Lan, kirill.shutemov,
sathyanarayanan.kuppuswamy, ak, isaku.yamahata, dan.j.williams,
jane.chu, seanjc, tony.luck, x86, linux-kernel, linux-hyperv,
netdev, linux-pci, linux-arch, iommu
On Wed, Nov 16, 2022 at 10:41:29AM -0800, Michael Kelley wrote:
> 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.
I hope you're right. Our boot ordering is fragile as hell. But
mem_encrypt_init() doesn't do a whole lot of important setup - that has
happened a lot earlier already - so I'm not too worried.
But we'll see what breaks in wider testing.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 66+ messages in thread
* [Patch v3 07/14] x86/hyperv: Change vTOM handling to use standard coco mechanisms
2022-11-16 18:41 [Patch v3 00/14] Add PCI pass-thru support to Hyper-V Confidential VMs Michael Kelley
` (5 preceding siblings ...)
2022-11-16 18:41 ` [Patch v3 06/14] init: Call mem_encrypt_init() after Hyper-V hypercall init is done Michael Kelley
@ 2022-11-16 18:41 ` Michael Kelley
2022-11-17 2:59 ` Tianyu Lan
2022-11-21 15:03 ` Borislav Petkov
2022-11-16 18:41 ` [Patch v3 08/14] swiotlb: Remove bounce buffer remapping for Hyper-V Michael Kelley
` (6 subsequent siblings)
13 siblings, 2 replies; 66+ messages in thread
From: Michael Kelley @ 2022-11-16 18:41 UTC (permalink / raw)
To: hpa, kys, haiyangz, wei.liu, decui, luto, peterz, davem, edumazet,
kuba, pabeni, lpieralisi, robh, kw, bhelgaas, arnd, hch,
m.szyprowski, robin.murphy, thomas.lendacky, brijesh.singh, tglx,
mingo, bp, dave.hansen, Tianyu.Lan, kirill.shutemov,
sathyanarayanan.kuppuswamy, ak, isaku.yamahata, dan.j.williams,
jane.chu, seanjc, tony.luck, x86, linux-kernel, linux-hyperv,
netdev, linux-pci, linux-arch, iommu
Cc: mikelley
Hyper-V guests on AMD SEV-SNP hardware have the option of using the
"virtual Top Of Memory" (vTOM) feature specified by the SEV-SNP
architecture. With vTOM, shared vs. private memory accesses are
controlled by splitting the guest physical address space into two
halves. vTOM is the dividing line where the uppermost bit of the
physical address space is set; e.g., with 47 bits of guest physical
address space, vTOM is 0x40000000000 (bit 46 is set). Guest phyiscal
memory is accessible at two parallel physical addresses -- one below
vTOM and one above vTOM. Accesses below vTOM are private (encrypted)
while accesses above vTOM are shared (decrypted). In this sense, vTOM
is like the GPA.SHARED bit in Intel TDX.
Support for Hyper-V guests using vTOM was added to the Linux kernel in
two patch sets[1][2]. This support treats the vTOM bit as part of
the physical address. For accessing shared (decrypted) memory, these
patch sets create a second kernel virtual mapping that maps to physical
addresses above vTOM.
A better approach is to treat the vTOM bit as a protection flag, not
as part of the physical address. This new approach is like the approach
for the GPA.SHARED bit in Intel TDX. Rather than creating a second kernel
virtual mapping, the existing mapping is updated using recently added
coco mechanisms. When memory is changed between private and shared using
set_memory_decrypted() and set_memory_encrypted(), the PTEs for the
existing kernel mapping are changed to add or remove the vTOM bit
in the guest physical address, just as with TDX. The hypercalls to
change the memory status on the host side are made using the existing
callback mechanism. Everything just works, with a minor tweak to map
the I/O APIC to use private accesses.
To accomplish the switch in approach, the following must be done in
in this single patch:
* Update Hyper-V initialization to set the cc_mask based on vTOM
and do other coco initialization.
* Update physical_mask so the vTOM bit is no longer treated as part
of the physical address
* Update cc_mkenc() and cc_mkdec() to be active for Hyper-V guests.
This makes the vTOM bit part of the protection flags.
* Code already exists to make hypercalls to inform Hyper-V about pages
changing between shared and private. Update this code to run as a
callback from __set_memory_enc_pgtable().
* Remove the Hyper-V special case from __set_memory_enc_dec()
* Remove the Hyper-V specific call to swiotlb_update_mem_attributes()
since mem_encrypt_init() will now do it.
[1] https://lore.kernel.org/all/20211025122116.264793-1-ltykernel@gmail.com/
[2] https://lore.kernel.org/all/20211213071407.314309-1-ltykernel@gmail.com/
Signed-off-by: Michael Kelley <mikelley@microsoft.com>
---
arch/x86/coco/core.c | 11 +++++++++-
arch/x86/hyperv/hv_init.c | 11 ----------
arch/x86/hyperv/ivm.c | 45 +++++++++++++++++++++++++++++++----------
arch/x86/include/asm/mshyperv.h | 8 ++------
arch/x86/kernel/cpu/mshyperv.c | 15 +++++++-------
arch/x86/mm/pat/set_memory.c | 3 ---
6 files changed, 53 insertions(+), 40 deletions(-)
diff --git a/arch/x86/coco/core.c b/arch/x86/coco/core.c
index 49b44f8..f5e1f2d 100644
--- a/arch/x86/coco/core.c
+++ b/arch/x86/coco/core.c
@@ -78,7 +78,14 @@ static bool amd_cc_platform_has(enum cc_attr attr)
static bool hyperv_cc_platform_has(enum cc_attr attr)
{
- return attr == CC_ATTR_GUEST_MEM_ENCRYPT;
+ switch (attr) {
+ case CC_ATTR_GUEST_MEM_ENCRYPT:
+ case CC_ATTR_MEM_ENCRYPT:
+ case CC_ATTR_EMULATED_IOAPIC:
+ return true;
+ default:
+ return false;
+ }
}
bool cc_platform_has(enum cc_attr attr)
@@ -108,6 +115,7 @@ u64 cc_mkenc(u64 val)
switch (vendor) {
case CC_VENDOR_AMD:
return val | cc_mask;
+ case CC_VENDOR_HYPERV:
case CC_VENDOR_INTEL:
return val & ~cc_mask;
default:
@@ -121,6 +129,7 @@ u64 cc_mkdec(u64 val)
switch (vendor) {
case CC_VENDOR_AMD:
return val & ~cc_mask;
+ case CC_VENDOR_HYPERV:
case CC_VENDOR_INTEL:
return val | cc_mask;
default:
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index f49bc3e..89a97d7 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -29,7 +29,6 @@
#include <linux/syscore_ops.h>
#include <clocksource/hyperv_timer.h>
#include <linux/highmem.h>
-#include <linux/swiotlb.h>
int hyperv_init_cpuhp;
u64 hv_current_partition_id = ~0ull;
@@ -504,16 +503,6 @@ void __init hyperv_init(void)
/* Query the VMs extended capability once, so that it can be cached. */
hv_query_ext_cap(0);
-#ifdef CONFIG_SWIOTLB
- /*
- * Swiotlb bounce buffer needs to be mapped in extra address
- * space. Map function doesn't work in the early place and so
- * call swiotlb_update_mem_attributes() here.
- */
- if (hv_is_isolation_supported())
- swiotlb_update_mem_attributes();
-#endif
-
return;
clean_guest_os_id:
diff --git a/arch/x86/hyperv/ivm.c b/arch/x86/hyperv/ivm.c
index e8be4c2..29ccbe8 100644
--- a/arch/x86/hyperv/ivm.c
+++ b/arch/x86/hyperv/ivm.c
@@ -13,6 +13,7 @@
#include <asm/svm.h>
#include <asm/sev.h>
#include <asm/io.h>
+#include <asm/coco.h>
#include <asm/mshyperv.h>
#include <asm/hypervisor.h>
@@ -233,7 +234,6 @@ void hv_ghcb_msr_read(u64 msr, u64 *value)
local_irq_restore(flags);
}
EXPORT_SYMBOL_GPL(hv_ghcb_msr_read);
-#endif
/*
* hv_mark_gp