RE: [PATCH] ACPI/IORT: fix the iort_id_map function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----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%2Finf
> >>> oc
> >>>
> >>
> enter.arm.com%2Fhelp%2Ftopic%2Fcom.arm.doc.den0049d%2FDEN0049D_IO
> >> _Rema
> >>>
> >>
> pping_Table.pdf&amp;data=02%7C01%7Cpankaj.bansal%40nxp.com%7C78d
> >> 82a560
> >>>
> >>
> 5714219196008d781db06a7%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%
> >> 7C1%7C6
> >>>
> >>
> 37120650018983814&amp;sdata=%2FRhATUKx%2FA2gPEx%2BNY9X%2F7kqV
> >> CrEeRnbE%
> >>> 2B2qlTkdGDc%3D&amp;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.

> 
> Thanks
> Hanjun





[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux