[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v6 01/11] pci: introduce a type to store a SBDF



>>> On 19.09.17 at 17:40, <Paul.Durrant@xxxxxxxxxx> wrote:
>> --- a/xen/arch/x86/hvm/io.c
>> +++ b/xen/arch/x86/hvm/io.c
>> @@ -257,17 +257,11 @@ void register_g2m_portio_handler(struct domain *d)
>>  }
>> 
>>  unsigned int hvm_pci_decode_addr(unsigned int cf8, unsigned int addr,
>> -                                 unsigned int *bus, unsigned int *slot,
>> -                                 unsigned int *func)
>> +                                 pci_sbdf_t *bdf)
> 
> I'd prefer the pointer name to be 'sbdf' rather than 'bdf', but otherwise...

Indeed. Or have a sub-type "struct pci_bdf_t", as the segment
(sadly) isn't relevant yet.

>>  {
>> -    unsigned int bdf;
>> -
>>      ASSERT(CF8_ENABLED(cf8));
>> 
>> -    bdf = CF8_BDF(cf8);
>> -    *bus = PCI_BUS(bdf);
>> -    *slot = PCI_SLOT(bdf);
>> -    *func = PCI_FUNC(bdf);
>> +    bdf->sbdf = CF8_BDF(cf8);

Filling ->bdf here and setting ->seg explicitly with zero may also
make the current limitation more obvious.

>> --- a/xen/arch/x86/hvm/ioreq.c
>> +++ b/xen/arch/x86/hvm/ioreq.c
>> @@ -1177,17 +1177,15 @@ struct hvm_ioreq_server
>> *hvm_select_ioreq_server(struct domain *d,
>>           (p->addr & ~3) == 0xcfc &&
>>           CF8_ENABLED(cf8) )
>>      {
>> -        uint32_t sbdf, x86_fam;
>> -        unsigned int bus, slot, func, reg;
>> +        uint32_t x86_fam;
>> +        pci_sbdf_t bdf;
>> +        unsigned int reg;
>> 
>> -        reg = hvm_pci_decode_addr(cf8, p->addr, &bus, &slot, &func);
>> +        reg = hvm_pci_decode_addr(cf8, p->addr, &bdf);
>> 
>>          /* PCI config data cycle */
>> -
>> -        sbdf = XEN_DMOP_PCI_SBDF(0, bus, slot, func);
>> -
>>          type = XEN_DMOP_IO_RANGE_PCI;
>> -        addr = ((uint64_t)sbdf << 32) | reg;
>> +        addr = ((uint64_t)bdf.bdf << 32) | reg;

I also wonder why the field used here is bdf instead of sbdf.
It would make for less future changes if you used .sbdf here
right away.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.