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

[Paolo Bonzini]

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
>