From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Xu Subject: Re: [PATCH kvm-unit-tests 06/17] pci: introduce struct pci_dev Date: Tue, 8 Nov 2016 10:33:12 -0500 Message-ID: <20161108153312.GE2793@pxdev.xzpeter.org> References: <1477468040-21034-1-git-send-email-peterx@redhat.com> <1477468040-21034-7-git-send-email-peterx@redhat.com> <20161104164101.ymyxzkjbbpsr4mbr@kamzik.brq.redhat.com> <20161107170510.GF3719@pxdev.xzpeter.org> <20161107180212.pcidrbzzvqwfdda6@kamzik.brq.redhat.com> <20161107194229.GN3719@pxdev.xzpeter.org> <20161108101611.2wl62qx32numogky@kamzik.brq.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Cc: kvm@vger.kernel.org, rkrcmar@redhat.com, agordeev@redhat.com, jan.kiszka@web.de, pbonzini@redhat.com To: Andrew Jones Return-path: Received: from mx1.redhat.com ([209.132.183.28]:33766 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751453AbcKHPdO (ORCPT ); Tue, 8 Nov 2016 10:33:14 -0500 Content-Disposition: inline In-Reply-To: <20161108101611.2wl62qx32numogky@kamzik.brq.redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: 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