[PATCH 4/8] hw/pci-host/q35: Remove Q35PCIHost::pci_hole64_fix field

Philippe Mathieu-Daudé posted 8 patches 9 months, 1 week ago
[PATCH 4/8] hw/pci-host/q35: Remove Q35PCIHost::pci_hole64_fix field
Posted by Philippe Mathieu-Daudé 9 months, 1 week ago
The Q35PCIHost::pci_hole64_fix boolean was only set in
the pc_compat_2_10[] array, via the 'x-pci-hole64-fix=off'
property. We removed all machines using that array, lets
remove that property and all the code around it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/pci-host/q35.h | 1 -
 hw/pci-host/q35.c         | 6 ++----
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h
index ddafc3f2e3d..75810528205 100644
--- a/include/hw/pci-host/q35.h
+++ b/include/hw/pci-host/q35.h
@@ -63,7 +63,6 @@ struct Q35PCIHost {
     PCIExpressHost parent_obj;
     /*< public >*/
 
-    bool pci_hole64_fix;
     MCHPCIState mch;
 };
 
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 1951ae440cc..f6e29cc4fc8 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -116,13 +116,12 @@ static void q35_host_get_pci_hole_end(Object *obj, Visitor *v,
 static uint64_t q35_host_get_pci_hole64_start_value(Object *obj)
 {
     PCIHostState *h = PCI_HOST_BRIDGE(obj);
-    Q35PCIHost *s = Q35_HOST_DEVICE(obj);
     Range w64;
     uint64_t value;
 
     pci_bus_get_w64_range(h->bus, &w64);
     value = range_is_empty(&w64) ? 0 : range_lob(&w64);
-    if (!value && s->pci_hole64_fix) {
+    if (!value) {
         value = pc_pci_hole64_start();
     }
     return value;
@@ -156,7 +155,7 @@ static void q35_host_get_pci_hole64_end(Object *obj, Visitor *v,
     pci_bus_get_w64_range(h->bus, &w64);
     value = range_is_empty(&w64) ? 0 : range_upb(&w64) + 1;
     hole64_end = ROUND_UP(hole64_start + s->mch.pci_hole64_size, 1ULL << 30);
-    if (s->pci_hole64_fix && value < hole64_end) {
+    if (value < hole64_end) {
         value = hole64_end;
     }
     visit_type_uint64(v, name, &value, errp);
@@ -181,7 +180,6 @@ static const Property q35_host_props[] = {
                      mch.above_4g_mem_size, 0),
     DEFINE_PROP_BOOL(PCI_HOST_PROP_SMM_RANGES, Q35PCIHost,
                      mch.has_smm_ranges, true),
-    DEFINE_PROP_BOOL("x-pci-hole64-fix", Q35PCIHost, pci_hole64_fix, true),
 };
 
 static void q35_host_class_init(ObjectClass *klass, const void *data)
-- 
2.47.1


Re: [PATCH 4/8] hw/pci-host/q35: Remove Q35PCIHost::pci_hole64_fix field
Posted by Daniel P. Berrangé 1 month ago
This patch did not get merged so far, so in case it is resurrected
I wanted to warn that even though we're going to be removing machine
types that use it, we might want to keep the property.

In this issue

  https://gitlab.com/qemu-project/qemu/-/issues/1947#note_2994552211

being able to disable the pci_hole64_fix is indicated as a possible
fix for a Windows XP  BSOD.

Perhaps there is a better way to avoid the root cause of the BSOD,
but if not this knob would be useful to retain.


On Fri, May 02, 2025 at 12:35:18AM +0200, Philippe Mathieu-Daudé wrote:
> The Q35PCIHost::pci_hole64_fix boolean was only set in
> the pc_compat_2_10[] array, via the 'x-pci-hole64-fix=off'
> property. We removed all machines using that array, lets
> remove that property and all the code around it.



> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  include/hw/pci-host/q35.h | 1 -
>  hw/pci-host/q35.c         | 6 ++----
>  2 files changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h
> index ddafc3f2e3d..75810528205 100644
> --- a/include/hw/pci-host/q35.h
> +++ b/include/hw/pci-host/q35.h
> @@ -63,7 +63,6 @@ struct Q35PCIHost {
>      PCIExpressHost parent_obj;
>      /*< public >*/
>  
> -    bool pci_hole64_fix;
>      MCHPCIState mch;
>  };
>  
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index 1951ae440cc..f6e29cc4fc8 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -116,13 +116,12 @@ static void q35_host_get_pci_hole_end(Object *obj, Visitor *v,
>  static uint64_t q35_host_get_pci_hole64_start_value(Object *obj)
>  {
>      PCIHostState *h = PCI_HOST_BRIDGE(obj);
> -    Q35PCIHost *s = Q35_HOST_DEVICE(obj);
>      Range w64;
>      uint64_t value;
>  
>      pci_bus_get_w64_range(h->bus, &w64);
>      value = range_is_empty(&w64) ? 0 : range_lob(&w64);
> -    if (!value && s->pci_hole64_fix) {
> +    if (!value) {
>          value = pc_pci_hole64_start();
>      }
>      return value;
> @@ -156,7 +155,7 @@ static void q35_host_get_pci_hole64_end(Object *obj, Visitor *v,
>      pci_bus_get_w64_range(h->bus, &w64);
>      value = range_is_empty(&w64) ? 0 : range_upb(&w64) + 1;
>      hole64_end = ROUND_UP(hole64_start + s->mch.pci_hole64_size, 1ULL << 30);
> -    if (s->pci_hole64_fix && value < hole64_end) {
> +    if (value < hole64_end) {
>          value = hole64_end;
>      }
>      visit_type_uint64(v, name, &value, errp);
> @@ -181,7 +180,6 @@ static const Property q35_host_props[] = {
>                       mch.above_4g_mem_size, 0),
>      DEFINE_PROP_BOOL(PCI_HOST_PROP_SMM_RANGES, Q35PCIHost,
>                       mch.has_smm_ranges, true),
> -    DEFINE_PROP_BOOL("x-pci-hole64-fix", Q35PCIHost, pci_hole64_fix, true),
>  };
>  
>  static void q35_host_class_init(ObjectClass *klass, const void *data)
> -- 
> 2.47.1
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|