| Message ID | 1476325947-28268-1-git-send-email-caoj.fnst@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
ping On 10/13/2016 10:32 AM, Cao jin wrote: > log_max have no chance to be PCIE_AER_LOG_MAX_UNSET, unless user specify it. > > Bonus: > 1. remove unnecessary local variable. > 2. fix a typo. > > Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com> > --- > hw/pci/pcie_aer.c | 10 +--------- > include/hw/pci/pcie_aer.h | 2 +- > 2 files changed, 2 insertions(+), 10 deletions(-) > > diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c > index ac47f34..6cf088b 100644 > --- a/hw/pci/pcie_aer.c > +++ b/hw/pci/pcie_aer.c > @@ -99,18 +99,10 @@ static void aer_log_clear_all_err(PCIEAERLog *aer_log) > int pcie_aer_init(PCIDevice *dev, uint8_t cap_ver, uint16_t offset, > uint16_t size) > { > - PCIExpressDevice *exp; > - > pcie_add_capability(dev, PCI_EXT_CAP_ID_ERR, cap_ver, > offset, size); > - exp = &dev->exp; > - exp->aer_cap = offset; > + dev->exp.aer_cap = offset; > > - /* log_max is property */ > - if (dev->exp.aer_log.log_max == PCIE_AER_LOG_MAX_UNSET) { > - dev->exp.aer_log.log_max = PCIE_AER_LOG_MAX_DEFAULT; > - } > - /* clip down the value to avoid unreasobale memory usage */ > if (dev->exp.aer_log.log_max > PCIE_AER_LOG_MAX_LIMIT) { > return -EINVAL; > } > diff --git a/include/hw/pci/pcie_aer.h b/include/hw/pci/pcie_aer.h > index c373591..31755ef 100644 > --- a/include/hw/pci/pcie_aer.h > +++ b/include/hw/pci/pcie_aer.h > @@ -40,7 +40,7 @@ struct PCIEAERLog { > * The specified value will be clipped down to PCIE_AER_LOG_MAX_LIMIT > * to avoid unreasonable memory usage. > * I bet that 128 log size would be big enough, otherwise too many errors > - * for system to function normaly. But could consecutive errors occur? > + * for system to function normally. But could consecutive errors occur? > */ > #define PCIE_AER_LOG_MAX_DEFAULT 8 > #define PCIE_AER_LOG_MAX_LIMIT 128 >
On 10/13/2016 05:32 AM, Cao jin wrote: > log_max have no chance to be PCIE_AER_LOG_MAX_UNSET, unless user specify it. > Hi, > Bonus: > 1. remove unnecessary local variable. > 2. fix a typo. > > Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com> > --- > hw/pci/pcie_aer.c | 10 +--------- > include/hw/pci/pcie_aer.h | 2 +- > 2 files changed, 2 insertions(+), 10 deletions(-) > > diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c > index ac47f34..6cf088b 100644 > --- a/hw/pci/pcie_aer.c > +++ b/hw/pci/pcie_aer.c > @@ -99,18 +99,10 @@ static void aer_log_clear_all_err(PCIEAERLog *aer_log) > int pcie_aer_init(PCIDevice *dev, uint8_t cap_ver, uint16_t offset, > uint16_t size) > { > - PCIExpressDevice *exp; > - > pcie_add_capability(dev, PCI_EXT_CAP_ID_ERR, cap_ver, > offset, size); > - exp = &dev->exp; > - exp->aer_cap = offset; > + dev->exp.aer_cap = offset; > > - /* log_max is property */ > - if (dev->exp.aer_log.log_max == PCIE_AER_LOG_MAX_UNSET) { > - dev->exp.aer_log.log_max = PCIE_AER_LOG_MAX_DEFAULT; > - } OK, maybe this check is not needed, the default value is PCIE_AER_LOG_MAX_DEFAULT -> 8. The user can pass -device ioh3420,aer_log_max=0xffff and until now would be automatically modified to PCIE_AER_LOG_MAX_DEFAULT -> 15, from now on he will fail. However the behavior change is OK. > - /* clip down the value to avoid unreasobale memory usage */ > if (dev->exp.aer_log.log_max > PCIE_AER_LOG_MAX_LIMIT) { > return -EINVAL; > } If you already started working on it, maybe you can fix the error message when the user passes aer_log_max over PCIE_AER_LOG_MAX_LIMIT. For the moment we get: qemu-system-x86_64: -device ioh3420,aer_log_max=0xfff: Device initialization failed And I would suggest a more clear patch subject. Thanks, Marcel > diff --git a/include/hw/pci/pcie_aer.h b/include/hw/pci/pcie_aer.h > index c373591..31755ef 100644 > --- a/include/hw/pci/pcie_aer.h > +++ b/include/hw/pci/pcie_aer.h > @@ -40,7 +40,7 @@ struct PCIEAERLog { > * The specified value will be clipped down to PCIE_AER_LOG_MAX_LIMIT > * to avoid unreasonable memory usage. > * I bet that 128 log size would be big enough, otherwise too many errors > - * for system to function normaly. But could consecutive errors occur? > + * for system to function normally. But could consecutive errors occur? > */ > #define PCIE_AER_LOG_MAX_DEFAULT 8 > #define PCIE_AER_LOG_MAX_LIM
diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c index ac47f34..6cf088b 100644 --- a/hw/pci/pcie_aer.c +++ b/hw/pci/pcie_aer.c @@ -99,18 +99,10 @@ static void aer_log_clear_all_err(PCIEAERLog *aer_log) int pcie_aer_init(PCIDevice *dev, uint8_t cap_ver, uint16_t offset, uint16_t size) { - PCIExpressDevice *exp; - pcie_add_capability(dev, PCI_EXT_CAP_ID_ERR, cap_ver, offset, size); - exp = &dev->exp; - exp->aer_cap = offset; + dev->exp.aer_cap = offset; - /* log_max is property */ - if (dev->exp.aer_log.log_max == PCIE_AER_LOG_MAX_UNSET) { - dev->exp.aer_log.log_max = PCIE_AER_LOG_MAX_DEFAULT; - } - /* clip down the value to avoid unreasobale memory usage */ if (dev->exp.aer_log.log_max > PCIE_AER_LOG_MAX_LIMIT) { return -EINVAL; } diff --git a/include/hw/pci/pcie_aer.h b/include/hw/pci/pcie_aer.h index c373591..31755ef 100644 --- a/include/hw/pci/pcie_aer.h +++ b/include/hw/pci/pcie_aer.h @@ -40,7 +40,7 @@ struct PCIEAERLog { * The specified value will be clipped down to PCIE_AER_LOG_MAX_LIMIT * to avoid unreasonable memory usage. * I bet that 128 log size would be big enough, otherwise too many errors - * for system to function normaly. But could consecutive errors occur? + * for system to function normally. But could consecutive errors occur? */ #define PCIE_AER_LOG_MAX_DEFAULT 8 #define PCIE_AER_LOG_MAX_LIMIT 128
log_max have no chance to be PCIE_AER_LOG_MAX_UNSET, unless user specify it. Bonus: 1. remove unnecessary local variable. 2. fix a typo. Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com> --- hw/pci/pcie_aer.c | 10 +--------- include/hw/pci/pcie_aer.h | 2 +- 2 files changed, 2 insertions(+), 10 deletions(-)