qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH-for-10.0 0/2] hw/pci-host/designware: Fix ATU_UPPER_TARGET register access
@ 2025-03-31 15:20 Philippe Mathieu-Daudé
  2025-03-31 15:20 ` [PATCH-for-10.0 1/2] hw/pci-host/designware: Fix access to ATU_UPPER_TARGET register Philippe Mathieu-Daudé
  2025-03-31 15:20 ` [PATCH-for-10.1 2/2] hw/pci-host/designware: Use deposit/extract API Philippe Mathieu-Daudé
  0 siblings, 2 replies; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-31 15:20 UTC (permalink / raw)
  To: qemu-devel
  Cc: Gustavo Romero, Joey, qemu-arm, Peter Maydell, Andrey Smirnov,
	Philippe Mathieu-Daudé

Trivial fix for issue #2861.

Philippe Mathieu-Daudé (2):
  hw/pci-host/designware: Fix access to ATU_UPPER_TARGET register
  hw/pci-host/designware: Use deposit/extract API

 hw/pci-host/designware.c | 47 ++++++++++++++--------------------------
 1 file changed, 16 insertions(+), 31 deletions(-)

-- 
2.47.1



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH-for-10.0 1/2] hw/pci-host/designware: Fix access to ATU_UPPER_TARGET register
  2025-03-31 15:20 [PATCH-for-10.0 0/2] hw/pci-host/designware: Fix ATU_UPPER_TARGET register access Philippe Mathieu-Daudé
@ 2025-03-31 15:20 ` Philippe Mathieu-Daudé
  2025-03-31 16:15   ` Gustavo Romero
  2025-03-31 15:20 ` [PATCH-for-10.1 2/2] hw/pci-host/designware: Use deposit/extract API Philippe Mathieu-Daudé
  1 sibling, 1 reply; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-31 15:20 UTC (permalink / raw)
  To: qemu-devel
  Cc: Gustavo Romero, Joey, qemu-arm, Peter Maydell, Andrey Smirnov,
	Philippe Mathieu-Daudé, qemu-stable

Fix copy/paste error writing to the ATU_UPPER_TARGET
register, we want to update the upper 32 bits.

Cc: qemu-stable@nongnu.org
Reported-by: Joey <jeundery@gmail.com>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2861
Fixes: d64e5eabc4c ("pci: Add support for Designware IP block")
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/pci-host/designware.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
index c07740bfaa4..5598d18f478 100644
--- a/hw/pci-host/designware.c
+++ b/hw/pci-host/designware.c
@@ -371,7 +371,7 @@ static void designware_pcie_root_config_write(PCIDevice *d, uint32_t address,
 
     case DESIGNWARE_PCIE_ATU_UPPER_TARGET:
         viewport->target &= 0x00000000FFFFFFFFULL;
-        viewport->target |= val;
+        viewport->target |= (uint64_t)val << 32;
         break;
 
     case DESIGNWARE_PCIE_ATU_LIMIT:
-- 
2.47.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH-for-10.1 2/2] hw/pci-host/designware: Use deposit/extract API
  2025-03-31 15:20 [PATCH-for-10.0 0/2] hw/pci-host/designware: Fix ATU_UPPER_TARGET register access Philippe Mathieu-Daudé
  2025-03-31 15:20 ` [PATCH-for-10.0 1/2] hw/pci-host/designware: Fix access to ATU_UPPER_TARGET register Philippe Mathieu-Daudé
@ 2025-03-31 15:20 ` Philippe Mathieu-Daudé
  2025-03-31 16:16   ` Gustavo Romero
  1 sibling, 1 reply; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-31 15:20 UTC (permalink / raw)
  To: qemu-devel
  Cc: Gustavo Romero, Joey, qemu-arm, Peter Maydell, Andrey Smirnov,
	Philippe Mathieu-Daudé

Prefer the safer (less bug-prone) deposit/extract API
to access lower/upper 32-bit of 64-bit registers.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/pci-host/designware.c | 47 ++++++++++++++--------------------------
 1 file changed, 16 insertions(+), 31 deletions(-)

diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
index 5598d18f478..3f2282b2596 100644
--- a/hw/pci-host/designware.c
+++ b/hw/pci-host/designware.c
@@ -22,6 +22,7 @@
 #include "qapi/error.h"
 #include "qemu/module.h"
 #include "qemu/log.h"
+#include "qemu/bitops.h"
 #include "hw/pci/msi.h"
 #include "hw/pci/pci_bridge.h"
 #include "hw/pci/pci_host.h"
@@ -162,11 +163,9 @@ designware_pcie_root_config_read(PCIDevice *d, uint32_t address, int len)
         break;
 
     case DESIGNWARE_PCIE_MSI_ADDR_LO:
-        val = root->msi.base;
-        break;
-
     case DESIGNWARE_PCIE_MSI_ADDR_HI:
-        val = root->msi.base >> 32;
+        val = extract64(root->msi.base,
+                        address == DESIGNWARE_PCIE_MSI_ADDR_LO ? 0 : 32, 32);
         break;
 
     case DESIGNWARE_PCIE_MSI_INTR0_ENABLE:
@@ -190,19 +189,15 @@ designware_pcie_root_config_read(PCIDevice *d, uint32_t address, int len)
         break;
 
     case DESIGNWARE_PCIE_ATU_LOWER_BASE:
-        val = viewport->base;
-        break;
-
     case DESIGNWARE_PCIE_ATU_UPPER_BASE:
-        val = viewport->base >> 32;
+        val = extract64(viewport->base,
+                        address == DESIGNWARE_PCIE_ATU_LOWER_BASE ? 0 : 32, 32);
         break;
 
     case DESIGNWARE_PCIE_ATU_LOWER_TARGET:
-        val = viewport->target;
-        break;
-
     case DESIGNWARE_PCIE_ATU_UPPER_TARGET:
-        val = viewport->target >> 32;
+        val = extract64(viewport->target,
+                        address == DESIGNWARE_PCIE_ATU_LOWER_TARGET ? 0 : 32, 32);
         break;
 
     case DESIGNWARE_PCIE_ATU_LIMIT:
