Re: [PATCH v9 3/8] x86/vmware: Introduce VMware hypercall API
On Mon, May 06, 2024 at 02:53:00PM -0700, Alexey Makhalov wrote:
> +#define VMWARE_HYPERCALL \
> + ALTERNATIVE_3("cmpb $" \
> + __stringify(CPUID_VMWARE_FEATURES_ECX_VMMCALL) \
> + ", %[mode]\n\t" \
> + "jg 2f\n\t" \
> + "je 1f\n\t" \
> + "movw %[port], %%dx\n\t" \
> + "inl (%%dx), %%eax\n\t" \
> + "jmp 3f\n\t" \
> + "1: vmmcall\n\t" \
> + "jmp 3f\n\t" \
> + "2: vmcall\n\t" \
> + "3:\n\t", \
> + "movw %[port], %%dx\n\t" \
> + "inl (%%dx), %%eax", X86_FEATURE_HYPERVISOR, \
That's a bunch of insns and their size would inadvertently go into the final
image.
What you should try to do is something like this:
ALTERNATIVE_3("jmp .Lend_legacy_call", "", X86_FEATURE_HYPERVISOR,
"vmcall; jmp .Lend_legacy_call", X86_FEATURE_VMCALL,
"vmmcall; jmp .Lend_legacy_call", X86_FEATURE_VMW_VMMCALL)
/* bunch of conditional branches and INs and V*MCALLs, etc go
here */
.Lend_legacy_call:
so that you don't have these 26 bytes, as you say, of alternatives to patch but
only the JMPs and the VM*CALLs.
See for an example the macros in arch/x86/entry/calling.h which simply jump
over the code when not needed.
Also, you could restructure the alternative differently so that that bunch of
insns call is completely out-of-line because all current machines support
VM*CALL so you won't even need to patch. You only get to patch when running on
some old rust and there you can just as well go completely out-of-line.
Something along those lines, anyway.
> - * The high bandwidth in call. The low word of edx is presumed to have the
> - * HB bit set.
> + * High bandwidth calls are not supported on encrypted memory guests.
> + * The caller should check cc_platform_has(CC_ATTR_MEM_ENCRYPT) and use
> + * low bandwidth hypercall it memory encryption is set.
s/it/if/
> -#define VMWARE_PORT(cmd, eax, ebx, ecx, edx) \
> - __asm__("inl (%%dx), %%eax" : \
> - "=a"(eax), "=c"(ecx), "=d"(edx), "=b"(ebx) :\
> - "a"(VMWARE_HYPERVISOR_MAGIC), \
> - "c"(VMWARE_CMD_##cmd), \
> - "d"(VMWARE_HYPERVISOR_PORT), "b"(UINT_MAX) :\
> - "memory")
> -
> -#define VMWARE_VMCALL(cmd, eax, ebx, ecx, edx)
> \
> - __asm__("vmcall" : \
> - "=a"(eax), "=c"(ecx), "=d"(edx), "=b"(ebx) :\
> - "a"(VMWARE_HYPERVISOR_MAGIC), \
> - "c"(VMWARE_CMD_##cmd), \
> - "d"(0), "b"(UINT_MAX) : \
> - "memory")
> -
> -#define VMWARE_VMMCALL(cmd, eax, ebx, ecx, edx)
> \
> - __asm__("vmmcall" : \
> - "=a"(eax), "=c"(ecx), "=d"(edx), "=b"(ebx) :\
> - "a"(VMWARE_HYPERVISOR_MAGIC), \
> - "c"(VMWARE_CMD_##cmd), \
> - "d"(0), "b"(UINT_MAX) : \
> - "memory")
> -
> -#define VMWARE_CMD(cmd, eax, ebx, ecx, edx) do { \
> - switch (vmware_hypercall_mode) {\
> - case CPUID_VMWARE_FEATURES_ECX_VMCALL: \
> - VMWARE_VMCALL(cmd, eax, ebx, ecx, edx); \
> - break; \
> - case CPUID_VMWARE_FEATURES_ECX_VMMCALL: \
> - VMWARE_VMMCALL(cmd, eax, ebx, ecx, edx);\
> - break; \
> - default:\
> - VMWARE_PORT(cmd, eax, ebx, ecx, edx); \
> - break; \
> - } \
> - } while (0)
You're kidding, right?
You went to all that trouble in patch 1 to move those to the header only to
*remove* them here?
You do realize that that is a unnecessary churn for no good reason, right?
So that set needs to be
Re: [PATCH v9 1/8] x86/vmware: Move common macros to vmware.h
On Mon, May 06, 2024 at 02:52:58PM -0700, Alexey Makhalov wrote: > +#define VMWARE_HYPERVISOR_PORT 0x5658 > +#define VMWARE_HYPERVISOR_PORT_HB(VMWARE_HYPERVISOR_PORT | \ > + VMWARE_HYPERVISOR_HB) You can't help yourself not sneaking in any changes which are not code movement, can ya? The purpose of a sole code movement patch is to ease the review. Not to have to look at the code movement *and* some *additional* changes which you've done in-flight. Just because you felt like it. But which is nasty to review. Maybe you'll understand that better when you get to review someone else's patch which does crap like that. Make sure you remember that in the future, when sending patches. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v9 2/8] x86/vmware: Move common macros to vmware.h
On Wed, Apr 24, 2024 at 04:14:07PM -0700, Alexey Makhalov wrote: > +#define VMWARE_CMD_GETVERSION10 > +#define VMWARE_CMD_GETHZ 45 > +#define VMWARE_CMD_GETVCPU_INFO 68 > +#define VMWARE_CMD_STEALCLOCK91 Ok, what part in "* first patch: sole code movement, no changes whatsoever" wasn't clear? You're adding those macros above to a patch which claims that it does code movement only. Don't do that. Let me try again: * first patch: *ONLY* code movement. No new code, no new defines, no new functions, no nada! Only move code and do fixups so that it still builds like the export you're doing. * follow-on patches: other changes. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v9 1/8] x86/vmware: Correct macro names
On Wed, Apr 24, 2024 at 04:14:06PM -0700, Alexey Makhalov wrote: > VCPU_RESERVED and LEGACY_X2APIC are not VMware hypercall commands. > These are bits in return value of VMWARE_CMD_GETVCPU_INFO command. > Change VMWARE_CMD_ prefix to GETVCPU_INFO_ one. And move bit-shift > operation to the macro body. I don't understand: $ git grep GETVCPU_INFO arch/x86/kernel/cpu/vmware.c:51:#define VMWARE_CMD_GETVCPU_INFO 68 arch/x86/kernel/cpu/vmware.c:478: VMWARE_CMD(GETVCPU_INFO, eax, ebx, ecx, edx); so that's a VMWARE_CMD 68, at least the prefix says so. And those two are *bits* in that eax which that hypercall returns. Or are those two bits generic but defined in a vmware-specific hypercall? Hm. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v8 1/7] x86/vmware: Move common macros to vmware.h
On Mon, Apr 22, 2024 at 03:56:50PM -0700, Alexey Makhalov wrote:
> Move VMware hypercall macros to vmware.h. This is a prerequisite for
> the introduction of vmware_hypercall API. No functional changes besides
> exporting vmware_hypercall_mode symbol.
Well, I see more.
So code movement patches should be done this way:
* first patch: sole code movement, no changes whatsoever
* follow-on patches: add changes and explain them
Because... (follow me down)...
> @@ -476,8 +431,8 @@ static bool __init vmware_legacy_x2apic_available(void)
> {
> uint32_t eax, ebx, ecx, edx;
> VMWARE_CMD(GETVCPU_INFO, eax, ebx, ecx, edx);
> - return !(eax & BIT(VMWARE_CMD_VCPU_RESERVED)) &&
> - (eax & BIT(VMWARE_CMD_LEGACY_X2APIC));
> + return !(eax & BIT(VCPU_RESERVED)) &&
> + (eax & BIT(VCPU_LEGACY_X2APIC));
... what is that change for?
Those bit definitions are clearly vmware-specific. So why are you
changing them to something generic-ish?
In any case, this patch needs to be split as outlined above.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
amdgpu kmemleaks
Hi folks, anyone interested in a bunch of amdgpu kmemleak reports from latest Linus tree + tip? GPU is: [ 11.317312] [drm] amdgpu kernel modesetting enabled. [ 11.363627] [drm] initializing kernel modesetting (CARRIZO 0x1002:0x9874 0x103C:0x807E 0xC4). [ 11.364077] [drm] register mmio base: 0xD0C0 [ 11.364547] [drm] register mmio size: 262144 [ 11.365347] [drm] add ip block number 0 [ 11.365580] [drm] add ip block number 1 [ 11.365840] [drm] add ip block number 2 [ 11.366047] [drm] add ip block number 3 [ 11.366263] [drm] add ip block number 4 [ 11.366470] [drm] add ip block number 5 [ 11.32] [drm] add ip block number 6 [ 11.366835] [drm] add ip block number 7 [ 11.367022] [drm] add ip block number 8 [ 11.382774] [drm] BIOS signature incorrect 5b 7 [ 11.383002] resource: resource sanity check: requesting [mem 0x000c-0x000d], which spans more than PCI Bus :00 [mem 0x000c-0x000cbfff window] [ 11.383655] caller pci_map_rom+0x68/0x1d0 mapping multiple BARs [ 11.384009] amdgpu :00:01.0: amdgpu: Fetched VBIOS from ROM BAR [ 11.384402] amdgpu: ATOM BIOS: SWBRT27354.001 [ 11.385827] [drm] UVD is enabled in physical mode [ 11.386063] [drm] VCE enabled in physical mode [ 11.386886] amdgpu :00:01.0: vgaarb: deactivate vga console [ 11.389089] Console: switching to colour dummy device 80x25 [ 11.389543] amdgpu :00:01.0: amdgpu: Trusted Memory Zone (TMZ) feature not supported [ 11.390482] [drm] vm size is 64 GB, 2 levels, block size is 10-bit, fragment size is 9-bit [ 11.390793] amdgpu :00:01.0: amdgpu: VRAM: 512M 0x00F4 - 0x00F41FFF (512M used) [ 11.391129] amdgpu :00:01.0: amdgpu: GART: 1024M 0x00FF - 0x00FF3FFF [ 11.391456] [drm] Detected VRAM RAM=512M, BAR=512M [ 11.391632] [drm] RAM width 128bits UNKNOWN [ 11.394546] [drm] amdgpu: 512M of VRAM memory ready [ 11.394751] [drm] amdgpu: 7622M of GTT memory ready. [ 11.395299] [drm] GART: num cpu pages 262144, num gpu pages 262144 [ 11.395813] [drm] PCIE GART of 1024M enabled (table at 0x00F400A0). [ 11.404914] amdgpu: hwmgr_sw_init smu backed is smu8_smu [ 11.407177] [drm] Found UVD firmware Version: 1.91 Family ID: 11 [ 11.407670] [drm] UVD ENC is disabled [ 11.409969] [drm] Found VCE firmware Version: 52.4 Binary ID: 3 [ 11.412601] amdgpu: smu version 18.62.00 [ 11.419275] [drm] DM_PPLIB: values for Engine clock [ 11.419480] [drm] DM_PPLIB: 30 [ 11.419610] [drm] DM_PPLIB: 36 [ 11.419740] [drm] DM_PPLIB: 423530 [ 11.419869] [drm] DM_PPLIB: 514290 [ 11.419998] [drm] DM_PPLIB: 626090 [ 11.420127] [drm] DM_PPLIB: 72 [ 11.420327] [drm] DM_PPLIB: Validation clocks: [ 11.420536] [drm] DM_PPLIB:engine_max_clock: 72000 [ 11.420722] [drm] DM_PPLIB:memory_max_clock: 8 [ 11.420907] [drm] DM_PPLIB:level : 8 [ 11.421083] [drm] DM_PPLIB: values for Display clock [ 11.421266] [drm] DM_PPLIB: 30 [ 11.421395] [drm] DM_PPLIB: 40 [ 11.421524] [drm] DM_PPLIB: 496560 [ 11.421652] [drm] DM_PPLIB: 626090 [ 11.421781] [drm] DM_PPLIB: 685720 [ 11.421910] [drm] DM_PPLIB: 757900 [ 11.422039] [drm] DM_PPLIB: Validation clocks: [ 11.422201] [drm] DM_PPLIB:engine_max_clock: 72000 [ 11.422386] [drm] DM_PPLIB:memory_max_clock: 8 [ 11.422572] [drm] DM_PPLIB:level : 8 [ 11.422746] [drm] DM_PPLIB: values for Memory clock [ 11.422923] [drm] DM_PPLIB: 333000 [ 11.423052] [drm] DM_PPLIB: 80 [ 11.423181] [drm] DM_PPLIB: Validation clocks: [ 11.423342] [drm] DM_PPLIB:engine_max_clock: 72000 [ 11.423528] [drm] DM_PPLIB:memory_max_clock: 8 [ 11.423713] [drm] DM_PPLIB:level : 8 [ 11.424561] [drm] Display Core v3.2.266 initialized on DCE 11.0 [ 11.516117] [drm] UVD initialized successfully. [ 11.716119] [drm] VCE initialized successfully. unreferenced object 0x88810e6faa80 (size 128): comm "systemd-udevd", pid 1219, jiffies 4294895080 hex dump (first 32 bytes): 18 cb 03 00 00 0a 28 0a 48 0a c8 0a 00 00 a0 05 ..(.H... aa 05 b4 05 dc 05 00 00 0a 00 00 00 00 00 00 00 backtrace (crc 5201319b): [<6e1e4989>] kmalloc_trace+0x25a/0x300 [<7b61fcfc>] do_detailed_mode+0x323/0x670 [<79955120>] drm_for_each_detailed_block.part.0+0x34/0x180 [<9a087c6a>] _drm_edid_connector_add_modes.part.0+0x8f/0x10b0 [] drm_add_edid_modes+0x14e/0x160 [<0a49b747>] amdgpu_dm_connector_get_modes+0x13b/0x470 [amdgpu] [<5f5da5a5>] amdgpu_dm_init.isra.0+0x12ed/0x1e50 [amdgpu] [ ] dm_hw_init+0xe/0x20 [amdgpu] [ ] amdgpu_device_init+0x1f17/0x2530 [amdgpu] [<9c22ce56>] amdgpu_driver_load_kms+0x23/0x1a0 [amdgpu] [<8bc75f74>] amdgpu_pci_probe+0x1b5/0x550
Re: [PATCH v6 0/7] VMware hypercalls enhancements
On Tue, Jan 09, 2024 at 12:40:45AM -0800, Alexey Makhalov wrote: > v5->v6 change: > - Added ack by Kirill A. Shutemov in patch 7. Please do not spam. Adding someone's Ack does not mean you have to resend the whole thing immediately again. While waiting, please read Documentation/process/submitting-patches.rst and especially: "Don't get discouraged - or impatient After you have submitted your change, be patient and wait. Reviewers are busy people and may not get to your patch right away. Once upon a time, patches used to disappear into the void without comment, but the development process works more smoothly than that now. You should receive comments within a few weeks (typically 2-3); if that does not happen, make sure that you have sent your patches to the right place. Wait for a minimum of one week before resubmitting or pinging reviewers - possibly longer during busy times like merge windows." -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] drm/nouveau/kms/nv50-: Don't allow inheritance of headless iors
On Wed, Dec 13, 2023 at 07:43:57PM -0500, Lyude Paul wrote: > Turns out we made a silly mistake when coming up with OR inheritance on > nouveau. On pre-DCB 4.1, iors are statically routed to output paths via the > DCB. On later generations iors are only routed to an output path if they're > actually being used. Unfortunately, it appears with NVIF_OUTP_INHERIT_V0 we > make the mistake of assuming the later is true on all generations, which is > currently leading us to return bogus ior -> head assignments through nvif, > which causes WARN_ON(). > > So - fix this by verifying that we actually know that there's a head > assigned to an ior before allowing it to be inherited through nvif. This > -should- hopefully fix the WARN_ON on GT218 reported by Borislav. > > Signed-off-by: Lyude Paul > Cc: Borislav Petkov > --- > drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c > b/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c > index e4279f1772a1b..377d0e0cef848 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c > +++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c > @@ -385,7 +385,7 @@ nvkm_uoutp_mthd_inherit(struct nvkm_outp *outp, void > *argv, u32 argc) > > /* Ensure an ior is hooked up to this outp already */ > ior = outp->func->inherit(outp); > - if (!ior) > + if (!ior || !ior->arm.head) > return -ENODEV; > > /* With iors, there will be a separate output path for each type of > connector - and all of > -- Thanks, that fixes it! Reported-by: Borislav Petkov (AMD) Tested-by: Borislav Petkov (AMD) -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: nouveau 0000:01:00.0: drm_WARN_ON(!found_head)
On Wed, Dec 13, 2023 at 12:39:36PM +0100, Borislav Petkov wrote: > We're getting close to releasing so I guess we either debug this or shut > up the WARN. Not only that - panic_on_warn turns this into an explosion so you don't want that in a released kernel. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: nouveau 0000:01:00.0: drm_WARN_ON(!found_head)
On Tue, Dec 12, 2023 at 10:35:51PM -0500, Paul Dufresne wrote:
> https://gitlab.freedesktop.org/drm/nouveau/-/issues/282
Let's add more folks who were involved in
1b477f42285e ("drm/nouveau/kms: Add INHERIT ioctl to nvkm/nvif for reading IOR
state")
Apparently, someone wants to know that the loop over the crtcs in
nv50_display_read_hw_or_state() didn't find a head.
Holler if you need me to run a debug patch to figure out why.
We're getting close to releasing so I guess we either debug this or shut
up the WARN.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: nouveau 0000:01:00.0: drm_WARN_ON(!found_head)
On Sat, Nov 11, 2023 at 01:03:23PM +0100, Borislav Petkov wrote: > Hi, > > this is ontop of Linus' tree from the 4th (lemme know if I should try > the latest) on one of my test boxes: > > nouveau :01:00.0: vgaarb: deactivate vga console > Console: switching to colour dummy device 80x25 > nouveau :01:00.0: NVIDIA GT218 (0a8280b1) > CE: hpet increased min_delta_ns to 20115 nsec > nouveau :01:00.0: bios: version 70.18.49.00.00 > nouveau :01:00.0: fb: 1024 MiB DDR3 > nouveau :01:00.0: DRM: VRAM: 1024 MiB > nouveau :01:00.0: DRM: GART: 1048576 MiB > nouveau :01:00.0: DRM: TMDS table version 2.0 > nouveau :01:00.0: DRM: MM: using COPY for buffer copies > [ cut here ] > nouveau :01:00.0: drm_WARN_ON(!found_head) > WARNING: CPU: 4 PID: 786 at drivers/gpu/drm/nouveau/dispnv50/disp.c:2731 > nv50_display_init+0x28c/0x4f0 [nouveau] > Modules linked in: nouveau(+) drm_ttm_helper ttm video drm_exec drm_gpuvm > gpu_sched drm_display_helper wmi > CPU: 4 PID: 786 Comm: systemd-udevd Not tainted 6.6.0+ #1 This still fires on -rc5: [4.577348] nouveau :01:00.0: vgaarb: deactivate vga console [4.584482] Console: switching to colour dummy device 80x25 [4.590120] nouveau :01:00.0: NVIDIA GT218 (0a8280b1) [4.718171] nouveau :01:00.0: bios: version 70.18.49.00.00 [4.724788] nouveau :01:00.0: fb: 1024 MiB DDR3 [6.047984] nouveau :01:00.0: DRM: VRAM: 1024 MiB [6.053031] nouveau :01:00.0: DRM: GART: 1048576 MiB [6.058340] nouveau :01:00.0: DRM: TMDS table version 2.0 [6.065892] nouveau :01:00.0: DRM: MM: using COPY for buffer copies [6.078375] [ cut here ] [6.082994] nouveau :01:00.0: drm_WARN_ON(!found_head) [6.083023] WARNING: CPU: 3 PID: 779 at drivers/gpu/drm/nouveau/dispnv50/disp.c:2731 nv50_display_init+0x28c/0x4f0 [nouve au] [6.099800] Modules linked in: nouveau(+) drm_ttm_helper ttm video drm_exec drm_gpuvm gpu_sched drm_display_helper wmi [6.110490] CPU: 3 PID: 779 Comm: systemd-udevd Not tainted 6.7.0-rc5+ #2 [6.117272] Hardware name: MICRO-STAR INTERNATIONAL CO.,LTD MS-7599/870-C45 (MS-7599), BIOS V1.15 03/04/2011 [6.127087] RIP: 0010:nv50_display_init+0x28c/0x4f0 [nouveau] [6.132915] Code: 4c 8b 6f 50 4d 85 ed 75 03 4c 8b 2f e8 cd 16 37 e1 48 c7 c1 4c 55 2d a0 48 89 c6 4c 89 ea 48 c7 c7 42 5 5 2d a0 e8 44 5a e8 e0 <0f> 0b 48 8b 43 08 49 39 c6 48 8d 58 f8 0f 85 41 ff ff ff 48 8d 7c [6.151660] RSP: 0018:c936ba98 EFLAGS: 00010286 [6.156885] RAX: 002e RBX: 8881009fbc00 RCX: [6.164013] RDX: 0002 RSI: c936b9b0 RDI: 0001 [6.171141] RBP: 888103fc8ad0 R08: 888136ffdfe8 R09: 0058 [6.178263] R10: 027a R11: 888136401b70 R12: 888103fc8800 [6.185393] R13: 888100abddf0 R14: 888103fc8ab0 R15: [6.192521] FS: 7fdc144858c0() GS:88812f4c() knlGS: [6.200601] CS: 0010 DS: ES: CR0: 80050033 [6.206339] CR2: 55676cc01000 CR3: 000103f6c000 CR4: 06f0 [6.213466] Call Trace: [6.215921] [6.218015] ? __warn+0x96/0x160 [6.221240] ? nv50_display_init+0x28c/0x4f0 [nouveau] [6.226461] ? report_bug+0x1ec/0x200 [6.230119] ? handle_bug+0x3c/0x70 [6.233611] ? exc_invalid_op+0x1f/0x90 [6.237442] ? asm_exc_invalid_op+0x16/0x20 [6.241622] ? nv50_display_init+0x28c/0x4f0 [nouveau] [6.246840] ? nv50_display_init+0x28c/0x4f0 [nouveau] [6.252058] ? sched_set_fifo+0x46/0x60 [6.255897] nouveau_display_init+0xa0/0xd0 [nouveau] [6.261031] nouveau_drm_device_init+0x42a/0x990 [nouveau] [6.266604] nouveau_drm_probe+0x105/0x240 [nouveau] [6.271651] ? __pm_runtime_resume+0x68/0xa0 [6.275920] pci_device_probe+0xaa/0x140 [6.279840] really_probe+0xc2/0x2d0 [6.283411] __driver_probe_device+0x73/0x120 [6.287761] driver_probe_device+0x2c/0xb0 [6.291851] __driver_attach+0xa0/0x150 [6.295683] ? __device_attach_driver+0xc0/0xc0 [6.300205] bus_for_each_dev+0x67/0xa0 [6.304044] bus_add_driver+0x10e/0x210 [6.307874] driver_register+0x5c/0x120 [6.311706] ? 0xa0336000 [6.315017] do_one_initcall+0x44/0x200 [6.318851] ? kmalloc_trace+0x37/0xc0 [6.322595] do_init_module+0x64/0x230 [6.326344] init_module_from_file+0x8d/0xd0 [6.330609] idempotent_init_module+0x15a/0x210 [6.335136] __x64_sys_finit_module+0x67/0xb0 [6.339490] do_syscall_64+0x41/0xf0 [6.343066] entry_SYSCALL_64_after_hwframe+0x4b/0x53 [6.348118] RIP: 0033:0x7fdc14947ee9 [6.351691] Code: 08 44 89 e0 5b 41 5c c3 66 0f 1f 84 00 00 00 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0
Re: [PATCH v2 5/6] drm/vmwgfx: Use vmware_hypercall API
On Fri, Dec 01, 2023 at 03:24:51PM -0800, Alexey Makhalov wrote:
> Switch from VMWARE_HYPERCALL macro to vmware_hypercall API.
> Eliminate arch specific code.
>
> drivers/gpu/drm/vmwgfx/vmwgfx_msg_arm64.h: implement arm64 variant
> of vmware_hypercall here for now. The move of these functions to
> arch/arm64/include/asm/vmware.h as well as removal of
> drivers/gpu/drm/vmwgfx/vmwgfx_msg_{x86,arm64}.h header files will
> be performed in the follow up patchset.
Same note as for patch 1 - no commit order in git.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v2 2/6] x86/vmware: Introduce vmware_hypercall API
On Fri, Dec 01, 2023 at 03:24:48PM -0800, Alexey Makhalov wrote:
> Introducing vmware_hypercall family of functions as a common
> implementation to be used by the VMware guest code and virtual
> device drivers in architecture independent manner.
>
> By analogy with KVM hypercall API, vmware_hypercallX and
> vmware_hypercall_hb_{out,in} set of functions was added to
> achieve that. Architecture specific implementation should be
> hidden inside.
Pls read section "2) Describe your changes" in
Documentation/process/submitting-patches.rst for more details on how to
formulate your commit messages.
Also, see section "Changelog" in Documentation/process/maintainer-tip.rst
More specifically:
"Describe your changes in imperative mood, e.g. "make xyzzy do frotz"
instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy
to do frotz", as if you are giving orders to the codebase to change
its behaviour."
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v2 1/6] x86/vmware: Move common macros to vmware.h
On Fri, Dec 01, 2023 at 03:24:47PM -0800, Alexey Makhalov wrote: > Move VMware hypercall macros to vmware.h as a preparation step > for the next commit. No functional changes besides exporting "next commit" in git is ambiguous. Get rid of such formulations. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v2 6/6] x86/vmware: Add TDX hypercall support
On Fri, Dec 01, 2023 at 03:24:52PM -0800, Alexey Makhalov wrote:
> +#ifdef CONFIG_INTEL_TDX_GUEST
> +/* __tdx_hypercall() is not exported. So, export the wrapper */
> +void vmware_tdx_hypercall_args(struct tdx_module_args *args)
> +{
> + __tdx_hypercall(args);
> +}
> +EXPORT_SYMBOL_GPL(vmware_tdx_hypercall_args);
Uuuh, lovely. I'd like to see what the TDX folks think about this
export first.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
nouveau 0000:01:00.0: drm_WARN_ON(!found_head)
Hi, this is ontop of Linus' tree from the 4th (lemme know if I should try the latest) on one of my test boxes: nouveau :01:00.0: vgaarb: deactivate vga console Console: switching to colour dummy device 80x25 nouveau :01:00.0: NVIDIA GT218 (0a8280b1) CE: hpet increased min_delta_ns to 20115 nsec nouveau :01:00.0: bios: version 70.18.49.00.00 nouveau :01:00.0: fb: 1024 MiB DDR3 nouveau :01:00.0: DRM: VRAM: 1024 MiB nouveau :01:00.0: DRM: GART: 1048576 MiB nouveau :01:00.0: DRM: TMDS table version 2.0 nouveau :01:00.0: DRM: MM: using COPY for buffer copies [ cut here ] nouveau :01:00.0: drm_WARN_ON(!found_head) WARNING: CPU: 4 PID: 786 at drivers/gpu/drm/nouveau/dispnv50/disp.c:2731 nv50_display_init+0x28c/0x4f0 [nouveau] Modules linked in: nouveau(+) drm_ttm_helper ttm video drm_exec drm_gpuvm gpu_sched drm_display_helper wmi CPU: 4 PID: 786 Comm: systemd-udevd Not tainted 6.6.0+ #1 Hardware name: MICRO-STAR INTERNATIONAL CO.,LTD MS-7599/870-C45 (MS-7599), BIOS V1.15 03/04/2011 RIP: 0010:nv50_display_init+0x28c/0x4f0 [nouveau] Code: 4c 8b 6f 50 4d 85 ed 75 03 4c 8b 2f e8 6d 47 37 e1 48 c7 c1 4c 55 2d a0 48 89 c6 4c 89 ea 48 c7 c7 42 55 2d a0 e8 44 83 e8 e0 <0f> 0b 48 8b 43 08 49 39 c6 48 8d 58 f8 0f 85 41 ff ff ff 48 8d 7c RSP: 0018:c931ba98 EFLAGS: 00010286 RAX: 002e RBX: 888100a21400 RCX: RDX: 0002 RSI: c931b9b0 RDI: 0001 RBP: 888104eadad0 R08: 888136ffdfe8 R09: 0058 R10: 0289 R11: 888136401cd8 R12: 888104ead800 R13: 888100abddf0 R14: 888104eadab0 R15: FS: 7fef9c8e38c0() GS:88812f50() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 7fef9ce9d258 CR3: 00010330e000 CR4: 06f0 Call Trace: ? __warn+0x97/0x160 ? nv50_display_init+0x28c/0x4f0 [nouveau] ? report_bug+0x1ec/0x200 ? handle_bug+0x3c/0x70 ? exc_invalid_op+0x1f/0x90 ? asm_exc_invalid_op+0x16/0x20 ? nv50_display_init+0x28c/0x4f0 [nouveau] ? nv50_display_init+0x28c/0x4f0 [nouveau] ? sched_set_fifo+0x46/0x60 nouveau_display_init+0xa0/0xd0 [nouveau] nouveau_drm_device_init+0x42a/0x990 [nouveau] nouveau_drm_probe+0x105/0x240 [nouveau] ? __pm_runtime_resume+0x68/0xa0 pci_device_probe+0xaa/0x140 really_probe+0xc2/0x2d0 __driver_probe_device+0x73/0x120 driver_probe_device+0x2c/0xb0 __driver_attach+0xa0/0x150 ? __device_attach_driver+0xc0/0xc0 bus_for_each_dev+0x67/0xa0 bus_add_driver+0x10e/0x210 driver_register+0x5c/0x120 ? 0xa0336000 do_one_initcall+0x44/0x200 ? kmalloc_trace+0x37/0xc0 do_init_module+0x64/0x230 init_module_from_file+0x8d/0xd0 idempotent_init_module+0x15a/0x210 __x64_sys_finit_module+0x67/0xb0 do_syscall_64+0x41/0xf0 entry_SYSCALL_64_after_hwframe+0x4b/0x53 RIP: 0033:0x7fef9cda5ee9 Code: 08 44 89 e0 5b 41 5c c3 66 0f 1f 84 00 00 00 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d f7 ee 0e 00 f7 d8 64 89 01 48 RSP: 002b:7ffeb60299e8 EFLAGS: 0246 ORIG_RAX: 0139 RAX: ffda RBX: 55b322ddf5a0 RCX: 7fef9cda5ee9 RDX: RSI: 7fef9cf45e2d RDI: 0012 RBP: 0002 R08: R09: 55b322ddf690 R10: 0012 R11: 0246 R12: 7fef9cf45e2d R13: R14: 55b322dcb940 R15: 55b322ddf5a0 ---[ end trace ]--- [drm] Initialized nouveau 1.4.0 20120801 for :01:00.0 on minor 0 fbcon: nouveaudrmfb (fb0) is primary device Console: switching to colour frame buffer device 210x65 nouveau :01:00.0: [drm] fb0: nouveaudrmfb frame buffer device -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] drm/nouveau/disp: fix use-after-free in error handling of nouveau_connector_create
On Thu, Aug 17, 2023 at 12:24:45PM +0200, Karol Herbst wrote:
> simply throw a
>
> printk(KERN_WARNING "nvkm_uconn_uevent %u\n", outp->info.location);
>
> inside drivers/gpu/drm/nouveau/nvkm/engine/disp/uconn.c:104 after that
> mentioned comment.
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/uconn.c
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/uconn.c
index 46b057fe1412..661fd0cf3b3b 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/uconn.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/uconn.c
@@ -101,6 +101,7 @@ nvkm_uconn_uevent(struct nvkm_object *object, void *argv,
u32 argc, struct nvkm_
if (args->v0.types & NVIF_CONN_EVENT_V0_UNPLUG) bits |= NVKM_GPIO_LO;
if (args->v0.types & NVIF_CONN_EVENT_V0_IRQ) {
/* TODO: support DP IRQ on ANX9805 and remove this hack. */
+ printk(KERN_WARNING "nvkm_uconn_uevent %u\n",
outp->info.location);
if (!outp->info.location)
return -EINVAL;
}
result:
[ 10.566759] ACPI: bus type drm_connector registered
[ 10.591171] Console: switching to colour dummy device 80x25
[ 10.598472] nouveau :03:00.0: vgaarb: deactivate vga console
[ 10.607121] nouveau :03:00.0: NVIDIA GT218 (0a8c00b1)
[ 10.728361] nouveau :03:00.0: bios: version 70.18.83.00.08
[ 10.742137] nouveau :03:00.0: fb: 512 MiB DDR3
[ 11.059848] nouveau :03:00.0: DRM: VRAM: 512 MiB
[ 11.064911] nouveau :03:00.0: DRM: GART: 1048576 MiB
[ 11.070302] nouveau :03:00.0: DRM: TMDS table version 2.0
[ 11.076126] nouveau :03:00.0: DRM: DCB version 4.0
[ 11.081335] nouveau :03:00.0: DRM: DCB outp 00: 02000360
[ 11.087865] nouveau :03:00.0: DRM: DCB outp 01: 02000362 00020010
[ 11.094395] nouveau :03:00.0: DRM: DCB outp 02: 028003a6 0f220010
[ 11.100912] nouveau :03:00.0: DRM: DCB outp 03: 01011380
[ 11.107422] nouveau :03:00.0: DRM: DCB outp 04: 08011382 00020010
[ 11.113940] nouveau :03:00.0: DRM: DCB outp 05: 088113c6 0f220010
[ 11.120457] nouveau :03:00.0: DRM: DCB conn 00: 00101064
[ 11.126182] nouveau :03:00.0: DRM: DCB conn 01: 00202165
[ 11.138865] nouveau :03:00.0: DRM: MM: using COPY for buffer copies
[ 11.151291] nvkm_uconn_uevent 0
[ 11.154643] nvkm_uconn_uevent 0
[ 11.157975] nvkm_uconn_uevent 0
[ 11.161298] nvkm_uconn_uevent 0
[ 11.164616] nvkm_uconn_uevent 0
[ 11.167943] nvkm_uconn_uevent 0
[ 11.176010] [drm] Initialized nouveau 1.3.1 20120801 for :03:00.0 on
minor 0
[ 11.184186] nouveau :03:00.0: [drm] Cannot find any crtc or sizes
[ 11.260527] megasas: 07.725.01.00-rc1
[ 11.264555] st: Version 20160209, fixed bufsize 32768, s/g segs 256
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] drm/nouveau/disp: fix use-after-free in error handling of nouveau_connector_create
On Thu, Aug 17, 2023 at 12:00:47PM +0200, Karol Herbst wrote: > btw, what would help is to know where `nvkm_uconn_uevent` actually > fails, or rather, are you running into this "/* TODO: support DP IRQ > on ANX9805 and remove this hack. */" condition? Send me a diff, I'll run it here and catch output over serial. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] drm/nouveau/disp: fix use-after-free in error handling of nouveau_connector_create
On Thu, Aug 17, 2023 at 01:18:12AM +0200, Karol Herbst wrote: > do you have one of these? https://en.wikipedia.org/wiki/DMS-59 Ah, DMS == Dual Monitor Solution :-) Yap, that's exactly what the GPU has. And the Y-cable is 2xDVI. It is a Dell workstation and it came this way, meaning I haven't done any changes there. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] drm/nouveau/disp: fix use-after-free in error handling of nouveau_connector_create
On Wed, Aug 16, 2023 at 11:27:05PM +0200, Karol Herbst wrote: > that GPU has only a `DMS-59` connector, is that right? No clue. How do I figure that out? -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] drm/nouveau/disp: fix use-after-free in error handling of nouveau_connector_create
On Wed, Aug 16, 2023 at 04:57:28PM +0200, Karol Herbst wrote: > Do you have any connectors listed in "/sys/class/drm"? tree /sys/class/drm/ /sys/class/drm/ ├── card0 -> ../../devices/pci:00/:00:02.0/:03:00.0/drm/card0 ├── renderD128 -> ../../devices/pci:00/:00:02.0/:03:00.0/drm/renderD128 └── version > Also, mind sharing your vbios.rom file from > "/sys/kernel/debug/dri/0/vbios.rom"? Attached. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette vbios.rom.gz Description: application/gzip
Re: [PATCH] drm/nouveau/disp: fix use-after-free in error handling of nouveau_connector_create
On Wed, Aug 16, 2023 at 11:51:50AM +0200, Karol Herbst wrote: > Mind sharing your kernel logs with that patch applied? I suspect your > system boots up but you might just not have the connector available or > something? It could be that you have one of those GPUs affected by the > original change and then we'd have to figure out what to do with that. Close. With your patch applied, the machine is up and I can log in and use it. However, the output on the connected monitor stops after... [6.815167] ACPI: \_PR_.CP05: Found 4 idle states [6.825438] ACPI: \_PR_.CP06: Found 4 idle states [6.835661] ACPI: \_PR_.CP07: Found 4 idle states [7.280093] Freeing initrd memory: 8328K [7.601986] tsc: Refined TSC clocksource calibration: 3591.346 MHz [7.608360] clocksource: tsc: mask: 0x max_cycles: 0x33c46403b59, max_idle_ns: 440795293818 ns [7.620254] clocksource: Switched to clocksource tsc [8.337724] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled [8.350553] 00:05: ttyS0 at I/O 0x3f8 (irq = 4, base_baud = 115200) is a 16550A [8.375311] serial :00:16.3: enabling device ( -> 0003) [8.403681] :00:16.3: ttyS1 at I/O 0xf0a0 (irq = 17, base_baud = 115200) is a 16550A [8.424951] Linux agpgart interface v0.103 [8.432456] ACPI: bus type drm_connector registered ... this line here above. It is the last one output. What you see here below what I'm catching from serial. [8.456734] Console: switching to colour dummy device 80x25 [8.464414] nouveau :03:00.0: vgaarb: deactivate vga console [8.473063] nouveau :03:00.0: NVIDIA GT218 (0a8c00b1) [8.594096] nouveau :03:00.0: bios: version 70.18.83.00.08 [8.607906] nouveau :03:00.0: fb: 512 MiB DDR3 [8.926721] nouveau :03:00.0: DRM: VRAM: 512 MiB [8.931763] nouveau :03:00.0: DRM: GART: 1048576 MiB [8.937156] nouveau :03:00.0: DRM: TMDS table version 2.0 [8.942969] nouveau :03:00.0: DRM: DCB version 4.0 [8.948173] nouveau :03:00.0: DRM: DCB outp 00: 02000360 [8.954696] nouveau :03:00.0: DRM: DCB outp 01: 02000362 00020010 [8.961211] nouveau :03:00.0: DRM: DCB outp 02: 028003a6 0f220010 [8.967739] nouveau :03:00.0: DRM: DCB outp 03: 01011380 [8.974261] nouveau :03:00.0: DRM: DCB outp 04: 08011382 00020010 [8.980769] nouveau :03:00.0: DRM: DCB outp 05: 088113c6 0f220010 [8.987293] nouveau :03:00.0: DRM: DCB conn 00: 00101064 [8.993015] nouveau :03:00.0: DRM: DCB conn 01: 00202165 [9.005724] nouveau :03:00.0: DRM: MM: using COPY for buffer copies [9.023889] [drm] Initialized nouveau 1.3.1 20120801 for :03:00.0 on minor 0 [9.032044] nouveau :03:00.0: [drm] Cannot find any crtc or sizes [9.162909] megasas: 07.725.01.00-rc1 [9.167537] st: Version 20160209, fixed bufsize 32768, s/g segs 256 [9.176058] ahci :00:1f.2: version 3.0 [9.194078] ahci :00:1f.2: AHCI 0001.0300 32 slots 6 ports 6 Gbps 0x3 impl SATA mode [9.202487] ahci :00:1f.2: flags: 64bit ncq sntf pm led clo pio slum part ems apst [9.243154] scsi host0: ahci [9.252090] scsi host1: ahci [9.260389] scsi host2: ahci [9.268061] scsi host3: ahci [9.273542] scsi host4: ahci [9.279071] scsi host5: ahci ... and so on until full boot. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] drm/nouveau/disp: fix use-after-free in error handling of nouveau_connector_create
On Wed, Aug 16, 2023 at 12:11:57PM +0200, Borislav Petkov wrote: > Does that help? Btw, note that this is *plain* -rc5, without your patch. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] drm/nouveau/disp: fix use-after-free in error handling of nouveau_connector_create
On Wed, Aug 16, 2023 at 12:03:44PM +0200, Borislav Petkov wrote: > On Wed, Aug 16, 2023 at 11:51:50AM +0200, Karol Herbst wrote: > > Mind sharing your kernel logs with that patch applied? I suspect your > > system boots up but you might just not have the connector available or > > something? It could be that you have one of those GPUs affected by the > > original change and then we'd have to figure out what to do with that. > > Lemme do the KASAN run you requested first. It is an old and slooow box, > the grandma. :-) Does that help? [0.00] microcode: updated early: 0x710 -> 0x718, date = 2019-05-21 [0.00] Linux version 6.5.0-rc5 (root@gondor) (gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.1) #3 SMP PREEMPT_DYNAMIC Wed Aug 16 11:39:30 CEST 2023 [0.00] Command line: BOOT_IMAGE=/boot/vmlinuz-6.5.0-rc5 root=/dev/sda7 ro earlyprintk=ttyS0,115200 console=ttyS0,115200 console=tty0 ras=cec_disable root=/dev/sda7 log_buf_len=10M resume=/dev/sda5 no_console_suspend ignore_loglevel [0.00] KERNEL supported cpus: [0.00] Intel GenuineIntel [0.00] AMD AuthenticAMD [0.00] Centaur CentaurHauls [0.00] BIOS-provided physical RAM map: [0.00] BIOS-e820: [mem 0x-0x0009] usable [0.00] BIOS-e820: [mem 0x0010-0x18ebafff] usable [0.00] BIOS-e820: [mem 0x18ebb000-0x18fe7fff] ACPI NVS [0.00] BIOS-e820: [mem 0x18fe8000-0x18fe8fff] usable [0.00] BIOS-e820: [mem 0x18fe9000-0x18ff] ACPI NVS [0.00] BIOS-e820: [mem 0x1900-0x1dffcfff] usable [0.00] BIOS-e820: [mem 0x1dffd000-0x1dff] ACPI data [0.00] BIOS-e820: [mem 0x1e00-0xac77cfff] usable [0.00] BIOS-e820: [mem 0xac77d000-0xac77] type 20 [0.00] BIOS-e820: [mem 0xac78-0xac780fff] reserved [0.00] BIOS-e820: [mem 0xac781000-0xac782fff] type 20 [0.00] BIOS-e820: [mem 0xac783000-0xac7d9fff] reserved [0.00] BIOS-e820: [mem 0xac7da000-0xac7dafff] type 20 [0.00] BIOS-e820: [mem 0xac7db000-0xac7dcfff] reserved [0.00] BIOS-e820: [mem 0xac7dd000-0xac7e7fff] type 20 [0.00] BIOS-e820: [mem 0xac7e8000-0xac7f1fff] reserved [0.00] BIOS-e820: [mem 0xac7f2000-0xac7f5fff] type 20 [0.00] BIOS-e820: [mem 0xac7f6000-0xac7f9fff] reserved [0.00] BIOS-e820: [mem 0xac7fa000-0xac7fafff] type 20 [0.00] BIOS-e820: [mem 0xac7fb000-0xac803fff] reserved [0.00] BIOS-e820: [mem 0xac804000-0xac810fff] type 20 [0.00] BIOS-e820: [mem 0xac811000-0xac813fff] reserved [0.00] BIOS-e820: [mem 0xac814000-0xad7f] usable [0.00] BIOS-e820: [mem 0xb000-0xb3ff] reserved [0.00] BIOS-e820: [mem 0xfed2-0xfed3] reserved [0.00] BIOS-e820: [mem 0xfed5-0xfed8] reserved [0.00] BIOS-e820: [mem 0xffa0-0xffa3] reserved [0.00] BIOS-e820: [mem 0x0001-0x00044fff] usable [0.00] printk: bootconsole [earlyser0] enabled [0.00] printk: debug: ignoring loglevel setting. [0.00] NX (Execute Disable) protection: active [0.00] efi: EFI v2.0 by American Megatrends [0.00] efi: ACPI 2.0=0x1d98 SMBIOS=0xac811018 [0.00] efi: Remove mem57: MMIO range=[0xb000-0xb3ff] (64MB) from e820 map [0.00] e820: remove [mem 0xb000-0xb3ff] reserved [0.00] efi: Not removing mem58: MMIO range=[0xfed2-0xfed3] (128KB) from e820 map [0.00] efi: Remove mem59: MMIO range=[0xfed5-0xfed8] (0MB) from e820 map [0.00] e820: remove [mem 0xfed5-0xfed8] reserved [0.00] efi: Remove mem60: MMIO range=[0xffa0-0xffa3] (0MB) from e820 map [0.00] e820: remove [mem 0xffa0-0xffa3] reserved [0.00] SMBIOS 2.6 present. [0.00] DMI: Dell Inc. Precision T3600/0PTTT9, BIOS A13 05/11/2014 [0.00] tsc: Fast TSC calibration using PIT [0.00] tsc: Detected 3591.677 MHz processor [0.001418] e820: update [mem 0x-0x0fff] usable ==> reserved [0.007973] e820: remove [mem 0x000a-0x000f] usable [0.013555] last_pfn = 0x45 max_arch_pfn = 0x4 [0.019024] total RAM covered: 16352M [0.023219] Found optimal setting for mtrr clean up [0.027935] gran_size: 64K chunk_size: 64M num_reg: 8 lose cover RAM: 0G [0.035067] MTRR map: 6 entries (3 fixed + 3 variable; max 23), built from
Re: [PATCH] drm/nouveau/disp: fix use-after-free in error handling of nouveau_connector_create
On Wed, Aug 16, 2023 at 11:51:50AM +0200, Karol Herbst wrote: > Mind sharing your kernel logs with that patch applied? I suspect your > system boots up but you might just not have the connector available or > something? It could be that you have one of those GPUs affected by the > original change and then we'd have to figure out what to do with that. Lemme do the KASAN run you requested first. It is an old and slooow box, the grandma. :-) -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] drm/nouveau/disp: fix use-after-free in error handling of nouveau_connector_create
On Mon, Aug 14, 2023 at 04:49:32PM +0200, Karol Herbst wrote:
> We can't simply free the connector after calling drm_connector_init on it.
> We need to clean up the drm side first.
>
> It might not fix all regressions from 2b5d1c29f6c4 ("drm/nouveau/disp:
> PIOR DP uses GPIO for HPD, not PMGR AUX interrupts"), but at least it
> fixes a memory corruption in error handling related to that commit.
>
> Link:
> https://lore.kernel.org/lkml/20230806213107.GFZNARG6moWpFuSJ9W@fat_crate.local/
> Fixes: 95983aea8003 ("drm/nouveau/disp: add connector class")
> Signed-off-by: Karol Herbst
> ---
> drivers/gpu/drm/nouveau/nouveau_connector.c | 11 +++
> 1 file changed, 7 insertions(+), 4 deletions(-)
This one ontop of -rc5 doesn't help, unfortunately.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: 2b5d1c29f6c4 ("drm/nouveau/disp: PIOR DP uses GPIO for HPD, not PMGR AUX interrupts")
On Tue, Aug 08, 2023 at 12:39:32PM +0200, Karol Herbst wrote:
> ahh, that would have been good to know :)
Yeah, I didn't see it before - it would only freeze. Only after I added
the printk you requested.
> Mind figuring out what's exactly NULL inside nvif_object_mthd? Or
> rather what line `nvif_object_mthd+0x136` belongs to, then it should
> be easy to figure out what's wrong here.
That looks like this:
816ddfee: e8 8d 04 4e 00 callq 81bbe480
<__memcpy>
816ddff3: 41 8d 56 20 lea0x20(%r14),%edx
816ddff7: 49 8b 44 24 08 mov0x8(%r12),%rax
816ddffc: 83 fa 17cmp$0x17,%edx
816ddfff: 76 7d jbe816de07e
816de001: 49 39 c4cmp%rax,%r12
816de004: 74 45 je 816de04b
<--- RIP points here.
The 0x20 also fits the deref address: 0020.
Which means %rax is 0. Yap.
816de006: 48 8b 78 20 mov0x20(%rax),%rdi
816de00a: 4c 89 64 24 10 mov%r12,0x10(%rsp)
816de00f: 48 8b 40 38 mov0x38(%rax),%rax
816de013: c6 44 24 06 ff movb $0xff,0x6(%rsp)
816de018: 31 c9 xor%ecx,%ecx
816de01a: 48 89 e6mov%rsp,%rsi
816de01d: 48 8b 40 28 mov0x28(%rax),%rax
816de021: e8 3a 0c 4f 00 callq 81bcec60
<__x86_indirect_thunk_array>
Now, the preprocessed asm version of nvif/object.c says around here:
callmemcpy #
# drivers/gpu/drm/nouveau/nvif/object.c:160:ret = nvif_object_ioctl(object,
args, sizeof(*args) + size, NULL);
leal32(%r14), %edx #, _108
# drivers/gpu/drm/nouveau/nvif/object.c:33: struct nvif_client *client =
object->client;
movq8(%r12), %rax # object_19(D)->client, client
# drivers/gpu/drm/nouveau/nvif/object.c:38: if (size >= sizeof(*args) &&
args->v0.version == 0) {
cmpl$23, %edx #, _108
jbe .L69#,
# drivers/gpu/drm/nouveau/nvif/object.c:39: if (object !=
>object)
cmpq%rax, %r12 # client, object
je .L70#,
# drivers/gpu/drm/nouveau/nvif/object.c:47: return
client->driver->ioctl(client->object.priv, data, size, hack);
movq32(%rax), %rdi # client_109->object.priv,
client_109->object.priv
So I'd say that client is NULL. IINM.
movq%r12, 16(%rsp) # object, MEM[(union *)].v0.object
# drivers/gpu/drm/nouveau/nvif/object.c:47: return
client->driver->ioctl(client->object.priv, data, size, hack);
movq56(%rax), %rax # client_109->driver, client_109->driver
# drivers/gpu/drm/nouveau/nvif/object.c:43: args->v0.owner =
NVIF_IOCTL_V0_OWNER_ANY;
movb$-1, 6(%rsp)#, MEM[(union *)].v0.owner
.L64:
# drivers/gpu/drm/nouveau/nvif/object.c:47: return
client->driver->ioctl(client->object.priv, data, size, hack);
xorl%ecx, %ecx #
movq%rsp, %rsi #,
movq40(%rax), %rax #, _77->ioctl
call__x86_indirect_thunk_rax
# drivers/gpu/drm/nouveau/nvif/object.c:161:memcpy(data, args->mthd.data,
size);
> > [4.144676] #PF: supervisor read access in kernel mode
> > [4.144676] #PF: error_code(0x) - not-present page
> > [4.144676] PGD 0 P4D 0
> > [4.144676] Oops: [#1] PREEMPT SMP PTI
> > [4.144676] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 6.5.0-rc5-dirty #1
> > [4.144676] Hardware name: Dell Inc. Precision T3600/0PTTT9, BIOS A13
> > 05/11/2014
> > [4.144676] RIP: 0010:nvif_object_mthd+0x136/0x1e0
> > [4.144676] Code: f2 4c 89 ee 48 8d 7c 24 20 66 89 04 24 c6 44 24 18 00
> > e8 8d 04 4e 00 41 8d 56 20 49 8b 44 24 08 83 fa 17 76 7d 49 39 c4 74 45
> > <48> 8b 78 20 4c 89 64 24 10 48 8b 40 38 c6 44 24 06 ff 31 c9 48 89
Opcode bytes around RIP look correct too:
./scripts/decodecode < /tmp/oops
[ 4.144676] Code: f2 4c 89 ee 48 8d 7c 24 20 66 89 04 24 c6 44 24 18 00 e8 8d
04 4e 00 41 8d 56 20 49 8b 44 24 08 83 fa 17 76 7d 49 39 c4 74 45 <48> 8b 78 20
4c 89 64 24 10 48 8b 40 38 c6 44 24 06 ff 31 c9 48 89
All code
0: f2 4c 89 ee repnz mov %r13,%rsi
4: 48 8d 7c 24 20 lea0x20(%rsp),%rdi
9: 66 89 04 24 mov%ax,(%rsp)
d: c6 44 24 18 00 movb $0x0,0x18(%rsp)
12: e8 8d 04 4e 00 callq 0x4e04a4
17: 41 8d 56 20 lea0x20(%r14),%edx
1b: 49 8b 44 24 08 mov0x8(%r12),%rax
20: 83 fa 17cmp$0x17,%edx
23: 76 7d jbe0xa2
25: 49 39 c4cmp%rax,%r12
28: 74 45 je 0x6f
2a:* 48 8b 78 20 mov0x20(%rax),%rdi <-- trapping
Re: 2b5d1c29f6c4 ("drm/nouveau/disp: PIOR DP uses GPIO for HPD, not PMGR AUX interrupts")
On Mon, Aug 07, 2023 at 01:49:42PM +0200, Karol Herbst wrote: > in what way does it stop? Just not progressing? That would be kinda > concerning. Mind tracing with what arguments `nvkm_uevent_add` is > called with and without that patch? Well, me dumping those args I guess made the box not freeze before catching a #PF over serial. Does that help? [3.410135] Unpacking initramfs... [3.416319] software IO TLB: mapped [mem 0xa877d000-0xac77d000] (64MB) [3.418227] Initialise system trusted keyrings [3.432273] workingset: timestamp_bits=56 max_order=22 bucket_order=0 [3.439006] ntfs: driver 2.1.32 [Flags: R/W]. [3.443368] fuse: init (API version 7.38) [3.447601] 9p: Installing v9fs 9p2000 file system support [3.453223] Key type asymmetric registered [3.457332] Asymmetric key parser 'x509' registered [3.462236] Block layer SCSI generic (bsg) driver version 0.4 loaded (major 250) [3.475865] efifb: probing for efifb [3.479458] efifb: framebuffer at 0xf900, using 1920k, total 1920k [3.485969] efifb: mode is 800x600x32, linelength=3200, pages=1 [3.491872] efifb: scrolling: redraw [3.495438] efifb: Truecolor: size=8:8:8:8, shift=24:16:8:0 [3.502349] Console: switching to colour frame buffer device 100x37 [3.509564] fb0: EFI VGA frame buffer device [3.514013] ACPI: \_PR_.CP00: Found 4 idle states [3.518850] ACPI: \_PR_.CP01: Found 4 idle states [3.523687] ACPI: \_PR_.CP02: Found 4 idle states [3.528515] ACPI: \_PR_.CP03: Found 4 idle states [3.533346] ACPI: \_PR_.CP04: Found 4 idle states [3.538173] ACPI: \_PR_.CP05: Found 4 idle states [3.543003] ACPI: \_PR_.CP06: Found 4 idle states [3.544219] Freeing initrd memory: 8196K [3.547844] ACPI: \_PR_.CP07: Found 4 idle states [3.609542] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled [3.616224] 00:05: ttyS0 at I/O 0x3f8 (irq = 4, base_baud = 115200) is a 16550A [3.625552] serial :00:16.3: enabling device ( -> 0003) [3.633034] :00:16.3: ttyS1 at I/O 0xf0a0 (irq = 17, base_baud = 115200) is a 16550A [3.642451] Linux agpgart interface v0.103 [3.647141] ACPI: bus type drm_connector registered [3.653261] Console: switching to colour dummy device 80x25 [3.659092] nouveau :03:00.0: vgaarb: deactivate vga console [3.665174] nouveau :03:00.0: NVIDIA GT218 (0a8c00b1) [3.784585] nouveau :03:00.0: bios: version 70.18.83.00.08 [3.792244] nouveau :03:00.0: fb: 512 MiB DDR3 [3.948786] nouveau :03:00.0: DRM: VRAM: 512 MiB [3.953755] nouveau :03:00.0: DRM: GART: 1048576 MiB [3.959073] nouveau :03:00.0: DRM: TMDS table version 2.0 [3.964808] nouveau :03:00.0: DRM: DCB version 4.0 [3.969938] nouveau :03:00.0: DRM: DCB outp 00: 02000360 [3.976367] nouveau :03:00.0: DRM: DCB outp 01: 02000362 00020010 [3.982792] nouveau :03:00.0: DRM: DCB outp 02: 028003a6 0f220010 [3.989223] nouveau :03:00.0: DRM: DCB outp 03: 01011380 [3.995647] nouveau :03:00.0: DRM: DCB outp 04: 08011382 00020010 [4.002076] nouveau :03:00.0: DRM: DCB outp 05: 088113c6 0f220010 [4.008511] nouveau :03:00.0: DRM: DCB conn 00: 00101064 [4.014151] nouveau :03:00.0: DRM: DCB conn 01: 00202165 [4.021710] nvkm_uevent_add: uevent: 0x888100242100, event: 0x8881022de1a0, id: 0x0, bits: 0x1, func: 0x [4.033680] nvkm_uevent_add: uevent: 0x888100242300, event: 0x8881022de1a0, id: 0x0, bits: 0x1, func: 0x [4.045429] nouveau :03:00.0: DRM: MM: using COPY for buffer copies [4.052059] stackdepot: allocating hash table of 1048576 entries via kvcalloc [4.067191] nvkm_uevent_add: uevent: 0x888100242800, event: 0x888104b3e260, id: 0x0, bits: 0x1, func: 0x [4.078936] nvkm_uevent_add: uevent: 0x888100242900, event: 0x888104b3e260, id: 0x1, bits: 0x1, func: 0x [4.090514] nvkm_uevent_add: uevent: 0x888100242a00, event: 0x888102091f28, id: 0x1, bits: 0x3, func: 0x8177b700 [4.102118] tsc: Refined TSC clocksource calibration: 3591.345 MHz [4.108342] clocksource: tsc: mask: 0x max_cycles: 0x33c4635c383, max_idle_ns: 440795314831 ns [4.108401] nvkm_uevent_add: uevent: 0x8881020b6000, event: 0x888102091f28, id: 0xf, bits: 0x3, func: 0x8177b700 [4.129864] clocksource: Switched to clocksource tsc [4.131478] [drm] Initialized nouveau 1.3.1 20120801 for :03:00.0 on minor 0 [4.143806] BUG: kernel NULL pointer dereference, address: 0020 [4.144676] #PF: supervisor read access in kernel mode [4.144676] #PF: error_code(0x) - not-present page [4.144676] PGD 0 P4D 0 [4.144676] Oops: [#1] PREEMPT SMP PTI [4.144676] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 6.5.0-rc5-dirty #1 [4.144676]
2b5d1c29f6c4 ("drm/nouveau/disp: PIOR DP uses GPIO for HPD, not PMGR AUX interrupts")
Hi folks, the patch in $Subject breaks booting here on one of my test boxes, see below. Reverting it ontop of -rc4 fixes the issue. Thx. [3.580535] ACPI: \_PR_.CP04: Found 4 idle states [3.585694] ACPI: \_PR_.CP05: Found 4 idle states [3.590852] ACPI: \_PR_.CP06: Found 4 idle states [3.596037] ACPI: \_PR_.CP07: Found 4 idle states [3.644065] Freeing initrd memory: 6740K [3.742932] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled [3.750409] 00:05: ttyS0 at I/O 0x3f8 (irq = 4, base_baud = 115200) is a 16550A [3.762111] serial :00:16.3: enabling device ( -> 0003) [3.771589] :00:16.3: ttyS1 at I/O 0xf0a0 (irq = 17, base_baud = 115200) is a 16550A [3.782503] Linux agpgart interface v0.103 [3.787805] ACPI: bus type drm_connector registered <--- boot stops here. It should continue with this: [3.795491] Console: switching to colour dummy device 80x25 [3.801933] nouveau :03:00.0: vgaarb: deactivate vga console [3.808303] nouveau :03:00.0: NVIDIA GT218 (0a8c00b1) [3.931002] nouveau :03:00.0: bios: version 70.18.83.00.08 [3.941731] nouveau :03:00.0: fb: 512 MiB DDR3 [4.110348] tsc: Refined TSC clocksource calibration: 3591.349 MHz [4.116627] clocksource: tsc: mask: 0x max_cycles: 0x33c466a1ab5, max_idle_ns: 440795209767 ns [4.126871] clocksource: Switched to clocksource tsc [4.252013] nouveau :03:00.0: DRM: VRAM: 512 MiB [4.257088] nouveau :03:00.0: DRM: GART: 1048576 MiB [4.262501] nouveau :03:00.0: DRM: TMDS table version 2.0 [4.268333] nouveau :03:00.0: DRM: DCB version 4.0 [4.273561] nouveau :03:00.0: DRM: DCB outp 00: 02000360 [4.280104] nouveau :03:00.0: DRM: DCB outp 01: 02000362 00020010 [4.286630] nouveau :03:00.0: DRM: DCB outp 02: 028003a6 0f220010 [4.293176] nouveau :03:00.0: DRM: DCB outp 03: 01011380 [4.299711] nouveau :03:00.0: DRM: DCB outp 04: 08011382 00020010 [4.306243] nouveau :03:00.0: DRM: DCB outp 05: 088113c6 0f220010 [4.312772] nouveau :03:00.0: DRM: DCB conn 00: 00101064 [4.318520] nouveau :03:00.0: DRM: DCB conn 01: 00202165 [4.329488] nouveau :03:00.0: DRM: MM: using COPY for buffer copies [4.336261] stackdepot: allocating hash table of 1048576 entries via kvcalloc ... -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] drm/radeon: Disable outputs when releasing fbdev client
On Fri, Jun 09, 2023 at 04:03:56PM +0200, Thomas Zimmermann wrote: > Disable the modesetting pipeline before release the radeon's fbdev > client. Fixes the following error: > > [ 17.217408] WARNING: CPU: 5 PID: 1464 at drivers/gpu/drm/ttm/ttm_bo.c:326 > ttm_bo_release+0x27e/0x2d0 [ttm] > [ 17.217418] Modules linked in: edac_mce_amd radeon(+) drm_ttm_helper ttm > video drm_suballoc_helper drm_display_helper kvm irqbypass drm_kms_helper > syscopyarea crc32_pclmul sysfillrect sha512_ssse3 sysimgblt sha512_generic > cfbfillrect cfbimgblt wmi_bmof aesni_intel cfbcopyarea crypto_simd cryptd > k10temp acpi_cpufreq wmi dm_mod > [ 17.217432] CPU: 5 PID: 1464 Comm: systemd-udevd Not tainted 6.4.0-rc4+ #1 > [ 17.217436] Hardware name: Micro-Star International Co., Ltd. > MS-7A38/B450M PRO-VDH MAX (MS-7A38), BIOS B.G0 07/26/2022 > [ 17.217438] RIP: 0010:ttm_bo_release+0x27e/0x2d0 [ttm] > [ 17.217444] Code: 48 89 43 38 48 89 43 40 48 8b 5c 24 30 48 8b b5 40 08 00 > 00 48 8b 6c 24 38 48 83 c4 58 e9 7a 49 f7 e0 48 89 ef e9 6c fe ff ff <0f> 0b > 48 83 7b 20 00 0f 84 b7 fd ff ff 0f 0b 0f 1f 00 e9 ad fd ff > [ 17.217448] RSP: 0018:c995fbb0 EFLAGS: 00010202 > [ 17.217451] RAX: 0001 RBX: 8881052c8de0 RCX: > > [ 17.217453] RDX: 0001 RSI: RDI: > 8881052c8de0 > [ 17.217455] RBP: 888104a66e00 R08: 8881052c8de0 R09: > 888104a7cf08 > [ 17.217457] R10: c995fbe0 R11: c995fbe8 R12: > 8881052c8c78 > [ 17.217458] R13: 8881052c8c78 R14: dead0100 R15: > 88810528b108 > [ 17.217460] FS: 7f319fcbb8c0() GS:1a54() > knlGS: > [ 17.217463] CS: 0010 DS: ES: CR0: 80050033 > [ 17.217464] CR2: 55dc8b0224a0 CR3: 00010373d000 CR4: > 00750ee0 > [ 17.217466] PKRU: 5554 > [ 17.217468] Call Trace: > [ 17.217470] > [ 17.217472] ? __warn+0x97/0x160 > [ 17.217476] ? ttm_bo_release+0x27e/0x2d0 [ttm] > [ 17.217481] ? report_bug+0x1ec/0x200 > [ 17.217487] ? handle_bug+0x3c/0x70 > [ 17.217490] ? exc_invalid_op+0x1f/0x90 > [ 17.217493] ? preempt_count_sub+0xb5/0x100 > [ 17.217496] ? asm_exc_invalid_op+0x16/0x20 > [ 17.217500] ? ttm_bo_release+0x27e/0x2d0 [ttm] > [ 17.217505] ? ttm_resource_move_to_lru_tail+0x1ab/0x1d0 [ttm] > [ 17.217511] radeon_bo_unref+0x1a/0x30 [radeon] > [ 17.217547] radeon_gem_object_free+0x20/0x30 [radeon] > [ 17.217579] radeon_fbdev_fb_destroy+0x57/0x90 [radeon] > [ 17.217616] unregister_framebuffer+0x72/0x110 > [ 17.217620] drm_client_dev_unregister+0x6d/0xe0 > [ 17.217623] drm_dev_unregister+0x2e/0x90 > [ 17.217626] drm_put_dev+0x26/0x90 > [ 17.217628] pci_device_remove+0x44/0xc0 > [ 17.217631] really_probe+0x257/0x340 > [ 17.217635] __driver_probe_device+0x73/0x120 > [ 17.217638] driver_probe_device+0x2c/0xb0 > [ 17.217641] __driver_attach+0xa0/0x150 > [ 17.217643] ? __pfx___driver_attach+0x10/0x10 > [ 17.217646] bus_for_each_dev+0x67/0xa0 > [ 17.217649] bus_add_driver+0x10e/0x210 > [ 17.217651] driver_register+0x5c/0x120 > [ 17.217653] ? __pfx_radeon_module_init+0x10/0x10 [radeon] > [ 17.217681] do_one_initcall+0x44/0x220 > [ 17.217684] ? kmalloc_trace+0x37/0xc0 > [ 17.217688] do_init_module+0x64/0x240 > [ 17.217691] __do_sys_finit_module+0xb2/0x100 > [ 17.217694] do_syscall_64+0x3b/0x90 > [ 17.217697] entry_SYSCALL_64_after_hwframe+0x72/0xdc > [ 17.217700] RIP: 0033:0x7f319feaa5a9 > [ 17.217702] Code: 08 89 e8 5b 5d c3 66 2e 0f 1f 84 00 00 00 00 00 90 48 89 > f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d > 01 f0 ff ff 73 01 c3 48 8b 0d 27 08 0d 00 f7 d8 64 89 01 48 > [ 17.217706] RSP: 002b:7ffc6bf3e7f8 EFLAGS: 0246 ORIG_RAX: > 0139 > [ 17.217709] RAX: ffda RBX: 5607204f3170 RCX: > 7f319feaa5a9 > [ 17.217710] RDX: RSI: 7f31a002eefd RDI: > 0018 > [ 17.217712] RBP: 7f31a002eefd R08: R09: > 5607204f1860 > [ 17.217714] R10: 0018 R11: 0246 R12: > 0002 > [ 17.217716] R13: R14: 560720522450 R15: > 560720255899 > [ 17.217718] > [ 17.217719] ---[ end trace ]--- > > The buffer object backing the fbdev emulation got pinned twice: by the > fb_probe helper radeon_fbdev_create_pinned_object() and the modesetting > code when the framebuffer got displayed. It only got unpinned once by > the fbdev helper radeon_fbdev_destroy_pinned_object(). Hence TTM's BO- > release function complains a
WARNING: CPU: 5 PID: 1464 at drivers/gpu/drm/ttm/ttm_bo.c:326 ttm_bo_release+0x27e/0x2d0 [ttm]
Hi,
this below triggers with the latest Linus tree:
51f269a6ecc7 ("Merge tag 'probes-fixes-6.4-rc4' of
git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace")
...
[ 16.173593] [drm] radeon kernel modesetting enabled.
[ 16.173743] radeon :29:00.0: vgaarb: deactivate vga console
[ 16.174300] MCE: In-kernel MCE decoding enabled.
[ 16.175695] EDAC DEBUG: umc_read_base_mask: DCSB0[0]=0x0001 reg:
0x5
[ 16.175698] EDAC DEBUG: umc_read_base_mask: DCSB_SEC0[0]=0x reg:
0x50010
[ 16.175700] EDAC DEBUG: umc_read_base_mask: DCSB0[1]=0x reg:
0x50004
[ 16.175702] EDAC DEBUG: umc_read_base_mask: DCSB_SEC0[1]=0x reg:
0x50014
[ 16.175703] EDAC DEBUG: umc_read_base_mask: DCSB0[2]=0x0201 reg:
0x50008
[ 16.175705] EDAC DEBUG: umc_read_base_mask: DCSB_SEC0[2]=0x reg:
0x50018
[ 16.175706] EDAC DEBUG: umc_read_base_mask: DCSB0[3]=0x reg:
0x5000c
[ 16.175707] EDAC DEBUG: umc_read_base_mask: DCSB_SEC0[3]=0x reg:
0x5001c
[ 16.175709] EDAC DEBUG: umc_read_base_mask: DCSM0[0]=0x03fffdfe reg:
0x50020
[ 16.175710] EDAC DEBUG: umc_read_base_mask: DCSM_SEC0[0]=0x reg:
0x50028
[ 16.175712] EDAC DEBUG: umc_read_base_mask: DCSM0[1]=0x03fffdfe reg:
0x50024
[ 16.175713] EDAC DEBUG: umc_read_base_mask: DCSM_SEC0[1]=0x reg:
0x5002c
[ 16.175715] EDAC DEBUG: umc_read_base_mask: DCSB1[0]=0x0001 reg:
0x15
[ 16.175716] EDAC DEBUG: umc_read_base_mask: DCSB_SEC1[0]=0x reg:
0x150010
[ 16.175718] EDAC DEBUG: umc_read_base_mask: DCSB1[1]=0x reg:
0x150004
[ 16.175719] EDAC DEBUG: umc_read_base_mask: DCSB_SEC1[1]=0x reg:
0x150014
[ 16.175720] EDAC DEBUG: umc_read_base_mask: DCSB1[2]=0x0201 reg:
0x150008
[ 16.175722] EDAC DEBUG: umc_read_base_mask: DCSB_SEC1[2]=0x reg:
0x150018
[ 16.175723] EDAC DEBUG: umc_read_base_mask: DCSB1[3]=0x reg:
0x15000c
[ 16.175725] EDAC DEBUG: umc_read_base_mask: DCSB_SEC1[3]=0x reg:
0x15001c
[ 16.175726] EDAC DEBUG: umc_read_base_mask: DCSM1[0]=0x03fffdfe reg:
0x150020
[ 16.175728] EDAC DEBUG: umc_read_base_mask: DCSM_SEC1[0]=0x reg:
0x150028
[ 16.175729] EDAC DEBUG: umc_read_base_mask: DCSM1[1]=0x03fffdfe reg:
0x150024
[ 16.175730] EDAC DEBUG: umc_read_base_mask: DCSM_SEC1[1]=0x reg:
0x15002c
[ 16.175741] EDAC DEBUG: umc_determine_memory_type: UMC0 DIMM type:
Unbuffered-DDR4
[ 16.175742] EDAC DEBUG: umc_determine_memory_type: UMC1 DIMM type:
Unbuffered-DDR4
[ 16.177514] Console: switching to colour dummy device 80x25
[ 16.177693] [drm] initializing kernel modesetting (CEDAR 0x1002:0x68E1
0x174B:0x3000 0x00).
[ 16.177733] ATOM BIOS: AMD
[ 16.177795] radeon :29:00.0: VRAM: 1024M 0x -
0x3FFF (1024M used)
[ 16.177798] radeon :29:00.0: GTT: 1024M 0x4000 -
0x7FFF
[ 16.177800] [drm] Detected VRAM RAM=1024M, BAR=256M
[ 16.177802] [drm] RAM width 64bits DDR
[ 16.177835] [drm] radeon: 1024M of VRAM memory ready
[ 16.177836] [drm] radeon: 1024M of GTT memory ready.
[ 16.177839] [drm] Loading CEDAR Microcode
[ 16.179106] [drm] Internal thermal controller without fan control
[ 16.199812] [drm] radeon: dpm initialized
[ 16.200179] [drm] GART: num cpu pages 262144, num gpu pages 262144
[ 16.200399] [drm] enabling PCIE gen 2 link speeds, disable with
radeon.pcie_gen2=0
[ 16.218135] [drm] PCIE GART of 1024M enabled (table at 0x0014C000).
[ 16.218239] radeon :29:00.0: WB enabled
[ 16.218240] radeon :29:00.0: fence driver on ring 0 use gpu addr
0x4c00
[ 16.218242] radeon :29:00.0: fence driver on ring 3 use gpu addr
0x4c0c
[ 16.218606] radeon :29:00.0: fence driver on ring 5 use gpu addr
0x0005c418
[ 16.218657] radeon :29:00.0: radeon: MSI limited to 32-bit
[ 16.218689] radeon :29:00.0: radeon: using MSI.
[ 16.218707] [drm] radeon: irq initialized.
[ 16.234730] [drm] ring test on 0 succeeded in 0 usecs
[ 16.234738] [drm] ring test on 3 succeeded in 2 usecs
[ 16.317725] r8169 :25:00.0 eth0: Link is Down
[ 16.410486] [drm] ring test on 5 succeeded in 1 usecs
[ 16.410492] [drm] UVD initialized successfully.
[ 16.410555] [drm] ib test on ring 0 succeeded in 0 usecs
[ 16.410596] [drm] ib test on ring 3 succeeded in 0 usecs
[ 17.077422] [drm] ib test on ring 5 succeeded
[ 17.077581] [drm] Radeon Display Connectors
[ 17.077584] [drm] Connector 0:
[ 17.077585] [drm] HDMI-A-1
[ 17.077586] [drm] HPD4
[ 17.077588] [drm] DDC: 0x6440 0x6440 0x6444 0x6444 0x6448 0x6448 0x644c
0x644c
[ 17.077590] [drm] Encoders:
[ 17.077591] [drm] DFP1: INTERNAL_UNIPHY1
[ 17.077593] [drm] Connector 1:
[ 17.077594] [drm] DVI-I-1
[ 17.077595] [drm] HPD1
[ 17.077597] [drm] DDC: 0x6460 0x6460 0x6464 0x6464 0x6468 0x6468
Re: [RESUBMIT][PATCH] x86/mm: Fix PAT bit missing from page protection modify mask
On Thu, Jun 01, 2023 at 10:47:39AM +0200, Juergen Gross wrote: > As described in the commit message, this only works on bare metal due to the > PAT bit not being needed for WC mappings. > > Making this patch Xen specific would try to cure the symptoms without fixing > the underlying problem: _PAGE_PAT should be regarded the same way as the bits > for caching mode (_PAGE_CHG_MASK). So why isn't _PAGE_PAT part of _PAGE_CHG_MASK? It says above it "Set of bits not changed in pte_modify." And I don't see pte_modify() changing that bit either. Right now this "fix" looks like, "let's OR these two masks so that we can take care of _PAGE_PAT too". But it doesn't make a whole lotta sense to me... -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [RESUBMIT][PATCH] x86/mm: Fix PAT bit missing from page protection modify mask
On Fri, May 19, 2023 at 08:36:34PM +0200, Janusz Krzysztofik wrote:
> diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
> index 15ae4d6ba4768..56466afd04307 100644
> --- a/arch/x86/include/asm/pgtable.h
> +++ b/arch/x86/include/asm/pgtable.h
> @@ -654,8 +654,10 @@ static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t
> newprot)
> #define pgprot_modify pgprot_modify
> static inline pgprot_t pgprot_modify(pgprot_t oldprot, pgprot_t newprot)
> {
> - pgprotval_t preservebits = pgprot_val(oldprot) & _PAGE_CHG_MASK;
> - pgprotval_t addbits = pgprot_val(newprot) & ~_PAGE_CHG_MASK;
> + unsigned long mask = _PAGE_CHG_MASK | _PAGE_CACHE_MASK;
> +
> + pgprotval_t preservebits = pgprot_val(oldprot) & mask;
> + pgprotval_t addbits = pgprot_val(newprot) & ~mask;
> return __pgprot(preservebits | addbits);
> }
>
> --
This certainly needs Jürgen and he's on CC already, moving him to To:.
Also, why isn't this a Xen-specific fix but you're keeping _PAGE_PAT for
baremetal too, i.e., modifying the generic function?
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
call to __compiletime_assert_1441 declared with attribute error: FIELD_PREP: mask is not constant
Hi, I'm getting this on latest Linus master with gcc (SUSE Linux) 7.5.0: DESCEND objtool CALLscripts/checksyscalls.sh CC [M] drivers/gpu/drm/i915/gt/uc/intel_guc_submission.o In file included from :0:0: In function ‘__guc_context_policy_add_priority.isra.45’, inlined from ‘__guc_context_set_prio.isra.46’ at drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c:3198:3, inlined from ‘guc_context_set_prio’ at drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c:3226:2: ././include/linux/compiler_types.h:357:38: error: call to ‘__compiletime_assert_1441’ declared with attribute error: FIELD_PREP: mask is not constant _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) ^ ././include/linux/compiler_types.h:338:4: note: in definition of macro ‘__compiletime_assert’ prefix ## suffix();\ ^~ ././include/linux/compiler_types.h:357:2: note: in expansion of macro ‘_compiletime_assert’ _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) ^~~ ./include/linux/build_bug.h:39:37: note: in expansion of macro ‘compiletime_assert’ #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg) ^~ ./include/linux/bitfield.h:65:3: note: in expansion of macro ‘BUILD_BUG_ON_MSG’ BUILD_BUG_ON_MSG(!__builtin_constant_p(_mask), \ ^~~~ ./include/linux/bitfield.h:114:3: note: in expansion of macro ‘__BF_FIELD_CHECK’ __BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: "); \ ^~~~ drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c:2399:3: note: in expansion of macro ‘FIELD_PREP’ FIELD_PREP(GUC_KLV_0_KEY, GUC_CONTEXT_POLICIES_KLV_ID_##id) | \ ^~ drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c:2407:1: note: in expansion of macro ‘MAKE_CONTEXT_POLICY_ADD’ MAKE_CONTEXT_POLICY_ADD(priority, SCHEDULING_PRIORITY) ^~~ In function ‘__guc_context_policy_add_priority.isra.45’, inlined from ‘__guc_add_request’ at drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c:2437:2: ././include/linux/compiler_types.h:357:38: error: call to ‘__compiletime_assert_1441’ declared with attribute error: FIELD_PREP: mask is not constant _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) ^ ././include/linux/compiler_types.h:338:4: note: in definition of macro ‘__compiletime_assert’ prefix ## suffix();\ ^~ ././include/linux/compiler_types.h:357:2: note: in expansion of macro ‘_compiletime_assert’ _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) ^~~ ./include/linux/build_bug.h:39:37: note: in expansion of macro ‘compiletime_assert’ #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg) ^~ ./include/linux/bitfield.h:65:3: note: in expansion of macro ‘BUILD_BUG_ON_MSG’ BUILD_BUG_ON_MSG(!__builtin_constant_p(_mask), \ ^~~~ ./include/linux/bitfield.h:114:3: note: in expansion of macro ‘__BF_FIELD_CHECK’ __BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: "); \ ^~~~ drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c:2399:3: note: in expansion of macro ‘FIELD_PREP’ FIELD_PREP(GUC_KLV_0_KEY, GUC_CONTEXT_POLICIES_KLV_ID_##id) | \ ^~ drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c:2407:1: note: in expansion of macro ‘MAKE_CONTEXT_POLICY_ADD’ MAKE_CONTEXT_POLICY_ADD(priority, SCHEDULING_PRIORITY) ^~~ In function ‘__guc_context_policy_add_priority.isra.45’, inlined from ‘register_context’ at drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c:2437:2: ././include/linux/compiler_types.h:357:38: error: call to ‘__compiletime_assert_1441’ declared with attribute error: FIELD_PREP: mask is not constant _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) ^ ././include/linux/compiler_types.h:338:4: note: in definition of macro ‘__compiletime_assert’ prefix ## suffix();\ ^~ ././include/linux/compiler_types.h:357:2: note: in expansion of macro ‘_compiletime_assert’ _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__) ^~~ ./include/linux/build_bug.h:39:37: note: in expansion of macro ‘compiletime_assert’ #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg) ^~ ./include/linux/bitfield.h:65:3: note: in expansion of macro ‘BUILD_BUG_ON_MSG’ BUILD_BUG_ON_MSG(!__builtin_constant_p(_mask), \ ^~~~ ./include/linux/bitfield.h:114:3: note: in expansion of macro ‘__BF_FIELD_CHECK’ __BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: "); \ ^~~~ drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c:2399:3: note: in expansion of macro ‘FIELD_PREP’
[PATCH 11/11] drm/i915: Fix undefined behavior due to shift overflowing the constant
From: Borislav Petkov
Fix:
In file included from :0:0:
drivers/gpu/drm/i915/gt/uc/intel_guc.c: In function ‘intel_guc_send_mmio’:
././include/linux/compiler_types.h:352:38: error: call to
‘__compiletime_assert_1047’ \
declared with attribute error: FIELD_PREP: mask is not constant
_compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
and other build errors due to shift overflowing values.
See https://lore.kernel.org/r/ykwq6%2btih8gqp...@zn.tnic for the gory
details as to why it triggers with older gccs only.
Signed-off-by: Borislav Petkov
Cc: Jani Nikula
Cc: Joonas Lahtinen
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: David Airlie
Cc: Daniel Vetter
Cc: intel-...@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
---
.../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h | 2 +-
.../i915/gt/uc/abi/guc_communication_ctb_abi.h | 2 +-
.../gpu/drm/i915/gt/uc/abi/guc_messages_abi.h | 2 +-
drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h | 2 +-
drivers/gpu/drm/i915/i915_reg.h| 18 +-
5 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
index 7afdadc7656f..e835f28c0020 100644
--- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
+++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h
@@ -50,7 +50,7 @@
#define HOST2GUC_SELF_CFG_REQUEST_MSG_LEN
(GUC_HXG_REQUEST_MSG_MIN_LEN + 3u)
#define HOST2GUC_SELF_CFG_REQUEST_MSG_0_MBZ
GUC_HXG_REQUEST_MSG_0_DATA0
-#define HOST2GUC_SELF_CFG_REQUEST_MSG_1_KLV_KEY(0x << 16)
+#define HOST2GUC_SELF_CFG_REQUEST_MSG_1_KLV_KEY(0xU << 16)
#define HOST2GUC_SELF_CFG_REQUEST_MSG_1_KLV_LEN(0x << 0)
#define HOST2GUC_SELF_CFG_REQUEST_MSG_2_VALUE32
GUC_HXG_REQUEST_MSG_n_DATAn
#define HOST2GUC_SELF_CFG_REQUEST_MSG_3_VALUE64
GUC_HXG_REQUEST_MSG_n_DATAn
diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h
b/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h
index c9086a600bce..df83c1cc7c7a 100644
--- a/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h
+++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h
@@ -82,7 +82,7 @@ static_assert(sizeof(struct guc_ct_buffer_desc) == 64);
#define GUC_CTB_HDR_LEN1u
#define GUC_CTB_MSG_MIN_LENGUC_CTB_HDR_LEN
#define GUC_CTB_MSG_MAX_LEN256u
-#define GUC_CTB_MSG_0_FENCE(0x << 16)
+#define GUC_CTB_MSG_0_FENCE(0xU << 16)
#define GUC_CTB_MSG_0_FORMAT (0xf << 12)
#define GUC_CTB_FORMAT_HXG 0u
#define GUC_CTB_MSG_0_RESERVED (0xf << 8)
diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h
b/drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h
index 29ac823acd4c..7d5ba4d97d70 100644
--- a/drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h
+++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_messages_abi.h
@@ -40,7 +40,7 @@
*/
#define GUC_HXG_MSG_MIN_LEN1u
-#define GUC_HXG_MSG_0_ORIGIN (0x1 << 31)
+#define GUC_HXG_MSG_0_ORIGIN (0x1U << 31)
#define GUC_HXG_ORIGIN_HOST 0u
#define GUC_HXG_ORIGIN_GUC 1u
#define GUC_HXG_MSG_0_TYPE (0x7 << 28)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
b/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
index 66027a42cda9..ad570fa002a6 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
@@ -28,7 +28,7 @@
#define GS_MIA_HALT_REQUESTED (0x02 << GS_MIA_SHIFT)
#define GS_MIA_ISR_ENTRY (0x04 << GS_MIA_SHIFT)
#define GS_AUTH_STATUS_SHIFT 30
-#define GS_AUTH_STATUS_MASK(0x03 << GS_AUTH_STATUS_SHIFT)
+#define GS_AUTH_STATUS_MASK(0x03U << GS_AUTH_STATUS_SHIFT)
#define GS_AUTH_STATUS_BAD (0x01 << GS_AUTH_STATUS_SHIFT)
#define GS_AUTH_STATUS_GOOD(0x02 << GS_AUTH_STATUS_SHIFT)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 3c87d77d2cf6..f3ba3d0a430b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7555,19 +7555,19 @@ enum skl_power_gate {
#define PORT_CLK_SEL_LCPLL_810(2 << 29)
#define PORT_CLK_SEL_SPLL (3 << 29)
#define PORT_CLK_SEL_WRPLL(pll) (((pll) + 4) << 29)
-#define PORT_CLK_SEL_WRPLL1 (4 << 29)
-#define PORT_CLK_SEL_WRPLL2 (5 << 29)
-#define PORT_CLK_SEL_NONE (7 << 29)
-#define PORT_CLK_SEL_MASK (7 << 29)
+#define PORT_CLK_SEL_
[PATCH 04/11] drm/r128: Fix undefined behavior due to shift overflowing the constant
From: Borislav Petkov Fix: drivers/gpu/drm/r128/r128_cce.c: In function ‘r128_do_init_cce’: drivers/gpu/drm/r128/r128_cce.c:417:2: error: case label does not reduce to an integer constant case R128_PM4_64BM_64VCBM_64INDBM: ^~~~ drivers/gpu/drm/r128/r128_cce.c:418:2: error: case label does not reduce to an integer constant case R128_PM4_64PIO_64VCPIO_64INDPIO: ^~~~ See https://lore.kernel.org/r/ykwq6%2btih8gqp...@zn.tnic for the gory details as to why it triggers with older gccs only. Signed-off-by: Borislav Petkov Cc: David Airlie Cc: Daniel Vetter Cc: Alex Deucher Cc: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/r128/r128_drv.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/r128/r128_drv.h b/drivers/gpu/drm/r128/r128_drv.h index 2e1bc01aa5c9..970e192b0d51 100644 --- a/drivers/gpu/drm/r128/r128_drv.h +++ b/drivers/gpu/drm/r128/r128_drv.h @@ -300,8 +300,8 @@ extern long r128_compat_ioctl(struct file *filp, unsigned int cmd, # define R128_PM4_64PIO_128INDBM (5 << 28) # define R128_PM4_64BM_128INDBM (6 << 28) # define R128_PM4_64PIO_64VCBM_64INDBM(7 << 28) -# define R128_PM4_64BM_64VCBM_64INDBM (8 << 28) -# define R128_PM4_64PIO_64VCPIO_64INDPIO (15 << 28) +# define R128_PM4_64BM_64VCBM_64INDBM (8U << 28) +# define R128_PM4_64PIO_64VCPIO_64INDPIO (15U << 28) # define R128_PM4_BUFFER_CNTL_NOUPDATE(1 << 27) #define R128_PM4_BUFFER_WM_CNTL0x0708 -- 2.35.1
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 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 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 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 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
[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 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 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
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: BUG: unable to handle kernel paging request in drm_fb_helper_damage_work
On Mon, Sep 20, 2021 at 08:55:28PM +0800, Hao Sun wrote: > Hello, > > When using Healer to fuzz the latest Linux kernel, the following crash Your Healer thing - or whatever that next automated thing is which is trying to be smart - is not CCing the proper people: $ ./scripts/get_maintainer.pl -f drivers/gpu/drm/drm_fb_helper.c --no-rolestats Maarten Lankhorst Maxime Ripard Thomas Zimmermann David Airlie Daniel Vetter dri-devel@lists.freedesktop.org linux-ker...@vger.kernel.org I'll Cc them now but you should fix it. The syzcaller mails at least Cc more people and I'm sure you can figure out how to do that when you have the stack trace and get_maintainer.pl. > was triggered. > > HEAD commit: 4357f03d6611 Merge tag 'pm-5.15-rc2 > git tree: upstream > console output: > https://drive.google.com/file/d/13NUxvBLIswpoS8NOOAaq9PjOKgTYN19K/view?usp=sharing > kernel config: > https://drive.google.com/file/d/1HKZtF_s3l6PL3OoQbNq_ei9CdBus-Tz0/view?usp=sharing > > Sorry, I don't have a reproducer for this crash, hope the symbolized > report can help. > If you fix this issue, please add the following tag to the commit: > Reported-by: Hao Sun > > BUG: unable to handle page fault for address: c90003d79000 > #PF: supervisor read access in kernel mode > #PF: error_code(0x) - not-present page > PGD 8c00067 P4D 8c00067 PUD 8d63067 PMD 104409067 PTE 0 > Oops: [#1] PREEMPT SMP > CPU: 2 PID: 3032 Comm: kworker/2:2 Not tainted 5.15.0-rc1+ #19 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS > rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/01/2014 > Workqueue: events drm_fb_helper_damage_work > RIP: 0010:rep_movs arch/x86/lib/iomem.c:12 [inline] > RIP: 0010:memcpy_toio+0x48/0xa0 arch/x86/lib/iomem.c:57 > Code: 01 75 41 e8 4a 0d 04 ff 49 83 fc 01 76 0a e8 3f 0d 04 ff f6 c3 > 02 75 44 e8 35 0d 04 ff 4c 89 e1 48 89 df 48 89 ee 48 c1 e9 02 a5 > 41 f6 c4 02 74 02 66 a5 41 f6 c4 01 74 01 a4 5b 5d 41 5c e9 > RSP: 0018:c988fda8 EFLAGS: 00010206 > RAX: RBX: c90005aff000 RCX: 0100 > RDX: 88800f132240 RSI: c90003d79000 RDI: c90005b0 > RBP: c90003d78000 R08: 0001 R09: > R10: c988fdc8 R11: 0004 R12: 1400 > R13: 888101fc7000 R14: 02ff R15: c90003d78000 > FS: () GS:88807dd0() knlGS: > CS: 0010 DS: ES: CR0: 80050033 > CR2: c90003d79000 CR3: 00010ea77000 CR4: 00750ee0 > DR0: DR1: DR2: > DR3: DR6: fffe0ff0 DR7: 0400 > PKRU: 5554 > Call Trace: > dma_buf_map_memcpy_to include/linux/dma-buf-map.h:245 [inline] > drm_fb_helper_damage_blit_real drivers/gpu/drm/drm_fb_helper.c:388 [inline] > drm_fb_helper_damage_blit drivers/gpu/drm/drm_fb_helper.c:419 [inline] > drm_fb_helper_damage_work+0x30e/0x380 drivers/gpu/drm/drm_fb_helper.c:450 > process_one_work+0x359/0x850 kernel/workqueue.c:2297 > worker_thread+0x41/0x4d0 kernel/workqueue.c:2444 > kthread+0x178/0x1b0 kernel/kthread.c:319 > ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:295 > Modules linked in: > Dumping ftrace buffer: >(ftrace buffer empty) > CR2: c90003d79000 > ---[ end trace e1f0ecb0884517c4 ]--- > RIP: 0010:rep_movs arch/x86/lib/iomem.c:12 [inline] > RIP: 0010:memcpy_toio+0x48/0xa0 arch/x86/lib/iomem.c:57 > Code: 01 75 41 e8 4a 0d 04 ff 49 83 fc 01 76 0a e8 3f 0d 04 ff f6 c3 > 02 75 44 e8 35 0d 04 ff 4c 89 e1 48 89 df 48 89 ee 48 c1 e9 02 a5 > 41 f6 c4 02 74 02 66 a5 41 f6 c4 01 74 01 a4 5b 5d 41 5c e9 > RSP: 0018:c988fda8 EFLAGS: 00010206 > RAX: RBX: c90005aff000 RCX: 0100 > RDX: 88800f132240 RSI: c90003d79000 RDI: c90005b0 > RBP: c90003d78000 R08: 0001 R09: > R10: c988fdc8 R11: 0004 R12: 1400 > R13: 888101fc7000 R14: 02ff R15: c90003d78000 > FS: () GS:88807dd0() knlGS: > CS: 0010 DS: ES: CR0: 80050033 > CR2: c90003d79000 CR3: 00010ea77000 CR4: 00750ee0 > DR0: DR1: DR2: > DR3: DR6: fffe0ff0 DR7: 0400 > PKRU: 5554 > > Code disassembly (best guess): >0: 01 75 41add%esi,0x41(%rbp) >3: e8 4a 0d 04 ff callq 0xff040d52 >8: 49 83 fc 01 cmp$0x1,%r12 >c: 76 0a jbe0x18 >e: e8 3f 0d 04 ff callq 0xff040d52 > 13: f6 c3 02test $0x2,%bl > 16: 75 44 jne0x5c > 18: e8 35 0d 04 ff callq 0xff040d52 > 1d: 4c 89 e1mov%r12,%rcx > 20: 48 89 dfmov%rbx,%rdi > 23: 48 89 eemov
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 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 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 v2 0/2] allow simple{fb,drm} drivers to be used on non-x86 EFI platforms
On Tue, Jun 01, 2021 at 04:59:10PM +0200, Javier Martinez Canillas wrote: > The series touches different subystems and will need coordination between > maintainers. Ard Biesheuvel said that can be merged through the EFI tree. I'm always happy when code from arch/x86/ moves away so Acked-by: Borislav Petkov Btw, for the future, please CC everyone on the whole patchset - I had to go look at your 2/2 on lore to see what it does because I had only 1/2 in my mbox. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [syzbot] BUG: unable to handle kernel paging request in drm_fb_helper_damage_work (2)
Looks DRM to me. CCed... On Mon, May 31, 2021 at 12:13:22AM -0700, syzbot wrote: > Hello, > > syzbot found the following issue on: > > HEAD commit:7ac3a1c1 Merge tag 'mtd/fixes-for-5.13-rc4' of git://git.k.. > git tree: upstream > console output: https://syzkaller.appspot.com/x/log.txt?x=1619b4b5d0 > kernel config: https://syzkaller.appspot.com/x/.config?x=266cda122a0b56c > dashboard link: https://syzkaller.appspot.com/bug?extid=545dc60af42828d1e70b > userspace arch: i386 > > Unfortunately, I don't have any reproducer for this issue yet. > > IMPORTANT: if you fix the issue, please add the following tag to the commit: > Reported-by: syzbot+545dc60af42828d1e...@syzkaller.appspotmail.com > > BUG: unable to handle page fault for address: c9000dc68008 > #PF: supervisor write access in kernel mode > #PF: error_code(0x0002) - not-present page > PGD 1167 P4D 1167 PUD 111b3067 PMD 1ba2c067 PTE 0 > Oops: 0002 [#1] PREEMPT SMP KASAN > CPU: 2 PID: 16890 Comm: kworker/2:36 Not tainted 5.13.0-rc3-syzkaller #0 > Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.14.0-2 04/01/2014 > Workqueue: events drm_fb_helper_damage_work > RIP: 0010:rep_movs arch/x86/lib/iomem.c:12 [inline] > RIP: 0010:memcpy_toio+0x83/0xe0 arch/x86/lib/iomem.c:57 > Code: 8c fd 49 89 dd 31 ff 41 83 e5 02 4c 89 ee e8 c4 c2 8c fd 4d 85 ed 75 2e > e8 9a ba 8c fd 48 89 e9 48 89 df 4c 89 e6 48 c1 e9 02 a5 40 f6 c5 02 74 > 02 66 a5 40 f6 c5 01 74 01 a4 5b 5d 41 5c 41 > RSP: 0018:c9000e73fbc8 EFLAGS: 00010202 > RAX: RBX: c9000dc68008 RCX: 00fe > RDX: 88801534 RSI: c9000bdd9008 RDI: c9000dc68008 > RBP: 03f8 R08: R09: 0001 > R10: 83e81e1c R11: R12: c9000bdd9008 > R13: R14: c9000bdd9008 R15: 0001 > FS: () GS:88802cc0() knlGS: > CS: 0010 DS: ES: CR0: 80050033 > CR2: c9000dc68008 CR3: 6c062000 CR4: 00152ee0 > DR0: DR1: DR2: > DR3: DR6: fffe0ff0 DR7: 0400 > Call Trace: > dma_buf_map_memcpy_to include/linux/dma-buf-map.h:245 [inline] > drm_fb_helper_damage_blit_real drivers/gpu/drm/drm_fb_helper.c:388 [inline] > drm_fb_helper_damage_blit drivers/gpu/drm/drm_fb_helper.c:419 [inline] > drm_fb_helper_damage_work+0x733/0xac0 drivers/gpu/drm/drm_fb_helper.c:450 > process_one_work+0x98d/0x1600 kernel/workqueue.c:2276 > worker_thread+0x64c/0x1120 kernel/workqueue.c:2422 > kthread+0x3b1/0x4a0 kernel/kthread.c:313 > ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:294 > Modules linked in: > CR2: c9000dc68008 > ---[ end trace 7f8625a9b15be223 ]--- > RIP: 0010:rep_movs arch/x86/lib/iomem.c:12 [inline] > RIP: 0010:memcpy_toio+0x83/0xe0 arch/x86/lib/iomem.c:57 > Code: 8c fd 49 89 dd 31 ff 41 83 e5 02 4c 89 ee e8 c4 c2 8c fd 4d 85 ed 75 2e > e8 9a ba 8c fd 48 89 e9 48 89 df 4c 89 e6 48 c1 e9 02 a5 40 f6 c5 02 74 > 02 66 a5 40 f6 c5 01 74 01 a4 5b 5d 41 5c 41 > RSP: 0018:c9000e73fbc8 EFLAGS: 00010202 > RAX: RBX: c9000dc68008 RCX: 00fe > RDX: 88801534 RSI: c9000bdd9008 RDI: c9000dc68008 > RBP: 03f8 R08: R09: 0001 > R10: 83e81e1c R11: R12: c9000bdd9008 > R13: R14: c9000bdd9008 R15: 0001 > FS: () GS:88802cc0() knlGS: > CS: 0010 DS: ES: CR0: 80050033 > CR2: c9000dc68008 CR3: 6c062000 CR4: 00152ee0 > DR0: DR1: DR2: > DR3: DR6: fffe0ff0 DR7: 0400 > > > --- > This report is generated by a bot. It may contain errors. > See https://goo.gl/tpsmEJ for more information about syzbot. > syzbot engineers can be reached at syzkal...@googlegroups.com. > > syzbot will keep track of this issue. See: > https://goo.gl/tpsmEJ#status for how to communicate with syzbot. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH 1/2] drm/radeon: stop re-init the TTM page pool
On Tue, Jan 05, 2021 at 07:23:08PM +0100, Christian König wrote:
> Drivers are not supposed to init the page pool directly any more.
>
> Signed-off-by: Christian König
> ---
> drivers/gpu/drm/radeon/radeon_ttm.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c
> b/drivers/gpu/drm/radeon/radeon_ttm.c
> index d4328ff57757..35b715f82ed8 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -729,9 +729,6 @@ int radeon_ttm_init(struct radeon_device *rdev)
> }
> rdev->mman.initialized = true;
>
> - ttm_pool_init(>mman.bdev.pool, rdev->dev, rdev->need_swiotlb,
> - dma_addressing_limited(>pdev->dev));
> -
> r = radeon_ttm_init_vram(rdev);
> if (r) {
> DRM_ERROR("Failed initializing VRAM heap.\n");
> --
Was finally able to test those during workstation hw maintenance so I
was able to install a new kernel and reboot.
Reported-by: Borislav Petkov
Tested-by: Borislav Petkov
Thanks for the fixes!
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: 5.11-rc1 TTM list corruption
On Tue, Jan 05, 2021 at 07:08:52PM +0800, Huang Rui wrote: > Ah, this asic is a bit old and still use radeon driver. So we didn't > reproduce it on amdgpu driver. I don't have such the old asic in my hand. > May we know whether this issue can be duplicated after SI which is used > amdgpu module (not sure whether you have recent APU or GPU)? The latest I have (I think it is the latest) is: [1.826102] [drm] initializing kernel modesetting (RENOIR 0x1002:0x1636 0x17AA:0x5099 0xD1). and so far that hasn't triggered it. Which makes sense because that thing uses amdgpu: [1.810260] [drm] amdgpu kernel modesetting enabled. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: 5.11-rc1 TTM list corruption
Hi, On Tue, Jan 05, 2021 at 12:12:13PM +0800, Huang Rui wrote: > I am reproducing this issue as well, are you using a Raven board? I have no clue what Raven is. The workstation I triggered it once on, has: [7.563968] [drm] radeon kernel modesetting enabled. [7.581417] [drm] initializing kernel modesetting (CAICOS 0x1002:0x6779 0x174B:0xE164 0x00). [7.609217] [drm] Detected VRAM RAM=2048M, BAR=256M [7.614031] [drm] RAM width 64bits DDR [7.639665] [drm] radeon: 2048M of VRAM memory ready [7.644557] [drm] radeon: 1024M of GTT memory ready. [7.649451] [drm] Loading CAICOS Microcode [7.653548] [drm] Internal thermal controller without fan control [7.661221] [drm] radeon: dpm initialized [7.665227] [drm] GART: num cpu pages 262144, num gpu pages 262144 [7.671821] [drm] enabling PCIE gen 2 link speeds, disable with radeon.pcie_gen2=0 [7.703858] [drm] PCIE GART of 1024M enabled (table at 0x00162000). [7.749689] [drm] radeon: irq initialized. [7.769826] [drm] ring test on 0 succeeded in 1 usecs [7.774797] [drm] ring test on 3 succeeded in 3 usecs [7.955500] [drm] ring test on 5 succeeded in 1 usecs [7.960468] [drm] UVD initialized successfully. [7.965047] [drm] ib test on ring 0 succeeded in 0 usecs [7.970316] [drm] ib test on ring 3 succeeded in 0 usecs [8.626877] [drm] ib test on ring 5 succeeded [8.631376] [drm] Radeon Display Connectors [8.635496] [drm] Connector 0: [8.638503] [drm] HDMI-A-1 [8.641339] [drm] HPD2 [8.643835] [drm] DDC: 0x6440 0x6440 0x6444 0x6444 0x6448 0x6448 0x644c 0x644c [8.651102] [drm] Encoders: [8.654022] [drm] DFP1: INTERNAL_UNIPHY1 [8.658224] [drm] Connector 1: [8.661232] [drm] DVI-D-1 [8.663982] [drm] HPD4 [8.666479] [drm] DDC: 0x6460 0x6460 0x6464 0x6464 0x6468 0x6468 0x646c 0x646c [8.673745] [drm] Encoders: [8.676665] [drm] DFP2: INTERNAL_UNIPHY [8.680782] [drm] Connector 2: [8.683789] [drm] VGA-1 [8.686369] [drm] DDC: 0x6430 0x6430 0x6434 0x6434 0x6438 0x6438 0x643c 0x643c [8.693636] [drm] Encoders: [8.696555] [drm] CRT1: INTERNAL_KLDSCP_DAC1 [8.788923] [drm] fb mappable at 0xE0363000 [8.793036] [drm] vram apper at 0xE000 [8.797064] [drm] size 9216000 [8.800071] [drm] fb depth is 24 [8.803249] [drm]pitch is 7680 [8.807106] fbcon: radeondrmfb (fb0) is primary device [8.918927] radeon :1d:00.0: [drm] fb0: radeondrmfb frame buffer device [8.938461] [drm] Initialized radeon 2.50.0 20080528 for :1d:00.0 on minor 0 HTH. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: 5.11-rc1 TTM list corruption
On Fri, Jan 01, 2021 at 03:34:28PM +0100, Christian König wrote: > Going to double check the code, but can you reproduce this issue > reliable? Lemme find a test box which can trigger it too - the splat happened on my workstation and I'd like to avoid debugging there for obvious reasons. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
5.11-rc1 TTM list corruption
Hi folks, got this when trying to suspend my workstation to disk, it was still responsive so I could catch the splat: [22020.334381] [ cut here ] [22020.339057] list_del corruption. next->prev should be 8b7a9a40, but was 8881020bced0 [22020.347764] WARNING: CPU: 12 PID: 13134 at lib/list_debug.c:54 __list_del_entry_valid+0x8a/0x90 [22020.356397] Modules linked in: fuse essiv authenc nft_counter nf_tables libcrc32c nfnetlink loop dm_crypt dm_mod amd64_edac edac_mce_amd kvm_amd snd_hda_codec_realtek snd_hda_codec_generic led_class kvm ledtrig_audio snd_hda_codec_hdmi snd_hda_intel snd_intel_dspcfg snd_hda_codec snd_hda_core snd_pcm snd_timer irqbypass crct10dif_pclmul snd crc32_pclmul crc32c_intel ghash_clmulni_intel pcspkr k10temp soundcore gpio_amdpt gpio_generic acpi_cpufreq radeon aesni_intel glue_helper crypto_simd cryptd pinctrl_amd [22020.400855] CPU: 12 PID: 13134 Comm: hib.sh Not tainted 5.11.0-rc1+ #2 [22020.400857] Hardware name: Micro-Star International Co., Ltd. MS-7B79/X470 GAMING PRO (MS-7B79), BIOS 1.70 01/23/2019 [22020.400858] RIP: 0010:__list_del_entry_valid+0x8a/0x90 [22020.400861] Code: 46 00 0f 0b 31 c0 c3 48 89 f2 48 89 fe 48 c7 c7 78 30 0f 82 e8 24 6c 46 00 0f 0b 31 c0 c3 48 c7 c7 b8 30 0f 82 e8 13 6c 46 00 <0f> 0b 31 c0 c3 cc 48 85 d2 89 f8 74 20 48 8d 0c 16 0f b6 16 48 ff [22020.400863] RSP: 0018:c90001fbbcf8 EFLAGS: 00010292 [22020.441503] RAX: 0054 RBX: 8b7a9a40 RCX: [22020.441505] RDX: 8887fef26600 RSI: 8887fef17450 RDI: 8887fef17450 [22020.441505] RBP: 3f82 R08: 8887fef17450 R09: c90001fbbb38 [22020.441506] R10: 0001 R11: 0001 R12: [22020.441507] R13: 0080 R14: 0480 R15: 019b [22020.441508] FS: 7f51c72f9740() GS:8887fef0() knlGS: [22020.490045] CS: 0010 DS: ES: CR0: 80050033 [22020.490046] CR2: 5557afb81018 CR3: 00012099e000 CR4: 003506e0 [22020.490047] Call Trace: [22020.490048] ttm_pool_shrink+0x61/0xd0 [22020.508965] ttm_pool_shrinker_scan+0xa/0x20 [22020.508966] shrink_slab.part.0.constprop.0+0x1a1/0x330 [22020.508970] drop_slab_node+0x37/0x50 [22020.522011] drop_slab+0x33/0x60 [22020.522012] drop_caches_sysctl_handler+0x70/0x80 [22020.522015] proc_sys_call_handler+0x140/0x220 [22020.534286] new_sync_write+0x10b/0x190 [22020.534289] vfs_write+0x1b7/0x290 [22020.534291] ksys_write+0x60/0xe0 [22020.544762] do_syscall_64+0x33/0x40 [22020.544765] entry_SYSCALL_64_after_hwframe+0x44/0xa9 [22020.553320] RIP: 0033:0x7f51c73eaff3 [22020.553322] Code: 8b 15 a1 ee 0c 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b7 0f 1f 00 64 8b 04 25 18 00 00 00 85 c0 75 14 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 55 c3 0f 1f 40 00 48 83 ec 28 48 89 54 24 18 [22020.553324] RSP: 002b:7ffd0a748ef8 EFLAGS: 0246 ORIG_RAX: 0001 [22020.553325] RAX: ffda RBX: 0002 RCX: 7f51c73eaff3 [22020.553326] RDX: 0002 RSI: 56039fd0ee70 RDI: 0001 [22020.553327] RBP: 56039fd0ee70 R08: 000a R09: 0001 [22020.553327] R10: 56039fd0e770 R11: 0246 R12: 0002 [22020.611218] R13: 7f51c74bb6a0 R14: 0002 R15: 7f51c74bb8a0 [22020.611220] ---[ end trace f7ea94a6ddb98f71 ]--- -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/8] edac: ghes: use krealloc_array()
On Tue, Oct 27, 2020 at 01:17:22PM +0100, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski
>
> Use the helper that checks for overflows internally instead of manually
> calculating the size of the new array.
>
> Signed-off-by: Bartosz Golaszewski
> ---
> drivers/edac/ghes_edac.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c
> index a918ca93e4f7..6d1ddecbf0da 100644
> --- a/drivers/edac/ghes_edac.c
> +++ b/drivers/edac/ghes_edac.c
> @@ -207,8 +207,8 @@ static void enumerate_dimms(const struct dmi_header *dh,
> void *arg)
> if (!hw->num_dimms || !(hw->num_dimms % 16)) {
> struct dimm_info *new;
>
> - new = krealloc(hw->dimms, (hw->num_dimms + 16) * sizeof(struct
> dimm_info),
> - GFP_KERNEL);
> + new = krealloc_array(hw->dimms, hw->num_dimms + 16,
> + sizeof(struct dimm_info), GFP_KERNEL);
> if (!new) {
> WARN_ON_ONCE(1);
> return;
> --
Sure, why not.
Acked-by: Borislav Petkov
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
first bad commit: [fc8c70526bd30733ea8667adb8b8ffebea30a8ed] drm/radeon: Prefer lower feedback dividers
Hi, this patch breaks X on my box - it is fully responsive and I can log in into it from another machine but both monitors are black and show this: "The current input timing is not supported by the monitor display. Please change your input timing to 1920x1200@60Hz or any other monitor listed timing as per the monitor specifications." Reverting it fixes the issue. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 00/17] Drop uses of pci_read_config_*() return value
On Sun, Aug 02, 2020 at 02:14:06PM -0500, Bjorn Helgaas wrote: > Wait, I'm not convinced yet. I know that if a PCI read fails, you > normally get ~0 data because the host bridge fabricates it to complete > the CPU load. > > But what guarantees that a PCI config register cannot contain ~0? Well, I don't think you can differentiate that case, right? I guess this is where the driver knowledge comes into play: if the read returns ~0, the pci_read_config* should probably return in that case something like: PCIBIOS_READ_MAYBE_FAILED to denote it is all 1s and then the caller should be able to determine, based on any of domain:bus:slot.func and whatever else the driver knows about its hardware, whether the 1s are a valid value or an error. Hopefully. Or something better of which I cannot think of right now... -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 00/17] Drop uses of pci_read_config_*() return value
On Sun, Aug 02, 2020 at 07:28:00PM +0200, Saheed Bolarinwa wrote:
> Because the value ~0 has a meaning to some drivers and only
No, ~0 means that the PCI read failed. For *every* PCI device I know.
Here's me reading from 0xf0 offset of my hostbridge:
# setpci -s 00:00.0 0xf0.l
0100
That device doesn't have extended config space, so the last valid byte
is 0xff. Let's read beyond that:
# setpci -s 00:00.0 0x100.l
> Again, only the drivers can determine if ~0 is a valid value. This
> information is not available inside pci_config_read*().
Of course it is.
*every* change you've done in 6/17 - this is the only patch I have
received - checks for == ~0. So that check can just as well be moved
inside pci_config_read_*().
Here's how one could do it:
#define PCI_OP_READ(size, type, len) \
int noinline pci_bus_read_config_##size \
(struct pci_bus *bus, unsigned int devfn, int pos, type *value) \
{ \
int res;\
unsigned long flags;\
u32 data = 0; \
if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
pci_lock_config(flags); \
res = bus->ops->read(bus, devfn, pos, len, ); \
/* Check we actually read something which is not all 1s.*/
if (data == ~0)
return PCIBIOS_READ_FAILED;
*value = (type)data;\
pci_unlock_config(flags); \
return res; \
}
Also, I'd prefer a function to *not* return void but return either
an error or success. In the success case, the @value argument can be
consumed by the caller and otherwise not.
In any case, that change is a step in the wrong direction and I don't
like it, sorry.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 00/17] Drop uses of pci_read_config_*() return value
On Sat, Aug 01, 2020 at 01:24:29PM +0200, Saheed O. Bolarinwa wrote:
> The return value of pci_read_config_*() may not indicate a device error.
> However, the value read by these functions is more likely to indicate
> this kind of error. This presents two overlapping ways of reporting
> errors and complicates error checking.
So why isn't the *value check done in the pci_read_config_* functions
instead of touching gazillion callers?
For example, pci_conf{1,2}_read() could check whether the u32 *value it
just read depending on the access method, whether that value is ~0 and
return proper PCIBIOS_ error in that case.
The check you're replicating
if (val32 == (u32)~0)
everywhere, instead, is just ugly and tests a naked value ~0 which
doesn't mean anything...
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] 2dd4d163cd9c ("drm/nouveau: remove open-coded version of remove_conflicting_pci_framebuffers()")
On Thu, Jun 18, 2020 at 10:39:07PM +0200, Borislav Petkov wrote:
> I'll redo the bisection tomorrow, on a fresh head, to check.
Ok, just did it again, similar bisection log, points at the same commit.
The commit before it:
fb172f5fe880 ("drm/nouveau/gr/gk20a: move MODULE_FIRMWARE firmware definitions")
boots ok but
2dd4d163cd9c ("drm/nouveau: remove open-coded version of
remove_conflicting_pci_framebuffers()")
doesn't.
Ideas?
git bisect start
# good: [3d77e6a8804abcc0504c904bd6e5cdf3a5cf8162] Linux 5.7
git bisect good 3d77e6a8804abcc0504c904bd6e5cdf3a5cf8162
# bad: [5e857ce6eae7ca21b2055cca4885545e29228fe2] Merge branch 'hch' (maccess
patches from Christoph Hellwig)
git bisect bad 5e857ce6eae7ca21b2055cca4885545e29228fe2
# bad: [a98f670e41a99f53acb1fb33cee9c6abbb2e6f23] Merge tag 'media/v5.8-1' of
git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media
git bisect bad a98f670e41a99f53acb1fb33cee9c6abbb2e6f23
# bad: [5be993432821750f382809df5e20bf4c129b24f7] mm/hugetlb: define a generic
fallback for arch_clear_hugepage_flags()
git bisect bad 5be993432821750f382809df5e20bf4c129b24f7
# good: [cfa3b8068b09f25037146bfd5eed041b78878bee] Merge tag 'for-linus-hmm' of
git://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma
git bisect good cfa3b8068b09f25037146bfd5eed041b78878bee
# good: [a1fb548962397bb8609bb46e566809a9a1b30044] Merge tag
'drm-intel-next-2020-04-30' of git://anongit.freedesktop.org/drm/drm-intel into
drm-next
git bisect good a1fb548962397bb8609bb46e566809a9a1b30044
# bad: [750a02ab8d3c49ca7d23102be90d3d1db19e2827] Merge tag
'for-5.8/block-2020-06-01' of git://git.kernel.dk/linux-block
git bisect bad 750a02ab8d3c49ca7d23102be90d3d1db19e2827
# good: [e20bb857dea2f620ff37ae541ed8aee70e3c89f1] Merge tag
'exynos-drm-next-for-v5.8' of
git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos into drm-next
git bisect good e20bb857dea2f620ff37ae541ed8aee70e3c89f1
# good: [e6e7abffe386b614a194ec32457a00c304c980f4] blk-mq: simplify the
blk_mq_get_request calling convention
git bisect good e6e7abffe386b614a194ec32457a00c304c980f4
# bad: [7dbbdd37f2ae7dd4175ba3f86f4335c463b18403] drm/nouveau: use correct
conflicting framebuffer API
git bisect bad 7dbbdd37f2ae7dd4175ba3f86f4335c463b18403
# bad: [0f85bbb6ae517d9a4308527188afe35c2012bbc9] drm/nouveau/device: use
regular PRI accessors in chipset detection
git bisect bad 0f85bbb6ae517d9a4308527188afe35c2012bbc9
# good: [fa4f4c213f5f7807360c41f2501a3031a9940f3a] drm/nouveau/kms: Support
NVIDIA format modifiers
git bisect good fa4f4c213f5f7807360c41f2501a3031a9940f3a
# bad: [e3d8b08904694e9ccae5163d0bb7d35fa66e5bdc] drm/nouveau/svm: map pages
after migration
git bisect bad e3d8b08904694e9ccae5163d0bb7d35fa66e5bdc
# good: [fb172f5fe880cd0ddb4370b2fcc9ad4848c98bbb] drm/nouveau/gr/gk20a: move
MODULE_FIRMWARE firmware definitions
git bisect good fb172f5fe880cd0ddb4370b2fcc9ad4848c98bbb
# bad: [b950c8c5d082d822b0134d1fc058101ab346e503] drm/nouveau/bios: move ACPI
_ROM handling
git bisect bad b950c8c5d082d822b0134d1fc058101ab346e503
# bad: [2dd4d163cd9c15432524aa9863155bc03a821361] drm/nouveau: remove
open-coded version of remove_conflicting_pci_framebuffers()
git bisect bad 2dd4d163cd9c15432524aa9863155bc03a821361
# first bad commit: [2dd4d163cd9c15432524aa9863155bc03a821361] drm/nouveau:
remove open-coded version of remove_conflicting_pci_framebuffers()
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] 2dd4d163cd9c ("drm/nouveau: remove open-coded version of remove_conflicting_pci_framebuffers()")
Hi, On Thu, Jun 18, 2020 at 04:19:02PM -0400, Ilia Mirkin wrote: > Hi Boris, > > There was a fixup to that patch that you'll also have to revert first > -- 7dbbdd37f2ae7dd4175ba3f86f4335c463b18403. I guess there's some > subtle difference between the old open-coded logic and the helper, > they were supposed to be identical. Thanks, that's a good point. I reverted both but it still hangs. So either my bisection is wrong or reverting those two is not enough due to other changes. I'll redo the bisection tomorrow, on a fresh head, to check. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
2dd4d163cd9c ("drm/nouveau: remove open-coded version of remove_conflicting_pci_framebuffers()")
Hi, my test box won't boot 5.8-rc1 all the way but stops at ... fb0: switching to nouveaufb from EFI VGA <-- EOF I've bisected it to the commit in $Subject, see below. Unfortunately, it doesn't revert cleanly so I can't really do the final test of reverting it ontop of 5.8-rc1 to confirm that this one is really causing it. Any ideas? GPU is: [5.678614] fb0: switching to nouveaufb from EFI VGA [5.685577] Console: switching to colour dummy device 80x25 [5.691865] nouveau :03:00.0: NVIDIA GT218 (0a8c00b1) [5.814409] nouveau :03:00.0: bios: version 70.18.83.00.08 [5.823559] nouveau :03:00.0: fb: 512 MiB DDR3 [6.096680] [TTM] Zone kernel: Available graphics memory: 8158364 KiB [6.103327] [TTM] Zone dma32: Available graphics memory: 2097152 KiB [6.109951] [TTM] Initializing pool allocator [6.114405] [TTM] Initializing DMA pool allocator [6.119256] nouveau :03:00.0: DRM: VRAM: 512 MiB [6.124285] nouveau :03:00.0: DRM: GART: 1048576 MiB [6.129677] nouveau :03:00.0: DRM: TMDS table version 2.0 [6.135534] nouveau :03:00.0: DRM: DCB version 4.0 [6.140755] nouveau :03:00.0: DRM: DCB outp 00: 02000360 [6.147273] nouveau :03:00.0: DRM: DCB outp 01: 02000362 00020010 [6.153782] nouveau :03:00.0: DRM: DCB outp 02: 028003a6 0f220010 [6.160292] nouveau :03:00.0: DRM: DCB outp 03: 01011380 [6.166810] nouveau :03:00.0: DRM: DCB outp 04: 08011382 00020010 [6.173306] nouveau :03:00.0: DRM: DCB outp 05: 088113c6 0f220010 [6.179829] nouveau :03:00.0: DRM: DCB conn 00: 00101064 [6.185553] nouveau :03:00.0: DRM: DCB conn 01: 00202165 [6.196145] nouveau :03:00.0: DRM: MM: using COPY for buffer copies [6.233659] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). [6.311939] nouveau :03:00.0: DRM: allocated 1920x1080 fb: 0x7, bo (ptrval) [6.320736] fbcon: nouveaudrmfb (fb0) is primary device [6.392722] tsc: Refined TSC clocksource calibration: 3591.346 MHz [6.392788] clocksource: tsc: mask: 0x max_cycles: 0x33c46403b59, max_idle_ns: 440795293818 ns [6.392930] clocksource: Switched to clocksource tsc [6.509946] Console: switching to colour frame buffer device 240x67 [6.546287] nouveau :03:00.0: fb0: nouveaudrmfb frame buffer device [6.555021] [drm] Initialized nouveau 1.3.1 20120801 for :03:00.0 on minor 0 Thx. git bisect start # good: [3d77e6a8804abcc0504c904bd6e5cdf3a5cf8162] Linux 5.7 git bisect good 3d77e6a8804abcc0504c904bd6e5cdf3a5cf8162 # bad: [b3a9e3b9622ae10064826dccb4f7a52bd88c7407] Linux 5.8-rc1 git bisect bad b3a9e3b9622ae10064826dccb4f7a52bd88c7407 # bad: [ee01c4d72adffb7d424535adf630f2955748fa8b] Merge branch 'akpm' (patches from Andrew) git bisect bad ee01c4d72adffb7d424535adf630f2955748fa8b # bad: [16d91548d1057691979de4686693f0ff92f46000] Merge tag 'xfs-5.8-merge-8' of git://git.kernel.org/pub/scm/fs/xfs/xfs-linux git bisect bad 16d91548d1057691979de4686693f0ff92f46000 # good: [cfa3b8068b09f25037146bfd5eed041b78878bee] Merge tag 'for-linus-hmm' of git://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma git bisect good cfa3b8068b09f25037146bfd5eed041b78878bee # good: [3fd911b69b3117e03181262fc19ae6c3ef6962ce] Merge tag 'drm-misc-next-2020-05-07' of git://anongit.freedesktop.org/drm/drm-misc into drm-next git bisect good 3fd911b69b3117e03181262fc19ae6c3ef6962ce # bad: [1966391fa576e1fb2701be8bcca197d8f72737b7] mm/migrate.c: attach_page_private already does the get_page git bisect bad 1966391fa576e1fb2701be8bcca197d8f72737b7 # good: [43c8546bcd854806736d8a635a0d696504dd4c21] drm/amdgpu: Add a UAPI flag for user to call mem_sync git bisect good 43c8546bcd854806736d8a635a0d696504dd4c21 # good: [6cf991611bc72c077f0cc64e23987341ad7ef41e] Merge tag 'drm-intel-next-2020-05-15' of git://anongit.freedesktop.org/drm/drm-intel into drm-next git bisect good 6cf991611bc72c077f0cc64e23987341ad7ef41e # bad: [dc455f4c888365595c0a13da445e092422d55b8d] drm/nouveau/dispnv50: fix runtime pm imbalance on error git bisect bad dc455f4c888365595c0a13da445e092422d55b8d # bad: [2dd4d163cd9c15432524aa9863155bc03a821361] drm/nouveau: remove open-coded version of remove_conflicting_pci_framebuffers() git bisect bad 2dd4d163cd9c15432524aa9863155bc03a821361 # good: [c41219fda6e04255c44d37fd2c0d898c1c46abf1] Merge tag 'drm-intel-next-fixes-2020-05-20' of git://anongit.freedesktop.org/drm/drm-intel into drm-next git bisect good c41219fda6e04255c44d37fd2c0d898c1c46abf1 # good: [fd44028ff145ffb2d03c877d74f479da04ac2c62] drm/nouveau/acr: ensure falcon providing acr functions is bootstrapped first git bisect good fd44028ff145ffb2d03c877d74f479da04ac2c62 # good: [fa4f4c213f5f7807360c41f2501a3031a9940f3a] drm/nouveau/kms: Support NVIDIA format modifiers git bisect good fa4f4c213f5f7807360c41f2501a3031a9940f3a # good: [d2bcfce7f8a4ba8df28d3bebb81225bd7f9c046f] drm/nouveau/ibus: use
Re: [PATCH v2 2/4] x86/vmware: Add a header file for hypercall definitions
On Tue, Aug 27, 2019 at 09:19:03PM +0200, Thomas Hellström (VMware) wrote: > It should be correct. The flags VMWARE_HYPERVISOR_HB and > VMWARE_HYPERVISOR_OUT are only valid for the vmcall / vmmcall versions. > > For the legacy version, the direction is toggled by the instruction (in vs > out) and LB vs HB is toggled by the port number (0x5658 vs 0x5659) > > So in essence the low word definition of %edx is different in the two > versions. I've chosen to use the new vmcall/vmmcall definition in the driver > code. Ah, ok, I see what you mean. The old method would overwrite the low word of %edx but the new one would have the flags already prepared and *not* overwrite them so all good. Can you please document that more explicitly in the comment in arch/x86/include/asm/vmware.h? Something like: "... The new vmcall interface instead uses a set of flags to select bandwidth mode and transfer direction. The set of flags is already loaded into %edx by the macros which use VMWARE_HYPERCALL* and only when the guest must use the old VMWARE_HYPERVISOR_PORT* method, the low word is overwritten by the respective port number." Anyway, something along those lines. We want to have the alternatives code as clear and as transparent as possible because, well, of obvious reasons. :-) Thx. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/4] x86/vmware: Add a header file for hypercall definitions
On Fri, Aug 23, 2019 at 10:13:14AM +0200, Thomas Hellström (VMware) wrote:
> +/*
> + * The high bandwidth out call. The low word of edx is presumed to have the
> + * HB and OUT bits set.
> + */
> +#define VMWARE_HYPERCALL_HB_OUT
> \
> + ALTERNATIVE_2("movw $" VMWARE_HYPERVISOR_PORT_HB ", %%dx; rep outsb", \
Hmm, that looks fishy:
This call in vmw_port_hb_out(), for example, gets converted to the asm
below (I've left in the asm touching only rDX).
# drivers/gpu/drm/vmwgfx/vmwgfx_msg.c:160: VMW_PORT_HB_OUT(
#NO_APP
movzwl 0(%rbp), %edx # channel_20(D)->channel_id,
channel_20(D)->channel_id
...
sall$16, %edx #, tmp172
orl $3, %edx#, tmp173
this is adding channel_id and flags:
VMWARE_HYPERVISOR_HB | (channel->channel_id << 16) |
VMWARE_HYPERVISOR_OUT,
the $3 being (VMWARE_HYPERVISOR_HB | VMWARE_HYPERVISOR_OUT).
movslq %edx, %rdx # tmp173, tmp174
Here it is sign-extending it.
#APP
# 160 "drivers/gpu/drm/vmwgfx/vmwgfx_msg.c" 1
push %rbp;mov %r8, %rbp;# ALT: oldinstr2# bp
661:
movw $0x5659, %dx; rep outsb
And now here you're overwriting the low word of %edx. And now it
contains:
0x[channel_id]5659
and the low word doesn't contain the 3, i.e., (VMWARE_HYPERVISOR_HB |
VMWARE_HYPERVISOR_OUT) anymore. And that's before you do the hypercall
so I'm guessing that cannot be right.
Or?
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
Re: [PATCH v2 2/4] x86/vmware: Add a header file for hypercall definitions
On Fri, Aug 23, 2019 at 10:13:14AM +0200, Thomas Hellström (VMware) wrote: > From: Thomas Hellstrom > > The new header is intended to be used by drivers using the backdoor. > Follow the kvm example using alternatives self-patching to > choose between vmcall, vmmcall and io instructions. > > Also define two new CPU feature flags to indicate hypervisor support > for vmcall- and vmmcall instructions. I could use some of the explanation why we need two feature flags added here from: https://lkml.kernel.org/r/970d2bb6-ab29-315f-f5d8-5d1109585...@shipmail.org Thx. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: 1c74ca7a1a9a ("drm/fb-helper: call vga_remove_vgacon automatically.")
On Fri, Aug 09, 2019 at 10:54:41AM +0200, Gerd Hoffmann wrote: > A bit later: > >[8.198138] radeon :00:01.0: Direct firmware load for > radeon/PALM_pfp.bin failed with error -2 >[8.198351] r600_cp: Failed to load firmware "radeon/PALM_pfp.bin" >[8.198512] [drm:evergreen_init [radeon]] *ERROR* Failed to load > firmware! >[8.198590] radeon :00:01.0: Fatal error during GPU init > > So the radeon drm driver tries to load and fails due to missing firmware, > thats why you have a non-working display. > > So your options are: > (a) install linux-firmware, so the radeon driver can initialize, or > (b) boot your kernel with "nomodeset" command line option. The radeon > driver will not initialize then, and also not disable conflicting > display drivers (vgacon or fbcon @ vesafb/efifb). Damn firmware! I should've seen that message. ;-\ Yeah, did a) and that worked. Thanks! > Possibly it also makes sense to have the radeon driver try load the firmware > (from disk) rather early in the initialization process, before calling > drm_fb_helper_remove_conflicting_pci_framebuffers, so firmware not being > installed doesn't kill the display. That would've helped and saved a lot of time. Thanks again! -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
1c74ca7a1a9a ("drm/fb-helper: call vga_remove_vgacon automatically.")
Hi, for some unfathomable to me reason, the commit in $Subject breaks booting of the 32-bit partition of one of my test boxes. The box doesn't finish booting (normally it boots in text mode, there is no X server setup on it) but it is still responsible in the sense that I can reboot it with the Sysrq combination. No other keystrokes have effect. I bisected it to this patch and confirmed that it really is the culprit by reverting it ontop of latest Linus tree. With it reverted, it boots fine. .config is attached, lemme know if you need more info to debug this. Thx. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. # # Automatically generated file; DO NOT EDIT. # Linux/i386 5.3.0-rc2 Kernel Configuration # # # Compiler: gcc (Debian 8.3.0-2) 8.3.0 # CONFIG_CC_IS_GCC=y CONFIG_GCC_VERSION=80300 CONFIG_CLANG_VERSION=0 CONFIG_CC_CAN_LINK=y CONFIG_CC_HAS_ASM_GOTO=y CONFIG_CC_HAS_WARN_MAYBE_UNINITIALIZED=y CONFIG_IRQ_WORK=y CONFIG_BUILDTIME_EXTABLE_SORT=y CONFIG_THREAD_INFO_IN_TASK=y # # General setup # CONFIG_INIT_ENV_ARG_LIMIT=32 # CONFIG_COMPILE_TEST is not set # CONFIG_HEADER_TEST is not set CONFIG_LOCALVERSION="" # CONFIG_LOCALVERSION_AUTO is not set CONFIG_BUILD_SALT="" CONFIG_HAVE_KERNEL_GZIP=y CONFIG_HAVE_KERNEL_BZIP2=y CONFIG_HAVE_KERNEL_LZMA=y CONFIG_HAVE_KERNEL_XZ=y CONFIG_HAVE_KERNEL_LZO=y CONFIG_HAVE_KERNEL_LZ4=y CONFIG_KERNEL_GZIP=y # CONFIG_KERNEL_BZIP2 is not set # CONFIG_KERNEL_LZMA is not set # CONFIG_KERNEL_XZ is not set # CONFIG_KERNEL_LZO is not set # CONFIG_KERNEL_LZ4 is not set CONFIG_DEFAULT_HOSTNAME="x1" CONFIG_SWAP=y CONFIG_SYSVIPC=y CONFIG_SYSVIPC_SYSCTL=y # CONFIG_POSIX_MQUEUE is not set # CONFIG_CROSS_MEMORY_ATTACH is not set CONFIG_USELIB=y # CONFIG_AUDIT is not set CONFIG_HAVE_ARCH_AUDITSYSCALL=y # # IRQ subsystem # CONFIG_GENERIC_IRQ_PROBE=y CONFIG_GENERIC_IRQ_SHOW=y CONFIG_GENERIC_IRQ_EFFECTIVE_AFF_MASK=y CONFIG_GENERIC_PENDING_IRQ=y CONFIG_GENERIC_IRQ_MIGRATION=y CONFIG_IRQ_DOMAIN=y CONFIG_IRQ_DOMAIN_HIERARCHY=y CONFIG_GENERIC_MSI_IRQ=y CONFIG_GENERIC_MSI_IRQ_DOMAIN=y CONFIG_GENERIC_IRQ_MATRIX_ALLOCATOR=y CONFIG_GENERIC_IRQ_RESERVATION_MODE=y CONFIG_IRQ_FORCED_THREADING=y CONFIG_SPARSE_IRQ=y # CONFIG_GENERIC_IRQ_DEBUGFS is not set # end of IRQ subsystem CONFIG_CLOCKSOURCE_WATCHDOG=y CONFIG_ARCH_CLOCKSOURCE_DATA=y CONFIG_ARCH_CLOCKSOURCE_INIT=y CONFIG_CLOCKSOURCE_VALIDATE_LAST_CYCLE=y CONFIG_GENERIC_TIME_VSYSCALL=y CONFIG_GENERIC_CLOCKEVENTS=y CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=y CONFIG_GENERIC_CMOS_UPDATE=y # # Timers subsystem # CONFIG_TICK_ONESHOT=y CONFIG_NO_HZ_COMMON=y # CONFIG_HZ_PERIODIC is not set CONFIG_NO_HZ_IDLE=y CONFIG_NO_HZ=y CONFIG_HIGH_RES_TIMERS=y # end of Timers subsystem # CONFIG_PREEMPT_NONE is not set # CONFIG_PREEMPT_VOLUNTARY is not set CONFIG_PREEMPT=y CONFIG_PREEMPT_COUNT=y CONFIG_PREEMPTION=y # # CPU/Task time and stats accounting # CONFIG_TICK_CPU_ACCOUNTING=y # CONFIG_IRQ_TIME_ACCOUNTING is not set # CONFIG_BSD_PROCESS_ACCT is not set CONFIG_TASKSTATS=y CONFIG_TASK_DELAY_ACCT=y # CONFIG_TASK_XACCT is not set # CONFIG_PSI is not set # end of CPU/Task time and stats accounting # CONFIG_CPU_ISOLATION is not set # # RCU Subsystem # CONFIG_PREEMPT_RCU=y # CONFIG_RCU_EXPERT is not set CONFIG_SRCU=y CONFIG_TREE_SRCU=y CONFIG_TASKS_RCU=y CONFIG_RCU_STALL_COMMON=y CONFIG_RCU_NEED_SEGCBLIST=y # end of RCU Subsystem CONFIG_IKCONFIG=y CONFIG_IKCONFIG_PROC=y # CONFIG_IKHEADERS is not set CONFIG_LOG_BUF_SHIFT=21 CONFIG_LOG_CPU_MAX_BUF_SHIFT=12 CONFIG_PRINTK_SAFE_LOG_BUF_SHIFT=13 CONFIG_HAVE_UNSTABLE_SCHED_CLOCK=y # # Scheduler features # # end of Scheduler features CONFIG_ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH=y CONFIG_CGROUPS=y # CONFIG_MEMCG is not set # CONFIG_BLK_CGROUP is not set CONFIG_CGROUP_SCHED=y CONFIG_FAIR_GROUP_SCHED=y # CONFIG_CFS_BANDWIDTH is not set # CONFIG_RT_GROUP_SCHED is not set # CONFIG_CGROUP_PIDS is not set # CONFIG_CGROUP_RDMA is not set # CONFIG_CGROUP_FREEZER is not set # CONFIG_CGROUP_HUGETLB is not set # CONFIG_CPUSETS is not set # CONFIG_CGROUP_DEVICE is not set # CONFIG_CGROUP_CPUACCT is not set # CONFIG_CGROUP_PERF is not set # CONFIG_CGROUP_DEBUG is not set CONFIG_NAMESPACES=y # CONFIG_UTS_NS is not set # CONFIG_IPC_NS is not set # CONFIG_USER_NS is not set # CONFIG_PID_NS is not set # CONFIG_NET_NS is not set # CONFIG_CHECKPOINT_RESTORE is not set CONFIG_SCHED_AUTOGROUP=y # CONFIG_SYSFS_DEPRECATED is not set CONFIG_RELAY=y CONFIG_BLK_DEV_INITRD=y CONFIG_INITRAMFS_SOURCE="" CONFIG_RD_GZIP=y # CONFIG_RD_BZIP2 is not set # CONFIG_RD_LZMA is not set # CONFIG_RD_XZ is not set # CONFIG_RD_LZO is not set # CONFIG_RD_LZ4 is not set CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE=y # CONFIG_CC_OPTIMIZE_FOR_SIZE is not set CONFIG_SYSCTL=y CONFIG_HAVE_UID16=y CONFIG_SYSCTL_EXCEPTION_TRACE=y CONFIG_HAVE_PCSPKR_PLATFORM=y CONFIG_BPF=y CONFIG_EXPERT=y CONFIG_UID16=y CONFIG_MULTIUSER=y CONFIG_SGETMASK_SYSCALL=y CONFIG_SYSFS_SYSCALL=y CONFIG_SYSCTL_SYSCALL=y
Re: [PATCH v2 56/79] docs: Documentation/*.txt: rename all ReST files to *.rst
On Tue, Apr 23, 2019 at 04:06:40PM -0600, Jonathan Corbet wrote: > Remember that most of our docs are 99% RST even though they were written > by people who had never even heard of RST. I really don't think it's a > big deal - a far smaller cognitive load than trying to keep up with any > given subsystem's variable-declaration-ordering rules, for example :) Tztztz, this thing seems to have hit a nerve with people. Which means, I will enforce that even more now so that I annoy submitters more! :-P See, I can do my own "RST" too. :-P Srsly: ok, good. Sounds like we're on the same page then. > I'm trying to do the same in Documentation/, with an attempt to be > sympathetic toward our readers, sort things by intended audience, > and create (someday) a coherent whole. I agree that moving docs is > a short-term annoyance, but I'm hoping that it brings a long-term > benefit. Ok, that's fair. I've been moving files too, in the past. > Minimal markup is the policy (it's even documented :). Automating stuff > that can be automated is an area that has definitely not received > enough attention; hopefully some things can be done there in the very > near future. Sounds nice, thanks Jon! -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 56/79] docs: Documentation/*.txt: rename all ReST files to *.rst
On Wed, Apr 24, 2019 at 07:40:07AM -0300, Mauro Carvalho Chehab wrote: > Personally, I don't care much with monospaced fonts on this table. After > all, if I want to see it monospaced, I can simply click at the > "View page source" at the browser, and it will display the file as a > plain old monospaced text file. Goes to show why kernel people wouldn't want to look at that in the browser. Long hex numbers are hard to read as it is - that's why there's even the 4-digit separator in some docs, for example: 0x__8100_. Not having it monospaced makes the whole thing even less readable. That's why it is important for the markup not to get in the way of people looking at those files in an editor. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
