[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZUVUz8P6a0mzVsJP@swahl-home.5wahls.com>
Date: Fri, 3 Nov 2023 15:15:11 -0500
From: Steve Wahl <steve.wahl@....com>
To: Andrew Cooper <andrew.cooper3@...rix.com>
Cc: Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
"H. Peter Anvin" <hpa@...or.com>, Steve Wahl <steve.wahl@....com>,
Justin Ernst <justin.ernst@....com>,
Kyle Meyer <kyle.meyer@....com>,
Dimitri Sivanich <dimitri.sivanich@....com>,
Russ Anderson <russ.anderson@....com>,
Darren Hart <dvhart@...radead.org>,
Andy Shevchenko <andy@...radead.org>,
"K. Y. Srinivasan" <kys@...rosoft.com>,
Haiyang Zhang <haiyangz@...rosoft.com>,
Wei Liu <wei.liu@...nel.org>, Dexuan Cui <decui@...rosoft.com>,
Lorenzo Pieralisi <lpieralisi@...nel.org>,
Krzysztof WilczyĆski <kw@...ux.com>,
Rob Herring <robh@...nel.org>,
Bjorn Helgaas <bhelgaas@...gle.com>,
linux-kernel@...r.kernel.org, platform-driver-x86@...r.kernel.org,
linux-hyperv@...r.kernel.org, linux-pci@...r.kernel.org
Subject: Re: [PATCH 1/3] x86/apic: Drop apic::delivery_mode
On Thu, Nov 02, 2023 at 12:26:19PM +0000, Andrew Cooper wrote:
> This field is set to APIC_DELIVERY_MODE_FIXED in all cases, and is read
> exactly once. Fold the constant in uv_program_mmr() and drop the field.
>
> Searching for the origin of the stale HyperV comment reveals commit
> a31e58e129f7 ("x86/apic: Switch all APICs to Fixed delivery mode") which
> notes:
>
> As a consequence of this change, the apic::irq_delivery_mode field is
> now pointless, but this needs to be cleaned up in a separate patch.
>
> 6 years is long enough for this technical debt to have survived.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@...rix.com>
Reveiewed-by: Steve Wahl <steve.wahl@....com>
> ---
> arch/x86/include/asm/apic.h | 2 --
> arch/x86/kernel/apic/apic_flat_64.c | 2 --
> arch/x86/kernel/apic/apic_noop.c | 1 -
> arch/x86/kernel/apic/apic_numachip.c | 2 --
> arch/x86/kernel/apic/bigsmp_32.c | 1 -
> arch/x86/kernel/apic/probe_32.c | 1 -
> arch/x86/kernel/apic/x2apic_cluster.c | 1 -
> arch/x86/kernel/apic/x2apic_phys.c | 1 -
> arch/x86/kernel/apic/x2apic_uv_x.c | 1 -
> arch/x86/platform/uv/uv_irq.c | 2 +-
> drivers/pci/controller/pci-hyperv.c | 7 -------
> 11 files changed, 1 insertion(+), 20 deletions(-)
>
> diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
> index 5af4ec1a0f71..841afbd7bfe7 100644
> --- a/arch/x86/include/asm/apic.h
> +++ b/arch/x86/include/asm/apic.h
> @@ -272,8 +272,6 @@ struct apic {
> void (*send_IPI_all)(int vector);
> void (*send_IPI_self)(int vector);
>
> - enum apic_delivery_modes delivery_mode;
> -
> u32 disable_esr : 1,
> dest_mode_logical : 1,
> x2apic_set_max_apicid : 1;
> diff --git a/arch/x86/kernel/apic/apic_flat_64.c b/arch/x86/kernel/apic/apic_flat_64.c
> index 032a84e2c3cc..e526b226910b 100644
> --- a/arch/x86/kernel/apic/apic_flat_64.c
> +++ b/arch/x86/kernel/apic/apic_flat_64.c
> @@ -82,7 +82,6 @@ static struct apic apic_flat __ro_after_init = {
> .acpi_madt_oem_check = flat_acpi_madt_oem_check,
> .apic_id_registered = default_apic_id_registered,
>
> - .delivery_mode = APIC_DELIVERY_MODE_FIXED,
> .dest_mode_logical = true,
>
> .disable_esr = 0,
> @@ -153,7 +152,6 @@ static struct apic apic_physflat __ro_after_init = {
> .acpi_madt_oem_check = physflat_acpi_madt_oem_check,
> .apic_id_registered = default_apic_id_registered,
>
> - .delivery_mode = APIC_DELIVERY_MODE_FIXED,
> .dest_mode_logical = false,
>
> .disable_esr = 0,
> diff --git a/arch/x86/kernel/apic/apic_noop.c b/arch/x86/kernel/apic/apic_noop.c
> index 966d7cf10b95..70e7dfc3cc84 100644
> --- a/arch/x86/kernel/apic/apic_noop.c
> +++ b/arch/x86/kernel/apic/apic_noop.c
> @@ -45,7 +45,6 @@ static void noop_apic_write(u32 reg, u32 val)
> struct apic apic_noop __ro_after_init = {
> .name = "noop",
>
> - .delivery_mode = APIC_DELIVERY_MODE_FIXED,
> .dest_mode_logical = true,
>
> .disable_esr = 0,
> diff --git a/arch/x86/kernel/apic/apic_numachip.c b/arch/x86/kernel/apic/apic_numachip.c
> index 63f3d7be9dc7..8f5a42ad1f9f 100644
> --- a/arch/x86/kernel/apic/apic_numachip.c
> +++ b/arch/x86/kernel/apic/apic_numachip.c
> @@ -222,7 +222,6 @@ static const struct apic apic_numachip1 __refconst = {
> .probe = numachip1_probe,
> .acpi_madt_oem_check = numachip1_acpi_madt_oem_check,
>
> - .delivery_mode = APIC_DELIVERY_MODE_FIXED,
> .dest_mode_logical = false,
>
> .disable_esr = 0,
> @@ -259,7 +258,6 @@ static const struct apic apic_numachip2 __refconst = {
> .probe = numachip2_probe,
> .acpi_madt_oem_check = numachip2_acpi_madt_oem_check,
>
> - .delivery_mode = APIC_DELIVERY_MODE_FIXED,
> .dest_mode_logical = false,
>
> .disable_esr = 0,
> diff --git a/arch/x86/kernel/apic/bigsmp_32.c b/arch/x86/kernel/apic/bigsmp_32.c
> index 0e5535add4b5..863c3002a574 100644
> --- a/arch/x86/kernel/apic/bigsmp_32.c
> +++ b/arch/x86/kernel/apic/bigsmp_32.c
> @@ -80,7 +80,6 @@ static struct apic apic_bigsmp __ro_after_init = {
> .name = "bigsmp",
> .probe = probe_bigsmp,
>
> - .delivery_mode = APIC_DELIVERY_MODE_FIXED,
> .dest_mode_logical = false,
>
> .disable_esr = 1,
> diff --git a/arch/x86/kernel/apic/probe_32.c b/arch/x86/kernel/apic/probe_32.c
> index 9a06df6cdd68..f851ccf1e14f 100644
> --- a/arch/x86/kernel/apic/probe_32.c
> +++ b/arch/x86/kernel/apic/probe_32.c
> @@ -35,7 +35,6 @@ static struct apic apic_default __ro_after_init = {
> .probe = probe_default,
> .apic_id_registered = default_apic_id_registered,
>
> - .delivery_mode = APIC_DELIVERY_MODE_FIXED,
> .dest_mode_logical = true,
>
> .disable_esr = 0,
> diff --git a/arch/x86/kernel/apic/x2apic_cluster.c b/arch/x86/kernel/apic/x2apic_cluster.c
> index affbff65e497..7d15f6c3b718 100644
> --- a/arch/x86/kernel/apic/x2apic_cluster.c
> +++ b/arch/x86/kernel/apic/x2apic_cluster.c
> @@ -227,7 +227,6 @@ static struct apic apic_x2apic_cluster __ro_after_init = {
> .probe = x2apic_cluster_probe,
> .acpi_madt_oem_check = x2apic_acpi_madt_oem_check,
>
> - .delivery_mode = APIC_DELIVERY_MODE_FIXED,
> .dest_mode_logical = true,
>
> .disable_esr = 0,
> diff --git a/arch/x86/kernel/apic/x2apic_phys.c b/arch/x86/kernel/apic/x2apic_phys.c
> index 788cdb4ee394..8bb740e22b7d 100644
> --- a/arch/x86/kernel/apic/x2apic_phys.c
> +++ b/arch/x86/kernel/apic/x2apic_phys.c
> @@ -145,7 +145,6 @@ static struct apic apic_x2apic_phys __ro_after_init = {
> .probe = x2apic_phys_probe,
> .acpi_madt_oem_check = x2apic_acpi_madt_oem_check,
>
> - .delivery_mode = APIC_DELIVERY_MODE_FIXED,
> .dest_mode_logical = false,
>
> .disable_esr = 0,
> diff --git a/arch/x86/kernel/apic/x2apic_uv_x.c b/arch/x86/kernel/apic/x2apic_uv_x.c
> index 7d304ef1a7f5..ae4f0c1a7b43 100644
> --- a/arch/x86/kernel/apic/x2apic_uv_x.c
> +++ b/arch/x86/kernel/apic/x2apic_uv_x.c
> @@ -805,7 +805,6 @@ static struct apic apic_x2apic_uv_x __ro_after_init = {
> .probe = uv_probe,
> .acpi_madt_oem_check = uv_acpi_madt_oem_check,
>
> - .delivery_mode = APIC_DELIVERY_MODE_FIXED,
> .dest_mode_logical = false,
>
> .disable_esr = 0,
> diff --git a/arch/x86/platform/uv/uv_irq.c b/arch/x86/platform/uv/uv_irq.c
> index 4221259a5870..a379501b7a69 100644
> --- a/arch/x86/platform/uv/uv_irq.c
> +++ b/arch/x86/platform/uv/uv_irq.c
> @@ -35,7 +35,7 @@ static void uv_program_mmr(struct irq_cfg *cfg, struct uv_irq_2_mmr_pnode *info)
> mmr_value = 0;
> entry = (struct uv_IO_APIC_route_entry *)&mmr_value;
> entry->vector = cfg->vector;
> - entry->delivery_mode = apic->delivery_mode;
> + entry->delivery_mode = APIC_DELIVERY_MODE_FIXED;
> entry->dest_mode = apic->dest_mode_logical;
> entry->polarity = 0;
> entry->trigger = 0;
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index bed3cefdaf19..f5d2ef8572e7 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -650,13 +650,6 @@ static void hv_arch_irq_unmask(struct irq_data *data)
> PCI_FUNC(pdev->devfn);
> params->int_target.vector = hv_msi_get_int_vector(data);
>
> - /*
> - * Honoring apic->delivery_mode set to APIC_DELIVERY_MODE_FIXED by
> - * setting the HV_DEVICE_INTERRUPT_TARGET_MULTICAST flag results in a
> - * spurious interrupt storm. Not doing so does not seem to have a
> - * negative effect (yet?).
> - */
> -
> if (hbus->protocol_version >= PCI_PROTOCOL_VERSION_1_2) {
> /*
> * PCI_PROTOCOL_VERSION_1_2 supports the VP_SET version of the
>
> --
> 2.30.2
>
--
Steve Wahl, Hewlett Packard Enterprise
Powered by blists - more mailing lists