> -----Original Message----- > From: Pankaj Bansal > Sent: Monday, 16 December, 2019 10:48 PM > To: Hanjun Guo <guohanjun@xxxxxxxxxx>; Lorenzo Pieralisi > <lorenzo.pieralisi@xxxxxxx>; Sudeep Holla <sudeep.holla@xxxxxxx>; > Rafael J . Wysocki <rjw@xxxxxxxxxxxxx>; Len Brown <lenb@xxxxxxxxxx> > Cc: linux-acpi@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; > Jonathan Cameron <jonathan.cameron@xxxxxxxxxx> > Subject: RE: [PATCH] ACPI/IORT: fix the iort_id_map function > > > -----Original Message----- > > From: Hanjun Guo <guohanjun@xxxxxxxxxx> > > Sent: Monday, 16 December, 2019 05:24 PM > > To: Pankaj Bansal <pankaj.bansal@xxxxxxx>; Lorenzo Pieralisi > > <lorenzo.pieralisi@xxxxxxx>; Sudeep Holla <sudeep.holla@xxxxxxx>; > > Rafael J . Wysocki <rjw@xxxxxxxxxxxxx>; Len Brown <lenb@xxxxxxxxxx> > > Cc: linux-acpi@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; > > Jonathan Cameron <jonathan.cameron@xxxxxxxxxx> > > Subject: Re: [PATCH] ACPI/IORT: fix the iort_id_map function > > > > On 2019/12/16 13:14, Pankaj Bansal wrote: > > > Hi Hanjun, > > > > > > Thanks for replying. Please find my response inline > > > > > >> Hi Pankaj, > > >> > > >> On 2019/12/15 23:12, Pankaj Bansal wrote: > > >>> As per > > >>> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fi > > >>> nf > > >>> oc > > >>> > > >> > > > enter.arm.com%2Fhelp%2Ftopic%2Fcom.arm.doc.den0049d%2FDEN0049D_IO > > >> _Rema > > >>> > > >> > > > pping_Table.pdf&data=02%7C01%7Cpankaj.bansal%40nxp.com%7C78d > > >> 82a560 > > >>> > > >> > > 5714219196008d781db06a7%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0% > > >> 7C1%7C6 > > >>> > > >> > > > 37120650018983814&sdata=%2FRhATUKx%2FA2gPEx%2BNY9X%2F7kqV > > >> CrEeRnbE% > > >>> 2B2qlTkdGDc%3D&reserved=0 > > >>> in ID mappings: > > >>> Number of IDs = The number of IDs in the range minus one. > > >> > > >> Hmm, the spec is confusing, the spec may need to be updated, for > > >> example, for a PCI bus, device ID + function ID will take 8 bits > > >> and will be 256 IDs for that PCI bus, not sure why we need to minus one. > > >> > > > > > > I agree that this "minus one" thing is confusing. Not sure why It > > > was put in the spec like that. I guess they wanted the number of IDs > > > to be 0 > > based instead of 1 based. > > > > > >>> > > >>> Therefore, it's valid for ID mapping to contain single device > > >>> mapping which would have Number of IDs field 0. > > >> > > >> Why not use single mapping flag for this case? > > > > > > Actually single mapping flag doesn't mean that there is single > > > mapping in > > an ID mapping. > > > It means that Input ID should not be considered when looking for > > > Output > > ID. > > > If we put single id flag, then we cannot have a case where we have > > > an array of single mappings for one device. > > > e.g. an array of single mappings for one PCIe Root Complex, where we > > > have a unique output ID for a unique BDF(Input ID) > > > > Agreed, single mapping flag is not working for multi-entris of single > mappings. > > > > Do you have a real use case for this fix? I'm thinking if we will > > break any delivered platforms with this patch applied, since this code > > is not changed from 2016, and it's the key logic for mapping the IDs. > > We have this use case in our platform NXP LX2160A, where we provide the > array of single mappings in IORT table. Actually we can only have limited > number of output IDs for PCIe devices, so we allocate unique output ID to > each BDF connected to a PCIe root complex and pass these IDs in IORT table. > > > > > I checked Hisilicon's ARM64 server platform, Number of IDs is equal to > > the number of IDs in the range in the firmware, which is not doing the > > same as the spec said, but (rid_in > map->input_base + map->id_count) > > is still valid with this patch applied, not sure for other platforms. > > I don't think that this patch would break any platform which has IORT table > defined as per spec. Let me rephase this to persuade you. This patch is *increasing* the allowed input IDs. Therefore, this patch would *include* more platforms and none of the existing Platforms can be affected by it, because already their Input IDs would fall in the allowed IDs. > > > > > Thanks > > Hanjun
