Re: [SeaBIOS] [Qemu-devel] [PATCH v3 5/5] docs: update documentation considering PCIE-PCI bridge

2017-08-01 Thread Laszlo Ersek 
On 08/01/17 23:39, Michael S. Tsirkin wrote:
> On Wed, Aug 02, 2017 at 12:33:12AM +0300, Alexander Bezzubikov wrote:
>> 2017-08-01 23:31 GMT+03:00 Laszlo Ersek :
>>> (Whenever my comments conflict with Michael's or Marcel's, I defer to them.)
>>>
>>> On 07/29/17 01:37, Aleksandr Bezzubikov wrote:
 Signed-off-by: Aleksandr Bezzubikov 
 ---
  docs/pcie.txt|  46 ++
  docs/pcie_pci_bridge.txt | 121 
 +++
  2 files changed, 147 insertions(+), 20 deletions(-)
  create mode 100644 docs/pcie_pci_bridge.txt

 diff --git a/docs/pcie.txt b/docs/pcie.txt
 index 5bada24..338b50e 100644
 --- a/docs/pcie.txt
 +++ b/docs/pcie.txt
 @@ -46,7 +46,7 @@ Place only the following kinds of devices directly on 
 the Root Complex:
  (2) PCI Express Root Ports (ioh3420), for starting exclusively PCI 
 Express
  hierarchies.

 -(3) DMI-PCI Bridges (i82801b11-bridge), for starting legacy PCI
 +(3) PCIE-PCI Bridge (pcie-pci-bridge), for starting legacy PCI
  hierarchies.

  (4) Extra Root Complexes (pxb-pcie), if multiple PCI Express Root 
 Buses
>>>
>>> When reviewing previous patches modifying / adding this file, I
>>> requested that we spell out "PCI Express" every single time. I'd like to
>>> see the same in this patch, if possible.
>>
>> OK, I didn't know it.
>>
>>>
 @@ -55,18 +55,18 @@ Place only the following kinds of devices directly on 
 the Root Complex:
 pcie.0 bus
 
 
  |||  |
 -   ---   --   --   --
 -   | PCI Dev |   | PCIe Root Port |   | DMI-PCI Bridge |   |  pxb-pcie  |
 -   ---   --   --   --
 +   ---   --   ---   --
 +   | PCI Dev |   | PCIe Root Port |   | PCIE-PCI Bridge |   |  pxb-pcie  |
 +   ---   --   ---   --

  2.1.1 To plug a device into pcie.0 as a Root Complex Integrated Endpoint 
 use:
-device [,bus=pcie.0]
  2.1.2 To expose a new PCI Express Root Bus use:
-device pxb-pcie,id=pcie.1,bus_nr=x[,numa_node=y][,addr=z]
 -  Only PCI Express Root Ports and DMI-PCI bridges can be connected
 +  Only PCI Express Root Ports, PCIE-PCI bridges and DMI-PCI bridges 
 can be connected
>>>
>>> It would be nice if we could keep the flowing text wrapped to 80 chars.
>>>
>>> Also, here you add the "PCI Express-PCI" bridge to the list of allowed
>>> controllers (and you keep DMI-PCI as permitted), but above DMI was
>>> replaced. I think these should be made consistent -- we should make up
>>> our minds if we continue to recommend the DMI-PCI bridge or not. If not,
>>> then we should eradicate all traces of it. If we want to keep it at
>>> least for compatibility, then it should remain as fully documented as it
>>> is now.
>>
>> Now I'm beginning to think that we shouldn't keep the DMI-PCI bridge
>> even for compatibility and may want to use a new PCIE-PCI bridge
>> everywhere (of course, except some cases when users are
>> sure they need exactly DMI-PCI bridge for some reason)
> 
> Can dmi-pci support shpc? why doesn't it? For compatibility?

