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

2020-09-10 Thread 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é 
>>> ---
>>>  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
> 




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

2020-09-10 Thread Daniel P . Berrangé
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.

> 
> > Signed-off-by: Philippe Mathieu-Daudé 
> > ---
> >  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
-- 
|: 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 :|




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

2020-09-10 Thread Paolo Bonzini
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




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

2020-09-10 Thread Philippe Mathieu-Daudé
On 9/10/20 9:15 AM, 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?

Let's put some light in our address space then :)

But you right, the NASA does not consider renaming this one:
https://www.space.com/nasa-stops-racist-nicknames-cosmic-objects.html

Note than for electronic busses design, master/slave are also
well-known terms, but they might be considered offensive.
I changed this one too because I'm not sure about its offensiveness
threshold.

> 
>> Signed-off-by: Philippe Mathieu-Daudé 
>> ---
>>  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?

OK.

> 
>  Thomas
> 




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

2020-09-10 Thread Thomas Huth
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?

> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  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