linux-kernel - Re: [PATCH v2 1/1] x86/PCI: Clean up open-coded return code mangling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20231201210625.GA529122@bhelgaas>
Date:   Fri, 1 Dec 2023 15:06:25 -0600
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Cc:     Bjorn Helgaas <bhelgaas@...gle.com>,
        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>, linux-pci@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/1] x86/PCI: Clean up open-coded return code mangling

On Fri, Oct 27, 2023 at 12:31:58PM +0300, Ilpo Järvinen wrote:
> PCI BIOS gives a return code in 8 bits of eax register which is
> extracted by open-coded masks and shifting.
> 
> Name the return code bits with a define and introduce
> pcibios_get_return_code() helper to extract the return code to improve
> code readability. In addition, replace a zero test with
> PCIBIOS_SUCCESSFUL.
> 
> No function changes intended.
> 
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>

Applied to the PCI "enumeration" branch for v6.8.  x86 folks, if you'd
prefer to take this, let me know.

> ---
> 
> v2:
> - Improve changelog
> - Add helper to avoid repeating FIELD_GET()
> - Reuse existing ret variable in pcibios_get_irq_routing_table()
> 
>  arch/x86/pci/pcbios.c | 26 ++++++++++++++++++++------
>  1 file changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/pci/pcbios.c b/arch/x86/pci/pcbios.c
> index 4f15280732ed..01c94ac2fb70 100644
> --- a/arch/x86/pci/pcbios.c
> +++ b/arch/x86/pci/pcbios.c
> @@ -3,6 +3,8 @@
>   * BIOS32 and PCI BIOS handling.
>   */
>  
> +#include <linux/bits.h>
> +#include <linux/bitfield.h>
>  #include <linux/pci.h>
>  #include <linux/init.h>
>  #include <linux/slab.h>
> @@ -29,8 +31,19 @@
>  #define PCIBIOS_HW_TYPE1_SPEC		0x10
>  #define PCIBIOS_HW_TYPE2_SPEC		0x20
>  
> +/*
> + * Returned in EAX:
> + * - AH: return code
> + */
> +#define PCIBIOS_RETURN_CODE			GENMASK(15, 8)
> +
>  int pcibios_enabled;
>  
> +static u8 pcibios_get_return_code(u32 eax)
> +{
> +	return FIELD_GET(PCIBIOS_RETURN_CODE, eax);
> +}
> +
>  /* According to the BIOS specification at:
>   * http://members.datafast.net.au/dft0802/specs/bios21.pdf, we could
>   * restrict the x zone to some pages and make it ro. But this may be
> @@ -154,7 +167,7 @@ static int __init check_pcibios(void)
>  			: "memory");
>  		local_irq_restore(flags);
>  
> -		status = (eax >> 8) & 0xff;
> +		status = pcibios_get_return_code(eax);
>  		hw_mech = eax & 0xff;
>  		major_ver = (ebx >> 8) & 0xff;
>  		minor_ver = ebx & 0xff;
> @@ -227,7 +240,7 @@ static int pci_bios_read(unsigned int seg, unsigned int bus,
>  
>  	raw_spin_unlock_irqrestore(&pci_config_lock, flags);
>  
> -	return (int)((result & 0xff00) >> 8);
> +	return pcibios_get_return_code(result);
>  }
>  
>  static int pci_bios_write(unsigned int seg, unsigned int bus,
> @@ -269,7 +282,7 @@ static int pci_bios_write(unsigned int seg, unsigned int bus,
>  
>  	raw_spin_unlock_irqrestore(&pci_config_lock, flags);
>  
> -	return (int)((result & 0xff00) >> 8);
> +	return pcibios_get_return_code(result);
>  }
>  
>  
> @@ -385,8 +398,9 @@ struct irq_routing_table * pcibios_get_irq_routing_table(void)
>  		  "m" (opt)
>  		: "memory");
>  	DBG("OK  ret=%d, size=%d, map=%x\n", ret, opt.size, map);
> -	if (ret & 0xff00)
> -		printk(KERN_ERR "PCI: Error %02x when fetching IRQ routing table.\n", (ret >> 8) & 0xff);
> +	ret = pcibios_get_return_code(ret);
> +	if (ret)
> +		printk(KERN_ERR "PCI: Error %02x when fetching IRQ routing table.\n", ret);
>  	else if (opt.size) {
>  		rt = kmalloc(sizeof(struct irq_routing_table) + opt.size, GFP_KERNEL);
>  		if (rt) {
> @@ -415,7 +429,7 @@ int pcibios_set_irq_routing(struct pci_dev *dev, int pin, int irq)
>  		  "b" ((dev->bus->number << 8) | dev->devfn),
>  		  "c" ((irq << 8) | (pin + 10)),
>  		  "S" (&pci_indirect));
> -	return !(ret & 0xff00);
> +	return pcibios_get_return_code(ret) == PCIBIOS_SUCCESSFUL;
>  }
>  EXPORT_SYMBOL(pcibios_set_irq_routing);
>  
> -- 
> 2.30.2
>