Re: [Xen-devel] [RFC PATCH 11/12] hvmloader: use libacpi to build MCFG table

[Alexey G <x1917x@xxxxxxxxx>]

On Tue, 20 Mar 2018 03:36:57 -0600
"Jan Beulich" <JBeulich@xxxxxxxx> wrote:

>>>> On 19.03.18 at 22:20, <x1917x@xxxxxxxxx> wrote:  
>> On Mon, 19 Mar 2018 17:49:09 +0000
>> Roger Pau Monné <roger.pau@xxxxxxxxxx> wrote:  
>>>On Tue, Mar 13, 2018 at 04:33:56AM +1000, Alexey Gerasimenko wrote:  
>>>> --- a/tools/firmware/hvmloader/util.c
>>>> +++ b/tools/firmware/hvmloader/util.c
>>>> @@ -782,6 +782,69 @@ int get_pc_machine_type(void)
>>>>      return machine_type;
>>>>  }
>>>>  
>>>> +#define PCIEXBAR_ADDR_MASK_64MB     (~((1ULL << 26) - 1))
>>>> +#define PCIEXBAR_ADDR_MASK_128MB    (~((1ULL << 27) - 1))
>>>> +#define PCIEXBAR_ADDR_MASK_256MB    (~((1ULL << 28) - 1))
>>>> +#define PCIEXBAR_LENGTH_BITS(reg)   (((reg) >> 1) & 3)
>>>> +#define PCIEXBAREN                  1    
>>>
>>>PCIEXBAR_ENABLE maybe?  
>> 
>> PCIEXBAREN is just an official name of this bit from the
>> Intel datasheet. :) OK, will rename it to PCIEXBAR_ENABLE.  
>
>I think using names from the datasheet (where they exist) is
>preferable in cases like this one.

Leaving it intact then.

>>>> +    switch (PCIEXBAR_LENGTH_BITS(reg))
>>>> +    {
>>>> +    case 0:
>>>> +        base &= PCIEXBAR_ADDR_MASK_256MB;
>>>> +        break;
>>>> +    case 1:
>>>> +        base &= PCIEXBAR_ADDR_MASK_128MB;
>>>> +        break;
>>>> +    case 2:
>>>> +        base &= PCIEXBAR_ADDR_MASK_64MB;
>>>> +        break;    
>>>
>>>Missing newlines, plus this looks like it wants to use the defines
>>>introduced in patch 7 (PCIEXBAR_{64,128,256}_BUSES). Also any reason
>>>this patch and patch 7 cannot be put sequentially?  
>> 
>> I think all these #defines should find a way to pci_regs.h, it seems
>> like an appropriate place for them.  
>
>I don't think device specific defines belong into pci_regs.h.

Will gather all these #defines and macros in the new pci_regs_q35.h
file. It should not harm to include it from pci_regs.h I think, in
order to include pci_regs.h only in *.c.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel