linux-kernel - Re: [RFC] [PATCH] PCI mmconfig without ACPI

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] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 21 Feb 2009 00:55:15 -0500 (EST)
From:	Len Brown <lenb@...nel.org>
To:	Ingo Molnar <mingo@...e.hu>
Cc:	Ed Swierk <eswierk@...stanetworks.com>, tglx@...utronix.de,
	mingo@...hat.com, hpa@...or.com, linux-kernel@...r.kernel.org,
	linux-acpi@...r.kernel.org, jbarnes@...tuousgeek.org,
	linux-pci@...r.kernel.org
Subject: Re: [RFC] [PATCH] PCI mmconfig without ACPI


> > +#ifdef CONFIG_PCI_MMCONFIG
> > +
> > +struct acpi_mcfg_allocation {
> > +	u64 address;		/* Base address, processor-relative */
> > +	u16 pci_segment;	/* PCI segment group number */
> > +	u8 start_bus_number;	/* Starting PCI Bus number */
> > +	u8 end_bus_number;	/* Final PCI Bus number */
> > +	u32 reserved;
> > +};
> 
> Please rename this to "struct pci_mcfg_allocation" - there's nothing ACPI 
> about it anymore - mmcfg is a PCI feature and ACPI is an enumeration method.
> 
> Also, while touching it, please also use the opportunity to align structure 
> fields vertically:
> 
>  struct pci_mcfg_allocation {
> 	u64	address;		/* Base address, processor-relative */
> 	u16	pci_segment;		/* PCI segment group number */
> 	u8	start_bus_number;	/* Starting PCI Bus number */
> 	u8	end_bus_number;		/* Final PCI Bus number */
> 	u32	__reserved;
>  };
> 
> The whole layout of this structure becomes easier to read and nicer to look 
> at as well.
> 
> Another small detail: note how i renamed reserved to __reserved - that is a 
> standard way to de-emphasise the signficance of a structure field.
> 
> The reserved field there is for future expansion and to pad the structure to 
> 16 bytes - it doesnt really mean much and the underscores move it a bit out 
> of the default line of sight.
> 
> With a 'reserved' field people end up wondering whether it's perhaps some 
> _semantic_ 'reserved area' kind of thing (like for e820 maps, etc.) - so 
> it's never bad to make that distinction explicit via the double underscores.

struct acpi_mcfg_allocation is the structure that maps onto the MCFG
ACPI table as defined in the PCI firmware spec and provided by the ACPI 
BIOS.

I'd like it to stay in actbl1.h -- as that is part of ACPICA, which
tracks the standard tables.  (and I see you did this with your updated 
patch, thanks.)

FWIW, "reserved" here really does have a specific definition.
On read-only tables, such as this one, reserved fields are defined
to return 0 on reads for this version of the table, but may
return non-zero on future revisions.

thanks,
-Len

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/