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: Mon, 7 Nov 2016 14:42:29 -0500 Message-ID: <20161107194229.GN3719@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> 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]:47434 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751360AbcKGTmc (ORCPT ); Mon, 7 Nov 2016 14:42:32 -0500 Content-Disposition: inline In-Reply-To: <20161107180212.pcidrbzzvqwfdda6@kamzik.brq.redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: 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? [...] > > Yes, actually this is something of my own "bad" habit. I used to add > > xxx_ prefix for better tag generations. When I read some big C repos > > like Linux, I see lots of duplicated field names for different > > structs/unions. For example, when I want to see codes related to > > StructA.FieldX, cscope will always tell me something more than this > > (rather than giving me StructA.FieldX, it gives me > > Struct[ABCDE..].FieldX). Then I need to pick out where StructA is. > > That's sometimes annoying, which depends on the length of the tag > > list. So I prefer to add this kind of prefix in the codes. > > > > Actually when I read some other codes (IIUC FreeBSD kernel), I see > > that this "bad" technique is used as well with very short prefix, > > e.g., for a struct called "net_filter" and field called "users", it > > may be defined as: > > > > net_filter.nf_users > > > > So the "nf_" prefix is the short form of "net_filter", and it can be > > used to distinguish the net_filter "users" fields with other "users" > > ones. > > > > Here "pci" is short enough IMHO, so I prefixed it. > > > > Not sure whether it's a good idea though. > > Seems like overkill for kvm-unit-tests. Ok. Let me remove all pci_ prefix for pci_dev struct fields. Thanks, -- peterx