Re: [PATCH v2 1/1] x86/elf: Add a new .note section containing Xfeatures information to x86 core files
On Sun, May 26, 2024 at 10:24:41AM +0530, Balasubrmanian, Vignesh wrote: > If we can add a new enum only when we extend, then as Thomas suggested can > we use other kernel variables as in the first version of the patch until we > extend for other/new features? I assume by "other kernel variables" you mean CPUID? If so, can you change the layout of your buffer once you export it to userspace? -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v2 1/1] x86/elf: Add a new .note section containing Xfeatures information to x86 core files
On Thu, May 23, 2024 at 11:57:00AM +0530, Balasubrmanian, Vignesh wrote: > Currently, this enum is the same as XSAVE, but when we add other features, > this > enum might have a different value of the XSAVE features and can be modified > without disturbing the existing kernel code. We will do that when we cross that bridge, right? -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v2 1/1] x86/elf: Add a new .note section containing Xfeatures information to x86 core files
On Wed, May 22, 2024 at 06:42:55PM +0530, Balasubrmanian, Vignesh wrote:
> > > +enum custom_feature {
> > > + FEATURE_XSAVE_FP = 0,
> > > + FEATURE_XSAVE_SSE = 1,
> > > + FEATURE_XSAVE_YMM = 2,
> > > + FEATURE_XSAVE_BNDREGS = 3,
> > > + FEATURE_XSAVE_BNDCSR = 4,
> > > + FEATURE_XSAVE_OPMASK = 5,
> > > + FEATURE_XSAVE_ZMM_Hi256 = 6,
> > > + FEATURE_XSAVE_Hi16_ZMM = 7,
> > > + FEATURE_XSAVE_PT = 8,
> > > + FEATURE_XSAVE_PKRU = 9,
> > > + FEATURE_XSAVE_PASID = 10,
> > > + FEATURE_XSAVE_CET_USER = 11,
> > > + FEATURE_XSAVE_CET_SHADOW_STACK = 12,
> > > + FEATURE_XSAVE_HDC = 13,
> > > + FEATURE_XSAVE_UINTR = 14,
> > > + FEATURE_XSAVE_LBR = 15,
> > > + FEATURE_XSAVE_HWP = 16,
> > > + FEATURE_XSAVE_XTILE_CFG = 17,
> > > + FEATURE_XSAVE_XTILE_DATA = 18,
> > > + FEATURE_MAX,
> > > + FEATURE_XSAVE_EXTENDED_START = FEATURE_XSAVE_YMM,
> > > + FEATURE_XSAVE_EXTENDED_END = FEATURE_XSAVE_XTILE_DATA,
> > > +};
> > Why can't this use the existing 'enum xfeature' which is providing
> > exactly the same information already?
> First version of patch was similar to what you mentioned here and other
> review comments to use existing kernel definitions.
> https://lore.kernel.org/linux-mm/20240314112359.50713-1-vigba...@amd.com/T/
>
> As per the review comment
> https://lore.kernel.org/linux-mm/20240314162954.GAZfMmAnYQoRjRbRzc@fat_crate.local/
> , modified the patch to be a independent of kernel internal definitions.
> Though this enum and below function "get_sub_leaf" are not useful now, it
> will be required when we extend for a new/different features.
No, Thomas' sugggestion is to use the existing xfeature enum - not
define the same thing again.
Why do you need that enum custom_feature thing if you can use
/*
* List of XSAVE features Linux knows about:
*/
enum xfeature {
from arch/x86/include/asm/fpu/types.h
?
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v2 1/1] arch/fault: don't print logs for pte marker poison errors
On Wed, May 15, 2024 at 12:19:16PM -0700, Axel Rasmussen wrote: > An unprivileged process can allocate a VMA, use the userfaultfd API to > install one of these PTE markers, and then register a no-op SIGBUS > handler. Now it can access that address in a tight loop, Maybe the userfaultfd should not allow this, I dunno. You made me look at this thing and to me it all sounds weird. One thread does page fault handling for the other and that helps with live migration somehow. OMG, whaaat? Maybe I don't understand it and probably never will... But, for example, membarrier used do to a stupid thing of allowing one thread to hammer another with an IPI storm. Bad bad idea. So it got fixed. All I'm saying is, if unprivileged processes can do crap, they should be prevented from doing crap. Like ratelimiting the pagefaults or whatnot. One of the recovery action strategies from memory poison is, well, you kill the process. If you can detect the hammering process which installed that page marker, you kill it. Problem solved. But again, this userfaultfd thing sounds really weird so I could very well be way wrong. > Even in a non-contrived / non-malicious case, use of this API could > have similar effects. If nothing else, the log message can be > confusing to administrators: they state that an MCE occurred, whereas > with the simulated poison API, this is not the case; it isn't a "real" > MCE / hardware error. Yeah, I read that part in Documentation/admin-guide/mm/userfaultfd.rst Simulated poison huh? Another WTF. > In the KVM use case, the host can't just allocate a new page, because > it doesn't know what the guest might have had stored there. Best we Ok, let's think of real hw poison. When doing the recovery, you don't care what's stored there because as far as the hardware is concerned, if you consume that poison the *whole* machine might go down. So you lose the page. Plain and simple. And the guest can go visit the bureau of complaints and grievances. Still better than killing the guest or even the whole host with other guests running on it. > can do is propagate the poison into the guest, and let the guest OS > deal with it as it sees fit, and mark the page poisoned on the host. You mark the page as poison on the host and you yank it from under the guest. That physical frame is gone and the faster all the actors involved understand that, the better. > I don't disagree the guest *shouldn't* reaccess it in this case. :) > But if it did, it should get another poison event just as you say. Yes, it shouldn't. Look at memory_failure(). This will kill whole processes if it has to, depending on what the page is used for. > And, live migration between physical hosts should be transparent to > the guest. So if the guest gets a poison, and then we live migrate it, So if I were to design this, I'd do it this way: 0. guest gets hw poison injected 1. it runs memory_failure() and it kills the processes using the page. 2. page is marked poisoned on the host so no other guest gets it. That's it. No second accesses whatsoever. At least this is how it works on baremetal. This hw poisoning emulation is just silly and unnecessary. But again, I probably am missing some aspects. It all just sounded really weird to me that's why I thought I should ask what's behind all that. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v2 1/1] arch/fault: don't print logs for pte marker poison errors
On Wed, May 15, 2024 at 10:33:03AM -0700, Axel Rasmussen wrote: > Right, the goal is to still have the process get a SIGBUS, but to > avoid the "MCE error" log message. The basic issue is, unprivileged > users can set these markers up, and thereby completely spam up the > log. What is the real attack scenario you want to protect against? Or is this something hypothetical? > That said, one thing I'm not sure about is whether or not > VM_FAULT_SIGBUS is a viable alternative (returned for a new PTE marker > type specific to simulated poison). The goal of the simulated poison > feature is to "closely simulate" a real hardware poison event. If you > live migrate a VM from a host with real poisoned memory, to a new > host: you'd want to keep the same behavior if the guest accessed those > addresses again, so as not to confuse the guest about why it suddenly > became "un-poisoned". Well, the recovery action is to poison the page and the process should be resilient enough and allocate a new, clean page which doesn't trigger hw poison hopefully, if possible. It doesn't make a whole lotta sense if poison "remains". Hardware poison you don't want to touch a second time either - otherwise you might consume that poison and die. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v2 1/1] arch/fault: don't print logs for pte marker poison errors
On Fri, May 10, 2024 at 11:29:26AM -0700, Axel Rasmussen wrote: > @@ -3938,7 +3938,7 @@ static vm_fault_t handle_pte_marker(struct vm_fault > *vmf) > > /* Higher priority than uffd-wp when data corrupted */ > if (marker & PTE_MARKER_POISONED) > - return VM_FAULT_HWPOISON; > + return VM_FAULT_HWPOISON | VM_FAULT_HWPOISON_SILENT; If you know here that this poisoning should be silent, why do you have to make it all complicated and propagate it into arch code, waste a separate VM_FAULT flag just for that instead of simply returning here a VM_FAULT_COMPLETED or some other innocuous value which would stop processing the fault? -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH 1/1] x86/elf: Add a new .note section containing Xfeatures information to x86 core files
On Sat, Mar 16, 2024 at 12:51:28AM +0100, Thomas Gleixner wrote: > Anything which is not enumerated in CPUID does not exist in > XSTATE. Period and end of story. But why not have a simple buffer definition which doesn't need CPUID? Also, doing the CPUID thing would need extending the gdb remote protocol as explained here: https://lore.kernel.org/r/971d21b7-0309-439e-91b6-234f84da9...@freebsd.org The simple buffer layout won't. So regardless of where hw is going, I think a simple buffer definition is always better. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH 0/1] Add XSAVE layout description to Core files for debuggers to support varying XSAVE layouts
On Thu, Mar 14, 2024 at 04:25:44PM +, Willgerodt, Felix wrote:
> I am wondering if it wouldn't be easier for everyone if corefiles would just
> contain space for all possible XSAVE components?
You mean we should shuffle out from the kernel 8K of AMX state even if
nothing uses it or the machine doesn't even support it?
That's silly.
Please have a look at this:
+struct xfeat_component {
+ u32 xfeat_type;
+ u32 xfeat_sz;
+ u32 xfeat_off;
+ u32 xfeat_flags;
+} __packed;
What is wrong with having a blob of such xfeat_component things
describing the XSTATE buffer and parsing it in gdb?
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH 1/1] x86/elf: Add a new .note section containing Xfeatures information to x86 core files
On Thu, Mar 14, 2024 at 09:19:15AM -0700, Dave Hansen wrote: > Are you envisioning an *XSAVE* state component that's not described by > CPUID? I want to be prepared. You can imagine some of the short cuts and corners cutting hw guys would do so I'd want to be prepared there and not tie this to CPUID. > Or some _other_ (non-XSAVE) component in a core dump that isn't > described by CPUID? Yes, that too. Since the format of this buffer is so simple and machine-independent, it can be extended as needed without issues. > That argument breaks down a bit on the flags though: > > xc.xfeat_flags = xstate_flags[i]; > > Because it comes _directly_ from CPUID with zero filtering: > > cpuid_count(XSTATE_CPUID, i, , , , ); > ... > xstate_flags[i] = ecx; > > So this layout is quite dependent on what's in x86's CPUID. Yeah, no, this should not be copying CPUID flags - those flags should be *translated* to independently defined flags which describe those buffers. This is even more important if we change our xstate_flags[] machinery. This buffer should not use any kernel-internal definitions and structures but be completely self-describing. Vignesh, pls fix that. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH 1/1] x86/elf: Add a new .note section containing Xfeatures information to x86 core files
On Thu, Mar 14, 2024 at 08:37:09AM -0700, Dave Hansen wrote: > This is pretty close to just a raw dump of the XSAVE CPUID leaves. > Rather than come up with an XSAVE-specific ABI that depends on CPUID > *ANYWAY* (because it dumps the "flags" register aka. ECX), maybe we > should just bite the bullet and dump out (some of) the raw CPUID space. Funny you should say that. This was what they had done originally but if you dump CPUID and you want to add another component in the future which is *not* described by CPUID, your scheme breaks. So the idea is to have a self-describing buffers layout, independent from any x86-ism. You can extend this in a straight-forward way then later. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v7 13/41] mm: Make pte_mkwrite() take a VMA
On Mon, Feb 27, 2023 at 02:29:29PM -0800, Rick Edgecombe wrote: > [0] > https://lore.kernel.org/lkml/0e29a2d0-08d8-bcd6-ff26-4bea0e403...@redhat.com/#t I guess that sub-thread about how you arrived at this "pass a VMA" decision should be in the Link tag. But that's for the committer, I'd say. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v3 2/2] x86: Fix /proc/cpuinfo cpumask warning
On Thu, Nov 03, 2022 at 10:31:30AM -0700, Yury Norov wrote:
> Let's take for example cpu_llc_shared_mask() added by you in
> arch/x86/include/asm/smp.h recently:
>
> static inline struct cpumask *cpu_llc_shared_mask(int cpu)
> {
> return per_cpu(cpu_llc_shared_map, cpu);
> }
>
> It's in a global header and available to the rest of the kernel, just as
> well.
Just like
static inline struct cpumask *cpu_l2c_shared_mask(int cpu)
{
return per_cpu(cpu_l2c_shared_map, cpu);
}
should check != must check.
But it's perfectly fine if you're going to attempt to prove some bogus
argument of yours - I can safely ignore you.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v3 2/2] x86: Fix /proc/cpuinfo cpumask warning
On Thu, Nov 03, 2022 at 09:30:54AM -0700, yury.no...@gmail.com wrote:a > Callers should pass sane arguments into internal functions if they > expect sane output. What internal function? It's in a global header. > The API not exported to userspace shouldn't sanity-check all inputs > arguments. That doesn't have anything to do with userspace at all. APIs exported to the rest of the kernel should very well check their inputs. Otherwise they're not APIs - just some random functions which are visible to the compiler. > So, the portable code shouldn't expect from cpumasks more than > documentation said: for a _valid_ offset cpumask_next() returns next > set bit or >= nr_cpu_ids. Lemme quote from my previous mail: "First make sure cpumask_next()'s valid accepted range has been settled upon, has been explicitly documented" So where is that valid range documented? > cpumask_check() has been broken for years. Attempting to fix it faced > so much resistance, that I had to revert the patch. The suggestion on that thread made sense: you first fix the callers and then the interface. Just like any other "broken" kernel API. Nothing's stopping you from fixing it properly - it'll just take a while and if it is such a widely used interface, you probably should come up with a strategy first how to fix it without impacting current use. Interfaces and their in-kernel users get refactored constantly. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v3 2/2] x86: Fix /proc/cpuinfo cpumask warning
On Thu, Nov 03, 2022 at 04:34:04PM +0100, Andrew Jones wrote: > Indeed, but that's less of an issue with cpumask_next() than with > the way cpuinfo implements its start and next seq ops (next > unconditionally increments *pos and then calls start and start > must use *pos - 1 since the first time its called it needs to use > -1). Maybe because those are done wrongly... A ->next() function should not call the ->start() function. A ->start() function should, well, only start and nothing else. And a ->stop() function should maybe check *pos and say whether one should stop or not. But I haven't looked at seq_ops at least in a decade and I have no clue whether that would work. I'm just looking at the function pointers and am trying to spell out what looks most natural IMO. IOW, maybe this should be fixed "right" and not only "made to work". Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v3 2/2] x86: Fix /proc/cpuinfo cpumask warning
On Thu, Nov 03, 2022 at 01:59:45PM +0100, Andrew Jones wrote: > The patch I'm proposing ensures cpumask_next()'s range, which is actually > [-1, nr_cpus_ids - 1), Lemme make sure I understand it correctly: on the upper boundary, if you supply for n the value nr_cpu_ids - 2, then it will return potentially the last bit if the mask is set, i.e., the one at position (nr_cpu_ids - 1). If you supply nr_cpus_ids - 1, then it'll return nr_cpu_ids to signal no further bits set. Yes, no? > I'll send a v4 with another stab at the commit message. Yes, and it is still an unreadable mess: "A kernel compiled with commit ... but not its revert... " Nope. First make sure cpumask_next()'s valid accepted range has been settled upon, has been explicitly documented in a comment above it and then I'll take a patch that fixes whatever is there to fix. Callers should not have to filter values before passing them in - the function either returns an error or returns the next bit in the mask. This thing: if (*pos == nr_cpu_ids) but then to pass in pos - 1: *pos = cpumask_next(*pos - 1 looks to me like the interface needs more cooking. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v3 2/2] x86: Fix /proc/cpuinfo cpumask warning
On Mon, Oct 31, 2022 at 11:03:27AM +0100, Andrew Jones wrote: > Currently (after the revert of 78e5a3399421) After the revert? That commit is still in the latest Linus tree. > with DEBUG_PER_CPU_MAPS we'll get a warning splat when the cpu is > outside the range [-1, nr_cpu_ids) Yah, that range makes sense. > and cpumask_next() will call find_next_bit() with the input plus one anyway. > find_next_bit() doesn't explicity document what happens when an input is > outside the range, but it currently returns the bitmap size without any > side effects, which means cpumask_next() will return nr_cpu_ids. That is good to have in the commit message. > show_cpuinfo() doesn't try to show anything in that case and stops its > loop, or, IOW, things work fine now with an input of nr_cpu_ids - 1. But, > show_cpuinfo() is just getting away with a violated cpumask_next() > contract, which 78e5a3399421 exposed. How about a new commit message like > this You're making it sound more complex than it is. All you wanna say is: "Filter out invalid cpumask_next() inputs by checking its first argument against nr_cpu_ids because cpumask_next() will call find_next_bit() with the input plus one but the valid range for n is [-1, nr_cpu_ids)." But that thing with the revert above needs to be clarified first. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v3 2/2] x86: Fix /proc/cpuinfo cpumask warning
On Mon, Oct 31, 2022 at 09:06:04AM +0100, Andrew Jones wrote: > The valid cpumask range is [0, nr_cpu_ids) and cpumask_next() always > returns a CPU ID greater than its input, which results in its input > range being [-1, nr_cpu_ids - 1). Ensure showing CPU info avoids > triggering error conditions in cpumask_next() by stopping its loop What error conditions? What would happen if @n is outside of the valid range? -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v3 2/2] x86: Fix /proc/cpuinfo cpumask warning
On Fri, Oct 28, 2022 at 10:13:28AM -0500, Yury Norov wrote: > Because it's related to bitmap API usage and has been revealed after > some work in bitmaps. So first of all, that "fix" needs to explain what exactly it is fixing. Not "it fixes this and that warning" but why the input arg to cpumask_next() cannot be nr_cpu_ids because... yadda yadda... > And because nobody else cares. Why do you assume that? > If you're willing to move it yourself please go ahead. If it fixes a real issue, we are taking it. And pls note that x86 patches go through the tip tree. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v3 2/2] x86: Fix /proc/cpuinfo cpumask warning
On Fri, Oct 28, 2022 at 07:46:08AM -0700, Yury Norov wrote: > I'll take it in bitmap-for-next this weekend. Why? -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] EDAC/ppc_4xx: Reorder symbols to get rid of a few forward declarations
On Sun, Sep 18, 2022 at 01:20:13AM +0200, Uwe Kleine-König wrote: > When moving the definition of ppc4xx_edac_driver further down, the > forward declarations can just be dropped. > > Do this to reduce line needless repetition. > > Signed-off-by: Uwe Kleine-König > --- > drivers/edac/ppc4xx_edac.c | 23 +-- > 1 file changed, 9 insertions(+), 14 deletions(-) Applied, thanks. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v7 3/6] x86/kexec: Carry forward IMA measurement log on kexec
On Fri, Aug 12, 2022 at 01:14:38PM -0400, Stefan Berger wrote: > Yes, so this series can be tested by krobot. You mean Intel's 0day robot? I believe that thing has by now enough logic to figure out which branch to base patches ontop. Or maybe there's some magic incantation to tell it which base commit to use so that you can simply do your patches ontop of latest linux-next instead of having to carry upstreamed patches. Also, there's a little point in testing against 5.19 when you wanna test it against v6.0-rc1... -- Regards/Gruss, Boris. SUSE Software Solutions Germany GmbH GF: Ivo Totev, Andrew Myers, Andrew McDonald, Martje Boudien Moerman (HRB 36809, AG Nürnberg)
Re: [PATCH v7 3/6] x86/kexec: Carry forward IMA measurement log on kexec
On Fri, Aug 12, 2022 at 12:43:02PM -0400, Stefan Berger wrote: > From: Jonathan McDowell > > On kexec file load, the Integrity Measurement Architecture (IMA) > subsystem may verify the IMA signature of the kernel and initramfs, and > measure it. The command line parameters passed to the kernel in the > kexec call may also be measured by IMA. > > A remote attestation service can verify a TPM quote based on the TPM > event log, the IMA measurement list and the TPM PCR data. This can > be achieved only if the IMA measurement log is carried over from the > current kernel to the next kernel across the kexec call. > > PowerPC and ARM64 both achieve this using device tree with a > "linux,ima-kexec-buffer" node. x86 platforms generally don't make use of > device tree, so use the setup_data mechanism to pass the IMA buffer to > the new kernel. > > Signed-off-by: Jonathan McDowell > Signed-off-by: Borislav Petkov > Reviewed-by: Mimi Zohar # IMA function definitions > Link: https://lore.kernel.org/r/YmKyvlF3my1yWTvK@noodles-fedora-PC23Y6EG Is there any particular reason to keep sending a patch which is already upstream? -- Regards/Gruss, Boris. SUSE Software Solutions Germany GmbH GF: Ivo Totev, Andrew Myers, Andrew McDonald, Martje Boudien Moerman (HRB 36809, AG Nürnberg)
Re: [PATCH v3] random: handle archrandom with multiple longs
On Tue, Jul 19, 2022 at 03:02:07PM +0200, Jason A. Donenfeld wrote: > The archrandom interface was originally designed for x86, which supplies > RDRAND/RDSEED for receiving random words into registers, resulting in > one function to generate an int and another to generate a long. However, > other architectures don't follow this. > > On arm64, the SMCCC TRNG interface can return between 1 and 3 longs. On > s390, the CPACF TRNG interface can return arbitrary amounts, with 32 > longs having the same cost as one. On UML, the os_getrandom() interface > can return arbitrary amounts. > > So change the api signature to take a "max_longs" parameter designating > the maximum number of longs requested, and then return the number of > longs generated. > > Since callers need to check this return value and loop anyway, each arch > implementation does not bother implementing its own loop to try again to > fill the maximum number of longs. Additionally, all existing callers > pass in a constant max_longs parameter. Taken together, these two things > mean that the codegen doesn't really change much for one-word-at-a-time > platforms, while performance is greatly improved on platforms such as > s390. > > Cc: Will Deacon > Cc: Alexander Gordeev > Cc: Thomas Gleixner > Cc: H. Peter Anvin > Cc: Catalin Marinas > Cc: Borislav Petkov > Cc: Heiko Carstens > Cc: Johannes Berg > Cc: Mark Rutland > Cc: Harald Freudenberger > Acked-by: Michael Ellerman > Signed-off-by: Jason A. Donenfeld > --- > arch/arm64/include/asm/archrandom.h | 102 -- > arch/arm64/kernel/kaslr.c | 2 +- > arch/powerpc/include/asm/archrandom.h | 30 ++-- > arch/powerpc/kvm/book3s_hv.c | 2 +- > arch/s390/include/asm/archrandom.h| 29 ++-- > arch/um/include/asm/archrandom.h | 21 ++ > arch/x86/include/asm/archrandom.h | 41 +-- > arch/x86/kernel/espfix_64.c | 2 +- > drivers/char/random.c | 45 > include/asm-generic/archrandom.h | 18 + > include/linux/random.h| 12 +-- > 11 files changed, 116 insertions(+), 188 deletions(-) Acked-by: Borislav Petkov # for x86 Thx. -- Regards/Gruss, Boris. SUSE Software Solutions Germany GmbH GF: Ivo Totev, Andrew Myers, Andrew McDonald, Martje Boudien Moerman (HRB 36809, AG Nürnberg)
Re: [PATCH v3] random: handle archrandom with multiple longs
On Tue, Jul 19, 2022 at 03:02:07PM +0200, Jason A. Donenfeld wrote: > Since callers need to check this return value and loop anyway, each arch > implementation does not bother implementing its own loop to try again to > fill the maximum number of longs. Additionally, all existing callers > pass in a constant max_longs parameter. Hmm, maybe this has come up already but it reads weird. If I have a function arch_get_random_longs(), I'd expect it to give me the number of longs I requested or say, error. Why do the callers need to loop? If I have to loop, I'd call the "get me one long" function and loop N times. -- Regards/Gruss, Boris. SUSE Software Solutions Germany GmbH GF: Ivo Totev, Andrew Myers, Andrew McDonald, Martje Boudien Moerman (HRB 36809, AG Nürnberg)
Re: [PATCH] random: remove CONFIG_ARCH_RANDOM and "nordrand"
On Tue, Jul 05, 2022 at 02:50:34PM -0700, H. Peter Anvin wrote:
> It's just math. The only variable is your confidence level, i.e. at
> what level do you decide that the likelihood of pure chance is way
> smaller than the likelihood of hardware failure.
That might be but the likelyhood of certain BIOSes dropping the ball
after resume is 100%:
7879fc4bdc75 ("x86/rdrand: Sanity-check RDRAND output")
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] random: remove CONFIG_ARCH_RANDOM and "nordrand"
On Tue, Jul 05, 2022 at 09:44:17PM +0200, Jason A. Donenfeld wrote: > Oh, huh. Maybe in that case I should adjust the message to say "consider > using `random.trust_cpu=0`," which is the thing that would actually make > a security difference. Why isn't that option documented in Documentation/admin-guide/kernel-parameters.txt? > But actually, one thing that wasn't clear to me was: does `nordrand` > affect what userspace sees? While random.c is okay in lots of > circumstances, I could imagine `nordrand` playing a role in preventing > userspace from using it, which might be desirable. Is this the case? If > so, I can remove the nordrand chunk from this patch for v2. If not, I'll > adjust the text to mention `random.trust_cpu=0`. Unfortunately, it doesn't disable the instruction. It would be lovely if we had a switch like that... That's why this message is supposed to be noisy so that people can pay attention at least. > In the sense that random.c can handle mostly any input without making > the quality worse. So, you can't accidentally taint it. The only risk is > if it thinks RDRAND is good and trustable when it isn't, but that's what > `random.trust_cpu=0` is for. And that's why I'm saying that if you detect RDRAND returning the same thing over and over again, you should simply stop using it. Automatically. Not rely on the user to do anything. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH 2/4] ELF: Remove elf_core_copy_kernel_regs()
+ PPC ML as an FYI that this change will come through tip.
On Fri, Mar 25, 2022 at 11:39:51AM -0400, Brian Gerst wrote:
> x86-32 was the last architecture that implemented separate user and
> kernel registers.
>
> Signed-off-by: Brian Gerst
> ---
> arch/powerpc/kernel/fadump.c | 2 +-
> arch/powerpc/platforms/powernv/opal-core.c | 2 +-
> include/linux/elfcore.h| 9 -
> kernel/kexec_core.c| 2 +-
> 4 files changed, 3 insertions(+), 12 deletions(-)
>
> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> index 4fdb7c77fda1..c0cf17196d6c 100644
> --- a/arch/powerpc/kernel/fadump.c
> +++ b/arch/powerpc/kernel/fadump.c
> @@ -752,7 +752,7 @@ u32 *__init fadump_regs_to_elf_notes(u32 *buf, struct
> pt_regs *regs)
>* FIXME: How do i get PID? Do I really need it?
>* prstatus.pr_pid =
>*/
> - elf_core_copy_kernel_regs(_reg, regs);
> + elf_core_copy_regs(_reg, regs);
> buf = append_elf_note(buf, CRASH_CORE_NOTE_NAME, NT_PRSTATUS,
> , sizeof(prstatus));
> return buf;
> diff --git a/arch/powerpc/platforms/powernv/opal-core.c
> b/arch/powerpc/platforms/powernv/opal-core.c
> index 0331f1973f0e..dd6e99edff76 100644
> --- a/arch/powerpc/platforms/powernv/opal-core.c
> +++ b/arch/powerpc/platforms/powernv/opal-core.c
> @@ -112,7 +112,7 @@ static void __init fill_prstatus(struct elf_prstatus
> *prstatus, int pir,
> struct pt_regs *regs)
> {
> memset(prstatus, 0, sizeof(struct elf_prstatus));
> - elf_core_copy_kernel_regs(&(prstatus->pr_reg), regs);
> + elf_core_copy_regs(&(prstatus->pr_reg), regs);
>
> /*
>* Overload PID with PIR value.
> diff --git a/include/linux/elfcore.h b/include/linux/elfcore.h
> index f8e206e82476..346a8b56cdc8 100644
> --- a/include/linux/elfcore.h
> +++ b/include/linux/elfcore.h
> @@ -84,15 +84,6 @@ static inline void elf_core_copy_regs(elf_gregset_t
> *elfregs, struct pt_regs *re
> #endif
> }
>
> -static inline void elf_core_copy_kernel_regs(elf_gregset_t *elfregs, struct
> pt_regs *regs)
> -{
> -#ifdef ELF_CORE_COPY_KERNEL_REGS
> - ELF_CORE_COPY_KERNEL_REGS((*elfregs), regs);
> -#else
> - elf_core_copy_regs(elfregs, regs);
> -#endif
> -}
> -
> static inline int elf_core_copy_task_regs(struct task_struct *t,
> elf_gregset_t* elfregs)
> {
> #if defined (ELF_CORE_COPY_TASK_REGS)
> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> index 68480f731192..be4b54c2c615 100644
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -1078,7 +1078,7 @@ void crash_save_cpu(struct pt_regs *regs, int cpu)
> return;
> memset(, 0, sizeof(prstatus));
> prstatus.common.pr_pid = current->pid;
> - elf_core_copy_kernel_regs(_reg, regs);
> + elf_core_copy_regs(_reg, regs);
> buf = append_elf_note(buf, KEXEC_CORE_NOTE_NAME, NT_PRSTATUS,
> , sizeof(prstatus));
> final_note(buf);
> --
> 2.35.1
>
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v0 42/42] notifier: Return an error when callback is already registered
On Mon, Nov 08, 2021 at 03:59:26PM -0500, Alan Stern wrote: > Is there really any reason for returning an error code? For example, is > it anticipated that at some point in the future these registration calls > might fail? > > Currently, the only reason for failing... Right, I believe with not making it return void we're leaving the door open for some, *hypothetical* future return values if we decide we need to return them too, at some point. Yes, I can't think of another fact to state besides that the callback was already registered or return success but who knows what we wanna do in the future... And so if we change them all to void now, I think it'll be a lot more churn to switch back to returning a non-void value and having the callers who choose to handle that value, do so again. So, long story short, keeping the retval - albeit not very useful right now - is probably easier. I hope I'm making some sense here. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v0 00/42] notifiers: Return an error when callback is already registered
On Mon, Nov 08, 2021 at 11:23:13AM -0500, Steven Rostedt wrote: > Question, how often does this warning trigger? Is it common to see in > development? Yeah, haven't seen it myself yet. But we hashed it out over IRC. :-) -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v0 42/42] notifier: Return an error when callback is already registered
On Mon, Nov 08, 2021 at 05:12:16PM +0100, Geert Uytterhoeven wrote: > Returning void is the other extreme ;-) > > There are 3 levels (ignoring BUG_ON()/panic () inside the callee): > 1. Return void: no one can check success or failure, > 2. Return an error code: up to the caller to decide, > 3. Return a __must_check error code: every caller must check. > > I'm in favor of 2, as there are several places where it cannot fail. Makes sense to me. I'll do that in the next iteration. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v0 42/42] notifier: Return an error when callback is already registered
On Mon, Nov 08, 2021 at 04:25:47PM +0100, Geert Uytterhoeven wrote:
> I'm not against returning proper errors codes. I'm against forcing
> callers to check things that cannot fail and to add individual error
> printing to each and every caller.
If you're against checking things at the callers, then the registration
function should be void. IOW, those APIs are not optimally designed atm.
> Note that in other areas, we are moving in the other direction,
> to a centralized printing of error messages, cfr. e.g. commit
> 7723f4c5ecdb8d83 ("driver core: platform: Add an error message to
> platform_get_irq*()").
Yes, thus my other idea to add a lower level __notifier_chain_register()
to do the checking.
I'll see if I can convert those notifier registration functions to
return void, in the process. But let's see what the others think first.
Thanks for taking the time.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v0 00/42] notifiers: Return an error when callback is already registered
On Mon, Nov 08, 2021 at 03:24:39PM +0100, Borislav Petkov wrote:
> I guess I can add another indirection to notifier_chain_register() and
> avoid touching all the call sites.
IOW, something like this below.
This way I won't have to touch all the callsites and the registration
routines would still return a proper value instead of returning 0
unconditionally.
---
diff --git a/kernel/notifier.c b/kernel/notifier.c
index b8251dc0bc0f..04f08b2ef17f 100644
--- a/kernel/notifier.c
+++ b/kernel/notifier.c
@@ -19,14 +19,12 @@ BLOCKING_NOTIFIER_HEAD(reboot_notifier_list);
* are layered on top of these, with appropriate locking added.
*/
-static int notifier_chain_register(struct notifier_block **nl,
- struct notifier_block *n)
+static int __notifier_chain_register(struct notifier_block **nl,
+struct notifier_block *n)
{
while ((*nl) != NULL) {
- if (unlikely((*nl) == n)) {
- WARN(1, "double register detected");
- return 0;
- }
+ if (unlikely((*nl) == n))
+ return -EEXIST;
if (n->priority > (*nl)->priority)
break;
nl = &((*nl)->next);
@@ -36,6 +34,18 @@ static int notifier_chain_register(struct notifier_block
**nl,
return 0;
}
+static int notifier_chain_register(struct notifier_block **nl,
+ struct notifier_block *n)
+{
+ int ret = __notifier_chain_register(nl, n);
+
+ if (ret == -EEXIST)
+ WARN(1, "double register of notifier callback %ps detected",
+ n->notifier_call);
+
+ return ret;
+}
+
static int notifier_chain_unregister(struct notifier_block **nl,
struct notifier_block *n)
{
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v0 00/42] notifiers: Return an error when callback is already registered
On Mon, Nov 08, 2021 at 09:17:03AM -0500, Alan Stern wrote: > What reason is there for moving the check into the callers? It seems > like pointless churn. Why not add the error return code, change the > WARN to pr_warn, and leave the callers as they are? Wouldn't that end > up having exactly the same effect? > > For that matter, what sort of remedial action can a caller take if the > return code is -EEXIST? Is there any point in forcing callers to check > the return code if they can't do anything about it? See my reply to Geert from just now: https://lore.kernel.org/r/yykyueqcsowqm...@zn.tnic I guess I can add another indirection to notifier_chain_register() and avoid touching all the call sites. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v0 42/42] notifier: Return an error when callback is already registered
On Mon, Nov 08, 2021 at 03:07:03PM +0100, Geert Uytterhoeven wrote:
> I think the addition of __must_check is overkill, leading to the
> addition of useless error checks and message printing.
See the WARN in notifier_chain_register() - it will already do "message
printing".
> Many callers call this where it cannot fail, and where nothing can
> be done in the very unlikely event that the call would ever start to
> fail.
This is an attempt to remove this WARN() hack in
notifier_chain_register() and have the function return a proper error
value instead of this "Currently always returns zero." which is bad
design.
Some of the registration functions around the tree check that retval and
some don't. So if "it cannot fail" those registration either should not
return a value or callers should check that return value - what we have
now doesn't make a whole lot of sense.
Oh, and then fixing this should avoid stuff like:
+ if (notifier_registered == false) {
+ mce_register_decode_chain(_bad_page_nb);
+ notifier_registered = true;
+ }
from propagating in the code.
The other idea I have is to add another indirection in
notifier_chain_register() which will check the *proper* return value of
a lower level __notifier_chain_register() and then issue that warning.
That would definitely avoid touch so many call sites.
Bottom line is: what we have now needs cleaning up.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
[PATCH v0 42/42] notifier: Return an error when callback is already registered
From: Borislav Petkov
The notifier registration routine doesn't return a proper error value
when a callback has already been registered, leading people to track
whether that registration has happened at the call site:
https://lore.kernel.org/amd-gfx/20210512013058.6827-1-mukul.jo...@amd.com/
Which is unnecessary.
Return -EEXIST to signal that case so that callers can act accordingly.
Enforce callers to check the return value, leading to loud screaming
during build:
arch/x86/kernel/cpu/mce/core.c: In function ‘mce_register_decode_chain’:
arch/x86/kernel/cpu/mce/core.c:167:2: error: ignoring return value of \
‘blocking_notifier_chain_register’, declared with attribute
warn_unused_result [-Werror=unused-result]
blocking_notifier_chain_register(_mce_decoder_chain, nb);
^~~~
Drop the WARN too, while at it.
Suggested-by: Thomas Gleixner
Signed-off-by: Borislav Petkov
Cc: Arnd Bergmann
Cc: Ayush Sawal
Cc: Greg Kroah-Hartman
Cc: Rohit Maheshwari
Cc: Steven Rostedt
Cc: Vinay Kumar Yadav
Cc: alsa-de...@alsa-project.org
Cc: bcm-kernel-feedback-l...@broadcom.com
Cc: intel-...@lists.freedesktop.org
Cc: intel-gvt-...@lists.freedesktop.org
Cc: linux-al...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-...@vger.kernel.org
Cc: linux-cry...@vger.kernel.org
Cc: linux-e...@vger.kernel.org
Cc: linux-fb...@vger.kernel.org
Cc: linux-hyp...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Cc: linux-l...@vger.kernel.org
Cc: linux-m...@vger.kernel.org
Cc: linux-par...@vger.kernel.org
Cc: linux...@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-remotep...@vger.kernel.org
Cc: linux-renesas-...@vger.kernel.org
Cc: linux-s...@vger.kernel.org
Cc: linux-s...@vger.kernel.org
Cc: linux...@vger.kernel.org
Cc: linux-stag...@lists.linux.dev
Cc: linux-te...@vger.kernel.org
Cc: linux...@lists.infradead.org
Cc: linux-...@vger.kernel.org
Cc: linux-xte...@linux-xtensa.org
Cc: net...@vger.kernel.org
Cc: openipmi-develo...@lists.sourceforge.net
Cc: r...@vger.kernel.org
Cc: sparcli...@vger.kernel.org
Cc: x...@kernel.org
Cc: xen-de...@lists.xenproject.org
---
include/linux/notifier.h | 8
kernel/notifier.c| 36 +++-
2 files changed, 23 insertions(+), 21 deletions(-)
diff --git a/include/linux/notifier.h b/include/linux/notifier.h
index 87069b8459af..45cc5a8d0fd8 100644
--- a/include/linux/notifier.h
+++ b/include/linux/notifier.h
@@ -141,13 +141,13 @@ extern void srcu_init_notifier_head(struct
srcu_notifier_head *nh);
#ifdef __KERNEL__
-extern int atomic_notifier_chain_register(struct atomic_notifier_head *nh,
+extern int __must_check atomic_notifier_chain_register(struct
atomic_notifier_head *nh,
struct notifier_block *nb);
-extern int blocking_notifier_chain_register(struct blocking_notifier_head *nh,
+extern int __must_check blocking_notifier_chain_register(struct
blocking_notifier_head *nh,
struct notifier_block *nb);
-extern int raw_notifier_chain_register(struct raw_notifier_head *nh,
+extern int __must_check raw_notifier_chain_register(struct raw_notifier_head
*nh,
struct notifier_block *nb);
-extern int srcu_notifier_chain_register(struct srcu_notifier_head *nh,
+extern int __must_check srcu_notifier_chain_register(struct srcu_notifier_head
*nh,
struct notifier_block *nb);
extern int atomic_notifier_chain_unregister(struct atomic_notifier_head *nh,
diff --git a/kernel/notifier.c b/kernel/notifier.c
index b8251dc0bc0f..451ef3f73ad2 100644
--- a/kernel/notifier.c
+++ b/kernel/notifier.c
@@ -20,13 +20,11 @@ BLOCKING_NOTIFIER_HEAD(reboot_notifier_list);
*/
static int notifier_chain_register(struct notifier_block **nl,
- struct notifier_block *n)
+ struct notifier_block *n)
{
while ((*nl) != NULL) {
- if (unlikely((*nl) == n)) {
- WARN(1, "double register detected");
- return 0;
- }
+ if (unlikely((*nl) == n))
+ return -EEXIST;
if (n->priority > (*nl)->priority)
break;
nl = &((*nl)->next);
@@ -134,10 +132,11 @@ static int notifier_call_chain_robust(struct
notifier_block **nl,
*
* Adds a notifier to an atomic notifier chain.
*
- * Currently always returns zero.
+ * Returns 0 on success, %-EEXIST on error.
*/
-int atomic_notifier_chain_register(struct atomic_notifier_head *nh,
- struct notifier_block *n)
+int __must_check
+atomic_notifier_chain_register(struct atomic_notifier_head *nh,
+ struct notifier_block *n)
{
unsigned long flags;
int ret;
@@ -216,10 +215,11 @@ NOKPROBE_SYMBOL(atomic_notifier_call_chain);
* Adds a notifier to
[PATCH v0 34/42] powerpc: Check notifier registration return value
From: Borislav Petkov
Avoid homegrown notifier registration checks.
No functional changes.
Signed-off-by: Borislav Petkov
Cc: linuxppc-dev@lists.ozlabs.org
---
arch/powerpc/kernel/setup-common.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/setup-common.c
b/arch/powerpc/kernel/setup-common.c
index b1e43b69a559..32f9900cd4f4 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -725,14 +725,18 @@ static struct notifier_block kernel_offset_notifier = {
void __init setup_panic(void)
{
- if (IS_ENABLED(CONFIG_RANDOMIZE_BASE) && kaslr_offset() > 0)
- atomic_notifier_chain_register(_notifier_list,
- _offset_notifier);
+ if (IS_ENABLED(CONFIG_RANDOMIZE_BASE) && kaslr_offset() > 0) {
+ if (atomic_notifier_chain_register(_notifier_list,
+ _offset_notifier))
+ pr_warn("Kernel offset notifier already registered\n");
+ }
/* PPC64 always does a hard irq disable in its panic handler */
if (!IS_ENABLED(CONFIG_PPC64) && !ppc_md.panic)
return;
- atomic_notifier_chain_register(_notifier_list, _panic_block);
+
+ if (atomic_notifier_chain_register(_notifier_list,
_panic_block))
+ pr_warn("Panic notifier already registered\n");
}
#ifdef CONFIG_CHECK_CACHE_COHERENCY
--
2.29.2
[PATCH v0 32/42] macintosh/adb: Check notifier registration return value
From: Borislav Petkov
Avoid homegrown notifier registration checks.
No functional changes.
Reported-by: kernel test robot
Signed-off-by: Borislav Petkov
Cc: linuxppc-dev@lists.ozlabs.org
---
drivers/macintosh/adbhid.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/macintosh/adbhid.c b/drivers/macintosh/adbhid.c
index 994ba5cb3678..c8cbf8588186 100644
--- a/drivers/macintosh/adbhid.c
+++ b/drivers/macintosh/adbhid.c
@@ -1262,8 +1262,8 @@ static int __init adbhid_init(void)
adbhid_probe();
- blocking_notifier_chain_register(_client_list,
- _adb_notifier);
+ if (blocking_notifier_chain_register(_client_list,
_adb_notifier))
+ pr_warn("ADB message notifier already registered\n");
return 0;
}
--
2.29.2
[PATCH v0 00/42] notifiers: Return an error when callback is already registered
From: Borislav Petkov Hi all, this is a huge patchset for something which is really trivial - it changes the notifier registration routines to return an error value if a notifier callback is already present on the respective list of callbacks. For more details scroll to the last patch. Everything before it is converting the callers to check the return value of the registration routines and issue a warning, instead of the WARN() notifier_chain_register() does now. Before the last patch has been applied, though, that checking is a NOP which would make the application of those patches trivial - every maintainer can pick a patch at her/his discretion - only the last one enables the build warnings and that one will be queued only after the preceding patches have all been merged so that there are no build warnings. Due to the sheer volume of the patches, I have addressed the respective patch and the last one, which enables the warning, with addressees for each maintained area so as not to spam people unnecessarily. If people prefer I carry some through tip, instead, I'll gladly do so - your call. And, if you think the warning messages need to be more precise, feel free to adjust them before committing. Thanks! Cc: Arnd Bergmann Cc: Ayush Sawal Cc: Greg Kroah-Hartman Cc: Rohit Maheshwari Cc: Steven Rostedt Cc: Vinay Kumar Yadav Cc: alsa-de...@alsa-project.org Cc: bcm-kernel-feedback-l...@broadcom.com Cc: intel-...@lists.freedesktop.org Cc: intel-gvt-...@lists.freedesktop.org Cc: linux-al...@vger.kernel.org Cc: linux-arm-ker...@lists.infradead.org Cc: linux-arm-ker...@lists.infradead.org Cc: linux-...@vger.kernel.org Cc: linux-cry...@vger.kernel.org Cc: linux-e...@vger.kernel.org Cc: linux-fb...@vger.kernel.org Cc: linux-hyp...@vger.kernel.org Cc: linux-...@vger.kernel.org Cc: linux-l...@vger.kernel.org Cc: linux-m...@vger.kernel.org Cc: linux-par...@vger.kernel.org Cc: linux...@vger.kernel.org Cc: linuxppc-dev@lists.ozlabs.org Cc: linux-remotep...@vger.kernel.org Cc: linux-renesas-...@vger.kernel.org Cc: linux-s...@vger.kernel.org Cc: linux-s...@vger.kernel.org Cc: linux...@vger.kernel.org Cc: linux-stag...@lists.linux.dev Cc: linux-te...@vger.kernel.org Cc: linux...@lists.infradead.org Cc: linux-...@vger.kernel.org Cc: linux-xte...@linux-xtensa.org Cc: net...@vger.kernel.org Cc: openipmi-develo...@lists.sourceforge.net Cc: r...@vger.kernel.org Cc: sparcli...@vger.kernel.org Cc: x...@kernel.org Cc: xen-de...@lists.xenproject.org Borislav Petkov (42): x86: Check notifier registration return value xen/x86: Check notifier registration return value impi: Check notifier registration return value clk: renesas: Check notifier registration return value dca: Check notifier registration return value firmware: Check notifier registration return value drm/i915: Check notifier registration return value Drivers: hv: vmbus: Check notifier registration return value iio: proximity: cros_ec: Check notifier registration return value leds: trigger: Check notifier registration return value misc: Check notifier registration return value ethernet: chelsio: Check notifier registration return value power: reset: Check notifier registration return value remoteproc: Check notifier registration return value scsi: target: Check notifier registration return value USB: Check notifier registration return value drivers: video: Check notifier registration return value drivers/xen: Check notifier registration return value kernel/hung_task: Check notifier registration return value rcu: Check notifier registration return value tracing: Check notifier registration return value net: fib_notifier: Check notifier registration return value ASoC: soc-jack: Check notifier registration return value staging: olpc_dcon: Check notifier registration return value arch/um: Check notifier registration return value alpha: Check notifier registration return value bus: brcmstb_gisb: Check notifier registration return value soc: bcm: brcmstb: pm: pm-arm: Check notifier registration return value arm64: Check notifier registration return value soc/tegra: Check notifier registration return value parisc: Check notifier registration return value macintosh/adb: Check notifier registration return value mips: Check notifier registration return value powerpc: Check notifier registration return value sh: Check notifier registration return value s390: Check notifier registration return value sparc: Check notifier registration return value xtensa: Check notifier registration return value crypto: ccree - check notifier registration return value EDAC/altera: Check notifier registration return value power: supply: ab8500: Check notifier registration return value notifier: Return an error when callback is already registered arch/alpha/kernel/setup.c | 5 +-- arch/arm64/kernel/setup.c | 6 ++-- arch/mips/kernel/relocate.c
Re: [PATCH v4 0/8] Implement generic cc_platform_has() helper function
On Tue, Sep 28, 2021 at 02:01:57PM -0700, Kuppuswamy, Sathyanarayanan wrote: > Yes. But, since the check is related to TDX, I just want to confirm whether > you are fine with naming the function as intel_*(). Why is this such a big of a deal?! There's amd_cc_platform_has() and intel_cc_platform_has() will be the corresponding Intel version. > Since this patch is going to have dependency on TDX code, I will include > this patch in TDX patch set. Ok. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v4 0/8] Implement generic cc_platform_has() helper function
On Tue, Sep 28, 2021 at 01:48:46PM -0700, Kuppuswamy, Sathyanarayanan wrote: > Just read it. If you want to use cpuid_has_tdx_guest() directly in > cc_platform_has(), then you want to rename intel_cc_platform_has() to > tdx_cc_platform_has()? Why? You simply do: if (cpuid_has_tdx_guest()) intel_cc_platform_has(...); and lemme paste from that mail: " ...you should use cpuid_has_tdx_guest() instead but cache its result so that you don't call CPUID each time the kernel executes cc_platform_has()." Makes sense? -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v4 0/8] Implement generic cc_platform_has() helper function
On Tue, Sep 28, 2021 at 12:19:49PM -0700, Kuppuswamy, Sathyanarayanan wrote: > Intel CC support patch is not included in this series. You want me > to address the issue raised by Joerg before merging it? Did you not see my email to you today: https://lkml.kernel.org/r/yvl4zughfsh1q...@zn.tnic ? -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
[PATCH 8/8] treewide: Replace the use of mem_encrypt_active() with cc_platform_has()
From: Tom Lendacky
Replace uses of mem_encrypt_active() with calls to cc_platform_has() with
the CC_ATTR_MEM_ENCRYPT attribute.
Remove the implementation of mem_encrypt_active() across all arches.
For s390, since the default implementation of the cc_platform_has()
matches the s390 implementation of mem_encrypt_active(), cc_platform_has()
does not need to be implemented in s390 (the config option
ARCH_HAS_CC_PLATFORM is not set).
Signed-off-by: Tom Lendacky
Signed-off-by: Borislav Petkov
---
arch/powerpc/include/asm/mem_encrypt.h | 5 -
arch/powerpc/platforms/pseries/svm.c| 5 +++--
arch/s390/include/asm/mem_encrypt.h | 2 --
arch/x86/include/asm/mem_encrypt.h | 5 -
arch/x86/kernel/head64.c| 9 +++--
arch/x86/mm/ioremap.c | 4 ++--
arch/x86/mm/mem_encrypt.c | 2 +-
arch/x86/mm/pat/set_memory.c| 3 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 +++-
drivers/gpu/drm/drm_cache.c | 4 ++--
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 4 ++--
drivers/gpu/drm/vmwgfx/vmwgfx_msg.c | 6 +++---
drivers/iommu/amd/iommu.c | 3 ++-
drivers/iommu/amd/iommu_v2.c| 3 ++-
drivers/iommu/iommu.c | 3 ++-
fs/proc/vmcore.c| 6 +++---
include/linux/mem_encrypt.h | 4
kernel/dma/swiotlb.c| 4 ++--
18 files changed, 36 insertions(+), 40 deletions(-)
diff --git a/arch/powerpc/include/asm/mem_encrypt.h
b/arch/powerpc/include/asm/mem_encrypt.h
index ba9dab07c1be..2f26b8fc8d29 100644
--- a/arch/powerpc/include/asm/mem_encrypt.h
+++ b/arch/powerpc/include/asm/mem_encrypt.h
@@ -10,11 +10,6 @@
#include
-static inline bool mem_encrypt_active(void)
-{
- return is_secure_guest();
-}
-
static inline bool force_dma_unencrypted(struct device *dev)
{
return is_secure_guest();
diff --git a/arch/powerpc/platforms/pseries/svm.c
b/arch/powerpc/platforms/pseries/svm.c
index 87f001b4c4e4..c083ecbbae4d 100644
--- a/arch/powerpc/platforms/pseries/svm.c
+++ b/arch/powerpc/platforms/pseries/svm.c
@@ -8,6 +8,7 @@
#include
#include
+#include
#include
#include
#include
@@ -63,7 +64,7 @@ void __init svm_swiotlb_init(void)
int set_memory_encrypted(unsigned long addr, int numpages)
{
- if (!mem_encrypt_active())
+ if (!cc_platform_has(CC_ATTR_MEM_ENCRYPT))
return 0;
if (!PAGE_ALIGNED(addr))
@@ -76,7 +77,7 @@ int set_memory_encrypted(unsigned long addr, int numpages)
int set_memory_decrypted(unsigned long addr, int numpages)
{
- if (!mem_encrypt_active())
+ if (!cc_platform_has(CC_ATTR_MEM_ENCRYPT))
return 0;
if (!PAGE_ALIGNED(addr))
diff --git a/arch/s390/include/asm/mem_encrypt.h
b/arch/s390/include/asm/mem_encrypt.h
index 2542cbf7e2d1..08a8b96606d7 100644
--- a/arch/s390/include/asm/mem_encrypt.h
+++ b/arch/s390/include/asm/mem_encrypt.h
@@ -4,8 +4,6 @@
#ifndef __ASSEMBLY__
-static inline bool mem_encrypt_active(void) { return false; }
-
int set_memory_encrypted(unsigned long addr, int numpages);
int set_memory_decrypted(unsigned long addr, int numpages);
diff --git a/arch/x86/include/asm/mem_encrypt.h
b/arch/x86/include/asm/mem_encrypt.h
index da14ede311aa..2d4f5c17d79c 100644
--- a/arch/x86/include/asm/mem_encrypt.h
+++ b/arch/x86/include/asm/mem_encrypt.h
@@ -96,11 +96,6 @@ static inline void mem_encrypt_free_decrypted_mem(void) { }
extern char __start_bss_decrypted[], __end_bss_decrypted[],
__start_bss_decrypted_unused[];
-static inline bool mem_encrypt_active(void)
-{
- return sme_me_mask;
-}
-
static inline u64 sme_get_me_mask(void)
{
return sme_me_mask;
diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index de01903c3735..fc5371a7e9d1 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -19,7 +19,7 @@
#include
#include
#include
-#include
+#include
#include
#include
@@ -284,8 +284,13 @@ unsigned long __head __startup_64(unsigned long physaddr,
* The bss section will be memset to zero later in the initialization so
* there is no need to zero it after changing the memory encryption
* attribute.
+*
+* This is early code, use an open coded check for SME instead of
+* using cc_platform_has(). This eliminates worries about removing
+* instrumentation or checking boot_cpu_data in the cc_platform_has()
+* function.
*/
- if (mem_encrypt_active()) {
+ if (sme_get_me_mask()) {
vaddr = (unsigned long)__start_bss_decrypted;
vaddr_end = (unsigned long)__end_bss_decrypted;
for (; vaddr < vaddr_end; vaddr += PMD_SIZE) {
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index b59a5cbc6bc5..026031b3b782 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -694,7 +694,7 @@ static bool __i
[PATCH 5/8] x86/sme: Replace occurrences of sme_active() with cc_platform_has()
From: Tom Lendacky
Replace uses of sme_active() with the more generic cc_platform_has()
using CC_ATTR_HOST_MEM_ENCRYPT. If future support is added for other
memory encryption technologies, the use of CC_ATTR_HOST_MEM_ENCRYPT
can be updated, as required.
This also replaces two usages of sev_active() that are really geared
towards detecting if SME is active.
Signed-off-by: Tom Lendacky
Signed-off-by: Borislav Petkov
---
arch/x86/include/asm/kexec.h | 2 +-
arch/x86/include/asm/mem_encrypt.h | 2 --
arch/x86/kernel/machine_kexec_64.c | 15 ---
arch/x86/kernel/pci-swiotlb.c| 9 -
arch/x86/kernel/relocate_kernel_64.S | 2 +-
arch/x86/mm/ioremap.c| 6 +++---
arch/x86/mm/mem_encrypt.c| 13 -
arch/x86/mm/mem_encrypt_identity.c | 9 -
arch/x86/realmode/init.c | 5 +++--
drivers/iommu/amd/init.c | 7 ---
10 files changed, 36 insertions(+), 34 deletions(-)
diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h
index 0a6e34b07017..11b7c06e2828 100644
--- a/arch/x86/include/asm/kexec.h
+++ b/arch/x86/include/asm/kexec.h
@@ -129,7 +129,7 @@ relocate_kernel(unsigned long indirection_page,
unsigned long page_list,
unsigned long start_address,
unsigned int preserve_context,
- unsigned int sme_active);
+ unsigned int host_mem_enc_active);
#endif
#define ARCH_HAS_KIMAGE_ARCH
diff --git a/arch/x86/include/asm/mem_encrypt.h
b/arch/x86/include/asm/mem_encrypt.h
index 3fb9f5ebefa4..63c5b99ccae5 100644
--- a/arch/x86/include/asm/mem_encrypt.h
+++ b/arch/x86/include/asm/mem_encrypt.h
@@ -51,7 +51,6 @@ void __init mem_encrypt_free_decrypted_mem(void);
void __init mem_encrypt_init(void);
void __init sev_es_init_vc_handling(void);
-bool sme_active(void);
bool sev_active(void);
bool sev_es_active(void);
@@ -76,7 +75,6 @@ static inline void __init sme_encrypt_kernel(struct
boot_params *bp) { }
static inline void __init sme_enable(struct boot_params *bp) { }
static inline void sev_es_init_vc_handling(void) { }
-static inline bool sme_active(void) { return false; }
static inline bool sev_active(void) { return false; }
static inline bool sev_es_active(void) { return false; }
diff --git a/arch/x86/kernel/machine_kexec_64.c
b/arch/x86/kernel/machine_kexec_64.c
index 131f30fdcfbd..7040c0fa921c 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
#include
#include
@@ -358,7 +359,7 @@ void machine_kexec(struct kimage *image)
(unsigned long)page_list,
image->start,
image->preserve_context,
- sme_active());
+
cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT));
#ifdef CONFIG_KEXEC_JUMP
if (image->preserve_context)
@@ -569,12 +570,12 @@ void arch_kexec_unprotect_crashkres(void)
*/
int arch_kexec_post_alloc_pages(void *vaddr, unsigned int pages, gfp_t gfp)
{
- if (sev_active())
+ if (!cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT))
return 0;
/*
-* If SME is active we need to be sure that kexec pages are
-* not encrypted because when we boot to the new kernel the
+* If host memory encryption is active we need to be sure that kexec
+* pages are not encrypted because when we boot to the new kernel the
* pages won't be accessed encrypted (initially).
*/
return set_memory_decrypted((unsigned long)vaddr, pages);
@@ -582,12 +583,12 @@ int arch_kexec_post_alloc_pages(void *vaddr, unsigned int
pages, gfp_t gfp)
void arch_kexec_pre_free_pages(void *vaddr, unsigned int pages)
{
- if (sev_active())
+ if (!cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT))
return;
/*
-* If SME is active we need to reset the pages back to being
-* an encrypted mapping before freeing them.
+* If host memory encryption is active we need to reset the pages back
+* to being an encrypted mapping before freeing them.
*/
set_memory_encrypted((unsigned long)vaddr, pages);
}
diff --git a/arch/x86/kernel/pci-swiotlb.c b/arch/x86/kernel/pci-swiotlb.c
index c2cfa5e7c152..814ab46a0dad 100644
--- a/arch/x86/kernel/pci-swiotlb.c
+++ b/arch/x86/kernel/pci-swiotlb.c
@@ -6,7 +6,7 @@
#include
#include
#include
-#include
+#include
#include
#include
@@ -45,11 +45,10 @@ int __init pci_swiotlb_detect_4gb(void)
swiotlb = 1;
/*
-* If SME is active then swiotlb will be set to 1 so that bounce
-* buffers are allocated and used for devices that do not support
-* the addressing range required for the encryp
[PATCH 7/8] x86/sev: Replace occurrences of sev_es_active() with cc_platform_has()
From: Tom Lendacky
Replace uses of sev_es_active() with the more generic cc_platform_has()
using CC_ATTR_GUEST_STATE_ENCRYPT. If future support is added for other
memory encyrption techonologies, the use of CC_ATTR_GUEST_STATE_ENCRYPT
can be updated, as required.
Signed-off-by: Tom Lendacky
Signed-off-by: Borislav Petkov
---
arch/x86/include/asm/mem_encrypt.h | 2 --
arch/x86/kernel/sev.c | 6 +++---
arch/x86/mm/mem_encrypt.c | 24 +++-
arch/x86/realmode/init.c | 3 +--
4 files changed, 7 insertions(+), 28 deletions(-)
diff --git a/arch/x86/include/asm/mem_encrypt.h
b/arch/x86/include/asm/mem_encrypt.h
index a5a58ccd1ee3..da14ede311aa 100644
--- a/arch/x86/include/asm/mem_encrypt.h
+++ b/arch/x86/include/asm/mem_encrypt.h
@@ -51,7 +51,6 @@ void __init mem_encrypt_free_decrypted_mem(void);
void __init mem_encrypt_init(void);
void __init sev_es_init_vc_handling(void);
-bool sev_es_active(void);
#define __bss_decrypted __section(".bss..decrypted")
@@ -74,7 +73,6 @@ static inline void __init sme_encrypt_kernel(struct
boot_params *bp) { }
static inline void __init sme_enable(struct boot_params *bp) { }
static inline void sev_es_init_vc_handling(void) { }
-static inline bool sev_es_active(void) { return false; }
static inline int __init
early_set_memory_decrypted(unsigned long vaddr, unsigned long size) { return
0; }
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index a6895e440bc3..53a6837d354b 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -11,7 +11,7 @@
#include /* For show_regs() */
#include
-#include
+#include
#include
#include
#include
@@ -615,7 +615,7 @@ int __init sev_es_efi_map_ghcbs(pgd_t *pgd)
int cpu;
u64 pfn;
- if (!sev_es_active())
+ if (!cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT))
return 0;
pflags = _PAGE_NX | _PAGE_RW;
@@ -774,7 +774,7 @@ void __init sev_es_init_vc_handling(void)
BUILD_BUG_ON(offsetof(struct sev_es_runtime_data, ghcb_page) %
PAGE_SIZE);
- if (!sev_es_active())
+ if (!cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT))
return;
if (!sev_es_check_cpu_features())
diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index 932007a6913b..2d04c39bea1d 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -361,25 +361,6 @@ int __init early_set_memory_encrypted(unsigned long vaddr,
unsigned long size)
return early_set_memory_enc_dec(vaddr, size, true);
}
-/*
- * SME and SEV are very similar but they are not the same, so there are
- * times that the kernel will need to distinguish between SME and SEV. The
- * cc_platform_has() function is used for this. When a distinction isn't
- * needed, the CC_ATTR_MEM_ENCRYPT attribute can be used.
- *
- * The trampoline code is a good example for this requirement. Before
- * paging is activated, SME will access all memory as decrypted, but SEV
- * will access all memory as encrypted. So, when APs are being brought
- * up under SME the trampoline area cannot be encrypted, whereas under SEV
- * the trampoline area must be encrypted.
- */
-
-/* Needs to be called from non-instrumentable code */
-bool noinstr sev_es_active(void)
-{
- return sev_status & MSR_AMD64_SEV_ES_ENABLED;
-}
-
/* Override for DMA direct allocation check - ARCH_HAS_FORCE_DMA_UNENCRYPTED */
bool force_dma_unencrypted(struct device *dev)
{
@@ -449,7 +430,7 @@ static void print_mem_encrypt_feature_info(void)
pr_cont(" SEV");
/* Encrypted Register State */
- if (sev_es_active())
+ if (cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT))
pr_cont(" SEV-ES");
pr_cont("\n");
@@ -468,7 +449,8 @@ void __init mem_encrypt_init(void)
* With SEV, we need to unroll the rep string I/O instructions,
* but SEV-ES supports them through the #VC handler.
*/
- if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT) && !sev_es_active())
+ if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT) &&
+ !cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT))
static_branch_enable(_enable_key);
print_mem_encrypt_feature_info();
diff --git a/arch/x86/realmode/init.c b/arch/x86/realmode/init.c
index c878c5ee5a4c..4a3da7592b99 100644
--- a/arch/x86/realmode/init.c
+++ b/arch/x86/realmode/init.c
@@ -2,7 +2,6 @@
#include
#include
#include
-#include
#include
#include
@@ -48,7 +47,7 @@ static void sme_sev_setup_real_mode(struct trampoline_header
*th)
if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT))
th->flags |= TH_FLAGS_SME_ACTIVE;
- if (sev_es_active()) {
+ if (cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT)) {
/*
* Skip the call to verify_cpu() in secondary_startup_64 as it
[PATCH 6/8] x86/sev: Replace occurrences of sev_active() with cc_platform_has()
From: Tom Lendacky
Replace uses of sev_active() with the more generic cc_platform_has()
using CC_ATTR_GUEST_MEM_ENCRYPT. If future support is added for other
memory encryption technologies, the use of CC_ATTR_GUEST_MEM_ENCRYPT
can be updated, as required.
Signed-off-by: Tom Lendacky
Signed-off-by: Borislav Petkov
---
arch/x86/include/asm/mem_encrypt.h | 2 --
arch/x86/kernel/crash_dump_64.c| 4 +++-
arch/x86/kernel/kvm.c | 3 ++-
arch/x86/kernel/kvmclock.c | 4 ++--
arch/x86/kernel/machine_kexec_64.c | 4 ++--
arch/x86/kvm/svm/svm.c | 3 ++-
arch/x86/mm/ioremap.c | 6 +++---
arch/x86/mm/mem_encrypt.c | 21 -
arch/x86/platform/efi/efi_64.c | 9 +
9 files changed, 27 insertions(+), 29 deletions(-)
diff --git a/arch/x86/include/asm/mem_encrypt.h
b/arch/x86/include/asm/mem_encrypt.h
index 63c5b99ccae5..a5a58ccd1ee3 100644
--- a/arch/x86/include/asm/mem_encrypt.h
+++ b/arch/x86/include/asm/mem_encrypt.h
@@ -51,7 +51,6 @@ void __init mem_encrypt_free_decrypted_mem(void);
void __init mem_encrypt_init(void);
void __init sev_es_init_vc_handling(void);
-bool sev_active(void);
bool sev_es_active(void);
#define __bss_decrypted __section(".bss..decrypted")
@@ -75,7 +74,6 @@ static inline void __init sme_encrypt_kernel(struct
boot_params *bp) { }
static inline void __init sme_enable(struct boot_params *bp) { }
static inline void sev_es_init_vc_handling(void) { }
-static inline bool sev_active(void) { return false; }
static inline bool sev_es_active(void) { return false; }
static inline int __init
diff --git a/arch/x86/kernel/crash_dump_64.c b/arch/x86/kernel/crash_dump_64.c
index 045e82e8945b..a7f617a3981d 100644
--- a/arch/x86/kernel/crash_dump_64.c
+++ b/arch/x86/kernel/crash_dump_64.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
static ssize_t __copy_oldmem_page(unsigned long pfn, char *buf, size_t csize,
unsigned long offset, int userbuf,
@@ -73,5 +74,6 @@ ssize_t copy_oldmem_page_encrypted(unsigned long pfn, char
*buf, size_t csize,
ssize_t elfcorehdr_read(char *buf, size_t count, u64 *ppos)
{
- return read_from_oldmem(buf, count, ppos, 0, sev_active());
+ return read_from_oldmem(buf, count, ppos, 0,
+ cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT));
}
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index b656456c3a94..8863d1941f1b 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -418,7 +419,7 @@ static void __init sev_map_percpu_data(void)
{
int cpu;
- if (!sev_active())
+ if (!cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
return;
for_each_possible_cpu(cpu) {
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index ad273e5861c1..fc3930c5db1b 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -16,9 +16,9 @@
#include
#include
#include
+#include
#include
-#include
#include
#include
@@ -232,7 +232,7 @@ static void __init kvmclock_init_mem(void)
* hvclock is shared between the guest and the hypervisor, must
* be mapped decrypted.
*/
- if (sev_active()) {
+ if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT)) {
r = set_memory_decrypted((unsigned long) hvclock_mem,
1UL << order);
if (r) {
diff --git a/arch/x86/kernel/machine_kexec_64.c
b/arch/x86/kernel/machine_kexec_64.c
index 7040c0fa921c..f5da4a18070a 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -167,7 +167,7 @@ static int init_transition_pgtable(struct kimage *image,
pgd_t *pgd)
}
pte = pte_offset_kernel(pmd, vaddr);
- if (sev_active())
+ if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
prot = PAGE_KERNEL_EXEC;
set_pte(pte, pfn_pte(paddr >> PAGE_SHIFT, prot));
@@ -207,7 +207,7 @@ static int init_pgtable(struct kimage *image, unsigned long
start_pgtable)
level4p = (pgd_t *)__va(start_pgtable);
clear_page(level4p);
- if (sev_active()) {
+ if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT)) {
info.page_flag |= _PAGE_ENC;
info.kernpg_flag |= _PAGE_ENC;
}
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 05e8d4d27969..a2f78a8cfdaa 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
#include
#include
@@ -455,7 +456,7 @@ static int has_svm(void)
return 0;
}
- if (sev_active()) {
+ if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT)) {
pr_info("KVM is unsupported whe
[PATCH v4 0/8] Implement generic cc_platform_has() helper function
From: Borislav Petkov Hi all, here's v4 of the cc_platform_has() patchset with feedback incorporated. I'm going to route this through tip if there are no objections. Thx. Tom Lendacky (8): x86/ioremap: Selectively build arch override encryption functions arch/cc: Introduce a function to check for confidential computing features x86/sev: Add an x86 version of cc_platform_has() powerpc/pseries/svm: Add a powerpc version of cc_platform_has() x86/sme: Replace occurrences of sme_active() with cc_platform_has() x86/sev: Replace occurrences of sev_active() with cc_platform_has() x86/sev: Replace occurrences of sev_es_active() with cc_platform_has() treewide: Replace the use of mem_encrypt_active() with cc_platform_has() arch/Kconfig | 3 + arch/powerpc/include/asm/mem_encrypt.h | 5 -- arch/powerpc/platforms/pseries/Kconfig | 1 + arch/powerpc/platforms/pseries/Makefile | 2 + arch/powerpc/platforms/pseries/cc_platform.c | 26 ++ arch/powerpc/platforms/pseries/svm.c | 5 +- arch/s390/include/asm/mem_encrypt.h | 2 - arch/x86/Kconfig | 1 + arch/x86/include/asm/io.h| 8 ++ arch/x86/include/asm/kexec.h | 2 +- arch/x86/include/asm/mem_encrypt.h | 12 +-- arch/x86/kernel/Makefile | 6 ++ arch/x86/kernel/cc_platform.c| 69 +++ arch/x86/kernel/crash_dump_64.c | 4 +- arch/x86/kernel/head64.c | 9 +- arch/x86/kernel/kvm.c| 3 +- arch/x86/kernel/kvmclock.c | 4 +- arch/x86/kernel/machine_kexec_64.c | 19 +++-- arch/x86/kernel/pci-swiotlb.c| 9 +- arch/x86/kernel/relocate_kernel_64.S | 2 +- arch/x86/kernel/sev.c| 6 +- arch/x86/kvm/svm/svm.c | 3 +- arch/x86/mm/ioremap.c| 18 ++-- arch/x86/mm/mem_encrypt.c| 55 arch/x86/mm/mem_encrypt_identity.c | 9 +- arch/x86/mm/pat/set_memory.c | 3 +- arch/x86/platform/efi/efi_64.c | 9 +- arch/x86/realmode/init.c | 8 +- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 +- drivers/gpu/drm/drm_cache.c | 4 +- drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 4 +- drivers/gpu/drm/vmwgfx/vmwgfx_msg.c | 6 +- drivers/iommu/amd/init.c | 7 +- drivers/iommu/amd/iommu.c| 3 +- drivers/iommu/amd/iommu_v2.c | 3 +- drivers/iommu/iommu.c| 3 +- fs/proc/vmcore.c | 6 +- include/linux/cc_platform.h | 88 include/linux/mem_encrypt.h | 4 - kernel/dma/swiotlb.c | 4 +- 40 files changed, 310 insertions(+), 129 deletions(-) create mode 100644 arch/powerpc/platforms/pseries/cc_platform.c create mode 100644 arch/x86/kernel/cc_platform.c create mode 100644 include/linux/cc_platform.h -- 2.29.2
[PATCH 3/8] x86/sev: Add an x86 version of cc_platform_has()
From: Tom Lendacky
Introduce an x86 version of the cc_platform_has() function. This will be
used to replace vendor specific calls like sme_active(), sev_active(),
etc.
Signed-off-by: Tom Lendacky
Signed-off-by: Borislav Petkov
---
arch/x86/Kconfig | 1 +
arch/x86/include/asm/mem_encrypt.h | 1 +
arch/x86/kernel/Makefile | 6 +++
arch/x86/kernel/cc_platform.c | 69 ++
arch/x86/mm/mem_encrypt.c | 1 +
5 files changed, 78 insertions(+)
create mode 100644 arch/x86/kernel/cc_platform.c
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index ab83c22d274e..9f190ec4f953 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1518,6 +1518,7 @@ config AMD_MEM_ENCRYPT
select ARCH_HAS_FORCE_DMA_UNENCRYPTED
select INSTRUCTION_DECODER
select ARCH_HAS_RESTRICTED_VIRTIO_MEMORY_ACCESS
+ select ARCH_HAS_CC_PLATFORM
help
Say yes to enable support for the encryption of system memory.
This requires an AMD processor that supports Secure Memory
diff --git a/arch/x86/include/asm/mem_encrypt.h
b/arch/x86/include/asm/mem_encrypt.h
index 9c80c68d75b5..3fb9f5ebefa4 100644
--- a/arch/x86/include/asm/mem_encrypt.h
+++ b/arch/x86/include/asm/mem_encrypt.h
@@ -13,6 +13,7 @@
#ifndef __ASSEMBLY__
#include
+#include
#include
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 8f4e8fa6ed75..2ff3e600f426 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -21,6 +21,7 @@ CFLAGS_REMOVE_ftrace.o = -pg
CFLAGS_REMOVE_early_printk.o = -pg
CFLAGS_REMOVE_head64.o = -pg
CFLAGS_REMOVE_sev.o = -pg
+CFLAGS_REMOVE_cc_platform.o = -pg
endif
KASAN_SANITIZE_head$(BITS).o := n
@@ -29,6 +30,7 @@ KASAN_SANITIZE_dumpstack_$(BITS).o:= n
KASAN_SANITIZE_stacktrace.o:= n
KASAN_SANITIZE_paravirt.o := n
KASAN_SANITIZE_sev.o := n
+KASAN_SANITIZE_cc_platform.o := n
# With some compiler versions the generated code results in boot hangs, caused
# by several compilation units. To be safe, disable all instrumentation.
@@ -47,6 +49,7 @@ endif
KCOV_INSTRUMENT:= n
CFLAGS_head$(BITS).o += -fno-stack-protector
+CFLAGS_cc_platform.o += -fno-stack-protector
CFLAGS_irq.o := -I $(srctree)/$(src)/../include/asm/trace
@@ -147,6 +150,9 @@ obj-$(CONFIG_UNWINDER_FRAME_POINTER)+=
unwind_frame.o
obj-$(CONFIG_UNWINDER_GUESS) += unwind_guess.o
obj-$(CONFIG_AMD_MEM_ENCRYPT) += sev.o
+
+obj-$(CONFIG_ARCH_HAS_CC_PLATFORM) += cc_platform.o
+
###
# 64 bit specific files
ifeq ($(CONFIG_X86_64),y)
diff --git a/arch/x86/kernel/cc_platform.c b/arch/x86/kernel/cc_platform.c
new file mode 100644
index ..03bb2f343ddb
--- /dev/null
+++ b/arch/x86/kernel/cc_platform.c
@@ -0,0 +1,69 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Confidential Computing Platform Capability checks
+ *
+ * Copyright (C) 2021 Advanced Micro Devices, Inc.
+ *
+ * Author: Tom Lendacky
+ */
+
+#include
+#include
+#include
+
+#include
+
+static bool __maybe_unused intel_cc_platform_has(enum cc_attr attr)
+{
+#ifdef CONFIG_INTEL_TDX_GUEST
+ return false;
+#else
+ return false;
+#endif
+}
+
+/*
+ * SME and SEV are very similar but they are not the same, so there are
+ * times that the kernel will need to distinguish between SME and SEV. The
+ * cc_platform_has() function is used for this. When a distinction isn't
+ * needed, the CC_ATTR_MEM_ENCRYPT attribute can be used.
+ *
+ * The trampoline code is a good example for this requirement. Before
+ * paging is activated, SME will access all memory as decrypted, but SEV
+ * will access all memory as encrypted. So, when APs are being brought
+ * up under SME the trampoline area cannot be encrypted, whereas under SEV
+ * the trampoline area must be encrypted.
+ */
+static bool amd_cc_platform_has(enum cc_attr attr)
+{
+#ifdef CONFIG_AMD_MEM_ENCRYPT
+ switch (attr) {
+ case CC_ATTR_MEM_ENCRYPT:
+ return sme_me_mask;
+
+ case CC_ATTR_HOST_MEM_ENCRYPT:
+ return sme_me_mask && !(sev_status & MSR_AMD64_SEV_ENABLED);
+
+ case CC_ATTR_GUEST_MEM_ENCRYPT:
+ return sev_status & MSR_AMD64_SEV_ENABLED;
+
+ case CC_ATTR_GUEST_STATE_ENCRYPT:
+ return sev_status & MSR_AMD64_SEV_ES_ENABLED;
+
+ default:
+ return false;
+ }
+#else
+ return false;
+#endif
+}
+
+
+bool cc_platform_has(enum cc_attr attr)
+{
+ if (sme_me_mask)
+ return amd_cc_platform_has(attr);
+
+ return false;
+}
+EXPORT_SYMBOL_GPL(cc_platform_has);
diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index ff08dc463634..e29b1418d00c 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encr
[PATCH 1/8] x86/ioremap: Selectively build arch override encryption functions
From: Tom Lendacky
In preparation for other uses of the cc_platform_has() function
besides AMD's memory encryption support, selectively build the
AMD memory encryption architecture override functions only when
CONFIG_AMD_MEM_ENCRYPT=y. These functions are:
- early_memremap_pgprot_adjust()
- arch_memremap_can_ram_remap()
Additionally, routines that are only invoked by these architecture
override functions can also be conditionally built. These functions are:
- memremap_should_map_decrypted()
- memremap_is_efi_data()
- memremap_is_setup_data()
- early_memremap_is_setup_data()
And finally, phys_mem_access_encrypted() is conditionally built as well,
but requires a static inline version of it when CONFIG_AMD_MEM_ENCRYPT is
not set.
Signed-off-by: Tom Lendacky
Signed-off-by: Borislav Petkov
---
arch/x86/include/asm/io.h | 8
arch/x86/mm/ioremap.c | 2 +-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
index 841a5d104afa..5c6a4af0b911 100644
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -391,6 +391,7 @@ extern void arch_io_free_memtype_wc(resource_size_t start,
resource_size_t size)
#define arch_io_reserve_memtype_wc arch_io_reserve_memtype_wc
#endif
+#ifdef CONFIG_AMD_MEM_ENCRYPT
extern bool arch_memremap_can_ram_remap(resource_size_t offset,
unsigned long size,
unsigned long flags);
@@ -398,6 +399,13 @@ extern bool arch_memremap_can_ram_remap(resource_size_t
offset,
extern bool phys_mem_access_encrypted(unsigned long phys_addr,
unsigned long size);
+#else
+static inline bool phys_mem_access_encrypted(unsigned long phys_addr,
+unsigned long size)
+{
+ return true;
+}
+#endif
/**
* iosubmit_cmds512 - copy data to single MMIO location, in 512-bit units
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index 60ade7dd71bd..ccff76cedd8f 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -508,6 +508,7 @@ void unxlate_dev_mem_ptr(phys_addr_t phys, void *addr)
memunmap((void *)((unsigned long)addr & PAGE_MASK));
}
+#ifdef CONFIG_AMD_MEM_ENCRYPT
/*
* Examine the physical address to determine if it is an area of memory
* that should be mapped decrypted. If the memory is not part of the
@@ -746,7 +747,6 @@ bool phys_mem_access_encrypted(unsigned long phys_addr,
unsigned long size)
return arch_memremap_can_ram_remap(phys_addr, size, 0);
}
-#ifdef CONFIG_AMD_MEM_ENCRYPT
/* Remap memory with encryption */
void __init *early_memremap_encrypted(resource_size_t phys_addr,
unsigned long size)
--
2.29.2
[PATCH 2/8] arch/cc: Introduce a function to check for confidential computing features
From: Tom Lendacky
In preparation for other confidential computing technologies, introduce
a generic helper function, cc_platform_has(), that can be used to
check for specific active confidential computing attributes, like
memory encryption. This is intended to eliminate having to add multiple
technology-specific checks to the code (e.g. if (sev_active() ||
tdx_active() || ... ).
[ bp: s/_CC_PLATFORM_H/_LINUX_CC_PLATFORM_H/g ]
Co-developed-by: Andi Kleen
Signed-off-by: Andi Kleen
Co-developed-by: Kuppuswamy Sathyanarayanan
Signed-off-by: Kuppuswamy Sathyanarayanan
Signed-off-by: Tom Lendacky
Signed-off-by: Borislav Petkov
---
arch/Kconfig| 3 ++
include/linux/cc_platform.h | 88 +
2 files changed, 91 insertions(+)
create mode 100644 include/linux/cc_platform.h
diff --git a/arch/Kconfig b/arch/Kconfig
index 8df1c7102643..d1e69d6e8498 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -1234,6 +1234,9 @@ config RELR
config ARCH_HAS_MEM_ENCRYPT
bool
+config ARCH_HAS_CC_PLATFORM
+ bool
+
config HAVE_SPARSE_SYSCALL_NR
bool
help
diff --git a/include/linux/cc_platform.h b/include/linux/cc_platform.h
new file mode 100644
index ..a075b70b9a70
--- /dev/null
+++ b/include/linux/cc_platform.h
@@ -0,0 +1,88 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Confidential Computing Platform Capability checks
+ *
+ * Copyright (C) 2021 Advanced Micro Devices, Inc.
+ *
+ * Author: Tom Lendacky
+ */
+
+#ifndef _LINUX_CC_PLATFORM_H
+#define _LINUX_CC_PLATFORM_H
+
+#include
+#include
+
+/**
+ * enum cc_attr - Confidential computing attributes
+ *
+ * These attributes represent confidential computing features that are
+ * currently active.
+ */
+enum cc_attr {
+ /**
+* @CC_ATTR_MEM_ENCRYPT: Memory encryption is active
+*
+* The platform/OS is running with active memory encryption. This
+* includes running either as a bare-metal system or a hypervisor
+* and actively using memory encryption or as a guest/virtual machine
+* and actively using memory encryption.
+*
+* Examples include SME, SEV and SEV-ES.
+*/
+ CC_ATTR_MEM_ENCRYPT,
+
+ /**
+* @CC_ATTR_HOST_MEM_ENCRYPT: Host memory encryption is active
+*
+* The platform/OS is running as a bare-metal system or a hypervisor
+* and actively using memory encryption.
+*
+* Examples include SME.
+*/
+ CC_ATTR_HOST_MEM_ENCRYPT,
+
+ /**
+* @CC_ATTR_GUEST_MEM_ENCRYPT: Guest memory encryption is active
+*
+* The platform/OS is running as a guest/virtual machine and actively
+* using memory encryption.
+*
+* Examples include SEV and SEV-ES.
+*/
+ CC_ATTR_GUEST_MEM_ENCRYPT,
+
+ /**
+* @CC_ATTR_GUEST_STATE_ENCRYPT: Guest state encryption is active
+*
+* The platform/OS is running as a guest/virtual machine and actively
+* using memory encryption and register state encryption.
+*
+* Examples include SEV-ES.
+*/
+ CC_ATTR_GUEST_STATE_ENCRYPT,
+};
+
+#ifdef CONFIG_ARCH_HAS_CC_PLATFORM
+
+/**
+ * cc_platform_has() - Checks if the specified cc_attr attribute is active
+ * @attr: Confidential computing attribute to check
+ *
+ * The cc_platform_has() function will return an indicator as to whether the
+ * specified Confidential Computing attribute is currently active.
+ *
+ * Context: Any context
+ * Return:
+ * * TRUE - Specified Confidential Computing attribute is active
+ * * FALSE - Specified Confidential Computing attribute is not active
+ */
+bool cc_platform_has(enum cc_attr attr);
+
+#else /* !CONFIG_ARCH_HAS_CC_PLATFORM */
+
+static inline bool cc_platform_has(enum cc_attr attr) { return false; }
+
+#endif /* CONFIG_ARCH_HAS_CC_PLATFORM */
+
+#endif /* _LINUX_CC_PLATFORM_H */
--
2.29.2
[PATCH 4/8] powerpc/pseries/svm: Add a powerpc version of cc_platform_has()
From: Tom Lendacky
Introduce a powerpc version of the cc_platform_has() function. This will
be used to replace the powerpc mem_encrypt_active() implementation, so
the implementation will initially only support the CC_ATTR_MEM_ENCRYPT
attribute.
Signed-off-by: Tom Lendacky
Signed-off-by: Borislav Petkov
Acked-by: Michael Ellerman
---
arch/powerpc/platforms/pseries/Kconfig | 1 +
arch/powerpc/platforms/pseries/Makefile | 2 ++
arch/powerpc/platforms/pseries/cc_platform.c | 26
3 files changed, 29 insertions(+)
create mode 100644 arch/powerpc/platforms/pseries/cc_platform.c
diff --git a/arch/powerpc/platforms/pseries/Kconfig
b/arch/powerpc/platforms/pseries/Kconfig
index 5e037df2a3a1..2e57391e0778 100644
--- a/arch/powerpc/platforms/pseries/Kconfig
+++ b/arch/powerpc/platforms/pseries/Kconfig
@@ -159,6 +159,7 @@ config PPC_SVM
select SWIOTLB
select ARCH_HAS_MEM_ENCRYPT
select ARCH_HAS_FORCE_DMA_UNENCRYPTED
+ select ARCH_HAS_CC_PLATFORM
help
There are certain POWER platforms which support secure guests using
the Protected Execution Facility, with the help of an Ultravisor
diff --git a/arch/powerpc/platforms/pseries/Makefile
b/arch/powerpc/platforms/pseries/Makefile
index 4cda0ef87be0..41d8aee98da4 100644
--- a/arch/powerpc/platforms/pseries/Makefile
+++ b/arch/powerpc/platforms/pseries/Makefile
@@ -31,3 +31,5 @@ obj-$(CONFIG_FA_DUMP) += rtas-fadump.o
obj-$(CONFIG_SUSPEND) += suspend.o
obj-$(CONFIG_PPC_VAS) += vas.o
+
+obj-$(CONFIG_ARCH_HAS_CC_PLATFORM) += cc_platform.o
diff --git a/arch/powerpc/platforms/pseries/cc_platform.c
b/arch/powerpc/platforms/pseries/cc_platform.c
new file mode 100644
index ..e8021af83a19
--- /dev/null
+++ b/arch/powerpc/platforms/pseries/cc_platform.c
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Confidential Computing Platform Capability checks
+ *
+ * Copyright (C) 2021 Advanced Micro Devices, Inc.
+ *
+ * Author: Tom Lendacky
+ */
+
+#include
+#include
+
+#include
+#include
+
+bool cc_platform_has(enum cc_attr attr)
+{
+ switch (attr) {
+ case CC_ATTR_MEM_ENCRYPT:
+ return is_secure_guest();
+
+ default:
+ return false;
+ }
+}
+EXPORT_SYMBOL_GPL(cc_platform_has);
--
2.29.2
Re: [PATCH v3 5/8] x86/sme: Replace occurrences of sme_active() with cc_platform_has()
On Fri, Sep 24, 2021 at 12:41:32PM +0300, Kirill A. Shutemov wrote: > On Thu, Sep 23, 2021 at 08:21:03PM +0200, Borislav Petkov wrote: > > On Thu, Sep 23, 2021 at 12:05:58AM +0300, Kirill A. Shutemov wrote: > > > Unless we find other way to guarantee RIP-relative access, we must use > > > fixup_pointer() to access any global variables. > > > > Yah, I've asked compiler folks about any guarantees we have wrt > > rip-relative addresses but it doesn't look good. Worst case, we'd have > > to do the fixup_pointer() thing. > > > > In the meantime, Tom and I did some more poking at this and here's a > > diff ontop. > > > > The direction being that we'll stick both the AMD and Intel > > *cc_platform_has() call into cc_platform.c for which instrumentation > > will be disabled so no issues with that. > > > > And that will keep all that querying all together in a single file. > > And still do cc_platform_has() calls in __startup_64() codepath? > > It's broken. > > Intel detection in cc_platform_has() relies on boot_cpu_data.x86_vendor > which is not initialized until early_cpu_init() in setup_arch(). Given > that X86_VENDOR_INTEL is 0 it leads to false-positive. Yeah, Tom, I had the same question yesterday. /me looks in his direction. :-) -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v3 5/8] x86/sme: Replace occurrences of sme_active() with cc_platform_has()
On Thu, Sep 23, 2021 at 12:05:58AM +0300, Kirill A. Shutemov wrote:
> Unless we find other way to guarantee RIP-relative access, we must use
> fixup_pointer() to access any global variables.
Yah, I've asked compiler folks about any guarantees we have wrt
rip-relative addresses but it doesn't look good. Worst case, we'd have
to do the fixup_pointer() thing.
In the meantime, Tom and I did some more poking at this and here's a
diff ontop.
The direction being that we'll stick both the AMD and Intel
*cc_platform_has() call into cc_platform.c for which instrumentation
will be disabled so no issues with that.
And that will keep all that querying all together in a single file.
---
diff --git a/arch/x86/include/asm/mem_encrypt.h
b/arch/x86/include/asm/mem_encrypt.h
index a73712b6ee0e..2d4f5c17d79c 100644
--- a/arch/x86/include/asm/mem_encrypt.h
+++ b/arch/x86/include/asm/mem_encrypt.h
@@ -51,7 +51,6 @@ void __init mem_encrypt_free_decrypted_mem(void);
void __init mem_encrypt_init(void);
void __init sev_es_init_vc_handling(void);
-bool amd_cc_platform_has(enum cc_attr attr);
#define __bss_decrypted __section(".bss..decrypted")
@@ -74,7 +73,6 @@ static inline void __init sme_encrypt_kernel(struct
boot_params *bp) { }
static inline void __init sme_enable(struct boot_params *bp) { }
static inline void sev_es_init_vc_handling(void) { }
-static inline bool amd_cc_platform_has(enum cc_attr attr) { return false; }
static inline int __init
early_set_memory_decrypted(unsigned long vaddr, unsigned long size) { return
0; }
@@ -103,12 +101,6 @@ static inline u64 sme_get_me_mask(void)
return sme_me_mask;
}
-#if defined(CONFIG_CPU_SUP_INTEL) && defined(CONFIG_ARCH_HAS_CC_PLATFORM)
-bool intel_cc_platform_has(enum cc_attr attr);
-#else
-static inline bool intel_cc_platform_has(enum cc_attr attr) { return false; }
-#endif
-
#endif /* __ASSEMBLY__ */
#endif /* __X86_MEM_ENCRYPT_H__ */
diff --git a/arch/x86/kernel/cc_platform.c b/arch/x86/kernel/cc_platform.c
index da54a1805211..97ede7052f77 100644
--- a/arch/x86/kernel/cc_platform.c
+++ b/arch/x86/kernel/cc_platform.c
@@ -13,6 +13,52 @@
#include
+static bool intel_cc_platform_has(enum cc_attr attr)
+{
+#ifdef CONFIG_INTEL_TDX_GUEST
+ return false;
+#else
+ return false;
+#endif
+}
+
+/*
+ * SME and SEV are very similar but they are not the same, so there are
+ * times that the kernel will need to distinguish between SME and SEV. The
+ * cc_platform_has() function is used for this. When a distinction isn't
+ * needed, the CC_ATTR_MEM_ENCRYPT attribute can be used.
+ *
+ * The trampoline code is a good example for this requirement. Before
+ * paging is activated, SME will access all memory as decrypted, but SEV
+ * will access all memory as encrypted. So, when APs are being brought
+ * up under SME the trampoline area cannot be encrypted, whereas under SEV
+ * the trampoline area must be encrypted.
+ */
+static bool amd_cc_platform_has(enum cc_attr attr)
+{
+#ifdef CONFIG_AMD_MEM_ENCRYPT
+ switch (attr) {
+ case CC_ATTR_MEM_ENCRYPT:
+ return sme_me_mask;
+
+ case CC_ATTR_HOST_MEM_ENCRYPT:
+ return sme_me_mask && !(sev_status & MSR_AMD64_SEV_ENABLED);
+
+ case CC_ATTR_GUEST_MEM_ENCRYPT:
+ return sev_status & MSR_AMD64_SEV_ENABLED;
+
+ case CC_ATTR_GUEST_STATE_ENCRYPT:
+ return sev_status & MSR_AMD64_SEV_ES_ENABLED;
+
+ default:
+ return false;
+ }
+#else
+ return false;
+#endif
+}
+
+
bool cc_platform_has(enum cc_attr attr)
{
if (sme_me_mask)
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 53756ff12295..8321c43554a1 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -60,13 +60,6 @@ static u64 msr_test_ctrl_cache __ro_after_init;
*/
static bool cpu_model_supports_sld __ro_after_init;
-#ifdef CONFIG_ARCH_HAS_CC_PLATFORM
-bool intel_cc_platform_has(enum cc_attr attr)
-{
- return false;
-}
-#endif
-
/*
* Processors which have self-snooping capability can handle conflicting
* memory type across CPUs by snooping its own cache. However, there exists
diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index 9417d404ea92..23d54b810f08 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -361,38 +361,6 @@ int __init early_set_memory_encrypted(unsigned long vaddr,
unsigned long size)
return early_set_memory_enc_dec(vaddr, size, true);
}
-/*
- * SME and SEV are very similar but they are not the same, so there are
- * times that the kernel will need to distinguish between SME and SEV. The
- * cc_platform_has() function is used for this. When a distinction isn't
- * needed, the CC_ATTR_MEM_ENCRYPT attribute can be used.
- *
- * The trampoline code is a good example for this requirement. Before
- * paging is activated, SME will access all memory as decrypted, but SEV
- * will access all memory
Re: [PATCH v3 5/8] x86/sme: Replace occurrences of sme_active() with cc_platform_has()
On Wed, Sep 22, 2021 at 05:30:15PM +0300, Kirill A. Shutemov wrote: > Not fine, but waiting to blowup with random build environment change. Why is it not fine? Are you suspecting that the compiler might generate something else and not a rip-relative access? -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v3 5/8] x86/sme: Replace occurrences of sme_active() with cc_platform_has()
On Wed, Sep 22, 2021 at 12:20:59AM +0300, Kirill A. Shutemov wrote: > I still believe calling cc_platform_has() from __startup_64() is totally > broken as it lacks proper wrapping while accessing global variables. Well, one of the issues on the AMD side was using boot_cpu_data too early and the Intel side uses it too. Can you replace those checks with is_tdx_guest() or whatever was the helper's name which would check whether the the kernel is running as a TDX guest, and see if that helps? Thx. -- Regards/Gruss, Boris.
Re: [PATCH v3 5/8] x86/sme: Replace occurrences of sme_active() with cc_platform_has()
On Tue, Sep 21, 2021 at 12:04:58PM -0500, Tom Lendacky wrote: > Looks like instrumentation during early boot. I worked with Boris offline to > exclude arch/x86/kernel/cc_platform.c from some of the instrumentation and > that allowed an allyesconfig to boot. And here's the lineup I have so far, I'd appreciate it if ppc and s390 folks could run it too: https://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git/log/?h=rc2-cc Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [RFC PATCH 2/8] x86: add CPU field to struct thread_info
On Tue, Sep 14, 2021 at 02:10:30PM +0200, Ard Biesheuvel wrote:
> The CPU field will be moved back into thread_info even when
> THREAD_INFO_IN_TASK is enabled, so add it back to x86's definition of
> struct thread_info.
>
> Signed-off-by: Ard Biesheuvel
> ---
> arch/x86/include/asm/thread_info.h | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/x86/include/asm/thread_info.h
> b/arch/x86/include/asm/thread_info.h
> index cf132663c219..ebec69c35e95 100644
> --- a/arch/x86/include/asm/thread_info.h
> +++ b/arch/x86/include/asm/thread_info.h
> @@ -57,6 +57,9 @@ struct thread_info {
> unsigned long flags; /* low level flags */
> unsigned long syscall_work; /* SYSCALL_WORK_ flags */
> u32 status; /* thread synchronous flags */
> +#ifdef CONFIG_SMP
> + u32 cpu;/* current CPU */
> +#endif
> };
>
> #define INIT_THREAD_INFO(tsk)\
> --
Acked-by: Borislav Petkov
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v3 0/8] Implement generic cc_platform_has() helper function
On Wed, Sep 15, 2021 at 10:26:06AM -0700, Kuppuswamy, Sathyanarayanan wrote: > I have a Intel variant patch (please check following patch). But it includes > TDX changes as well. Shall I move TDX changes to different patch and just > create a separate patch for adding intel_cc_platform_has()? Yes, please, so that I can expedite that stuff separately and so that it can go in early in order for future work to be based ontop. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v3 4/8] powerpc/pseries/svm: Add a powerpc version of cc_platform_has()
On Wed, Sep 15, 2021 at 07:18:34PM +0200, Christophe Leroy wrote: > Could you please provide more explicit explanation why inlining such an > helper is considered as bad practice and messy ? Tom already told you to look at the previous threads. Let's read them together. This one, for example: https://lore.kernel.org/lkml/ysscwvpxevxw%2f...@infradead.org/ | > To take it out of line, I'm leaning towards the latter, creating a new | > file that is built based on the ARCH_HAS_PROTECTED_GUEST setting. | | Yes. In general everytime architectures have to provide the prototype | and not just the implementation of something we end up with a giant mess | sooner or later. In a few cases that is still warranted due to | performance concerns, but i don't think that is the case here. So I think what Christoph means here is that you want to have the generic prototype defined in a header and arches get to implement it exactly to the letter so that there's no mess. As to what mess exactly, I'd let him explain that. > Because as demonstrated in my previous response some days ago, taking that > outline ends up with an unneccessary ugly generated code and we don't > benefit front GCC's capability to fold in and opt out unreachable code. And this is real fast path where a couple of instructions matter or what? set_memory_encrypted/_decrypted doesn't look like one to me. > I can't see your point here. Inlining the function wouldn't add any > ifdeffery as far as I can see. If the function is touching defines etc, they all need to be visible. If that function needs to call other functions - which is the case on x86, perhaps not so much on power - then you need to either ifdef around them or provide stubs with ifdeffery in the headers. And you need to make them global functions instead of keeping them static to the same compilation unit, etc, etc. With a separate compilation unit, you don't need any of that and it is all kept in that single file. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v3 0/8] Implement generic cc_platform_has() helper function
On Wed, Sep 08, 2021 at 05:58:31PM -0500, Tom Lendacky wrote: > This patch series provides a generic helper function, cc_platform_has(), > to replace the sme_active(), sev_active(), sev_es_active() and > mem_encrypt_active() functions. > > It is expected that as new confidential computing technologies are > added to the kernel, they can all be covered by a single function call > instead of a collection of specific function calls all called from the > same locations. > > The powerpc and s390 patches have been compile tested only. Can the > folks copied on this series verify that nothing breaks for them. Also, > a new file, arch/powerpc/platforms/pseries/cc_platform.c, has been > created for powerpc to hold the out of line function. ... > > Tom Lendacky (8): > x86/ioremap: Selectively build arch override encryption functions > mm: Introduce a function to check for confidential computing features > x86/sev: Add an x86 version of cc_platform_has() > powerpc/pseries/svm: Add a powerpc version of cc_platform_has() > x86/sme: Replace occurrences of sme_active() with cc_platform_has() > x86/sev: Replace occurrences of sev_active() with cc_platform_has() > x86/sev: Replace occurrences of sev_es_active() with cc_platform_has() > treewide: Replace the use of mem_encrypt_active() with > cc_platform_has() Ok, modulo the minor things the plan is to take this through tip after -rc2 releases in order to pick up the powerpc build fix and have a clean base (-rc2) to base stuff on, at the same time. Pls holler if something's still amiss. Sathya, if you want to prepare the Intel variant intel_cc_platform_has() ontop of those and send it to me, that would be good because then I can integrate it all in one branch which can be used to base future work ontop. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v3 4/8] powerpc/pseries/svm: Add a powerpc version of cc_platform_has()
On Wed, Sep 15, 2021 at 10:28:59AM +1000, Michael Ellerman wrote: > I don't love it, a new C file and an out-of-line call to then call back > to a static inline that for most configuration will return false ... but > whatever :) Yeah, hch thinks it'll cause a big mess otherwise: https://lore.kernel.org/lkml/ysscwvpxevxw%2f...@infradead.org/ I guess less ifdeffery is nice too. > Acked-by: Michael Ellerman (powerpc) Thx. > Yeah, fixed in mainline today, thanks for trying to cross compile :) Always! :-) -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v3 5/8] x86/sme: Replace occurrences of sme_active() with cc_platform_has()
On Wed, Sep 08, 2021 at 05:58:36PM -0500, Tom Lendacky wrote:
> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
> index 18fe19916bc3..4b54a2377821 100644
> --- a/arch/x86/mm/mem_encrypt.c
> +++ b/arch/x86/mm/mem_encrypt.c
> @@ -144,7 +144,7 @@ void __init sme_unmap_bootdata(char *real_mode_data)
> struct boot_params *boot_data;
> unsigned long cmdline_paddr;
>
> - if (!sme_active())
> + if (!cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT))
> return;
>
> /* Get the command line address before unmapping the real_mode_data */
> @@ -164,7 +164,7 @@ void __init sme_map_bootdata(char *real_mode_data)
> struct boot_params *boot_data;
> unsigned long cmdline_paddr;
>
> - if (!sme_active())
> + if (!cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT))
> return;
>
> __sme_early_map_unmap_mem(real_mode_data, sizeof(boot_params), true);
> @@ -377,11 +377,6 @@ bool sev_active(void)
> {
> return sev_status & MSR_AMD64_SEV_ENABLED;
> }
> -
> -bool sme_active(void)
> -{
> - return sme_me_mask && !sev_active();
> -}
> EXPORT_SYMBOL_GPL(sev_active);
>
> /* Needs to be called from non-instrumentable code */
You forgot this hunk:
diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index 5635ca9a1fbe..a3a2396362a5 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -364,8 +364,9 @@ int __init early_set_memory_encrypted(unsigned long vaddr,
unsigned long size)
/*
* SME and SEV are very similar but they are not the same, so there are
* times that the kernel will need to distinguish between SME and SEV. The
- * sme_active() and sev_active() functions are used for this. When a
- * distinction isn't needed, the mem_encrypt_active() function can be used.
+ * PATTR_HOST_MEM_ENCRYPT and PATTR_GUEST_MEM_ENCRYPT flags to
+ * amd_prot_guest_has() are used for this. When a distinction isn't needed,
+ * the mem_encrypt_active() function can be used.
*
* The trampoline code is a good example for this requirement. Before
* paging is activated, SME will access all memory as decrypted, but SEV
because there's still a sme_active() mentioned there:
$ git grep sme_active
arch/x86/mm/mem_encrypt.c:367: * sme_active() and sev_active() functions are
used for this. When a
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v3 4/8] powerpc/pseries/svm: Add a powerpc version of cc_platform_has()
On Tue, Sep 14, 2021 at 04:47:41PM +0200, Christophe Leroy wrote: > Yes, see > https://lore.kernel.org/linuxppc-dev/20210914123919.58203...@canb.auug.org.au/T/#t Aha, more compiler magic stuff ;-\ Oh well, I guess that fix will land upstream soon. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v3 4/8] powerpc/pseries/svm: Add a powerpc version of cc_platform_has()
On Wed, Sep 08, 2021 at 05:58:35PM -0500, Tom Lendacky wrote:
> Introduce a powerpc version of the cc_platform_has() function. This will
> be used to replace the powerpc mem_encrypt_active() implementation, so
> the implementation will initially only support the CC_ATTR_MEM_ENCRYPT
> attribute.
>
> Cc: Michael Ellerman
> Cc: Benjamin Herrenschmidt
> Cc: Paul Mackerras
> Signed-off-by: Tom Lendacky
> ---
> arch/powerpc/platforms/pseries/Kconfig | 1 +
> arch/powerpc/platforms/pseries/Makefile | 2 ++
> arch/powerpc/platforms/pseries/cc_platform.c | 26
> 3 files changed, 29 insertions(+)
> create mode 100644 arch/powerpc/platforms/pseries/cc_platform.c
Michael,
can I get an ACK for the ppc bits to carry them through the tip tree
pls?
Btw, on a related note, cross-compiling this throws the following error here:
$ make
CROSS_COMPILE=/home/share/src/crosstool/gcc-9.4.0-nolibc/powerpc64-linux/bin/powerpc64-linux-
V=1 ARCH=powerpc
...
/home/share/src/crosstool/gcc-9.4.0-nolibc/powerpc64-linux/bin/powerpc64-linux-gcc
-Wp,-MD,arch/powerpc/boot/.crt0.o.d -D__ASSEMBLY__ -Wall -Wundef
-Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -O2 -msoft-float
-mno-altivec -mno-vsx -pipe -fomit-frame-pointer -fno-builtin -fPIC -nostdinc
-include ./include/linux/compiler_attributes.h -I./arch/powerpc/include
-I./arch/powerpc/include/generated -I./include -I./arch/powerpc/include/uapi
-I./arch/powerpc/include/generated/uapi -I./include/uapi
-I./include/generated/uapi -include ./include/linux/compiler-version.h -include
./include/linux/kconfig.h -m32 -isystem
/home/share/src/crosstool/gcc-9.4.0-nolibc/powerpc64-linux/bin/../lib/gcc/powerpc64-linux/9.4.0/include
-mbig-endian -nostdinc -c -o arch/powerpc/boot/crt0.o arch/powerpc/boot/crt0.S
In file included from :
././include/linux/compiler_attributes.h:62:5: warning: "__has_attribute" is not
defined, evaluates to 0 [-Wundef]
62 | #if __has_attribute(__assume_aligned__)
| ^~~
././include/linux/compiler_attributes.h:62:20: error: missing binary operator
before token "("
62 | #if __has_attribute(__assume_aligned__)
|^
././include/linux/compiler_attributes.h:88:5: warning: "__has_attribute" is not
defined, evaluates to 0 [-Wundef]
88 | #if __has_attribute(__copy__)
| ^~~
...
Known issue?
This __has_attribute() thing is supposed to be supported
in gcc since 5.1 and I'm using the crosstool stuff from
https://www.kernel.org/pub/tools/crosstool/ and gcc-9.4 above is pretty
new so that should not happen actually.
But it does...
Hmmm.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v3 3/8] x86/sev: Add an x86 version of cc_platform_has()
On Wed, Sep 08, 2021 at 05:58:34PM -0500, Tom Lendacky wrote:
> diff --git a/arch/x86/kernel/cc_platform.c b/arch/x86/kernel/cc_platform.c
> new file mode 100644
> index ..3c9bacd3c3f3
> --- /dev/null
> +++ b/arch/x86/kernel/cc_platform.c
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Confidential Computing Platform Capability checks
> + *
> + * Copyright (C) 2021 Advanced Micro Devices, Inc.
> + *
> + * Author: Tom Lendacky
> + */
> +
> +#include
> +#include
> +#include
> +
> +bool cc_platform_has(enum cc_attr attr)
> +{
> + if (sme_me_mask)
Why are you still checking the sme_me_mask here? AFAIR, we said that
we'll do that only when the KVM folks come with a valid use case...
> + return amd_cc_platform_has(attr);
> +
> + return false;
> +}
> +EXPORT_SYMBOL_GPL(cc_platform_has);
> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
> index ff08dc463634..18fe19916bc3 100644
> --- a/arch/x86/mm/mem_encrypt.c
> +++ b/arch/x86/mm/mem_encrypt.c
> @@ -20,6 +20,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -389,6 +390,26 @@ bool noinstr sev_es_active(void)
> return sev_status & MSR_AMD64_SEV_ES_ENABLED;
> }
>
> +bool amd_cc_platform_has(enum cc_attr attr)
> +{
> + switch (attr) {
> + case CC_ATTR_MEM_ENCRYPT:
> + return sme_me_mask != 0;
No need for the "!= 0"
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v3 2/8] mm: Introduce a function to check for confidential computing features
On Wed, Sep 08, 2021 at 05:58:33PM -0500, Tom Lendacky wrote: > In prep for other confidential computing technologies, introduce a generic preparation > helper function, cc_platform_has(), that can be used to check for specific > active confidential computing attributes, like memory encryption. This is > intended to eliminate having to add multiple technology-specific checks to > the code (e.g. if (sev_active() || tdx_active())). ... > diff --git a/include/linux/cc_platform.h b/include/linux/cc_platform.h > new file mode 100644 > index ..253f3ea66cd8 > --- /dev/null > +++ b/include/linux/cc_platform.h > @@ -0,0 +1,88 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Confidential Computing Platform Capability checks > + * > + * Copyright (C) 2021 Advanced Micro Devices, Inc. > + * > + * Author: Tom Lendacky > + */ > + > +#ifndef _CC_PLATFORM_H _LINUX_CC_PLATFORM_H > +#define _CC_PLATFORM_H -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v2 03/12] x86/sev: Add an x86 version of prot_guest_has()
On Thu, Aug 19, 2021 at 10:52:53AM +0100, Christoph Hellwig wrote: > Which suggest that the name is not good to start with. Maybe protected > hardware, system or platform might be a better choice? Yah, coming up with a proper name here hasn't been easy. prot_guest_has() is not the first variant. >From all three things you suggest above, I guess calling it a "platform" is the closest. As in, this is a confidential computing platform which provides host and guest facilities etc. So calling it confidential_computing_platform_has() is obviously too long. ccp_has() clashes with the namespace of drivers/crypto/ccp/ which is used by the technology too. coco_platform_has() is too unserious. So I guess cc_platform_has() ain't all that bad. Unless you have a better idea, ofc. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v2 06/12] x86/sev: Replace occurrences of sev_active() with prot_guest_has()
On Tue, Aug 17, 2021 at 10:26:18AM -0500, Tom Lendacky wrote: > >>/* > >> - * If SME is active we need to be sure that kexec pages are > >> - * not encrypted because when we boot to the new kernel the > >> + * If host memory encryption is active we need to be sure that kexec > >> + * pages are not encrypted because when we boot to the new kernel the > >> * pages won't be accessed encrypted (initially). > >> */ > > > > That hunk belongs logically into the previous patch which removes > > sme_active(). > > I was trying to keep the sev_active() changes separate... so even though > it's an SME thing, I kept it here. But I can move it to the previous > patch, it just might look strange. Oh I meant only the comment because it is a SME-related change. But not too important so whatever. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v2 05/12] x86/sme: Replace occurrences of sme_active() with prot_guest_has()
On Tue, Aug 17, 2021 at 09:46:58AM -0500, Tom Lendacky wrote: > I'm ok with letting the TDX folks make changes to these calls to be SME or > SEV specific, if necessary, later. Yap, exactly. Let's add the specific stuff only when really needed. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v2 03/12] x86/sev: Add an x86 version of prot_guest_has()
On Tue, Aug 17, 2021 at 10:22:52AM -0500, Tom Lendacky wrote: > I can change it to be an AMD/HYGON check... although, I'll have to check > to see if any (very) early use of the function will work with that. We can always change it later if really needed. It is just that I'm not a fan of such "preemptive" changes. > At a minimum, the check in arch/x86/kernel/head64.c will have to be > changed or removed. I'll take a closer look. Yeah, sme_me_mask, already discussed on IRC. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v2 09/12] mm: Remove the now unused mem_encrypt_active() function
On Tue, Aug 17, 2021 at 12:22:33PM +0200, Borislav Petkov wrote: > This one wants to be part of the previous patch. ... and the three following patches too - the treewide patch does a single atomic :) replacement and that's it. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v2 09/12] mm: Remove the now unused mem_encrypt_active() function
On Fri, Aug 13, 2021 at 11:59:28AM -0500, Tom Lendacky wrote:
> The mem_encrypt_active() function has been replaced by prot_guest_has(),
> so remove the implementation.
>
> Reviewed-by: Joerg Roedel
> Signed-off-by: Tom Lendacky
> ---
> include/linux/mem_encrypt.h | 4
> 1 file changed, 4 deletions(-)
>
> diff --git a/include/linux/mem_encrypt.h b/include/linux/mem_encrypt.h
> index 5c4a18a91f89..ae4526389261 100644
> --- a/include/linux/mem_encrypt.h
> +++ b/include/linux/mem_encrypt.h
> @@ -16,10 +16,6 @@
>
> #include
>
> -#else/* !CONFIG_ARCH_HAS_MEM_ENCRYPT */
> -
> -static inline bool mem_encrypt_active(void) { return false; }
> -
> #endif /* CONFIG_ARCH_HAS_MEM_ENCRYPT */
>
> #ifdef CONFIG_AMD_MEM_ENCRYPT
> --
This one wants to be part of the previous patch.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v2 07/12] x86/sev: Replace occurrences of sev_es_active() with prot_guest_has()
On Fri, Aug 13, 2021 at 11:59:26AM -0500, Tom Lendacky wrote: > Replace occurrences of sev_es_active() with the more generic > prot_guest_has() using PATTR_GUEST_PROT_STATE, except for in > arch/x86/kernel/sev*.c and arch/x86/mm/mem_encrypt*.c where PATTR_SEV_ES > will be used. If future support is added for other memory encyrption > techonologies, the use of PATTR_GUEST_PROT_STATE can be updated, as > required, to specifically use PATTR_SEV_ES. > > Cc: Thomas Gleixner > Cc: Ingo Molnar > Cc: Borislav Petkov > Signed-off-by: Tom Lendacky > --- > arch/x86/include/asm/mem_encrypt.h | 2 -- > arch/x86/kernel/sev.c | 6 +++--- > arch/x86/mm/mem_encrypt.c | 7 +++ > arch/x86/realmode/init.c | 3 +-- > 4 files changed, 7 insertions(+), 11 deletions(-) Same comments to this one as for the previous two. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v2 06/12] x86/sev: Replace occurrences of sev_active() with prot_guest_has()
On Fri, Aug 13, 2021 at 11:59:25AM -0500, Tom Lendacky wrote:
> diff --git a/arch/x86/kernel/machine_kexec_64.c
> b/arch/x86/kernel/machine_kexec_64.c
> index 8e7b517ad738..66ff788b79c9 100644
> --- a/arch/x86/kernel/machine_kexec_64.c
> +++ b/arch/x86/kernel/machine_kexec_64.c
> @@ -167,7 +167,7 @@ static int init_transition_pgtable(struct kimage *image,
> pgd_t *pgd)
> }
> pte = pte_offset_kernel(pmd, vaddr);
>
> - if (sev_active())
> + if (prot_guest_has(PATTR_GUEST_MEM_ENCRYPT))
> prot = PAGE_KERNEL_EXEC;
>
> set_pte(pte, pfn_pte(paddr >> PAGE_SHIFT, prot));
> @@ -207,7 +207,7 @@ static int init_pgtable(struct kimage *image, unsigned
> long start_pgtable)
> level4p = (pgd_t *)__va(start_pgtable);
> clear_page(level4p);
>
> - if (sev_active()) {
> + if (prot_guest_has(PATTR_GUEST_MEM_ENCRYPT)) {
> info.page_flag |= _PAGE_ENC;
> info.kernpg_flag |= _PAGE_ENC;
> }
> @@ -570,12 +570,12 @@ void arch_kexec_unprotect_crashkres(void)
> */
> int arch_kexec_post_alloc_pages(void *vaddr, unsigned int pages, gfp_t gfp)
> {
> - if (sev_active())
> + if (!prot_guest_has(PATTR_HOST_MEM_ENCRYPT))
> return 0;
>
> /*
> - * If SME is active we need to be sure that kexec pages are
> - * not encrypted because when we boot to the new kernel the
> + * If host memory encryption is active we need to be sure that kexec
> + * pages are not encrypted because when we boot to the new kernel the
>* pages won't be accessed encrypted (initially).
>*/
That hunk belongs logically into the previous patch which removes
sme_active().
> return set_memory_decrypted((unsigned long)vaddr, pages);
> @@ -583,12 +583,12 @@ int arch_kexec_post_alloc_pages(void *vaddr, unsigned
> int pages, gfp_t gfp)
>
> void arch_kexec_pre_free_pages(void *vaddr, unsigned int pages)
> {
> - if (sev_active())
> + if (!prot_guest_has(PATTR_HOST_MEM_ENCRYPT))
> return;
>
> /*
> - * If SME is active we need to reset the pages back to being
> - * an encrypted mapping before freeing them.
> + * If host memory encryption is active we need to reset the pages back
> + * to being an encrypted mapping before freeing them.
>*/
> set_memory_encrypted((unsigned long)vaddr, pages);
> }
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index e8ccab50ebf6..b69f5ac622d5 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -25,6 +25,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -457,7 +458,7 @@ static int has_svm(void)
> return 0;
> }
>
> - if (sev_active()) {
> + if (prot_guest_has(PATTR_SEV)) {
> pr_info("KVM is unsupported when running as an SEV guest\n");
> return 0;
Same question as for PATTR_SME. PATTR_GUEST_MEM_ENCRYPT should be enough.
> @@ -373,7 +373,7 @@ int __init early_set_memory_encrypted(unsigned long
> vaddr, unsigned long size)
> * up under SME the trampoline area cannot be encrypted, whereas under SEV
> * the trampoline area must be encrypted.
> */
> -bool sev_active(void)
> +static bool sev_active(void)
> {
> return sev_status & MSR_AMD64_SEV_ENABLED;
> }
> @@ -382,7 +382,6 @@ static bool sme_active(void)
> {
> return sme_me_mask && !sev_active();
> }
> -EXPORT_SYMBOL_GPL(sev_active);
Just get rid of it altogether.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v2 05/12] x86/sme: Replace occurrences of sme_active() with prot_guest_has()
On Fri, Aug 13, 2021 at 11:59:24AM -0500, Tom Lendacky wrote:
> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
> index edc67ddf065d..5635ca9a1fbe 100644
> --- a/arch/x86/mm/mem_encrypt.c
> +++ b/arch/x86/mm/mem_encrypt.c
> @@ -144,7 +144,7 @@ void __init sme_unmap_bootdata(char *real_mode_data)
> struct boot_params *boot_data;
> unsigned long cmdline_paddr;
>
> - if (!sme_active())
> + if (!amd_prot_guest_has(PATTR_SME))
> return;
>
> /* Get the command line address before unmapping the real_mode_data */
> @@ -164,7 +164,7 @@ void __init sme_map_bootdata(char *real_mode_data)
> struct boot_params *boot_data;
> unsigned long cmdline_paddr;
>
> - if (!sme_active())
> + if (!amd_prot_guest_has(PATTR_SME))
> return;
>
> __sme_early_map_unmap_mem(real_mode_data, sizeof(boot_params), true);
> @@ -378,7 +378,7 @@ bool sev_active(void)
> return sev_status & MSR_AMD64_SEV_ENABLED;
> }
>
> -bool sme_active(void)
> +static bool sme_active(void)
Just get rid of it altogether. Also, there's an
EXPORT_SYMBOL_GPL(sev_active);
which needs to go under the actual function. Here's a diff ontop:
---
diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index 5635ca9a1fbe..a3a2396362a5 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -364,8 +364,9 @@ int __init early_set_memory_encrypted(unsigned long vaddr,
unsigned long size)
/*
* SME and SEV are very similar but they are not the same, so there are
* times that the kernel will need to distinguish between SME and SEV. The
- * sme_active() and sev_active() functions are used for this. When a
- * distinction isn't needed, the mem_encrypt_active() function can be used.
+ * PATTR_HOST_MEM_ENCRYPT and PATTR_GUEST_MEM_ENCRYPT flags to
+ * amd_prot_guest_has() are used for this. When a distinction isn't needed,
+ * the mem_encrypt_active() function can be used.
*
* The trampoline code is a good example for this requirement. Before
* paging is activated, SME will access all memory as decrypted, but SEV
@@ -377,11 +378,6 @@ bool sev_active(void)
{
return sev_status & MSR_AMD64_SEV_ENABLED;
}
-
-static bool sme_active(void)
-{
- return sme_me_mask && !sev_active();
-}
EXPORT_SYMBOL_GPL(sev_active);
/* Needs to be called from non-instrumentable code */
@@ -398,7 +394,7 @@ bool amd_prot_guest_has(unsigned int attr)
case PATTR_SME:
case PATTR_HOST_MEM_ENCRYPT:
- return sme_active();
+ return sme_me_mask && !sev_active();
case PATTR_SEV:
case PATTR_GUEST_MEM_ENCRYPT:
> {
> return sme_me_mask && !sev_active();
> }
> @@ -428,7 +428,7 @@ bool force_dma_unencrypted(struct device *dev)
>* device does not support DMA to addresses that include the
>* encryption mask.
>*/
> - if (sme_active()) {
> + if (amd_prot_guest_has(PATTR_SME)) {
So I'm not sure: you add PATTR_SME which you call with
amd_prot_guest_has() and PATTR_HOST_MEM_ENCRYPT which you call with
prot_guest_has() and they both end up being the same thing on AMD.
So why even bother with PATTR_SME?
This is only going to cause confusion later and I'd say let's simply use
prot_guest_has(PATTR_HOST_MEM_ENCRYPT) everywhere...
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v2 04/12] powerpc/pseries/svm: Add a powerpc version of prot_guest_has()
On Fri, Aug 13, 2021 at 11:59:23AM -0500, Tom Lendacky wrote: > Introduce a powerpc version of the prot_guest_has() function. This will > be used to replace the powerpc mem_encrypt_active() implementation, so > the implementation will initially only support the PATTR_MEM_ENCRYPT > attribute. > > Cc: Michael Ellerman > Cc: Benjamin Herrenschmidt > Cc: Paul Mackerras > Signed-off-by: Tom Lendacky > --- > arch/powerpc/include/asm/protected_guest.h | 30 ++ > arch/powerpc/platforms/pseries/Kconfig | 1 + > 2 files changed, 31 insertions(+) > create mode 100644 arch/powerpc/include/asm/protected_guest.h > > diff --git a/arch/powerpc/include/asm/protected_guest.h > b/arch/powerpc/include/asm/protected_guest.h > new file mode 100644 > index ..ce55c2c7e534 > --- /dev/null > +++ b/arch/powerpc/include/asm/protected_guest.h > @@ -0,0 +1,30 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Protected Guest (and Host) Capability checks > + * > + * Copyright (C) 2021 Advanced Micro Devices, Inc. > + * > + * Author: Tom Lendacky > + */ > + > +#ifndef _POWERPC_PROTECTED_GUEST_H > +#define _POWERPC_PROTECTED_GUEST_H > + > +#include > + > +#ifndef __ASSEMBLY__ Same thing here. Pls audit the whole set whether those __ASSEMBLY__ guards are really needed and remove them if not. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v2 03/12] x86/sev: Add an x86 version of prot_guest_has()
On Sun, Aug 15, 2021 at 08:53:31AM -0500, Tom Lendacky wrote: > It's not a cross-vendor thing as opposed to a KVM or other hypervisor > thing where the family doesn't have to be reported as AMD or HYGON. What would be the use case? A HV starts a guest which is supposed to be encrypted using the AMD's confidential guest technology but the HV tells the guest that it is not running on an AMD SVM HV but something else? Is that even an actual use case? Or am I way off? I know we have talked about this in the past but this still sounds insane. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v2 03/12] x86/sev: Add an x86 version of prot_guest_has()
On Fri, Aug 13, 2021 at 11:59:22AM -0500, Tom Lendacky wrote:
> diff --git a/arch/x86/include/asm/protected_guest.h
> b/arch/x86/include/asm/protected_guest.h
> new file mode 100644
> index ..51e4eefd9542
> --- /dev/null
> +++ b/arch/x86/include/asm/protected_guest.h
> @@ -0,0 +1,29 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Protected Guest (and Host) Capability checks
> + *
> + * Copyright (C) 2021 Advanced Micro Devices, Inc.
> + *
> + * Author: Tom Lendacky
> + */
> +
> +#ifndef _X86_PROTECTED_GUEST_H
> +#define _X86_PROTECTED_GUEST_H
> +
> +#include
> +
> +#ifndef __ASSEMBLY__
> +
> +static inline bool prot_guest_has(unsigned int attr)
> +{
> +#ifdef CONFIG_AMD_MEM_ENCRYPT
> + if (sme_me_mask)
> + return amd_prot_guest_has(attr);
> +#endif
> +
> + return false;
> +}
> +
> +#endif /* __ASSEMBLY__ */
> +
> +#endif /* _X86_PROTECTED_GUEST_H */
I think this can be simplified more, diff ontop below:
- no need for the ifdeffery as amd_prot_guest_has() has versions for
both when CONFIG_AMD_MEM_ENCRYPT is set or not.
- the sme_me_mask check is pushed there too.
- and since this is vendor-specific, I'm checking the vendor bit. Yeah,
yeah, cross-vendor but I don't really believe that.
---
diff --git a/arch/x86/include/asm/protected_guest.h
b/arch/x86/include/asm/protected_guest.h
index 51e4eefd9542..8541c76d5da4 100644
--- a/arch/x86/include/asm/protected_guest.h
+++ b/arch/x86/include/asm/protected_guest.h
@@ -12,18 +12,13 @@
#include
-#ifndef __ASSEMBLY__
-
static inline bool prot_guest_has(unsigned int attr)
{
-#ifdef CONFIG_AMD_MEM_ENCRYPT
- if (sme_me_mask)
+ if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD ||
+ boot_cpu_data.x86_vendor == X86_VENDOR_HYGON)
return amd_prot_guest_has(attr);
-#endif
return false;
}
-#endif /* __ASSEMBLY__ */
-
#endif /* _X86_PROTECTED_GUEST_H */
diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index edc67ddf065d..5a0442a6f072 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -392,6 +392,9 @@ bool noinstr sev_es_active(void)
bool amd_prot_guest_has(unsigned int attr)
{
+ if (!sme_me_mask)
+ return false;
+
switch (attr) {
case PATTR_MEM_ENCRYPT:
return sme_me_mask != 0;
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v2 02/12] mm: Introduce a function to check for virtualization protection features
On Fri, Aug 13, 2021 at 11:59:21AM -0500, Tom Lendacky wrote: > In prep for other protected virtualization technologies, introduce a > generic helper function, prot_guest_has(), that can be used to check > for specific protection attributes, like memory encryption. This is > intended to eliminate having to add multiple technology-specific checks > to the code (e.g. if (sev_active() || tdx_active())). > > Reviewed-by: Joerg Roedel > Co-developed-by: Andi Kleen > Signed-off-by: Andi Kleen > Co-developed-by: Kuppuswamy Sathyanarayanan > > Signed-off-by: Kuppuswamy Sathyanarayanan > > Signed-off-by: Tom Lendacky > --- > arch/Kconfig| 3 +++ > include/linux/protected_guest.h | 35 + > 2 files changed, 38 insertions(+) > create mode 100644 include/linux/protected_guest.h > > diff --git a/arch/Kconfig b/arch/Kconfig > index 98db63496bab..bd4f60c581f1 100644 > --- a/arch/Kconfig > +++ b/arch/Kconfig > @@ -1231,6 +1231,9 @@ config RELR > config ARCH_HAS_MEM_ENCRYPT > bool > > +config ARCH_HAS_PROTECTED_GUEST > + bool > + > config HAVE_SPARSE_SYSCALL_NR > bool > help > diff --git a/include/linux/protected_guest.h b/include/linux/protected_guest.h > new file mode 100644 > index ..43d4dde94793 > --- /dev/null > +++ b/include/linux/protected_guest.h > @@ -0,0 +1,35 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Protected Guest (and Host) Capability checks > + * > + * Copyright (C) 2021 Advanced Micro Devices, Inc. > + * > + * Author: Tom Lendacky > + */ > + > +#ifndef _PROTECTED_GUEST_H > +#define _PROTECTED_GUEST_H > + > +#ifndef __ASSEMBLY__ ^ Do you really need that guard? It builds fine without it too. Or something coming later does need it...? -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] crypto: DRBG - select SHA512
On Fri, Jul 16, 2021 at 04:14:12PM +0800, Herbert Xu wrote: > Stephan Mueller wrote: > > With the swtich to use HMAC(SHA-512) as the default DRBG type, the > > configuration must now also select SHA-512. > > > > Fixes: 9b7b94683a9b "crypto: DRBG - switch to HMAC SHA512 DRBG as default > > DRBG" > > Reported-by: Sachin Sant > > Signed-off-by: Stephan Mueller > > --- > > crypto/Kconfig | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > Patch applied. Thanks. Is that patch going to Linus anytime soon? I still see it on latest rc5+: DRBG: could not allocate digest TFM handle: hmac(sha512) alg: drbg: Failed to reset rng alg: drbg: Test 0 failed for drbg_nopr_hmac_sha512 [ cut here ] alg: self-tests for drbg_nopr_hmac_sha512 (stdrng) failed (rc=-22) WARNING: CPU: 3 PID: 76 at crypto/testmgr.c:5652 alg_test.part.0+0x132/0x3c0 Modules linked in: CPU: 3 PID: 76 Comm: cryptomgr_test Not tainted 5.14.0-rc5+ #1 Hardware name: LENOVO 2320CTO/2320CTO, BIOS G2ET86WW (2.06 ) 11/13/2012 RIP: 0010:alg_test.part.0+0x132/0x3c0 Code: c0 74 2e 80 3d 7f 61 ad 02 00 0f 85 c0 64 5f 00 44 89 c1 4c 89 f2 4c 89 ee 44 89 44 24 04 48 c7 c7 f8 0a 11 82 e8 8c 57 5e 00 <0f> 0b 44 8b 44 24 04 48 8b 84 24 98 00 00 00 65 48 2b 04 25 28 00 RSP: :c978fe38 EFLAGS: 00010292 RAX: 0042 RBX: RCX: RDX: 0001 RSI: 810f520f RDI: 810f520f RBP: 0053 R08: 0001 R09: 0001 R10: 888219df9000 R11: 3fff R12: 0053 R13: 888100c0ee00 R14: 888100c0ee80 R15: 14c0 FS: () GS:888211f8() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: CR3: 02412001 CR4: 001706e0 Call Trace: ? lock_is_held_type+0xd5/0x130 ? find_held_lock+0x2b/0x80 ? preempt_count_sub+0x9b/0xd0 ? crypto_acomp_scomp_free_ctx+0x30/0x30 cryptomgr_test+0x27/0x50 kthread+0x144/0x170 ? set_kthread_struct+0x40/0x40 ret_from_fork+0x22/0x30 irq event stamp: 411 hardirqs last enabled at (419): [] console_unlock+0x332/0x570 hardirqs last disabled at (426): [] console_unlock+0x3df/0x570 softirqs last enabled at (234): [] __do_softirq+0x329/0x496 softirqs last disabled at (151): [] irq_exit_rcu+0xdd/0x130 ---[ end trace edfdfd51982deb2d ]--- -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v2 01/12] x86/ioremap: Selectively build arch override encryption functions
On Fri, Aug 13, 2021 at 11:59:20AM -0500, Tom Lendacky wrote: > In prep for other uses of the prot_guest_has() function besides AMD's > memory encryption support, selectively build the AMD memory encryption > architecture override functions only when CONFIG_AMD_MEM_ENCRYPT=y. These > functions are: > - early_memremap_pgprot_adjust() > - arch_memremap_can_ram_remap() > > Additionally, routines that are only invoked by these architecture > override functions can also be conditionally built. These functions are: > - memremap_should_map_decrypted() > - memremap_is_efi_data() > - memremap_is_setup_data() > - early_memremap_is_setup_data() > > And finally, phys_mem_access_encrypted() is conditionally built as well, > but requires a static inline version of it when CONFIG_AMD_MEM_ENCRYPT is > not set. > > Cc: Thomas Gleixner > Cc: Ingo Molnar > Cc: Borislav Petkov > Cc: Dave Hansen > Cc: Andy Lutomirski > Cc: Peter Zijlstra > Signed-off-by: Tom Lendacky > --- > arch/x86/include/asm/io.h | 8 > arch/x86/mm/ioremap.c | 2 +- > 2 files changed, 9 insertions(+), 1 deletion(-) LGTM. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] powerpc/svm: Don't issue ultracalls if !mem_encrypt_active()
On Mon, Aug 02, 2021 at 09:20:30PM +1000, Michael Ellerman wrote:
> Will Deacon writes:
> > Commit ad6c00283163 ("swiotlb: Free tbl memory in swiotlb_exit()")
> > introduced a set_memory_encrypted() call to swiotlb_exit() so that the
> > buffer pages are returned to an encrypted state prior to being freed.
> >
> > Sachin reports that this leads to the following crash on a Power server:
> >
> > [0.010799] software IO TLB: tearing down default memory pool
> > [0.010805] [ cut here ]
> > [0.010808] kernel BUG at arch/powerpc/kernel/interrupt.c:98!
> >
> > Nick spotted that this is because set_memory_encrypted() is issuing an
> > ultracall which doesn't exist for the processor, and should therefore
> > be gated by mem_encrypt_active() to mirror the x86 implementation.
> >
> > Cc: Konrad Rzeszutek Wilk
> > Cc: Claire Chang
> > Cc: Christoph Hellwig
> > Cc: Robin Murphy
> > Fixes: ad6c00283163 ("swiotlb: Free tbl memory in swiotlb_exit()")
> > Suggested-by: Nicholas Piggin
> > Reported-by: Sachin Sant
> > Tested-by: Sachin Sant
> > Tested-by: Nathan Chancellor
> > Link:
> > https://lore.kernel.org/r/1905cd70-7656-42ae-99e2-a31fc3812...@linux.vnet.ibm.com/
> > Signed-off-by: Will Deacon
> > ---
> > arch/powerpc/platforms/pseries/svm.c | 6 ++
> > 1 file changed, 6 insertions(+)
>
> Thanks.
>
> Acked-by: Michael Ellerman
>
>
> I assume Konrad will take this via the swiotlb tree?
Btw, we're currently reworking that whole "am I running as a
confidential guest" querying, see:
https://lkml.kernel.org/r/029791b24c6412f9427cfe6ec598156c64395964.1627424774.git.thomas.lenda...@amd.com
for example.
I see Konrad has queued this for 5.15 in his devel/for-linus-5.15 branch
so if he sends it to Linus in the upcoming merge window (right Konrad?)
then I can base the rework ontop, once 5.15-rc1 releases, so that there
are no build breakages...
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH 01/11] mm: Introduce a function to check for virtualization protection features
On Wed, Jul 28, 2021 at 02:17:27PM +0100, Christoph Hellwig wrote: > So common checks obviously make sense, but I really hate the stupid > multiplexer. Having one well-documented helper per feature is much > easier to follow. We had that in x86 - it was called cpu_has_ where xxx is the feature bit. It didn't scale with the sheer amount of feature bits that kept getting added so we do cpu_feature_enabled(X86_FEATURE_XXX) now. The idea behind this is very similar - those protected guest flags will only grow in the couple of tens range - at least - so having a multiplexer is a lot simpler, I'd say, than having a couple of tens of helpers. And those PATTR flags should have good, readable names, btw. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] EDAC/mv64x60: Remove orphan mv64x60 driver
On Mon, Dec 07, 2020 at 03:02:53PM +1100, Michael Ellerman wrote:
> The mv64x60 EDAC driver depends on CONFIG_MV64X60. But that symbol is
> not user-selectable, and the last code that selected it was removed
> with the C2K board support in 2018, see:
>
> 92c8c16f3457 ("powerpc/embedded6xx: Remove C2K board support")
>
> That means the driver is now dead code, so remove it.
>
> Suggested-by: Borislav Petkov
> Signed-off-by: Michael Ellerman
> ---
> drivers/edac/Kconfig| 7 -
> drivers/edac/Makefile | 1 -
> drivers/edac/mv64x60_edac.c | 883
> drivers/edac/mv64x60_edac.h | 114 -
> 4 files changed, 1005 deletions(-)
> delete mode 100644 drivers/edac/mv64x60_edac.c
> delete mode 100644 drivers/edac/mv64x60_edac.h
Gladly taken and applied, thanks!
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] EDAC, mv64x60: Fix error return code in mv64x60_pci_err_probe()
On Thu, Dec 03, 2020 at 10:27:25PM +1100, Michael Ellerman wrote: > It's dead code, so drop it. > > I can send a patch if no one else wants to. Yes please. I love patches removing code! :-) -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] EDAC, mv64x60: Fix error return code in mv64x60_pci_err_probe()
On Tue, Nov 24, 2020 at 02:30:09PM +0800, Wang ShaoBo wrote:
> Fix to return -ENODEV error code when edac_pci_add_device() failed instaed
> of 0 in mv64x60_pci_err_probe(), as done elsewhere in this function.
>
> Fixes: 4f4aeeabc061 ("drivers-edac: add marvell mv64x60 driver")
> Signed-off-by: Wang ShaoBo
> ---
> drivers/edac/mv64x60_edac.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/edac/mv64x60_edac.c b/drivers/edac/mv64x60_edac.c
> index 3c68bb525d5d..456b9ca1fe8d 100644
> --- a/drivers/edac/mv64x60_edac.c
> +++ b/drivers/edac/mv64x60_edac.c
> @@ -168,6 +168,7 @@ static int mv64x60_pci_err_probe(struct platform_device
> *pdev)
>
> if (edac_pci_add_device(pci, pdata->edac_idx) > 0) {
> edac_dbg(3, "failed edac_pci_add_device()\n");
> + res = -ENODEV;
> goto err;
> }
That driver depends on MV64X60 and I don't see anything in the tree
enabling it and I can't select it AFAICT:
config MV64X60
bool
select PPC_INDIRECT_PCI
select CHECK_CACHE_COHERENCY
PPC folks, what do we do here?
If not used anymore, I'd love to have one less EDAC driver.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH 2/2] dt: Remove booting-without-of.rst
On Thu, Oct 08, 2020 at 09:24:20AM -0500, Rob Herring wrote: > booting-without-of.rstt is an ancient document that first outlined > Flattened DeviceTree on PowerPC initially. The DT world has evolved a > lot in the 15 years since and booting-without-of.rst is pretty stale. > The name of the document itself is confusing if you don't understand the > evolution from real 'OpenFirmware'. Most of what booting-without-of.rst > contains is now in the DT specification (which evolved out of the > ePAPR). The few things that weren't documented in the DT specification > are now. > > All that remains is the boot entry details, so let's move these to arch > specific documents. The exception is arm which already has the same > details documented. > > Cc: Frank Rowand > Cc: Mauro Carvalho Chehab > Cc: Geert Uytterhoeven > Cc: Michael Ellerman > Cc: Thomas Bogendoerfer > Cc: Jonathan Corbet > Cc: Paul Mackerras > Cc: Yoshinori Sato > Cc: Rich Felker > Cc: Thomas Gleixner > Cc: Ingo Molnar > Cc: Borislav Petkov > Cc: "H. Peter Anvin" > Cc: x...@kernel.org > Cc: linuxppc-dev@lists.ozlabs.org > Cc: linux-m...@vger.kernel.org > Cc: linux-...@vger.kernel.org > Cc: linux...@vger.kernel.org > Acked-by: Benjamin Herrenschmidt > Signed-off-by: Rob Herring > --- > .../devicetree/booting-without-of.rst | 1585 - > Documentation/devicetree/index.rst|1 - > Documentation/mips/booting.rst| 28 + > Documentation/mips/index.rst |1 + > Documentation/powerpc/booting.rst | 110 ++ > Documentation/powerpc/index.rst |1 + > Documentation/sh/booting.rst | 12 + > Documentation/sh/index.rst|1 + > Documentation/x86/booting-dt.rst | 21 + > Documentation/x86/index.rst |1 + For x86: Acked-by: Borislav Petkov Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH 09/13] x86: Remove dev->archdata.iommu pointer
On Thu, Jun 25, 2020 at 03:08:32PM +0200, Joerg Roedel wrote:
> From: Joerg Roedel
>
> There are no users left, all drivers have been converted to use the
> per-device private pointer offered by IOMMU core.
>
> Signed-off-by: Joerg Roedel
> ---
> arch/x86/include/asm/device.h | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/arch/x86/include/asm/device.h b/arch/x86/include/asm/device.h
> index 49bd6cf3eec9..7c0a52ca2f4d 100644
> --- a/arch/x86/include/asm/device.h
> +++ b/arch/x86/include/asm/device.h
> @@ -3,9 +3,6 @@
> #define _ASM_X86_DEVICE_H
>
> struct dev_archdata {
> -#ifdef CONFIG_IOMMU_API
> - void *iommu; /* hook for IOMMU specific extension */
> -#endif
> };
>
> struct pdev_archdata {
> --
Acked-by: Borislav Petkov
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v13 2/6] seq_buf: Export seq_buf_printf
On Mon, Jun 15, 2020 at 06:14:03PM +0530, Vaibhav Jain wrote: > 'seq_buf' provides a very useful abstraction for writing to a string > buffer without needing to worry about it over-flowing. However even > though the API has been stable for couple of years now its still not > exported to kernel loadable modules limiting its usage. > > Hence this patch proposes update to 'seq_buf.c' to mark > seq_buf_printf() which is part of the seq_buf API to be exported to > kernel loadable GPL modules. This symbol will be used in later parts > of this patch-set to simplify content creation for a sysfs attribute. > > Cc: Piotr Maziarz > Cc: Cezary Rojewski > Cc: Christoph Hellwig > Cc: Steven Rostedt > Cc: Borislav Petkov > Acked-by: Steven Rostedt (VMware) > Signed-off-by: Vaibhav Jain > --- > Changelog: > > v12..v13: > * None > > v11..v12: > * None Can you please resend your patchset once a week like everyone else and not flood inboxes with it? -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v7 2/5] seq_buf: Export seq_buf_printf() to external modules
On Fri, May 08, 2020 at 05:30:31PM +0530, Vaibhav Jain wrote:
> I am referring to Kernel Loadable Modules with MODULE_LICENSE("GPL")
> here.
And what does "external" refer to? Because if it is out-of-tree, we
don't export symbols for out-of-tree modules.
Looks like you're exporting it for that papr_scm.c thing, which is fine.
But that is not "external".
So?
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v7 2/5] seq_buf: Export seq_buf_printf() to external modules
On Fri, May 08, 2020 at 04:19:19PM +0530, Vaibhav Jain wrote: > 'seq_buf' provides a very useful abstraction for writing to a string > buffer without needing to worry about it over-flowing. However even > though the API has been stable for couple of years now its stills not > exported to external modules limiting its usage. > > Hence this patch proposes update to 'seq_buf.c' to mark > seq_buf_printf() which is part of the seq_buf API to be exported to > external GPL modules. This symbol will be used in later parts of this What is "external GPL modules"? -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] x86: Fix early boot crash on gcc-10, next try
On Sat, Apr 25, 2020 at 02:15:49PM -0500, Segher Boessenkool wrote: > My point is that you should explain at *every use* of this why you cannot > have tail calls *there*. This is very unusual, after all. > > There are *very* few places where you want to prevent tail calls, that's > why there is no attribute for it. Well, there is only one reason *why* so far - to prevent the stack canary cookie from being checked before returning from the function which set it. That could be explained once over the macro definition so that it can be looked up. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] x86: Fix early boot crash on gcc-10, next try
On Sat, Apr 25, 2020 at 01:37:01PM -0500, Segher Boessenkool wrote:
> That is a lot more typing then
> asm("");
That's why a macro with a hopefully more descriptive name would be
telling more than a mere asm("").
> but more seriously, you probably should explain why you do not want a
> tail call *anyway*, and in such a comment you can say that is what the
> asm is for.
Yes, the final version will have a comment and the whole spiel. This
diff is just me polling the maintainers: "do you want this for your arch
too?" Well, the PPC maintainers only, actually.
The other call in init/main.c would be for everybody.
> I don't see anything that prevents the tailcall in current code either,
> fwiw.
Right, and I don't see a reason why gcc-10 would do that optimization on
x86 only but I better ask first.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] x86: Fix early boot crash on gcc-10, next try
On Sat, Apr 25, 2020 at 07:31:40PM +0200, Borislav Petkov wrote:
> Hmm, that's what I was afraid of - having to sprinkle this around. Yah, let's
> wait for compiler guys to have a look here and then maybe I'll convert that
> thing to a macro called
>
> compiler_prevent_tail_call_opt()
>
> or so, so that it can be sprinkled around. ;-\
IOW, something like this (ontop) which takes care of the xen case too.
If it needs to be used by all arches, then I'll split the patch:
---
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 73bf8450afa1..4f275ac7830b 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -273,7 +273,7 @@ static void notrace start_secondary(void *unused)
* boot_init_stack_canary() and must not be checked before tail calling
* another function.
*/
- asm ("");
+ prevent_tail_call_optimization();
}
/**
diff --git a/arch/x86/xen/smp_pv.c b/arch/x86/xen/smp_pv.c
index 8fb8a50a28b4..f2adb63b2d7c 100644
--- a/arch/x86/xen/smp_pv.c
+++ b/arch/x86/xen/smp_pv.c
@@ -93,6 +93,7 @@ asmlinkage __visible void cpu_bringup_and_idle(void)
cpu_bringup();
boot_init_stack_canary();
cpu_startup_entry(CPUHP_AP_ONLINE_IDLE);
+ prevent_tail_call_optimization();
}
void xen_smp_intr_free_pv(unsigned int cpu)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 034b0a644efc..73f889f64513 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -356,4 +356,7 @@ static inline void *offset_to_ptr(const int *off)
/* [0] degrades to a pointer: a different type from an array */
#define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
+
+#define prevent_tail_call_optimization() asm("")
+
#endif /* __LINUX_COMPILER_H */
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] x86: Fix early boot crash on gcc-10, next try
On Sat, Apr 25, 2020 at 11:04:40AM -0400, Arvind Sankar wrote:
> I'd put the clause about stack protector being disabled and the
> tail-call one together, to make clear that you still need the never
> return and always inline bits.
Done.
> Also, this function is implemented by multiple arch's and they all
> have similar comments -- might be better to consolidate the comment in
> the generic (dummy) one in include/linux/stackprotector.h laying out
> the restrictions that arch implementations should follow?
I'm not sure gcc-10 does the same thing on other arches - I'd let gcc
guys chime in here and other arch maintainers to decide what to do.
> There's also the one in init/main.c which is used by multiple
> architectures. On x86 at least, the call to arch_call_rest_init at the
> end of start_kernel does not get tail-call optimized by gcc-10, but I
> don't see anything that actually prevents that from happening. We should
> add the asm("") there as well I think, unless the compiler guys see
> something about this function that will always prevent the optimization?
Hmm, that's what I was afraid of - having to sprinkle this around. Yah, let's
wait for compiler guys to have a look here and then maybe I'll convert that
thing to a macro called
compiler_prevent_tail_call_opt()
or so, so that it can be sprinkled around. ;-\
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: linux-next: build failure after merge of the tip tree
On Mon, Mar 30, 2020 at 07:04:16PM +1100, Michael Ellerman wrote: > Or just squash the hunk Stephen posted into the commit, which is what I > thought would happen to begin with. > > You can have my ack for it: > > Acked-by: Michael Ellerman (powerpc) Thanks but considering how this is not really urgent stuff and it can take its time and get some wider testing before getting upstream, I'd prefer to delay it. -- Regards/Gruss, Boris. SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg
Re: linux-next: build failure after merge of the tip tree
On Mon, Mar 30, 2020 at 03:08:19PM +1100, Stephen Rothwell wrote: > What you really need is an Ack from the PowerPC people for the fix you > suggested and then tha fix should go in the same series that is now > causing the failure (preferably before the problematic (for PowerPC) > patch. I'll zap this commit from the tip lineup. There's always another merge window. -- Regards/Gruss, Boris. SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg
Re: [PATCH -v2] treewide: Rename "unencrypted" to "decrypted"
On Thu, Mar 19, 2020 at 06:25:49PM +0100, Thomas Gleixner wrote:
> TBH, I don't see how
>
> if (force_dma_decrypted(dev))
> set_memory_encrypted((unsigned long)cpu_addr, 1 << page_order);
>
> makes more sense than the above. It's both non-sensical unless there is
9087c37584fb ("dma-direct: Force unencrypted DMA under SME for certain DMA
masks")
> a big fat comment explaining what this is about.
ACK to that.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH -v2] treewide: Rename "unencrypted" to "decrypted"
On Thu, Mar 19, 2020 at 11:06:15AM +, Robin Murphy wrote: > Let me add another vote from a native English speaker that "unencrypted" is > the appropriate term to imply the *absence* of encryption, whereas > "decrypted" implies the *reversal* of applied encryption. > > Naming things is famously hard, for good reason - names are *important* for > understanding. Just because a decision was already made one way doesn't mean > that that decision was necessarily right. Churning one area to be > consistently inaccurate just because it's less work than churning another > area to be consistently accurate isn't really the best excuse. Well, the reason we chose "decrypted" vs something else is so to be as different from "encrypted" as possible. If we called it "unencrypted" you'd have stuff like: if (force_dma_unencrypted(dev)) set_memory_encrypted((unsigned long)cpu_addr, 1 << page_order); and I *betcha* people will misread this and maybe even introduce bugs. So I don't think renaming it to "unencrypted" is better. And yes, I'm deliberately putting the language semantics here on a second place because of readability examples like the one above. But ok, since people don't want this, we can leave it as is. It's not like I don't have anything better to do. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH -v2] treewide: Rename "unencrypted" to "decrypted"
On Thu, Mar 19, 2020 at 11:20:11AM +0100, Christoph Hellwig wrote: > I thought we agreed that decrypted is absolutely the wrong term. I don't think we did. At least I don't know where we did that. > So NAK - if you want to change things it needs to go the other way. We are already using "decrypted" everywhere in arch/x86/. Changing that would be a *lot* more churn. And it is just a term, for chrissakes, to denote memory which is not encrypted. And it would make our lifes easier if we had only *two* terms instead of three or more. Especially if the concept we denote with this is a binary one: encrypted memory and *not* encrypted memory. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
[PATCH -v2] treewide: Rename "unencrypted" to "decrypted"
Hi,
here's v2 with build breakage fixed on ppc and s390 (obviously I can't
grep :-\) and commit message extended to explain more why.
Thx.
---
From: Borislav Petkov
Date: Tue, 17 Mar 2020 12:03:05 +0100
Back then when the whole SME machinery started getting mainlined, it
was agreed that for simplicity, clarity and sanity's sake, the terms
denoting encrypted and not-encrypted memory should be "encrypted" and
"decrypted". And the majority of the code sticks to that convention
except those two. So rename them.
The intent of "encrypted" and "decrypted" is to represent the binary
concept of memory which is encrypted and of memory which is not.
The further differentiation between decrypted and unencrypted memory is
not needed in the code (for now) and therefore keep things simple by
using solely the two terms.
No functional changes.
[ Convert forgotten s390 and ppc function variants. ]
Reported-by: kbuild test robot
Signed-off-by: Borislav Petkov
---
arch/powerpc/include/asm/mem_encrypt.h | 2 +-
arch/powerpc/platforms/pseries/Kconfig | 2 +-
arch/s390/Kconfig | 2 +-
arch/s390/mm/init.c| 2 +-
arch/x86/Kconfig | 2 +-
arch/x86/mm/mem_encrypt.c | 4 ++--
include/linux/dma-direct.h | 8
kernel/dma/Kconfig | 2 +-
kernel/dma/direct.c| 14 +++---
kernel/dma/mapping.c | 2 +-
10 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/arch/powerpc/include/asm/mem_encrypt.h
b/arch/powerpc/include/asm/mem_encrypt.h
index ba9dab07c1be..f0705cd3b079 100644
--- a/arch/powerpc/include/asm/mem_encrypt.h
+++ b/arch/powerpc/include/asm/mem_encrypt.h
@@ -15,7 +15,7 @@ static inline bool mem_encrypt_active(void)
return is_secure_guest();
}
-static inline bool force_dma_unencrypted(struct device *dev)
+static inline bool force_dma_decrypted(struct device *dev)
{
return is_secure_guest();
}
diff --git a/arch/powerpc/platforms/pseries/Kconfig
b/arch/powerpc/platforms/pseries/Kconfig
index 24c18362e5ea..a78e2c3e1d92 100644
--- a/arch/powerpc/platforms/pseries/Kconfig
+++ b/arch/powerpc/platforms/pseries/Kconfig
@@ -151,7 +151,7 @@ config PPC_SVM
depends on PPC_PSERIES
select SWIOTLB
select ARCH_HAS_MEM_ENCRYPT
- select ARCH_HAS_FORCE_DMA_UNENCRYPTED
+ select ARCH_HAS_FORCE_DMA_DECRYPTED
help
There are certain POWER platforms which support secure guests using
the Protected Execution Facility, with the help of an Ultravisor
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 8abe77536d9d..ab1dbb7415b4 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -192,7 +192,7 @@ config S390
select VIRT_CPU_ACCOUNTING
select ARCH_HAS_SCALED_CPUTIME
select HAVE_NMI
- select ARCH_HAS_FORCE_DMA_UNENCRYPTED
+ select ARCH_HAS_FORCE_DMA_DECRYPTED
select SWIOTLB
select GENERIC_ALLOCATOR
diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
index ac44bd76db4b..5fed85f9fea6 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -157,7 +157,7 @@ int set_memory_decrypted(unsigned long addr, int numpages)
}
/* are we a protected virtualization guest? */
-bool force_dma_unencrypted(struct device *dev)
+bool force_dma_decrypted(struct device *dev)
{
return is_prot_virt_guest();
}
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index beea77046f9b..2ae904f505e1 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1525,7 +1525,7 @@ config AMD_MEM_ENCRYPT
depends on X86_64 && CPU_SUP_AMD
select DYNAMIC_PHYSICAL_MASK
select ARCH_USE_MEMREMAP_PROT
- select ARCH_HAS_FORCE_DMA_UNENCRYPTED
+ select ARCH_HAS_FORCE_DMA_DECRYPTED
---help---
Say yes to enable support for the encryption of system memory.
This requires an AMD processor that supports Secure Memory
diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index a03614bd3e1a..66d09f269e6d 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -350,8 +350,8 @@ bool sev_active(void)
return sme_me_mask && sev_enabled;
}
-/* Override for DMA direct allocation check - ARCH_HAS_FORCE_DMA_UNENCRYPTED */
-bool force_dma_unencrypted(struct device *dev)
+/* Override for DMA direct allocation check - ARCH_HAS_FORCE_DMA_DECRYPTED */
+bool force_dma_decrypted(struct device *dev)
{
/*
* For SEV, all DMA must be to unencrypted addresses.
diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
index 24b8684aa21d..9f955844e9c7 100644
--- a/include/linux/dma-direct.h
+++ b/include/linux/dma-direct.h
@@ -26,14 +26,14 @@ static inline phys_addr_t __dma_to_phys(struct device *dev,
dma_addr_t dev_addr)
}
#endif /* !CONFIG_ARCH_HAS_PHYS_TO_DMA */
-#ifdef CONFIG_ARCH_HAS_FORCE_D
