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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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.

>>> +    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.

Jan

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