[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 2/3] pci: add QEMU-specific PCI capability st
From: |
Alexander Bezzubikov |
Subject: |
Re: [Qemu-devel] [PATCH v3 2/3] pci: add QEMU-specific PCI capability structure |
Date: |
Mon, 31 Jul 2017 21:54:55 +0300 |
2017-07-31 17:09 GMT+03:00 Marcel Apfelbaum <address@hidden>:
> 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 <address@hidden>
>>> ---
>>> 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