Re: [Qemu-devel] [PATCH v6 7/7] hw/pci-bridge: format SeaBIOS-compliant OFW device node for PXB

[Laszlo Ersek]

On 06/17/15 23:50, Michael S. Tsirkin wrote:
> On Wed, Jun 17, 2015 at 09:44:07PM +0200, Laszlo Ersek wrote:
>> On 06/17/15 21:32, Michael S. Tsirkin wrote:
>>> On Wed, Jun 17, 2015 at 03:28:44PM -0400, Kevin O'Connor wrote:
>>>> On Wed, Jun 17, 2015 at 09:15:24PM +0200, Laszlo Ersek wrote:
>>>>> On 06/17/15 20:54, Michael S. Tsirkin wrote:
>>>>>> Right. But what I was discussing is a different issue.  The point is
>>>>>> that it does not make sense to have /address@hidden under two 
>>>>>> hierarchies:
>>>>>> it's the same register.  What happens is that you access /address@hidden 
>>>>>> and
>>>>>> then *through that* you access another pci root.  Not the other way
>>>>>> around.  The proposal thus is to switch to 
>>>>>> /address@hidden/address@hidden in
>>>>>> seabios,
>>>>>
>>>>> For me this is still Question 1 -- 'everything in that pattern that is
>>>>> not "N"'.
>>>>>
>>>>> You seem to care about the *semantics* of that OFW device path fragment.
>>>>> I don't. First, the relevant IEEE spec is prohibitively hard for me to
>>>>> interpret semantically. Second, there is no known firmware that actually
>>>>> looks at the "i0cf8" unit-address term and decides *based on that term*
>>>>> that it has to talk PCI via 0xCF8 and 0xCFC. In other words, the current
>>>>> second node is entirely opaque in my interpretation.
>>>>>
>>>>>> unconditionally - not if (QEMU).
>>>>>
>>>>> This might qualify as some kind of semantic cleanup, but it will
>>>>> nonetheless break the SeaBIOS boot options expressed in OFW notation
>>>>> that are already persistently stored in cbfs, on physical machines. (As
>>>>> far as I understood.) It might not break the Coreboot-SeaBIOS interface,
>>>>> but it might invalidate preexistent entries that exist in the prior form
>>>>> (wherever they exist on physical hardware).
>>>>>
>>>>>> And I thought Kevin agreed
>>>>>> it's a good idea.
>>>>>>
>>>>>> Kevin - is this a good summary of your opinion?
>>>>>
>>>>> Kevin, please do answer.
>>>>
>>>> It is true that it would "invalidate preexistent entries" for
>>>> coreboot/seabios users that upgrade, but I think that is manageable.
>>>> So I defer the syntax discussion and decisions to the QEMU developers
>>>> that are doing the bulk of the work.
>>>>
>>>> -Kevin
>>>
>>> I'm fine with either /address@hidden,%x or /address@hidden/address@hidden, 
>>> with a
>>> slight preference to the later - in particular it's easier
>>> to implement in QEMU.
>>>
>>> It means old bios won't boot from a pxb, but I think that's
>>> manageable - it works otherwise.
>>
>> I don't understand -- the second option you named
>> ("/address@hidden/address@hidden") is what this patch implements, and "old" 
>> (ie.
>> current) SeaBIOS does boot from it.
>>
>> Laszlo
> 
> Ouch. I meant /address@hidden//address@hidden
> As you see, it's confusing.

If you insist on /address@hidden/address@hidden, then all of SeaBIOS, QEMU, and
OVMF must be (further) modified. Please confirm if this is what you'd like.

(As I said, IMO this change is not warranted for; it just replaces one
opaque string with another opaque string, without semantic effects, but
it causes extra churn and even requires a patch for SeaBIOS.)

Laszlo