Re: [Qemu-devel] [PATCH 3/5] pci: designware: Update MSI mapping unconditionally

[Peter Maydell]

On Tue, 16 Apr 2019 at 02:39, Andrey Smirnov  wrote:
>
> Expression to calculate update_msi_mapping in code handling writes to
> DESIGNWARE_PCIE_MSI_INTR0_ENABLE is missing an ! operator and should
> be:
>
> !!root->msi.intr[0].enable ^ !!val;
>
> so that MSI mapping is updated when enabled transitions from either
> "none" -> "any" or "any" -> "none". Since that register shouldn't be
> written to very often, change the code to update MSI mapping
> unconditionally instead of trying to fix the update_msi_mapping logic.
>
> Signed-off-by: Andrey Smirnov 
> Cc: Peter Maydell 
> Cc: Michael S. Tsirkin 
> Cc: qemu-devel@nongnu.org
> Cc: qemu-...@nongnu.org
> ---

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH 3/5] pci: designware: Update MSI mapping unconditionally

[Michael S. Tsirkin]

On Mon, Apr 15, 2019 at 06:39:00PM -0700, Andrey Smirnov wrote:
> Expression to calculate update_msi_mapping in code handling writes to
> DESIGNWARE_PCIE_MSI_INTR0_ENABLE is missing an ! operator and should
> be:
> 
> !!root->msi.intr[0].enable ^ !!val;
> 
> so that MSI mapping is updated when enabled transitions from either
> "none" -> "any" or "any" -> "none". Since that register shouldn't be
> written to very often, change the code to update MSI mapping
> unconditionally instead of trying to fix the update_msi_mapping logic.
> 
> Signed-off-by: Andrey Smirnov 
> Cc: Peter Maydell 
> Cc: Michael S. Tsirkin 
> Cc: qemu-devel@nongnu.org
> Cc: qemu-...@nongnu.org

Acked-by: Michael S. Tsirkin 

> ---
>  hw/pci-host/designware.c | 10 ++
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
> index 29ea313798..6affe823c0 100644
> --- a/hw/pci-host/designware.c
> +++ b/hw/pci-host/designware.c
> @@ -296,16 +296,10 @@ static void designware_pcie_root_config_write(PCIDevice 
> *d, uint32_t address,
>  root->msi.base |= (uint64_t)val << 32;
>  break;
>  
> -case DESIGNWARE_PCIE_MSI_INTR0_ENABLE: {
> -const bool update_msi_mapping = !root->msi.intr[0].enable ^ !!val;
> -
> +case DESIGNWARE_PCIE_MSI_INTR0_ENABLE:
>  root->msi.intr[0].enable = val;
> -
> -if (update_msi_mapping) {
> -designware_pcie_root_update_msi_mapping(root);
> -}
> +designware_pcie_root_update_msi_mapping(root);
>  break;
> -}
>  
>  case DESIGNWARE_PCIE_MSI_INTR0_MASK:
>  root->msi.intr[0].mask = val;
> -- 
> 2.20.1

[Qemu-devel] [PATCH 3/5] pci: designware: Update MSI mapping unconditionally

[Andrey Smirnov]

Expression to calculate update_msi_mapping in code handling writes to
DESIGNWARE_PCIE_MSI_INTR0_ENABLE is missing an ! operator and should
be:

!!root->msi.intr[0].enable ^ !!val;

so that MSI mapping is updated when enabled transitions from either
"none" -> "any" or "any" -> "none". Since that register shouldn't be
written to very often, change the code to update MSI mapping
unconditionally instead of trying to fix the update_msi_mapping logic.

Signed-off-by: Andrey Smirnov 
Cc: Peter Maydell 
Cc: Michael S. Tsirkin 
Cc: qemu-devel@nongnu.org
Cc: qemu-...@nongnu.org
---
 hw/pci-host/designware.c | 10 ++
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
index 29ea313798..6affe823c0 100644
--- a/hw/pci-host/designware.c
+++ b/hw/pci-host/designware.c
@@ -296,16 +296,10 @@ static void designware_pcie_root_config_write(PCIDevice 
*d, uint32_t address,
 root->msi.base |= (uint64_t)val << 32;
 break;
 
-case DESIGNWARE_PCIE_MSI_INTR0_ENABLE: {
-const bool update_msi_mapping = !root->msi.intr[0].enable ^ !!val;
-
+case DESIGNWARE_PCIE_MSI_INTR0_ENABLE:
 root->msi.intr[0].enable = val;
-
-if (update_msi_mapping) {
-designware_pcie_root_update_msi_mapping(root);
-}
+designware_pcie_root_update_msi_mapping(root);
 break;
-}
 
 case DESIGNWARE_PCIE_MSI_INTR0_MASK:
 root->msi.intr[0].mask = val;
-- 
2.20.1