[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aBo5ZDvD-ME4dUc-@gmail.com>
Date: Tue, 6 May 2025 18:31:32 +0200
From: Ingo Molnar <mingo@...nel.org>
To: Rik van Riel <riel@...riel.com>
Cc: linux-kernel@...r.kernel.org, linux-mm@...ck.org, x86@...nel.org,
kernel-team@...a.com, dave.hansen@...ux.intel.com, luto@...nel.org,
peterz@...radead.org, tglx@...utronix.de, mingo@...hat.com,
bp@...en8.de, hpa@...or.com, Yu-cheng Yu <yu-cheng.yu@...el.com>
Subject: Re: [RFC PATCH 7/9] x86/mm: Introduce Remote Action Request
* Rik van Riel <riel@...riel.com> wrote:
> +++ b/arch/x86/include/asm/rar.h
> @@ -0,0 +1,69 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_X86_RAR_H
> +#define _ASM_X86_RAR_H
> +
> +/*
> + * RAR payload types
> + */
> +#define RAR_TYPE_INVPG 0
> +#define RAR_TYPE_INVPG_NO_CR3 1
> +#define RAR_TYPE_INVPCID 2
> +#define RAR_TYPE_INVEPT 3
> +#define RAR_TYPE_INVVPID 4
> +#define RAR_TYPE_WRMSR 5
> +
> +/*
> + * Subtypes for RAR_TYPE_INVLPG
> + */
> +#define RAR_INVPG_ADDR 0 /* address specific */
> +#define RAR_INVPG_ALL 2 /* all, include global */
> +#define RAR_INVPG_ALL_NO_GLOBAL 3 /* all, exclude global */
> +
> +/*
> + * Subtypes for RAR_TYPE_INVPCID
> + */
> +#define RAR_INVPCID_ADDR 0 /* address specific */
> +#define RAR_INVPCID_PCID 1 /* all of PCID */
> +#define RAR_INVPCID_ALL 2 /* all, include global */
> +#define RAR_INVPCID_ALL_NO_GLOBAL 3 /* all, exclude global */
> +
> +/*
> + * Page size for RAR_TYPE_INVLPG
> + */
> +#define RAR_INVLPG_PAGE_SIZE_4K 0
> +#define RAR_INVLPG_PAGE_SIZE_2M 1
> +#define RAR_INVLPG_PAGE_SIZE_1G 2
> +
> +/*
> + * Max number of pages per payload
> + */
> +#define RAR_INVLPG_MAX_PAGES 63
> +
> +typedef struct {
> + uint64_t for_sw : 8;
> + uint64_t type : 8;
> + uint64_t must_be_zero_1 : 16;
> + uint64_t subtype : 3;
> + uint64_t page_size: 2;
> + uint64_t num_pages : 6;
> + uint64_t must_be_zero_2 : 21;
> +
> + uint64_t must_be_zero_3;
> +
> + /*
> + * Starting address
> + */
> + uint64_t initiator_cr3;
> + uint64_t linear_address;
> +
> + /*
> + * Padding
> + */
> + uint64_t padding[4];
> +} rar_payload_t;
- Please don't use _t typedefs for complex types. 'struct rar_payload'
should be good enough.
- Please use u64/u32 for HW ABI definitions.
- Please align bitfield definitions vertically, for better readability:
u64 for_sw : 8;
u64 type : 8;
u64 must_be_zero_1 : 16;
u64 subtype : 3;
u64 page_size : 2;
u64 num_pages : 6;
u64 must_be_zero_2 : 21;
> +++ b/arch/x86/mm/rar.c
> @@ -0,0 +1,226 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * RAR Tlb shootdown
s/Tlb
/TLB
> +#include <linux/kgdb.h>
Is this really needed? There's nothing KGDB specific in here AFAICS.
> +static DEFINE_PER_CPU_ALIGNED(u64[(RAR_MAX_PAYLOADS + 8) / 8], rar_action);
> +static void set_action_entry(unsigned long idx, int target_cpu)
> +{
> + u8 *bitmap = (u8 *)per_cpu(rar_action, target_cpu);
> + u8 *bitmap = (u8 *)per_cpu(rar_action, target_cpu);
> + bitmap = (u8 *)per_cpu(rar_action, this_cpu);
So AFAICS all these ugly, forced type casts tp (u8 *) are needed only
because rar_action has the wrong type: if it were an u8[], then these
lines could be:
static DEFINE_PER_CPU_ALIGNED(u8[RAR_MAX_PAYLOADS], rar_action);
...
u8 *bitmap = per_cpu(rar_action, target_cpu);
right?
Thanks,
Ingo
Powered by blists - more mailing lists