| Message ID | 1455790054-1952-5-git-send-email-leonid.bloch@ravellosystems.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Feb 18, 2016 at 12:07:25PM +0200, Leonid Bloch wrote: > From: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com> > > Signed-off-by: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com> > Signed-off-by: Leonid Bloch <leonid.bloch@ravellosystems.com> > --- > hw/pci/pcie.c | 7 +++++++ > include/hw/pci/pcie.h | 1 + > include/hw/pci/pcie_regs.h | 4 ++++ > 3 files changed, 12 insertions(+) > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c > index be3a318..f7ac7d4 100644 > --- a/hw/pci/pcie.c > +++ b/hw/pci/pcie.c > @@ -690,3 +690,10 @@ void pcie_ari_init(PCIDevice *dev, uint16_t offset, uint16_t nextfn) > offset, PCI_ARI_SIZEOF); > pci_set_long(dev->config + offset + PCI_ARI_CAP, (nextfn & 0xff) << 8); > } > + > +void pcie_dsn_init(PCIDevice *dev, uint16_t offset, uint64_t val) > +{ > + pcie_add_capability(dev, PCI_EXT_CAP_ID_DSN, PCI_DSN_VER, > + offset, PCI_EXT_CAP_DSN_SIZEOF); > + pci_set_quad(dev->config + offset + PCI_DSN_CAP, val); > +} > diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h > index cbbf0c5..83a325c 100644 > --- a/include/hw/pci/pcie.h > +++ b/include/hw/pci/pcie.h > @@ -119,6 +119,7 @@ void pcie_add_capability(PCIDevice *dev, > uint16_t offset, uint16_t size); > > void pcie_ari_init(PCIDevice *dev, uint16_t offset, uint16_t nextfn); > +void pcie_dsn_init(PCIDevice *dev, uint16_t offset, uint64_t val); val is not a good parameter name. Please make it meaningful. E.g. serial_number. I would also write device_serial_number out fully since PCI spec does not say DSN anywhere. > > extern const VMStateDescription vmstate_pcie_device; > > diff --git a/include/hw/pci/pcie_regs.h b/include/hw/pci/pcie_regs.h > index a95522a..a891a45 100644 > --- a/include/hw/pci/pcie_regs.h > +++ b/include/hw/pci/pcie_regs.h > @@ -79,6 +79,10 @@ > #define PCI_ARI_VER 1 > #define PCI_ARI_SIZEOF 8 > > +/* DSN */ > +#define PCI_DSN_VER 1 > +#define PCI_DSN_CAP 0x04 > + > /* AER */ > #define PCI_ERR_VER 2 > #define PCI_ERR_SIZEOF 0x48 Again no need for this: you have a wrapper so they will never be used anywhere else. Just document it well where you use these numbers. > -- > 2.5.0
On Thu, Feb 18, 2016 at 12:41 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > On Thu, Feb 18, 2016 at 12:07:25PM +0200, Leonid Bloch wrote: >> From: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com> >> >> Signed-off-by: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com> >> Signed-off-by: Leonid Bloch <leonid.bloch@ravellosystems.com> >> --- >> hw/pci/pcie.c | 7 +++++++ >> include/hw/pci/pcie.h | 1 + >> include/hw/pci/pcie_regs.h | 4 ++++ >> 3 files changed, 12 insertions(+) >> >> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c >> index be3a318..f7ac7d4 100644 >> --- a/hw/pci/pcie.c >> +++ b/hw/pci/pcie.c >> @@ -690,3 +690,10 @@ void pcie_ari_init(PCIDevice *dev, uint16_t offset, uint16_t nextfn) >> offset, PCI_ARI_SIZEOF); >> pci_set_long(dev->config + offset + PCI_ARI_CAP, (nextfn & 0xff) << 8); >> } >> + >> +void pcie_dsn_init(PCIDevice *dev, uint16_t offset, uint64_t val) >> +{ >> + pcie_add_capability(dev, PCI_EXT_CAP_ID_DSN, PCI_DSN_VER, >> + offset, PCI_EXT_CAP_DSN_SIZEOF); >> + pci_set_quad(dev->config + offset + PCI_DSN_CAP, val); >> +} >> diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h >> index cbbf0c5..83a325c 100644 >> --- a/include/hw/pci/pcie.h >> +++ b/include/hw/pci/pcie.h >> @@ -119,6 +119,7 @@ void pcie_add_capability(PCIDevice *dev, >> uint16_t offset, uint16_t size); >> >> void pcie_ari_init(PCIDevice *dev, uint16_t offset, uint16_t nextfn); >> +void pcie_dsn_init(PCIDevice *dev, uint16_t offset, uint64_t val); > > > val is not a good parameter name. > Please make it meaningful. E.g. serial_number. > > I would also write device_serial_number out fully > since PCI spec does not say DSN anywhere. Changed to dev_ser_num and ser_num. > >> >> extern const VMStateDescription vmstate_pcie_device; >> >> diff --git a/include/hw/pci/pcie_regs.h b/include/hw/pci/pcie_regs.h >> index a95522a..a891a45 100644 >> --- a/include/hw/pci/pcie_regs.h >> +++ b/include/hw/pci/pcie_regs.h >> @@ -79,6 +79,10 @@ >> #define PCI_ARI_VER 1 >> #define PCI_ARI_SIZEOF 8 >> >> +/* DSN */ >> +#define PCI_DSN_VER 1 >> +#define PCI_DSN_CAP 0x04 >> + >> /* AER */ >> #define PCI_ERR_VER 2 >> #define PCI_ERR_SIZEOF 0x48 > > Again no need for this: you have a wrapper so > they will never be used anywhere else. Just document it well where > you use these numbers. Switched to static const ints in the function itself. There the "dsn" remained (pci_dsn_ver, pci_dsn_cap), because they are used only inside of a function that deals with the device serial number, so it should be clear enough. > >> -- >> 2.5.0
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c index be3a318..f7ac7d4 100644 --- a/hw/pci/pcie.c +++ b/hw/pci/pcie.c @@ -690,3 +690,10 @@ void pcie_ari_init(PCIDevice *dev, uint16_t offset, uint16_t nextfn) offset, PCI_ARI_SIZEOF); pci_set_long(dev->config + offset + PCI_ARI_CAP, (nextfn & 0xff) << 8); } + +void pcie_dsn_init(PCIDevice *dev, uint16_t offset, uint64_t val) +{ + pcie_add_capability(dev, PCI_EXT_CAP_ID_DSN, PCI_DSN_VER, + offset, PCI_EXT_CAP_DSN_SIZEOF); + pci_set_quad(dev->config + offset + PCI_DSN_CAP, val); +} diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h index cbbf0c5..83a325c 100644 --- a/include/hw/pci/pcie.h +++ b/include/hw/pci/pcie.h @@ -119,6 +119,7 @@ void pcie_add_capability(PCIDevice *dev, uint16_t offset, uint16_t size); void pcie_ari_init(PCIDevice *dev, uint16_t offset, uint16_t nextfn); +void pcie_dsn_init(PCIDevice *dev, uint16_t offset, uint64_t val); extern const VMStateDescription vmstate_pcie_device; diff --git a/include/hw/pci/pcie_regs.h b/include/hw/pci/pcie_regs.h index a95522a..a891a45 100644 --- a/include/hw/pci/pcie_regs.h +++ b/include/hw/pci/pcie_regs.h @@ -79,6 +79,10 @@ #define PCI_ARI_VER 1 #define PCI_ARI_SIZEOF 8 +/* DSN */ +#define PCI_DSN_VER 1 +#define PCI_DSN_CAP 0x04 + /* AER */ #define PCI_ERR_VER 2 #define PCI_ERR_SIZEOF 0x48