I don't know why, but the fact that it doesn't is the reason libvirt
settled on auto-creating a dmi-pci bridge and a pci-pci bridge under
that for Q35. The reasoning was (IIRC Laine's words correctly) that the
dmi-pci bridge cannot receive hotplugged devices, while the pci-pci
bridge cannot be connected to the root complex. So both were needed.

Thanks
Laszlo

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [Qemu-devel] [PATCH v3 5/5] docs: update documentation considering PCIE-PCI bridge

2017-08-01 Thread Michael S. Tsirkin 
On Wed, Aug 02, 2017 at 12:33:12AM +0300, Alexander Bezzubikov wrote:
> 2017-08-01 23:31 GMT+03:00 Laszlo Ersek :
> > (Whenever my comments conflict with Michael's or Marcel's, I defer to them.)
> >
> > On 07/29/17 01:37, Aleksandr Bezzubikov wrote:
> >> Signed-off-by: Aleksandr Bezzubikov 
> >> ---
> >>  docs/pcie.txt|  46 ++
> >>  docs/pcie_pci_bridge.txt | 121 
> >> +++
> >>  2 files changed, 147 insertions(+), 20 deletions(-)
> >>  create mode 100644 docs/pcie_pci_bridge.txt
> >>
> >> diff --git a/docs/pcie.txt b/docs/pcie.txt
> >> index 5bada24..338b50e 100644
> >> --- a/docs/pcie.txt
> >> +++ b/docs/pcie.txt
> >> @@ -46,7 +46,7 @@ Place only the following kinds of devices directly on 
> >> the Root Complex:
> >>  (2) PCI Express Root Ports (ioh3420), for starting exclusively PCI 
> >> Express
> >>  hierarchies.
> >>
> >> -(3) DMI-PCI Bridges (i82801b11-bridge), for starting legacy PCI
> >> +(3) PCIE-PCI Bridge (pcie-pci-bridge), for starting legacy PCI
> >>  hierarchies.
> >>
> >>  (4) Extra Root Complexes (pxb-pcie), if multiple PCI Express Root 
> >> Buses
> >
> > When reviewing previous patches modifying / adding this file, I
> > requested that we spell out "PCI Express" every single time. I'd like to
> > see the same in this patch, if possible.
> 
> OK, I didn't know it.
> 
> >
> >> @@ -55,18 +55,18 @@ Place only the following kinds of devices directly on 
> >> the Root Complex:
> >> pcie.0 bus
> >> 
> >> 
> >>  |||  |
> >> -   ---   --   --   --
> >> -   | PCI Dev |   | PCIe Root Port |   | DMI-PCI Bridge |   |  pxb-pcie  |
> >> -   ---   --   --   --
> >> +   ---   --   ---   --
> >> +   | PCI Dev |   | PCIe Root Port |   | PCIE-PCI Bridge |   |  pxb-pcie  |
> >> +   ---   --   ---   --
> >>
> >>  2.1.1 To plug a device into pcie.0 as a Root Complex Integrated Endpoint 
> >> use:
> >>-device [,bus=pcie.0]
> >>  2.1.2 To expose a new PCI Express Root Bus use:
> >>-device pxb-pcie,id=pcie.1,bus_nr=x[,numa_node=y][,addr=z]
> >> -  Only PCI Express Root Ports and DMI-PCI bridges can be connected
> >> +  Only PCI Express Root Ports, PCIE-PCI bridges and DMI-PCI bridges 
> >> can be connected
> >
> > It would be nice if we could keep the flowing text wrapped to 80 chars.
> >
> > Also, here you add the "PCI Express-PCI" bridge to the list of allowed
> > controllers (and you keep DMI-PCI as permitted), but above DMI was
> > replaced. I think these should be made consistent -- we should make up
> > our minds if we continue to recommend the DMI-PCI bridge or not. If not,
> > then we should eradicate all traces of it. If we want to keep it at
> > least for compatibility, then it should remain as fully documented as it
> > is now.
> 
> Now I'm beginning to think that we shouldn't keep the DMI-PCI bridge
> even for compatibility and may want to use a new PCIE-PCI bridge
> everywhere (of course, except some cases when users are
> sure they need exactly DMI-PCI bridge for some reason)

Can dmi-pci support shpc? why doesn't it? For compatibility?


> >
> >>to the pcie.1 bus:
> >>-device 
> >> ioh3420,id=root_port1[,bus=pcie.1][,chassis=x][,slot=y][,addr=z]   
> >>   \
> >> -  -device i82801b11-bridge,id=dmi_pci_bridge1,bus=pcie.1
> >> +  -device pcie-pci-bridge,id=pcie_pci_bridge1,bus=pcie.1
> >>
> >>
> >>  2.2 PCI Express only hierarchy
> >> @@ -130,21 +130,25 @@ Notes:
> >>  Legacy PCI devices can be plugged into pcie.0 as Integrated Endpoints,
> >>  but, as mentioned in section 5, doing so means the legacy PCI
> >>  device in question will be incapable of hot-unplugging.
> >> -Besides that use DMI-PCI Bridges (i82801b11-bridge) in combination
> >> +Besides that use PCIE-PCI Bridges (pcie-pci-bridge) in combination
> >>  with PCI-PCI Bridges (pci-bridge) to start PCI hierarchies.
> >> +Instead of the PCIE-PCI Bridge DMI-PCI one can be used,
> >> +but it doens't support hot-plug, is not crossplatform and since that
> >
> > s/doens't/doesn't/
> >
> > s/since that/therefore it/
> >
> >> +is obsolete and deprecated. Use the PCIE-PCI Bridge if you're not
> >> +absolutely sure you need the DMI-PCI Bridge.
> >>
> >> -Prefer flat hierarchies. For most scenarios a single DMI-PCI Bridge
> >> +Prefer flat hierarchies. For most scenarios a single PCIE-PCI Bridge
> >>  (having 32 slots) and several PCI-PCI Bridges attached to it
> >>  (each supporting also 32 slots) will support hundreds of legacy devices.
> >> -The recommendation is to

Re: [SeaBIOS] [Qemu-devel] [PATCH v3 5/5] docs: update documentation considering PCIE-PCI bridge

2017-08-01 Thread Alexander Bezzubikov 
2017-08-01 23:31 GMT+03:00 Laszlo Ersek :
> (Whenever my comments conflict with Michael's or Marcel's, I defer to them.)
>
> On 07/29/17 01:37, Aleksandr Bezzubikov wrote:
>> Signed-off-by: Aleksandr Bezzubikov 
>> ---
>>  docs/pcie.txt|  46 ++
>>  docs/pcie_pci_bridge.txt | 121 
>> +++
>>  2 files changed, 147 insertions(+), 20 deletions(-)
>>  create mode 100644 docs/pcie_pci_bridge.txt
>>
>> diff --git a/docs/pcie.txt b/docs/pcie.txt
>> index 5bada24..338b50e 100644
>> --- a/docs/pcie.txt
>> +++ b/docs/pcie.txt
>> @@ -46,7 +46,7 @@ Place only the following kinds of devices directly on the 
>> Root Complex:
>>  (2) PCI Express Root Ports (ioh3420), for starting exclusively PCI 
>> Express
>>  hierarchies.
>>
>> -(3) DMI-PCI Bridges (i82801b11-bridge), for starting legacy PCI
>> +(3) PCIE-PCI Bridge (pcie-pci-bridge), for starting legacy PCI
>>  hierarchies.
>>
>>  (4) Extra Root Complexes (pxb-pcie), if multiple PCI Express Root Buses
>
> When reviewing previous patches modifying / adding this file, I
> requested that we spell out "PCI Express" every single time. I'd like to
> see the same in this patch, if possible.

OK, I didn't know it.

>
>> @@ -55,18 +55,18 @@ Place only the following kinds of devices directly on 
>> the Root Complex:
>> pcie.0 bus
>> 
>> 
>>  |||  |
>> -   ---   --   --   --
>> -   | PCI Dev |   | PCIe Root Port |   | DMI-PCI Bridge |   |  pxb-pcie  |
>> -   ---   --   --   --
>> +   ---   --   ---   --
>> +   | PCI Dev |   | PCIe Root Port |   | PCIE-PCI Bridge |   |  pxb-pcie  |
>> +   ---   --   ---   --
>>
>>  2.1.1 To plug a device into pcie.0 as a Root Complex Integrated Endpoint 
>> use:
>>-device [,bus=pcie.0]
>>  2.1.2 To expose a new PCI Express Root Bus use:
>>-device pxb-pcie,id=pcie.1,bus_nr=x[,numa_node=y][,addr=z]
>> -  Only PCI Express Root Ports and DMI-PCI bridges can be connected
>> +  Only PCI Express Root Ports, PCIE-PCI bridges and DMI-PCI bridges can 
>> be connected
>
> It would be nice if we could keep the flowing text wrapped to 80 chars.
>
> Also, here you add the "PCI Express-PCI" bridge to the list of allowed
> controllers (and you keep DMI-PCI as permitted), but above DMI was
> replaced. I think these should be made consistent -- we should make up
> our minds if we continue to recommend the DMI-PCI bridge or not. If not,
> then we should eradicate all traces of it. If we want to keep it at
> least for compatibility, then it should remain as fully documented as it
> is now.

Now I'm beginning to think that we shouldn't keep the DMI-PCI bridge
even for compatibility and may want to use a new PCIE-PCI bridge
everywhere (of course, except some cases when users are
sure they need exactly DMI-PCI bridge for some reason)

>
>>to the pcie.1 bus:
>>-device 
>> ioh3420,id=root_port1[,bus=pcie.1][,chassis=x][,slot=y][,addr=z] 
>> \
>> -  -device i82801b11-bridge,id=dmi_pci_bridge1,bus=pcie.1
>> +  -device pcie-pci-bridge,id=pcie_pci_bridge1,bus=pcie.1
>>
>>
>>  2.2 PCI Express only hierarchy
>> @@ -130,21 +130,25 @@ Notes:
>>  Legacy PCI devices can be plugged into pcie.0 as Integrated Endpoints,
>>  but, as mentioned in section 5, doing so means the legacy PCI
>>  device in question will be incapable of hot-unplugging.
>> -Besides that use DMI-PCI Bridges (i82801b11-bridge) in combination
>> +Besides that use PCIE-PCI Bridges (pcie-pci-bridge) in combination
>>  with PCI-PCI Bridges (pci-bridge) to start PCI hierarchies.
>> +Instead of the PCIE-PCI Bridge DMI-PCI one can be used,
>> +but it doens't support hot-plug, is not crossplatform and since that
>
> s/doens't/doesn't/
>
> s/since that/therefore it/
>
>> +is obsolete and deprecated. Use the PCIE-PCI Bridge if you're not
>> +absolutely sure you need the DMI-PCI Bridge.
>>
>> -Prefer flat hierarchies. For most scenarios a single DMI-PCI Bridge
>> +Prefer flat hierarchies. For most scenarios a single PCIE-PCI Bridge
>>  (having 32 slots) and several PCI-PCI Bridges attached to it
>>  (each supporting also 32 slots) will support hundreds of legacy devices.
>> -The recommendation is to populate one PCI-PCI Bridge under the DMI-PCI 
>> Bridge
>> +The recommendation is to populate one PCI-PCI Bridge under the PCIE-PCI 
>> Bridge
>>  until is full and then plug a new PCI-PCI Bridge...
>>
>> pcie.0 bus
>> --
>>  ||
>> -   ---

Re: [SeaBIOS] [Qemu-devel] [PATCH v3 5/5] docs: update documentation considering PCIE-PCI bridge

2017-08-01 Thread Laszlo Ersek 
(Whenever my comments conflict with Michael's or Marcel's, I defer to them.)

On 07/29/17 01:37, Aleksandr Bezzubikov wrote:
> Signed-off-by: Aleksandr Bezzubikov 
> ---
>  docs/pcie.txt|  46 ++
>  docs/pcie_pci_bridge.txt | 121 
> +++
>  2 files changed, 147 insertions(+), 20 deletions(-)
>  create mode 100644 docs/pcie_pci_bridge.txt
> 
> diff --git a/docs/pcie.txt b/docs/pcie.txt
> index 5bada24..338b50e 100644
> --- a/docs/pcie.txt
> +++ b/docs/pcie.txt
> @@ -46,7 +46,7 @@ Place only the following kinds of devices directly on the 
> Root Complex:
>  (2) PCI Express Root Ports (ioh3420), for starting exclusively PCI 
> Express
>  hierarchies.
>  
> -(3) DMI-PCI Bridges (i82801b11-bridge), for starting legacy PCI
> +(3) PCIE-PCI Bridge (pcie-pci-bridge), for starting legacy PCI
>  hierarchies.
>  
>  (4) Extra Root Complexes (pxb-pcie), if multiple PCI Express Root Buses

When reviewing previous patches modifying / adding this file, I
requested that we spell out "PCI Express" every single time. I'd like to
see the same in this patch, if possible.

> @@ -55,18 +55,18 @@ Place only the following kinds of devices directly on the 
> Root Complex:
> pcie.0 bus
> 
> 
>  |||  |
> -   ---   --   --   --
> -   | PCI Dev |   | PCIe Root Port |   | DMI-PCI Bridge |   |  pxb-pcie  |
> -   ---   --   --   --
> +   ---   --   ---   --
> +   | PCI Dev |   | PCIe Root Port |   | PCIE-PCI Bridge |   |  pxb-pcie  |
> +   ---   --   ---   --
>  
>  2.1.1 To plug a device into pcie.0 as a Root Complex Integrated Endpoint use:
>-device [,bus=pcie.0]
>  2.1.2 To expose a new PCI Express Root Bus use:
>-device pxb-pcie,id=pcie.1,bus_nr=x[,numa_node=y][,addr=z]
> -  Only PCI Express Root Ports and DMI-PCI bridges can be connected
> +  Only PCI Express Root Ports, PCIE-PCI bridges and DMI-PCI bridges can 
> be connected

It would be nice if we could keep the flowing text wrapped to 80 chars.

Also, here you add the "PCI Express-PCI" bridge to the list of allowed
controllers (and you keep DMI-PCI as permitted), but above DMI was
replaced. I think these should be made consistent -- we should make up
our minds if we continue to recommend the DMI-PCI bridge or not. If not,
then we should eradicate all traces of it. If we want to keep it at
least for compatibility, then it should remain as fully documented as it
is now.

>to the pcie.1 bus:
>-device 
> ioh3420,id=root_port1[,bus=pcie.1][,chassis=x][,slot=y][,addr=z]  
>\
> -  -device i82801b11-bridge,id=dmi_pci_bridge1,bus=pcie.1
> +  -device pcie-pci-bridge,id=pcie_pci_bridge1,bus=pcie.1
>  
>  
>  2.2 PCI Express only hierarchy
> @@ -130,21 +130,25 @@ Notes:
>  Legacy PCI devices can be plugged into pcie.0 as Integrated Endpoints,
>  but, as mentioned in section 5, doing so means the legacy PCI
>  device in question will be incapable of hot-unplugging.
> -Besides that use DMI-PCI Bridges (i82801b11-bridge) in combination
> +Besides that use PCIE-PCI Bridges (pcie-pci-bridge) in combination
>  with PCI-PCI Bridges (pci-bridge) to start PCI hierarchies.
> +Instead of the PCIE-PCI Bridge DMI-PCI one can be used,
> +but it doens't support hot-plug, is not crossplatform and since that

s/doens't/doesn't/

s/since that/therefore it/

> +is obsolete and deprecated. Use the PCIE-PCI Bridge if you're not 
> +absolutely sure you need the DMI-PCI Bridge.
>  
> -Prefer flat hierarchies. For most scenarios a single DMI-PCI Bridge
> +Prefer flat hierarchies. For most scenarios a single PCIE-PCI Bridge
>  (having 32 slots) and several PCI-PCI Bridges attached to it
>  (each supporting also 32 slots) will support hundreds of legacy devices.
> -The recommendation is to populate one PCI-PCI Bridge under the DMI-PCI Bridge
> +The recommendation is to populate one PCI-PCI Bridge under the PCIE-PCI 
> Bridge
>  until is full and then plug a new PCI-PCI Bridge...
>  
> pcie.0 bus
> --
>  ||
> -   ---   --
> -   | PCI Dev |   | DMI-PCI BRIDGE |
> -   ----
> +   ---   ---
> +   | PCI Dev |   | PCIE-PCI BRIDGE |
> +   -----
> ||
>----
>| PCI-PCI Bridge ||

Re: [SeaBIOS] [seabios PATCH for qemu 2.10 0/2] seabios: build ACPI 1.0-compatible ACPI tables

2017-08-01 Thread Michael S. Tsirkin 
On Wed, Jul 26, 2017 at 11:42:33AM +0200, Paolo Bonzini wrote:
> Old operating systems would like to have a rev1 (ACPI 1.0) FADT, but
> new operating systems would like to have rev3 (ACPI 2.0).
> Since old operating systems do not know about XSDTs, the
> solution is to point the RSDT to a v1 FADT and the XSDT to a
> rev3 FADT.

So I think for 2.10 Igor's patch is much smaller and only
touches QEMU, I'm inclined to merge just that one.
Let's discuss this after 2.10 is out.

> 
> Paolo Bonzini (2):
>   seabios: build RSDT from XSDT
>   seabios: create rev1 FADT in compatibility RSDT
> 
>  src/fw/paravirt.c | 68 
> ++-
>  src/std/acpi.h| 11 +
>  2 files changed, 78 insertions(+), 1 deletion(-)
> 
> -- 
> 2.13.3

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios

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

2017-08-01 Thread Alexander Bezzubikov 
2017-08-01 16:38 GMT+03:00 Marcel Apfelbaum :
> On 31/07/2017 22:01, Alexander Bezzubikov wrote:
>>
>> 2017-07-31 21:57 GMT+03:00 Michael S. Tsirkin :
>>>
>>> On Mon, Jul 31, 2017 at 09:54:55PM +0300, Alexander Bezzubikov wrote:

 2017-07-31 17:09 GMT+03:00 Marcel Apfelbaum :
>
> 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 
>>> ---
>>>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 000..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?
>>>
>>>
>>> I would just use simeple 64 bit registers. PCI spec has an ugly format
>>> with fields spread all over the place but that is because of
>>> compatibility concerns. It makes not sense to spend cycles just
>>> to be similarly messy.
>>
>>
>> Then I suggest to use exactly one field of a maximum possible size
>> for each reserving object, and get rid of mutually exclusive fields.
>> Then it can be something like that (order and names can be changed):
>> u8 bus;
>> u16 non_pref;
>> u32 io;
>> u64 pref;
>>
>
> I think Michael suggested:
>
> u64 bus_res;
> u64 mem_res;
> u64 io_res;
> u64 mem_pref_res;
>
> OR:
> u32 bus_res;
> u32 mem_res;
> u32 io_res;
> u64 mem_pref_res;
>
>
> We can use 0XFFF..F as "not-set" value "merging" Gerd's and Michael's
> requests.

Let's dwell on the second option (with -1 as 'ignore' sign), if no new
objections.

>
> Thanks,
> Marcel
>
>
>>
>>>
>>>
>>
>>
>
> 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

Re: [SeaBIOS] [PATCH v3 2/5] hw/pci: introduce pcie-pci-bridge device

2017-08-01 Thread Marcel Apfelbaum 

On 01/08/2017 18:51, Michael S. Tsirkin wrote:

On Tue, Aug 01, 2017 at 06:45:13PM +0300, Marcel Apfelbaum wrote:

On 01/08/2017 18:32, Michael S. Tsirkin wrote:

On Mon, Jul 31, 2017 at 09:40:41PM +0300, Alexander Bezzubikov wrote:

+typedef struct PCIEPCIBridge {
+/*< private >*/
+PCIBridge parent_obj;
+
+bool msi_enable;



Please rename the msi_enable property to "msi" in order
to be aligned with the existent PCIBridgeDev and
consider making it OnOffAuto for the same reason.
(I am not sure about the last part though, we have
   no meaning for "auto" here)



Agreed about "msi", but OnOffAuto looks weird to me
as we always want MSI to be enabled.




Hi Michael,


Why even have a property then? Can't you enable it unconditionally?



Because of a current bug in Linux kernel:
https://www.spinics.net/lists/linux-pci/msg63052.html
msi will not work until the patch is merged. Even when
it will be merged, not all linux kernels will contain the patch.


You should Cc stable to make sure they all gain it eventually.



Right! thanks, we missed cc-ing stable.
Added stable to the mail thread.
Marcel



Disabling msi is a workaround for the above case.

Thanks,
Marcel


Really enabling MSI without bus master is a bug that I'm not 100% sure
it even worth working around. But I guess it's not too bad to have a
work-around given it's this simple.




___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH v3 2/5] hw/pci: introduce pcie-pci-bridge device

2017-08-01 Thread Michael S. Tsirkin 
On Tue, Aug 01, 2017 at 06:45:13PM +0300, Marcel Apfelbaum wrote:
> On 01/08/2017 18:32, Michael S. Tsirkin wrote:
> > On Mon, Jul 31, 2017 at 09:40:41PM +0300, Alexander Bezzubikov wrote:
> > > > > +typedef struct PCIEPCIBridge {
> > > > > +/*< private >*/
> > > > > +PCIBridge parent_obj;
> > > > > +
> > > > > +bool msi_enable;
> > > > 
> > > > 
> > > > Please rename the msi_enable property to "msi" in order
> > > > to be aligned with the existent PCIBridgeDev and
> > > > consider making it OnOffAuto for the same reason.
> > > > (I am not sure about the last part though, we have
> > > >   no meaning for "auto" here)
> > > > 
> > > 
> > > Agreed about "msi", but OnOffAuto looks weird to me
> > > as we always want MSI to be enabled.
> > 
> 
> Hi Michael,
> 
> > Why even have a property then? Can't you enable it unconditionally?
> > 
> 
> Because of a current bug in Linux kernel:
>https://www.spinics.net/lists/linux-pci/msg63052.html
> msi will not work until the patch is merged. Even when
> it will be merged, not all linux kernels will contain the patch.

You should Cc stable to make sure they all gain it eventually.

> Disabling msi is a workaround for the above case.
> 
> Thanks,
> Marcel

Really enabling MSI without bus master is a bug that I'm not 100% sure
it even worth working around. But I guess it's not too bad to have a
work-around given it's this simple.

-- 
MST

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH v3 2/5] hw/pci: introduce pcie-pci-bridge device

2017-08-01 Thread Marcel Apfelbaum 

On 01/08/2017 18:32, Michael S. Tsirkin wrote:

On Mon, Jul 31, 2017 at 09:40:41PM +0300, Alexander Bezzubikov wrote:

+typedef struct PCIEPCIBridge {
+/*< private >*/
+PCIBridge parent_obj;
+
+bool msi_enable;



Please rename the msi_enable property to "msi" in order
to be aligned with the existent PCIBridgeDev and
consider making it OnOffAuto for the same reason.
(I am not sure about the last part though, we have
  no meaning for "auto" here)



Agreed about "msi", but OnOffAuto looks weird to me
as we always want MSI to be enabled.




Hi Michael,


Why even have a property then? Can't you enable it unconditionally?



Because of a current bug in Linux kernel:
   https://www.spinics.net/lists/linux-pci/msg63052.html
msi will not work until the patch is merged. Even when
it will be merged, not all linux kernels will contain the patch.

Disabling msi is a workaround for the above case.

Thanks,
Marcel

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH v3 2/5] hw/pci: introduce pcie-pci-bridge device

2017-08-01 Thread Michael S. Tsirkin 
On Mon, Jul 31, 2017 at 09:40:41PM +0300, Alexander Bezzubikov wrote:
> >> +typedef struct PCIEPCIBridge {
> >> +/*< private >*/
> >> +PCIBridge parent_obj;
> >> +
> >> +bool msi_enable;
> >
> >
> > Please rename the msi_enable property to "msi" in order
> > to be aligned with the existent PCIBridgeDev and
> > consider making it OnOffAuto for the same reason.
> > (I am not sure about the last part though, we have
> >  no meaning for "auto" here)
> >
> 
> Agreed about "msi", but OnOffAuto looks weird to me
> as we always want MSI to be enabled.

Why even have a property then? Can't you enable it unconditionally?

-- 
MST

___
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios

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

2017-08-01 Thread Marcel Apfelbaum 

On 31/07/2017 22:01, Alexander Bezzubikov wrote:

2017-07-31 21:57 GMT+03:00 Michael S. Tsirkin :

On Mon, Jul 31, 2017 at 09:54:55PM +0300, Alexander Bezzubikov wrote:

2017-07-31 17:09 GMT+03:00 Marcel Apfelbaum :

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


I would just use simeple 64 bit registers. PCI spec has an ugly format
with fields spread all over the place but that is because of
compatibility concerns. It makes not sense to spend cycles just
to be similarly messy.


Then I suggest to use exactly one field of a maximum possible size
for each reserving object, and get rid of mutually exclusive fields.
Then it can be something like that (order and names can be changed):
u8 bus;
u16 non_pref;
u32 io;
u64 pref;



I think Michael suggested:

u64 bus_res;
u64 mem_res;
u64 io_res;
u64 mem_pref_res;

OR:
u32 bus_res;
u32 mem_res;
u32 io_res;
u64 mem_pref_res;


We can use 0XFFF..F as "not-set" value "merging" Gerd's and Michael's
requests.

Thanks,
Marcel











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_QEMU1
+
+
+/* Offsets of QEMU capability fields */
+#define QEMU_PCI_CAP_NON_PREF   4
+#define QEMU_PCI_CAP_BUS_RES6
+#define QEMU_PCI_CAP_IO_8   7
+#define QEMU_PCI_CAP_IO_32  8
+#define QEMU_PCI_CAP_PREF_3212
+#define QEMU_PCI_CAP_PREF_6416
+#define QEMU_PCI_CAP_SIZE   24
+