| Message ID | 20190606090146.77381-3-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | pci: expand usage of pci_sbdf_t | expand |
> -----Original Message----- > From: Xen-devel [mailto:xen-devel-bounces@lists.xenproject.org] On Behalf Of Roger Pau Monne > Sent: 06 June 2019 10:02 > To: xen-devel@lists.xenproject.org > Cc: Stefano Stabellini <sstabellini@kernel.org>; Wei Liu <wl@xen.org>; Konrad Rzeszutek Wilk > <konrad.wilk@oracle.com>; George Dunlap <George.Dunlap@citrix.com>; Andrew Cooper > <Andrew.Cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Tim (Xen.org) <tim@xen.org>; Julien > Grall <julien.grall@arm.com>; Jan Beulich <jbeulich@suse.com>; Roger Pau Monne <roger.pau@citrix.com> > Subject: [Xen-devel] [PATCH v2 02/12] pci: introduce a pci_sbdf_t field to pci_dev > > And use an union with the current seg, bus and devfn fields to make > fields point to the same underlying data. > > No functional change. > > Suggested-by: Jan Beulich <jbeulich@suse.com> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Paul Durrant <paul.durrant@citrix.com> > --- > Cc: Andrew Cooper <andrew.cooper3@citrix.com> > Cc: George Dunlap <George.Dunlap@eu.citrix.com> > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Jan Beulich <jbeulich@suse.com> > Cc: Julien Grall <julien.grall@arm.com> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > Cc: Stefano Stabellini <sstabellini@kernel.org> > Cc: Tim Deegan <tim@xen.org> > Cc: Wei Liu <wl@xen.org> > --- > xen/drivers/vpci/header.c | 7 ++----- > xen/include/xen/pci.h | 12 +++++++++--- > 2 files changed, 11 insertions(+), 8 deletions(-) > > diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c > index efb6ca90e3..046b1c610e 100644 > --- a/xen/drivers/vpci/header.c > +++ b/xen/drivers/vpci/header.c > @@ -461,9 +461,6 @@ static int init_bars(struct pci_dev *pdev) > unsigned int i, num_bars, rom_reg; > struct vpci_header *header = &pdev->vpci->header; > struct vpci_bar *bars = header->bars; > - pci_sbdf_t sbdf = { > - .sbdf = PCI_SBDF3(pdev->seg, pdev->bus, pdev->devfn), > - }; > int rc; > > switch ( pci_conf_read8(pdev->seg, pdev->bus, slot, func, PCI_HEADER_TYPE) > @@ -530,7 +527,7 @@ static int init_bars(struct pci_dev *pdev) > else > bars[i].type = VPCI_BAR_MEM32; > > - rc = pci_size_mem_bar(sbdf, reg, &addr, &size, > + rc = pci_size_mem_bar(pdev->sbdf, reg, &addr, &size, > (i == num_bars - 1) ? PCI_BAR_LAST : 0); > if ( rc < 0 ) > { > @@ -560,7 +557,7 @@ static int init_bars(struct pci_dev *pdev) > } > > /* Check expansion ROM. */ > - rc = pci_size_mem_bar(sbdf, rom_reg, &addr, &size, PCI_BAR_ROM); > + rc = pci_size_mem_bar(pdev->sbdf, rom_reg, &addr, &size, PCI_BAR_ROM); > if ( rc > 0 && size ) > { > struct vpci_bar *rom = &header->bars[num_bars]; > diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h > index ec98274675..f4908abd8b 100644 > --- a/xen/include/xen/pci.h > +++ b/xen/include/xen/pci.h > @@ -83,9 +83,15 @@ struct pci_dev { > struct arch_msix *msix; > > struct domain *domain; > - const u16 seg; > - const u8 bus; > - const u8 devfn; > + > + union { > + struct { > + const uint8_t devfn; > + const uint8_t bus; > + const uint16_t seg; > + }; > + const pci_sbdf_t sbdf; > + }; > > u8 phantom_stride; > > -- > 2.20.1 (Apple Git-117) > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xenproject.org > https://lists.xenproject.org/mailman/listinfo/xen-devel
>>> On 06.06.19 at 11:01, <roger.pau@citrix.com> wrote: > And use an union with the current seg, bus and devfn fields to make > fields point to the same underlying data. > > No functional change. > > Suggested-by: Jan Beulich <jbeulich@suse.com> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com> with one question: > --- a/xen/include/xen/pci.h > +++ b/xen/include/xen/pci.h > @@ -83,9 +83,15 @@ struct pci_dev { > struct arch_msix *msix; > > struct domain *domain; > - const u16 seg; > - const u8 bus; > - const u8 devfn; > + > + union { > + struct { > + const uint8_t devfn; > + const uint8_t bus; > + const uint16_t seg; > + }; > + const pci_sbdf_t sbdf; > + }; Doesn't sticking a single "const" at the union have the same effect? Jan
On Thu, Jun 06, 2019 at 06:37:04AM -0600, Jan Beulich wrote: > >>> On 06.06.19 at 11:01, <roger.pau@citrix.com> wrote: > > And use an union with the current seg, bus and devfn fields to make > > fields point to the same underlying data. > > > > No functional change. > > > > Suggested-by: Jan Beulich <jbeulich@suse.com> > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > > Acked-by: Jan Beulich <jbeulich@suse.com> > with one question: > > > --- a/xen/include/xen/pci.h > > +++ b/xen/include/xen/pci.h > > @@ -83,9 +83,15 @@ struct pci_dev { > > struct arch_msix *msix; > > > > struct domain *domain; > > - const u16 seg; > > - const u8 bus; > > - const u8 devfn; > > + > > + union { > > + struct { > > + const uint8_t devfn; > > + const uint8_t bus; > > + const uint16_t seg; > > + }; > > + const pci_sbdf_t sbdf; > > + }; > > Doesn't sticking a single "const" at the union have the same effect? Indeed, and I've already done so, I assume it's fine to keep your Ack. Thanks, Roger.
>>> On 06.06.19 at 16:54, <roger.pau@citrix.com> wrote: > On Thu, Jun 06, 2019 at 06:37:04AM -0600, Jan Beulich wrote: >> >>> On 06.06.19 at 11:01, <roger.pau@citrix.com> wrote: >> > --- a/xen/include/xen/pci.h >> > +++ b/xen/include/xen/pci.h >> > @@ -83,9 +83,15 @@ struct pci_dev { >> > struct arch_msix *msix; >> > >> > struct domain *domain; >> > - const u16 seg; >> > - const u8 bus; >> > - const u8 devfn; >> > + >> > + union { >> > + struct { >> > + const uint8_t devfn; >> > + const uint8_t bus; >> > + const uint16_t seg; >> > + }; >> > + const pci_sbdf_t sbdf; >> > + }; >> >> Doesn't sticking a single "const" at the union have the same effect? > > Indeed, and I've already done so, I assume it's fine to keep your Ack. Definitely. Jan
diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c index efb6ca90e3..046b1c610e 100644 --- a/xen/drivers/vpci/header.c +++ b/xen/drivers/vpci/header.c @@ -461,9 +461,6 @@ static int init_bars(struct pci_dev *pdev) unsigned int i, num_bars, rom_reg; struct vpci_header *header = &pdev->vpci->header; struct vpci_bar *bars = header->bars; - pci_sbdf_t sbdf = { - .sbdf = PCI_SBDF3(pdev->seg, pdev->bus, pdev->devfn), - }; int rc; switch ( pci_conf_read8(pdev->seg, pdev->bus, slot, func, PCI_HEADER_TYPE) @@ -530,7 +527,7 @@ static int init_bars(struct pci_dev *pdev) else bars[i].type = VPCI_BAR_MEM32; - rc = pci_size_mem_bar(sbdf, reg, &addr, &size, + rc = pci_size_mem_bar(pdev->sbdf, reg, &addr, &size, (i == num_bars - 1) ? PCI_BAR_LAST : 0); if ( rc < 0 ) { @@ -560,7 +557,7 @@ static int init_bars(struct pci_dev *pdev) } /* Check expansion ROM. */ - rc = pci_size_mem_bar(sbdf, rom_reg, &addr, &size, PCI_BAR_ROM); + rc = pci_size_mem_bar(pdev->sbdf, rom_reg, &addr, &size, PCI_BAR_ROM); if ( rc > 0 && size ) { struct vpci_bar *rom = &header->bars[num_bars]; diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h index ec98274675..f4908abd8b 100644 --- a/xen/include/xen/pci.h +++ b/xen/include/xen/pci.h @@ -83,9 +83,15 @@ struct pci_dev { struct arch_msix *msix; struct domain *domain; - const u16 seg; - const u8 bus; - const u8 devfn; + + union { + struct { + const uint8_t devfn; + const uint8_t bus; + const uint16_t seg; + }; + const pci_sbdf_t sbdf; + }; u8 phantom_stride;
And use an union with the current seg, bus and devfn fields to make fields point to the same underlying data. No functional change. Suggested-by: Jan Beulich <jbeulich@suse.com> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Cc: Andrew Cooper <andrew.cooper3@citrix.com> Cc: George Dunlap <George.Dunlap@eu.citrix.com> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Jan Beulich <jbeulich@suse.com> Cc: Julien Grall <julien.grall@arm.com> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Cc: Stefano Stabellini <sstabellini@kernel.org> Cc: Tim Deegan <tim@xen.org> Cc: Wei Liu <wl@xen.org> --- xen/drivers/vpci/header.c | 7 ++----- xen/include/xen/pci.h | 12 +++++++++--- 2 files changed, 11 insertions(+), 8 deletions(-)