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%2Finfoc >>> >> 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. 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. Thanks Hanjun