[SeaBIOS] [PATCH v3 2/3] pci: add QEMU-specific PCI capability structure

[Alexander Bezzubikov]

2017-07-31 17:09 GMT+03:00 Marcel Apfelbaum <marcel at redhat.com>:
> On 31/07/2017 17:00, Michael S. Tsirkin wrote:
>>
>> On Sat, Jul 29, 2017 at 02:34:31AM +0300, Aleksandr Bezzubikov wrote:
>>>
>>> On PCI init PCI bridge devices may need some
>>> extra info about bus number to reserve, IO, memory and
>>> prefetchable memory limits. QEMU can provide this
>>> with special vendor-specific PCI capability.
>>>
>>> This capability is intended to be used only
>>> for Red Hat PCI bridges, i.e. QEMU cooperation.
>>>
>>> Signed-off-by: Aleksandr Bezzubikov <zuban32s at gmail.com>
>>> ---
>>>   src/fw/dev-pci.h | 62
>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 62 insertions(+)
>>>   create mode 100644 src/fw/dev-pci.h
>>>
>>> diff --git a/src/fw/dev-pci.h b/src/fw/dev-pci.h
>>> new file mode 100644
>>> index 0000000..fbd49ed
>>> --- /dev/null
>>> +++ b/src/fw/dev-pci.h
>>> @@ -0,0 +1,62 @@
>>> +#ifndef _PCI_CAP_H
>>> +#define _PCI_CAP_H
>>> +
>>> +#include "types.h"
>>> +
>>> +/*
>>> +
>>> +QEMU-specific vendor(Red Hat)-specific capability.
>>> +It's intended to provide some hints for firmware to init PCI devices.
>>> +
>>> +Its is shown below:
>>> +
>>> +Header:
>>> +
>>> +u8 id;       Standard PCI Capability Header field
>>> +u8 next;     Standard PCI Capability Header field
>>> +u8 len;      Standard PCI Capability Header field
>>> +u8 type;     Red Hat vendor-specific capability type:
>>> +               now only REDHAT_QEMU_CAP 1 exists
>>> +Data:
>>> +
>>> +u16 non_prefetchable_16;     non-prefetchable memory limit
>>> +
>>> +u8 bus_res;  minimum bus number to reserve;
>>> +             this is necessary for PCI Express Root Ports
>>> +             to support PCIE-to-PCI bridge hotplug
>>> +
>>> +u8 io_8;     IO limit in case of 8-bit limit value
>>> +u32 io_32;   IO limit in case of 16-bit limit value
>>> +             io_8 and io_16 are mutually exclusive, in other words,
>>> +             they can't be non-zero simultaneously
>>> +
>>> +u32 prefetchable_32;         non-prefetchable memory limit
>>> +                             in case of 32-bit limit value
>>> +u64 prefetchable_64;         non-prefetchable memory limit
>>> +                             in case of 64-bit limit value
>>> +                             prefetachable_32 and prefetchable_64 are
>>> +                             mutually exclusive, in other words,
>>> +                             they can't be non-zero simultaneously
>>> +If any field in Data section is 0,
>>> +it means that such kind of reservation
>>> +is not needed.

I really don't like this 'mutually exclusive' fields approach because
IMHO it increases confusion level when undertanding this capability structure.
But - if we came to consensus on that, then IO fields should be used
in the same way,
because as I understand, this 'mutual exclusivity' serves to distinguish cases
when we employ only *_LIMIT register and both *_LIMIT an UPPER_*_LIMIT
registers.
And this is how both IO and PREFETCHABLE works, isn't it?

>>
>>
>
> Hi Michael,
>
>> We also want a way to say "no hint for this type".
>>
>> One way to achive this would be to have instead multiple
>> vendor specific capabilities, one for each of
>> bus#/io/mem/prefetch. 0 would mean do not reserve anything,
>> absence of capability would mean "no info, up to firmware".
>>
>
> First version of the series was implemented exactly like you propose,
> however Gerd preferred only one capability with multiple fields.
>
> I personally like the simplicity of vendor cap per io/mem/bus,
> even if it is on the expense of the limited PCI Config space.
>

Personally I agree with Marcel since all this fields express
reservations of some objects.

> We need a consensus here :)
>

Absolutely :)

> Thanks,
> Marcel
>
>
>>
>>> +
>>> +*/
>>> +
>>> +/* Offset of vendor-specific capability type field */
>>> +#define PCI_CAP_VNDR_SPEC_TYPE  3
>>
>>
>> This is a QEMU specific thing. Please name it as such.
>>
>>> +
>>> +/* List of valid Red Hat vendor-specific capability types */
>>> +#define REDHAT_CAP_TYPE_QEMU    1
>>> +
>>> +
>>> +/* Offsets of QEMU capability fields */
>>> +#define QEMU_PCI_CAP_NON_PREF   4
>>> +#define QEMU_PCI_CAP_BUS_RES    6
>>> +#define QEMU_PCI_CAP_IO_8       7
>>> +#define QEMU_PCI_CAP_IO_32      8
>>> +#define QEMU_PCI_CAP_PREF_32    12
>>> +#define QEMU_PCI_CAP_PREF_64    16
>>> +#define QEMU_PCI_CAP_SIZE       24
>>> +
>>> +#endif /* _PCI_CAP_H */
>>> --
>>> 2.7.4
>
>



-- 
Alexander Bezzubikov