On Tue, Nov 08, 2016 at 11:16:11AM +0100, Andrew Jones wrote: > On Mon, Nov 07, 2016 at 02:42:29PM -0500, Peter Xu wrote: > > On Mon, Nov 07, 2016 at 07:02:12PM +0100, Andrew Jones wrote: > > > > [...] > > > > > > I thought pcidevaddr_t means bdf? That's a wild guess of me from its > > > > size, as it's defined as: > > > > > > > > typedef uint16_t pcidevaddr_t; > > > > > > > > uint16_t makes it looks more like bdf rather than devfn. For devfn, > > > > uint8_t suites better. > > > > > > > > And what I mean here is to pass in the bdf of the device, not devfn > > > > only (in our case, devfn is always bdf though, since we are only > > > > supporting bus number zero). > > > > > > see lib/x86/asm/pci.h:pci_config_read how 'dev' gets used, it's only dev. > > > > But shouldn't "dev" the same as "bdf" here? > > Hmm, yes. We're only shifting it by 8, not 11, so you're right. The word > 'dev' in pcidevaddr_t threw me off and I didn't bother looking up CF8's > bits until just now. OK, let's try to get consistent naming going on, > and we need better comments (comments at all). I'd like to see one > briefly describing 'bdf' and then use that for the primary name, i.e. > rename pcidevaddr_t to something like 'pcibdfaddr_t' or whatever. Yes, indeed the naming is a little bit confusing here. I was always trying to use bdf/devfn in the whole series to avoid misunderstanding. Regarding to the naming of pcidevaddr_t: that's okay for me, but I agree that bdf is better and clearer. If you would not mind, I'll keep it as it is for now for this series. Thanks, -- peterx -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
