linux-kernel - Re: [RFC PATCH 1/3] pci, acpi: Match PCI config space accessors against platfrom specific ECAM quirks.

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]
Message-ID: <8e00d6fa-7ed6-e69f-d41c-c114375c98ae@linaro.org>
Date:	Mon, 6 Jun 2016 15:54:08 +0800
From:	Hanjun Guo <hanjun.guo@...aro.org>
To:	Gabriele Paoloni <gabriele.paoloni@...wei.com>,
	Jeffrey Hugo <jhugo@...eaurora.org>,
	Christopher Covington <cov@...eaurora.org>,
	Tomasz Nowicki <tn@...ihalf.com>,
	"helgaas@...nel.org" <helgaas@...nel.org>,
	"arnd@...db.de" <arnd@...db.de>,
	"will.deacon@....com" <will.deacon@....com>,
	"catalin.marinas@....com" <catalin.marinas@....com>,
	"rafael@...nel.org" <rafael@...nel.org>,
	"Lorenzo.Pieralisi@....com" <Lorenzo.Pieralisi@....com>,
	"okaya@...eaurora.org" <okaya@...eaurora.org>,
	"jchandra@...adcom.com" <jchandra@...adcom.com>
Cc:	"liudongdong (C)" <liudongdong3@...wei.com>,
	"linaro-acpi@...ts.linaro.org" <linaro-acpi@...ts.linaro.org>,
	"jcm@...hat.com" <jcm@...hat.com>,
	"dhdang@....com" <dhdang@....com>,
	"Liviu.Dudau@....com" <Liviu.Dudau@....com>,
	"ddaney@...iumnetworks.com" <ddaney@...iumnetworks.com>,
	"jeremy.linton@....com" <jeremy.linton@....com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
	"robert.richter@...iumnetworks.com" 
	<robert.richter@...iumnetworks.com>,
	"msalter@...hat.com" <msalter@...hat.com>,
	"Suravee.Suthikulpanit@....com" <Suravee.Suthikulpanit@....com>,
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	Wangyijing <wangyijing@...wei.com>,
	"mw@...ihalf.com" <mw@...ihalf.com>,
	"andrea.gallo@...aro.org" <andrea.gallo@...aro.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [RFC PATCH 1/3] pci, acpi: Match PCI config space accessors
 against platfrom specific ECAM quirks.

On 2016/6/6 15:27, Gabriele Paoloni wrote:
> Hi Jeffrey
>> On 6/3/2016 9:32 AM, Gabriele Paoloni wrote:
>>> Hi Cov
>>>
>>>> Hi Tomasz,
>>>>
>>>> Thanks for your work on this.
>>>>
>>>> On 06/02/2016 04:41 AM, Tomasz Nowicki wrote:
>>>>> Some platforms may not be fully compliant with generic set of PCI
>>>> config
>>>>> accessors. For these cases we implement the way to overwrite
>>>> accessors
>>>>> set. Algorithm traverses available quirk list, matches against
>>>>> <oem_id, oem_rev, domain, bus number> tuple and returns
>> corresponding
>>>>> PCI config ops. oem_id and oem_rev come from MCFG table standard
>>>> header.
>>>>> All quirks can be defined using DECLARE_ACPI_MCFG_FIXUP() macro and
>>>>> kept self contained. Example:
>>>>>
>>>>> /* Custom PCI config ops */
>>>>> static struct pci_generic_ecam_ops foo_pci_ops = {
>>>>> 	.bus_shift	= 24,
>>>>> 	.pci_ops = {
>>>>> 		.map_bus = pci_ecam_map_bus,
>>>>> 		.read = foo_ecam_config_read,
>>>>> 		.write = foo_ecam_config_write,
>>>>> 	}
>>>>> };
>>>>>
>>>>> DECLARE_ACPI_MCFG_FIXUP(&foo_pci_ops, <oem_id_str>, <oem_rev>,
>>>> <domain_nr>, <bus_nr>);
>>>>>
>>>>> Signed-off-by: Tomasz Nowicki <tn@...ihalf.com>
>>>>> ---
>>>>>  drivers/acpi/pci_mcfg.c           | 32
>>>> ++++++++++++++++++++++++++++++++
>>>>>  include/asm-generic/vmlinux.lds.h |  7 +++++++
>>>>>  include/linux/pci-acpi.h          | 19 +++++++++++++++++++
>>>>>  3 files changed, 58 insertions(+)
>>>>>
>>>>> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
>>>>> index 1847f74..f3d4570 100644
>>>>> --- a/drivers/acpi/pci_mcfg.c
>>>>> +++ b/drivers/acpi/pci_mcfg.c
>>>>> @@ -22,11 +22,43 @@
>>>>>  #include <linux/kernel.h>
>>>>>  #include <linux/pci.h>
>>>>>  #include <linux/pci-acpi.h>
>>>>> +#include <linux/pci-ecam.h>
>>>>>
>>>>>  /* Root pointer to the mapped MCFG table */
>>>>>  static struct acpi_table_mcfg *mcfg_table;
>>>>>  static int mcfg_entries;
>>>>>
>>>>> +extern struct pci_cfg_fixup __start_acpi_mcfg_fixups[];
>>>>> +extern struct pci_cfg_fixup __end_acpi_mcfg_fixups[];
>>>>> +
>>>>> +struct pci_ecam_ops *pci_mcfg_get_ops(struct acpi_pci_root *root)
>>>>> +{
>>>>> +	int bus_num = root->secondary.start;
>>>>> +	int domain = root->segment;
>>>>> +	struct pci_cfg_fixup *f;
>>>>> +
>>>>> +	if (!mcfg_table)
>>>>> +		return &pci_generic_ecam_ops;
>>>>> +
>>>>> +	/*
>>>>> +	 * Match against platform specific quirks and return
>>>> corresponding
>>>>> +	 * CAM ops.
>>>>> +	 *
>>>>> +	 * First match against PCI topology <domain:bus> then use OEM ID
>>>> and
>>>>> +	 * OEM revision from MCFG table standard header.
>>>>> +	 */
>>>>> +	for (f = __start_acpi_mcfg_fixups; f < __end_acpi_mcfg_fixups;
>>>> f++) {
>>>>> +		if ((f->domain == domain || f->domain ==
>>>> PCI_MCFG_DOMAIN_ANY) &&
>>>>> +		    (f->bus_num == bus_num || f->bus_num ==
>>>> PCI_MCFG_BUS_ANY) &&
>>>>> +		    (!strncmp(f->oem_id, mcfg_table->header.oem_id,
>>>>> +			      ACPI_OEM_ID_SIZE)) &&
>>>>> +		    (f->oem_revision == mcfg_table->header.oem_revision))
>>>>
>>>> Is this more likely to be updated between quirky and fixed platforms
>>>> than oem_table_id? What do folks think about using oem_table_id
>> instead
>>>> of, or in addition to, oem_revision?
>>>
>>> From my understanding we need to stick to this mechanism as
>> (otherwise)
>>> there are platforms out in the field that would need a FW update.
>>>
>>> So I don't think that using oem_table_id "instead" is possible; about
>>> "in addition" I think it is doable, but I do not see the advantage
>> much.
>>> I mean that if a platform gets fixed the oem revision should change
>> too,
>>> Right?
>>
>> Cov and I had a discussion about this, so hopefully I can bring a
>> slightly different perspective that will make sense.
>>
>> We forsee a situation where we have platform A that needs a quirk, and
>> platform B that does not.  The OEM id is the same for both platforms as
>> they are different platforms from the same OEM.  Using the OEM revision
>> field does not seem to be appropriate since these are different
>> platforms and the revision field appears to be for the purpose of
>> tracking differences within a single platform.  Therefore, Cov is
>> proposing using the OEM table id as a mechanism to distinguish platform
>> A (needs quirk applied) vs platform B (no quirks) from the same OEM.
>
> Ah yes I see now...
>
> Probably it should be ok to have a check on all three OEM fields.

Just for reference, x86 and IA64 use oem_id and oem_table_id to make a
difference between different platforms, see
acpi_madt_oem_check(char *oem_id, char *oem_table_id) for x86 and ia64,
that can apply to ARM64 on MCFG too.

Thanks
Hanjun