@@ -321,14 +316,10 @@ static void designware_pcie_root_config_write(PCIDevice *d, uint32_t address,
         break;
 
     case DESIGNWARE_PCIE_MSI_ADDR_LO:
-        root->msi.base &= 0xFFFFFFFF00000000ULL;
-        root->msi.base |= val;
-        designware_pcie_root_update_msi_mapping(root);
-        break;
-
     case DESIGNWARE_PCIE_MSI_ADDR_HI:
-        root->msi.base &= 0x00000000FFFFFFFFULL;
-        root->msi.base |= (uint64_t)val << 32;
+        root->msi.base = deposit64(root->msi.base,
+                                   address == DESIGNWARE_PCIE_MSI_ADDR_LO
+                                   ? 0 : 32, 32, val);
         designware_pcie_root_update_msi_mapping(root);
         break;
 
@@ -355,23 +346,17 @@ static void designware_pcie_root_config_write(PCIDevice *d, uint32_t address,
         break;
 
     case DESIGNWARE_PCIE_ATU_LOWER_BASE:
-        viewport->base &= 0xFFFFFFFF00000000ULL;
-        viewport->base |= val;
-        break;
-
     case DESIGNWARE_PCIE_ATU_UPPER_BASE:
-        viewport->base &= 0x00000000FFFFFFFFULL;
-        viewport->base |= (uint64_t)val << 32;
+        viewport->base = deposit64(root->msi.base,
+                                   address == DESIGNWARE_PCIE_ATU_LOWER_BASE
+                                   ? 0 : 32, 32, val);
         break;
 
     case DESIGNWARE_PCIE_ATU_LOWER_TARGET:
-        viewport->target &= 0xFFFFFFFF00000000ULL;
-        viewport->target |= val;
-        break;
-
     case DESIGNWARE_PCIE_ATU_UPPER_TARGET:
-        viewport->target &= 0x00000000FFFFFFFFULL;
-        viewport->target |= (uint64_t)val << 32;
+        viewport->target = deposit64(root->msi.base,
+                                     address == DESIGNWARE_PCIE_ATU_LOWER_TARGET
+                                     ? 0 : 32, 32, val);
         break;
 
     case DESIGNWARE_PCIE_ATU_LIMIT:
-- 
2.47.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH-for-10.0 1/2] hw/pci-host/designware: Fix access to ATU_UPPER_TARGET register
  2025-03-31 15:20 ` [PATCH-for-10.0 1/2] hw/pci-host/designware: Fix access to ATU_UPPER_TARGET register Philippe Mathieu-Daudé
@ 2025-03-31 16:15   ` Gustavo Romero
  0 siblings, 0 replies; 5+ messages in thread
From: Gustavo Romero @ 2025-03-31 16:15 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Joey, qemu-arm, Peter Maydell, Andrey Smirnov, qemu-stable

Hi Phil,

On 3/31/25 12:20, Philippe Mathieu-Daudé wrote:
> Fix copy/paste error writing to the ATU_UPPER_TARGET
> register, we want to update the upper 32 bits.
> 
> Cc: qemu-stable@nongnu.org
> Reported-by: Joey <jeundery@gmail.com>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2861
> Fixes: d64e5eabc4c ("pci: Add support for Designware IP block")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/pci-host/designware.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
> index c07740bfaa4..5598d18f478 100644
> --- a/hw/pci-host/designware.c
> +++ b/hw/pci-host/designware.c
> @@ -371,7 +371,7 @@ static void designware_pcie_root_config_write(PCIDevice *d, uint32_t address,
>   
>       case DESIGNWARE_PCIE_ATU_UPPER_TARGET:
>           viewport->target &= 0x00000000FFFFFFFFULL;
> -        viewport->target |= val;
> +        viewport->target |= (uint64_t)val << 32;
>           break;
>   
>       case DESIGNWARE_PCIE_ATU_LIMIT:

Reviewed-by: Gustavo Romero <gustavo.romero@linaro.org>


Cheers,
Gustavo


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH-for-10.1 2/2] hw/pci-host/designware: Use deposit/extract API
  2025-03-31 15:20 ` [PATCH-for-10.1 2/2] hw/pci-host/designware: Use deposit/extract API Philippe Mathieu-Daudé
@ 2025-03-31 16:16   ` Gustavo Romero
  0 siblings, 0 replies; 5+ messages in thread
From: Gustavo Romero @ 2025-03-31 16:16 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Joey, qemu-arm, Peter Maydell, Andrey Smirnov

Hi Phil,

On 3/31/25 12:20, Philippe Mathieu-Daudé wrote:
> Prefer the safer (less bug-prone) deposit/extract API
> to access lower/upper 32-bit of 64-bit registers.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/pci-host/designware.c | 47 ++++++++++++++--------------------------
>   1 file changed, 16 insertions(+), 31 deletions(-)
> 
> diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
> index 5598d18f478..3f2282b2596 100644
> --- a/hw/pci-host/designware.c
> +++ b/hw/pci-host/designware.c
> @@ -22,6 +22,7 @@
>   #include "qapi/error.h"
>   #include "qemu/module.h"
>   #include "qemu/log.h"
> +#include "qemu/bitops.h"
>   #include "hw/pci/msi.h"
>   #include "hw/pci/pci_bridge.h"
>   #include "hw/pci/pci_host.h"
> @@ -162,11 +163,9 @@ designware_pcie_root_config_read(PCIDevice *d, uint32_t address, int len)
>           break;
>   
>       case DESIGNWARE_PCIE_MSI_ADDR_LO:
> -        val = root->msi.base;
> -        break;
> -
>       case DESIGNWARE_PCIE_MSI_ADDR_HI:
> -        val = root->msi.base >> 32;
> +        val = extract64(root->msi.base,
> +                        address == DESIGNWARE_PCIE_MSI_ADDR_LO ? 0 : 32, 32);
>           break;
>   
>       case DESIGNWARE_PCIE_MSI_INTR0_ENABLE:
> @@ -190,19 +189,15 @@ designware_pcie_root_config_read(PCIDevice *d, uint32_t address, int len)
>           break;
>   
>       case DESIGNWARE_PCIE_ATU_LOWER_BASE:
> -        val = viewport->base;
> -        break;
> -
>       case DESIGNWARE_PCIE_ATU_UPPER_BASE:
> -        val = viewport->base >> 32;
> +        val = extract64(viewport->base,
> +                        address == DESIGNWARE_PCIE_ATU_LOWER_BASE ? 0 : 32, 32);
>           break;
>   
>       case DESIGNWARE_PCIE_ATU_LOWER_TARGET:
> -        val = viewport->target;
> -        break;
> -
>       case DESIGNWARE_PCIE_ATU_UPPER_TARGET:
> -        val = viewport->target >> 32;
> +        val = extract64(viewport->target,
> +                        address == DESIGNWARE_PCIE_ATU_LOWER_TARGET ? 0 : 32, 32);
>           break;
>   
>       case DESIGNWARE_PCIE_ATU_LIMIT:
> @@ -321,14 +316,10 @@ static void designware_pcie_root_config_write(PCIDevice *d, uint32_t address,
>           break;
>   
>       case DESIGNWARE_PCIE_MSI_ADDR_LO:
> -        root->msi.base &= 0xFFFFFFFF00000000ULL;
> -        root->msi.base |= val;
> -        designware_pcie_root_update_msi_mapping(root);
> -        break;
> -
>       case DESIGNWARE_PCIE_MSI_ADDR_HI:
> -        root->msi.base &= 0x00000000FFFFFFFFULL;
> -        root->msi.base |= (uint64_t)val << 32;
> +        root->msi.base = deposit64(root->msi.base,
> +                                   address == DESIGNWARE_PCIE_MSI_ADDR_LO
> +                                   ? 0 : 32, 32, val);
>           designware_pcie_root_update_msi_mapping(root);
>           break;
>   
> @@ -355,23 +346,17 @@ static void designware_pcie_root_config_write(PCIDevice *d, uint32_t address,
>           break;
>   
>       case DESIGNWARE_PCIE_ATU_LOWER_BASE:
> -        viewport->base &= 0xFFFFFFFF00000000ULL;
> -        viewport->base |= val;
> -        break;
> -
>       case DESIGNWARE_PCIE_ATU_UPPER_BASE:
> -        viewport->base &= 0x00000000FFFFFFFFULL;
> -        viewport->base |= (uint64_t)val << 32;
> +        viewport->base = deposit64(root->msi.base,
> +                                   address == DESIGNWARE_PCIE_ATU_LOWER_BASE
> +                                   ? 0 : 32, 32, val);
>           break;
>   
>       case DESIGNWARE_PCIE_ATU_LOWER_TARGET:
> -        viewport->target &= 0xFFFFFFFF00000000ULL;
> -        viewport->target |= val;
> -        break;
> -
>       case DESIGNWARE_PCIE_ATU_UPPER_TARGET:
> -        viewport->target &= 0x00000000FFFFFFFFULL;
> -        viewport->target |= (uint64_t)val << 32;
> +        viewport->target = deposit64(root->msi.base,
> +                                     address == DESIGNWARE_PCIE_ATU_LOWER_TARGET
> +                                     ? 0 : 32, 32, val);
>           break;
>   
>       case DESIGNWARE_PCIE_ATU_LIMIT:

Reviewed-by: Gustavo Romero <gustavo.romero@linaro.org>


Cheers,
Gustavo


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-03-31 16:16 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-31 15:20 [PATCH-for-10.0 0/2] hw/pci-host/designware: Fix ATU_UPPER_TARGET register access Philippe Mathieu-Daudé
2025-03-31 15:20 ` [PATCH-for-10.0 1/2] hw/pci-host/designware: Fix access to ATU_UPPER_TARGET register Philippe Mathieu-Daudé
2025-03-31 16:15   ` Gustavo Romero
2025-03-31 15:20 ` [PATCH-for-10.1 2/2] hw/pci-host/designware: Use deposit/extract API Philippe Mathieu-Daudé
2025-03-31 16:16   ` Gustavo Romero

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).