diff mbox series

[5/6] hw/pci-host/q35: Rename PCI 'black hole as '(memory) hole'

Message ID 20200910070131.435543-6-philmd@redhat.com
State New
Headers show
Series misc: Some inclusive terminology changes | expand

Commit Message

Philippe Mathieu-Daudé Sept. 10, 2020, 7:01 a.m. UTC 
In order to use inclusive terminology, rename "blackhole"
as "(memory)hole".

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/hw/pci-host/q35.h |  4 ++--
 hw/pci-host/q35.c         | 38 +++++++++++++++++++-------------------
 tests/qtest/q35-test.c    |  2 +-
 3 files changed, 22 insertions(+), 22 deletions(-)

Comments

Paolo Bonzini Sept. 10, 2020, 9:11 a.m. UTC | #1 
On 10/09/20 09:15, Thomas Huth wrote:
> On 10/09/2020 09.01, Philippe Mathieu-Daudé wrote:
>> In order to use inclusive terminology, rename "blackhole"
>> as "(memory)hole".
> A black hole is a well-known astronomical term, which is simply named
> that way since it absorbes all light. I doubt that anybody could get
> upset by this term?
> 

Agreed.  This is a memory region that absorbs all writes and always
reads as zero, the astronomical reference is obvious.

Paolo
Paolo Bonzini Sept. 10, 2020, 11:36 a.m. UTC | #2 
On 10/09/20 11:14, Daniel P. Berrangé wrote:
> On Thu, Sep 10, 2020 at 09:15:02AM +0200, Thomas Huth wrote:
>> On 10/09/2020 09.01, Philippe Mathieu-Daudé wrote:
>>> In order to use inclusive terminology, rename "blackhole"
>>> as "(memory)hole".
>>
>> A black hole is a well-known astronomical term, which is simply named
>> that way since it absorbes all light. I doubt that anybody could get
>> upset by this term?
> 
> In this particular case I think the change is the right thing to do
> simply because the astronomical analogy is not adding any value in
> understanding. Calling it a "memoryhole" is more descriptive in what
> is actually is.

Absolutely not.  A memory hole ("memoryhole" is not an English word)
would be easily confused with a hole in the memory map, which this is
not.  For example on x86 systems the "PCI hole" is a hole between the
end of low RAM and the bottom of the ROM that is reserved for memory
mapped devices.  The "PCI hole" is explicitly left free by board code so
that the OS can put PCI BARs in there.

These black hole MemoryRegions, instead, are present in the memory map
and their purpose is to absorbs all writes and only sends back zeros,
hiding the contents of SMRAM and TSEG from the guest.  Just like a black
hole they are "something that exists".

Therefore, both "memory hole" and "hole" as in Philippe's patch are
worse than the astronomical metaphor.

Paolo

>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> ---
>>>  include/hw/pci-host/q35.h |  4 ++--
>>>  hw/pci-host/q35.c         | 38 +++++++++++++++++++-------------------
>>>  tests/qtest/q35-test.c    |  2 +-
>>>  3 files changed, 22 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h
>>> index 070305f83df..0fb90aca18b 100644
>>> --- a/include/hw/pci-host/q35.h
>>> +++ b/include/hw/pci-host/q35.h
>>> @@ -48,8 +48,8 @@ typedef struct MCHPCIState {
>>>      PAMMemoryRegion pam_regions[13];
>>>      MemoryRegion smram_region, open_high_smram;
>>>      MemoryRegion smram, low_smram, high_smram;
>>> -    MemoryRegion tseg_blackhole, tseg_window;
>>> -    MemoryRegion smbase_blackhole, smbase_window;
>>> +    MemoryRegion tseg_hole, tseg_window;
>>> +    MemoryRegion smbase_hole, smbase_window;
>>
>> Maybe rather use smbase_memhole and tseg_memhole?
>>
>>  Thomas
>>
>>
> 
> Regards,
> Daniel
>
diff mbox series

Patch

diff --git a/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h
index 070305f83df..0fb90aca18b 100644
--- a/include/hw/pci-host/q35.h
+++ b/include/hw/pci-host/q35.h
@@ -48,8 +48,8 @@  typedef struct MCHPCIState {
     PAMMemoryRegion pam_regions[13];
     MemoryRegion smram_region, open_high_smram;
     MemoryRegion smram, low_smram, high_smram;
-    MemoryRegion tseg_blackhole, tseg_window;
-    MemoryRegion smbase_blackhole, smbase_window;
+    MemoryRegion tseg_hole, tseg_window;
+    MemoryRegion smbase_hole, smbase_window;
     bool has_smram_at_smbase;
     Range pci_hole;
     uint64_t below_4g_mem_size;
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index b67cb9c29f8..21f58ccfa28 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -266,20 +266,20 @@  static const TypeInfo q35_host_info = {
  * MCH D0:F0
  */
 
-static uint64_t blackhole_read(void *ptr, hwaddr reg, unsigned size)
+static uint64_t memoryhole_read(void *ptr, hwaddr reg, unsigned size)
 {
     return 0xffffffff;
 }
 
-static void blackhole_write(void *opaque, hwaddr addr, uint64_t val,
+static void memoryhole_write(void *opaque, hwaddr addr, uint64_t val,
                             unsigned width)
 {
     /* nothing */
 }
 
-static const MemoryRegionOps blackhole_ops = {
-    .read = blackhole_read,
-    .write = blackhole_write,
+static const MemoryRegionOps memoryhole_ops = {
+    .read = memoryhole_read,
+    .write = memoryhole_write,
     .endianness = DEVICE_NATIVE_ENDIAN,
     .valid.min_access_size = 1,
     .valid.max_access_size = 4,
@@ -393,12 +393,12 @@  static void mch_update_smram(MCHPCIState *mch)
     } else {
         tseg_size = 0;
     }
-    memory_region_del_subregion(mch->system_memory, &mch->tseg_blackhole);
-    memory_region_set_enabled(&mch->tseg_blackhole, tseg_size);
-    memory_region_set_size(&mch->tseg_blackhole, tseg_size);
+    memory_region_del_subregion(mch->system_memory, &mch->tseg_hole);
+    memory_region_set_enabled(&mch->tseg_hole, tseg_size);
+    memory_region_set_size(&mch->tseg_hole, tseg_size);
     memory_region_add_subregion_overlap(mch->system_memory,
                                         mch->below_4g_mem_size - tseg_size,
-                                        &mch->tseg_blackhole, 1);
+                                        &mch->tseg_hole, 1);
 
     memory_region_set_enabled(&mch->tseg_window, tseg_size);
     memory_region_set_size(&mch->tseg_window, tseg_size);
@@ -456,7 +456,7 @@  static void mch_update_smbase_smram(MCHPCIState *mch)
     } else {
         lck = false;
     }
-    memory_region_set_enabled(&mch->smbase_blackhole, lck);
+    memory_region_set_enabled(&mch->smbase_hole, lck);
     memory_region_set_enabled(&mch->smbase_window, lck);
     memory_region_transaction_commit();
 }
@@ -601,13 +601,13 @@  static void mch_realize(PCIDevice *d, Error **errp)
     memory_region_set_enabled(&mch->high_smram, true);
     memory_region_add_subregion(&mch->smram, 0xfeda0000, &mch->high_smram);
 
-    memory_region_init_io(&mch->tseg_blackhole, OBJECT(mch),
-                          &blackhole_ops, NULL,
-                          "tseg-blackhole", 0);
-    memory_region_set_enabled(&mch->tseg_blackhole, false);
+    memory_region_init_io(&mch->tseg_hole, OBJECT(mch),
+                          &memoryhole_ops, NULL,
+                          "tseg-hole", 0);
+    memory_region_set_enabled(&mch->tseg_hole, false);
     memory_region_add_subregion_overlap(mch->system_memory,
                                         mch->below_4g_mem_size,
-                                        &mch->tseg_blackhole, 1);
+                                        &mch->tseg_hole, 1);
 
     memory_region_init_alias(&mch->tseg_window, OBJECT(mch), "tseg-window",
                              mch->ram_memory, mch->below_4g_mem_size, 0);
@@ -619,13 +619,13 @@  static void mch_realize(PCIDevice *d, Error **errp)
      * This is not what hardware does, so it's QEMU specific hack.
      * See commit message for details.
      */
-    memory_region_init_io(&mch->smbase_blackhole, OBJECT(mch), &blackhole_ops,
-                          NULL, "smbase-blackhole",
+    memory_region_init_io(&mch->smbase_hole, OBJECT(mch), &memoryhole_ops,
+                          NULL, "smbase-hole",
                           MCH_HOST_BRIDGE_SMBASE_SIZE);
-    memory_region_set_enabled(&mch->smbase_blackhole, false);
+    memory_region_set_enabled(&mch->smbase_hole, false);
     memory_region_add_subregion_overlap(mch->system_memory,
                                         MCH_HOST_BRIDGE_SMBASE_ADDR,
-                                        &mch->smbase_blackhole, 1);
+                                        &mch->smbase_hole, 1);
 
     memory_region_init_alias(&mch->smbase_window, OBJECT(mch),
                              "smbase-window", mch->ram_memory,
diff --git a/tests/qtest/q35-test.c b/tests/qtest/q35-test.c
index b7cf1449906..7cddd0a7f61 100644
--- a/tests/qtest/q35-test.c
+++ b/tests/qtest/q35-test.c
@@ -231,7 +231,7 @@  static void test_smram_smbase_lock(void)
         qpci_config_writeb(pcidev, MCH_HOST_BRIDGE_F_SMBASE, i);
         g_assert(qpci_config_readb(pcidev, MCH_HOST_BRIDGE_F_SMBASE) == 0x02);
 
-        /* RAM access should go into black hole */
+        /* RAM access should go into memory hole */
         qtest_writeb(qts, SMBASE, SMRAM_TEST_PATTERN);
         g_assert_cmpint(qtest_readb(qts, SMBASE), ==, 0xff);
     }