qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: Marcel Apfelbaum <marcel@redhat.com>,
	Kevin O'Connor <kevin@koconnor.net>
Cc: seabios@seabios.org, qemu-devel@nongnu.org, mst@redhat.com
Subject: Re: [Qemu-devel] [PATCH V2] pci: fixes to allow booting from extra root pci buses.
Date: Thu, 11 Jun 2015 17:00:58 +0200	[thread overview]
Message-ID: <5579A2AA.3030201@redhat.com> (raw)
In-Reply-To: <55799CD6.1060506@redhat.com>

On 06/11/15 16:36, Marcel Apfelbaum wrote:
> On 06/11/2015 05:24 PM, Kevin O'Connor wrote:
>> On Thu, Jun 11, 2015 at 05:12:33PM +0300, Marcel Apfelbaum wrote:
>>> On 06/11/2015 04:58 PM, Kevin O'Connor wrote:
>>>> On Thu, Jun 11, 2015 at 04:37:08PM +0300, Marcel Apfelbaum wrote:
>>>>> The fixes solves the following issue:
>>>>> The PXB device exposes a new  pci root bridge with the
>>>>> fw path:  /pci-root@4/..., in which 4 is the root bus number.
>>>>> Before this patch the fw path was wrongly computed:
>>>>>      /pci-root@1/pci@i0cf8/...
>>>>> Fix the above issues: Correct the bus number and remove the
>>>>> extra host bridge description.
>>>>
>>>> Why is that wrong?  The previous path looks correct to me.
>>> The prev path includes both the extra root bridge and *then* the
>>> usual host bridge.
>>>   /pci-root@1/pci@i0cf8/   ...
>>>      ^ new       ^ regular  ^ devices
>>>
>>> Since the new pci root bridge (and bus) is on "paralel" with the
>>> regular one.
>>> it is not correct to add it to the path.
>>>
>>> The architecture is:
>>>   /<host bridge>/devices...
>>>   /extra root bridge/devices...
>>>   /extra root bridge/devices...
>>> And not
>>> /extra root bridge//<host bridge>/devices
>>
>> Your patch changed both the "/extra root bridge/devices..." part and
>> the "@1" part.  The change of the "@1" in "/pci-root@1/" is not
>> correct IMO.
> Why? @1 should be the unit address which is the text representation
> of the physical address, in our case the slot. Since the bus number
> in our case is 4, I think /pci-root@4/ is the 'correct' address.
>>
>> Does open-firmware have any examples for PCI paths and in particular
>> PCI paths when there are multiple root-buses?
> Maybe Laszlo can say more, but we both agreed that this would be the
> berst representation of extra root buses on both OVMF and Seabios.

The

  PCI Bus Binding to:
  IEEE Std 1275-1994 Standard for Boot
  (Initialization Configuration) Firmware

document (binding) does speak about this, as far as I can see, in

  2.2.1. Physical Address Formats

It first gives a "Numerical Representation" in device tree format (same
thing as in DTB / FDT), and then a "Text Representation" with references
to "Numerical Representation". It is *completely* Greek to me. It took
me minutes of staring just to vaguely understand how the current

  i0cf8

unit address comes together.

I've always treated the OFW devpaths that QEMU generates only
*syntactically* conformant to the (base) OFW spec, and never considered
the particular bindings 100% binding. That said, if someone finds where
the PCI binding defines unit addresses for *root* buses, please let me
know, just for reference.

>> It's possible to replace the "pci@i0cf8" with "pci-root@1" but that
>> seems odd as the extra root bus is accessible via io accesses to
>> 0x0cf8.
> While this is true, /pci-root@[...]/ may represent also other kind of host
> bridges not only PXBs. But we can change this of course, as long as OVMF
> can also
> work with it.
> 
>>
>> Another option would be to place the pci-root@1 behind the pci@i0cf8
>> as in "/pci@i0cf8/pci-root@1/...".  Or, the root bus could be appended
>> to the host bridge as in "/pci@i0cf8,1/...".
> The latest representation makes sense to me,  but "/pci@i0cf8,4/...",
> after comma
> the bus number.
> 
> Laszlo, will this work for OVMF?

With the v3 patchset for QEMU, we could probably easily generate the
"i0cf8,4" unit address inside the PXB device model itself. (Of course
exactly what number should stand after the comma remains a question.)

Parsing it in OVMF is doable, albeit somewhat ugly.

In any case, I'm not convinced at all why this is a better idea than the
proposal in this patch.

Laszlo

  reply	other threads:[~2015-06-11 15:01 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-11 13:37 [Qemu-devel] [PATCH V2] pci: fixes to allow booting from extra root pci buses Marcel Apfelbaum
2015-06-11 13:57 ` Laszlo Ersek
2015-06-11 13:58 ` Kevin O'Connor
2015-06-11 14:12   ` Marcel Apfelbaum
2015-06-11 14:24     ` Kevin O'Connor
2015-06-11 14:36       ` Marcel Apfelbaum
2015-06-11 15:00         ` Laszlo Ersek [this message]
2015-06-11 16:54         ` Kevin O'Connor
2015-06-11 17:46           ` Marcel Apfelbaum
2015-06-11 18:34             ` Laszlo Ersek
2015-06-11 19:24               ` Kevin O'Connor
2015-06-12  9:25                 ` Laszlo Ersek
2015-06-12 13:03                   ` Kevin O'Connor
2015-06-12 15:45                     ` Laszlo Ersek
2015-06-12 18:40                       ` Kevin O'Connor
2015-06-12 20:13                         ` Laszlo Ersek
2015-06-14 12:05                         ` Michael S. Tsirkin
2015-06-14 14:50                           ` Kevin O'Connor
2015-06-14 18:06                             ` Michael S. Tsirkin
2015-06-14 18:21                               ` Kevin O'Connor
2015-06-14 21:39                               ` Benjamin Herrenschmidt
2015-06-14 21:59                                 ` Kevin O'Connor
2015-06-15  2:50                                   ` Benjamin Herrenschmidt
2015-06-15  8:22                                     ` Michael S. Tsirkin
2015-06-11 19:10             ` Kevin O'Connor
2015-06-12  6:00               ` Gerd Hoffmann
2015-06-12 12:17                 ` Marcel Apfelbaum
2015-06-12 13:23                   ` Kevin O'Connor
2015-06-15  6:01                     ` Gerd Hoffmann
2015-06-15  6:50                       ` Gerd Hoffmann
2015-06-15  9:02                         ` Marcel Apfelbaum
2015-06-15  9:43                       ` Michael S. Tsirkin
2015-06-15 10:18                         ` Gerd Hoffmann
2015-06-15 10:26                           ` Michael S. Tsirkin
2015-06-11 14:35   ` Laszlo Ersek
2015-06-11 16:48     ` Kevin O'Connor
2015-06-11 18:38       ` [Qemu-devel] [SeaBIOS] " Laszlo Ersek
2015-06-14 12:10       ` [Qemu-devel] " Michael S. Tsirkin
2015-06-14 13:47         ` Kevin O'Connor

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5579A2AA.3030201@redhat.com \
    --to=lersek@redhat.com \
    --cc=kevin@koconnor.net \
    --cc=marcel@redhat.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=seabios@seabios.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).