diff mbox

[Qemu-ppc,56/72] PPC: e500: Use new MPIC dt format

Message ID 61CF2032-367D-4F3A-A845-BB92B35C334B@suse.de
State New
Headers show

Commit Message

Alexander Graf Aug. 8, 2012, 9:16 p.m. UTC
On 24.06.2012, at 01:07, Alexander Graf wrote:

> Due to popular demand, we're updating the way we generate the MPIC
> node and interrupt lines based on what the current state of art is.
> 
> Requested-by: Scott Wood <scottwood@freescale.com>
> Signed-off-by: Alexander Graf <agraf@suse.de>

Hey Scott,

This patch breaks SMP for me. The reason for the breakage is that Linux does some things differently when it finds an fsl,mpic instead of a generic openpic. I have assembled logs between a working version (compatible openpic) and a broken version (compatible fsl,mpic) with guest and host debug turned on.

Maybe you have an idea what's going wrong.


Alex

Comments

Scott Wood Aug. 8, 2012, 10:40 p.m. UTC | #1
On 08/08/2012 04:16 PM, Alexander Graf wrote:
> 
> On 24.06.2012, at 01:07, Alexander Graf wrote:
> 
>> Due to popular demand, we're updating the way we generate the MPIC
>> node and interrupt lines based on what the current state of art is.
>>
>> Requested-by: Scott Wood <scottwood@freescale.com>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
> 
> Hey Scott,
> 
> This patch breaks SMP for me. The reason for the breakage is that
> Linux does some things differently when it finds an fsl,mpic instead
> of a generic openpic. I have assembled logs between a working version
> (compatible openpic) and a broken version (compatible fsl,mpic) with
> guest and host debug turned on.
> 
> Maybe you have an idea what's going wrong.

IIRC QEMU is missing support for large vectors, which is probably
breaking IPIs.  A recent change to Linux has it assuming it can use
large vectors when it sees fsl,mpic, as we're running out of vectors (on
p4080 MSIs collided with the arbitrarily chosen timer vector, and on
t4240 the normal internal interrupts alone go beyond 256).

We need to get the enhancements from our internal KVM MPIC back into QEMU.

-Scott
Alexander Graf Aug. 9, 2012, 8:48 p.m. UTC | #2
On 09.08.2012, at 00:40, Scott Wood wrote:

> On 08/08/2012 04:16 PM, Alexander Graf wrote:
>> 
>> On 24.06.2012, at 01:07, Alexander Graf wrote:
>> 
>>> Due to popular demand, we're updating the way we generate the MPIC
>>> node and interrupt lines based on what the current state of art is.
>>> 
>>> Requested-by: Scott Wood <scottwood@freescale.com>
>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> 
>> Hey Scott,
>> 
>> This patch breaks SMP for me. The reason for the breakage is that
>> Linux does some things differently when it finds an fsl,mpic instead
>> of a generic openpic. I have assembled logs between a working version
>> (compatible openpic) and a broken version (compatible fsl,mpic) with
>> guest and host debug turned on.
>> 
>> Maybe you have an idea what's going wrong.
> 
> IIRC QEMU is missing support for large vectors, which is probably
> breaking IPIs.  A recent change to Linux has it assuming it can use
> large vectors when it sees fsl,mpic, as we're running out of vectors (on
> p4080 MSIs collided with the arbitrarily chosen timer vector, and on
> t4240 the normal internal interrupts alone go beyond 256).
> 
> We need to get the enhancements from our internal KVM MPIC back into QEMU.

Ok, so the quick fix for 1.2 would be to revert to the old compatible name. Can we leave the 4-field interrupt numbers or do we need to revert the whole patch?


Alex
Scott Wood Aug. 9, 2012, 8:50 p.m. UTC | #3
On 08/09/2012 03:48 PM, Alexander Graf wrote:
> 
> On 09.08.2012, at 00:40, Scott Wood wrote:
> 
>> On 08/08/2012 04:16 PM, Alexander Graf wrote:
>>>
>>> On 24.06.2012, at 01:07, Alexander Graf wrote:
>>>
>>>> Due to popular demand, we're updating the way we generate the MPIC
>>>> node and interrupt lines based on what the current state of art is.
>>>>
>>>> Requested-by: Scott Wood <scottwood@freescale.com>
>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>
>>> Hey Scott,
>>>
>>> This patch breaks SMP for me. The reason for the breakage is that
>>> Linux does some things differently when it finds an fsl,mpic instead
>>> of a generic openpic. I have assembled logs between a working version
>>> (compatible openpic) and a broken version (compatible fsl,mpic) with
>>> guest and host debug turned on.
>>>
>>> Maybe you have an idea what's going wrong.
>>
>> IIRC QEMU is missing support for large vectors, which is probably
>> breaking IPIs.  A recent change to Linux has it assuming it can use
>> large vectors when it sees fsl,mpic, as we're running out of vectors (on
>> p4080 MSIs collided with the arbitrarily chosen timer vector, and on
>> t4240 the normal internal interrupts alone go beyond 256).
>>
>> We need to get the enhancements from our internal KVM MPIC back into QEMU.
> 
> Ok, so the quick fix for 1.2 would be to revert to the old compatible
> name. Can we leave the 4-field interrupt numbers or do we need to
> revert the whole patch?

In theory you shouldn't have 4-cell interrupt numbers without fsl,mpic,
but I don't think it will actually break in Linux -- the extra cells
should just be ignored.

-Scott
Alexander Graf Aug. 9, 2012, 8:52 p.m. UTC | #4
On 09.08.2012, at 22:50, Scott Wood wrote:

> On 08/09/2012 03:48 PM, Alexander Graf wrote:
>> 
>> On 09.08.2012, at 00:40, Scott Wood wrote:
>> 
>>> On 08/08/2012 04:16 PM, Alexander Graf wrote:
>>>> 
>>>> On 24.06.2012, at 01:07, Alexander Graf wrote:
>>>> 
>>>>> Due to popular demand, we're updating the way we generate the MPIC
>>>>> node and interrupt lines based on what the current state of art is.
>>>>> 
>>>>> Requested-by: Scott Wood <scottwood@freescale.com>
>>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>> 
>>>> Hey Scott,
>>>> 
>>>> This patch breaks SMP for me. The reason for the breakage is that
>>>> Linux does some things differently when it finds an fsl,mpic instead
>>>> of a generic openpic. I have assembled logs between a working version
>>>> (compatible openpic) and a broken version (compatible fsl,mpic) with
>>>> guest and host debug turned on.
>>>> 
>>>> Maybe you have an idea what's going wrong.
>>> 
>>> IIRC QEMU is missing support for large vectors, which is probably
>>> breaking IPIs.  A recent change to Linux has it assuming it can use
>>> large vectors when it sees fsl,mpic, as we're running out of vectors (on
>>> p4080 MSIs collided with the arbitrarily chosen timer vector, and on
>>> t4240 the normal internal interrupts alone go beyond 256).
>>> 
>>> We need to get the enhancements from our internal KVM MPIC back into QEMU.
>> 
>> Ok, so the quick fix for 1.2 would be to revert to the old compatible
>> name. Can we leave the 4-field interrupt numbers or do we need to
>> revert the whole patch?
> 
> In theory you shouldn't have 4-cell interrupt numbers without fsl,mpic,
> but I don't think it will actually break in Linux -- the extra cells
> should just be ignored.

So I suppose the best would be to revert the whole patch and simply go back to the old format then, to make sure we don't introduce more oddness for non-Linux guests (not that I'm aware we're capable of running any, especially any that would use the dtb).

Once we have working large vectors in our MPIC emulation, we can easily put the patch into place again and generate dt's that show an fsl mpic.


Alex
Scott Wood Aug. 9, 2012, 8:58 p.m. UTC | #5
On 08/09/2012 03:52 PM, Alexander Graf wrote:
> 
> On 09.08.2012, at 22:50, Scott Wood wrote:
> 
>> On 08/09/2012 03:48 PM, Alexander Graf wrote:
>>>
>>> On 09.08.2012, at 00:40, Scott Wood wrote:
>>>
>>>> On 08/08/2012 04:16 PM, Alexander Graf wrote:
>>>>>
>>>>> On 24.06.2012, at 01:07, Alexander Graf wrote:
>>>>>
>>>>>> Due to popular demand, we're updating the way we generate the MPIC
>>>>>> node and interrupt lines based on what the current state of art is.
>>>>>>
>>>>>> Requested-by: Scott Wood <scottwood@freescale.com>
>>>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>>>
>>>>> Hey Scott,
>>>>>
>>>>> This patch breaks SMP for me. The reason for the breakage is that
>>>>> Linux does some things differently when it finds an fsl,mpic instead
>>>>> of a generic openpic. I have assembled logs between a working version
>>>>> (compatible openpic) and a broken version (compatible fsl,mpic) with
>>>>> guest and host debug turned on.
>>>>>
>>>>> Maybe you have an idea what's going wrong.
>>>>
>>>> IIRC QEMU is missing support for large vectors, which is probably
>>>> breaking IPIs.  A recent change to Linux has it assuming it can use
>>>> large vectors when it sees fsl,mpic, as we're running out of vectors (on
>>>> p4080 MSIs collided with the arbitrarily chosen timer vector, and on
>>>> t4240 the normal internal interrupts alone go beyond 256).
>>>>
>>>> We need to get the enhancements from our internal KVM MPIC back into QEMU.
>>>
>>> Ok, so the quick fix for 1.2 would be to revert to the old compatible
>>> name. Can we leave the 4-field interrupt numbers or do we need to
>>> revert the whole patch?
>>
>> In theory you shouldn't have 4-cell interrupt numbers without fsl,mpic,
>> but I don't think it will actually break in Linux -- the extra cells
>> should just be ignored.
> 
> So I suppose the best would be to revert the whole patch and simply
> go back to the old format then, to make sure we don't introduce more
> oddness for non-Linux guests (not that I'm aware we're capable of
> running any, especially any that would use the dtb).
> 
> Once we have working large vectors in our MPIC emulation, we can
> easily put the patch into place again and generate dt's that show an
> fsl mpic.

Additionally, we should consider adding extra compatibles with the major
QEMU version in them, so that QEMU-aware target code can work around
QEMU limitations even if it's been fixed in a more recent QEMU.

I think this is what you were getting at in the e500 platform
discussion, when you pointed at the PC versioning, but it's not about
documenting semantics so much as identifying the actual implementation.

-Scott
Alexander Graf Aug. 9, 2012, 9:01 p.m. UTC | #6
On 09.08.2012, at 22:58, Scott Wood wrote:

> On 08/09/2012 03:52 PM, Alexander Graf wrote:
>> 
>> On 09.08.2012, at 22:50, Scott Wood wrote:
>> 
>>> On 08/09/2012 03:48 PM, Alexander Graf wrote:
>>>> 
>>>> On 09.08.2012, at 00:40, Scott Wood wrote:
>>>> 
>>>>> On 08/08/2012 04:16 PM, Alexander Graf wrote:
>>>>>> 
>>>>>> On 24.06.2012, at 01:07, Alexander Graf wrote:
>>>>>> 
>>>>>>> Due to popular demand, we're updating the way we generate the MPIC
>>>>>>> node and interrupt lines based on what the current state of art is.
>>>>>>> 
>>>>>>> Requested-by: Scott Wood <scottwood@freescale.com>
>>>>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>>>> 
>>>>>> Hey Scott,
>>>>>> 
>>>>>> This patch breaks SMP for me. The reason for the breakage is that
>>>>>> Linux does some things differently when it finds an fsl,mpic instead
>>>>>> of a generic openpic. I have assembled logs between a working version
>>>>>> (compatible openpic) and a broken version (compatible fsl,mpic) with
>>>>>> guest and host debug turned on.
>>>>>> 
>>>>>> Maybe you have an idea what's going wrong.
>>>>> 
>>>>> IIRC QEMU is missing support for large vectors, which is probably
>>>>> breaking IPIs.  A recent change to Linux has it assuming it can use
>>>>> large vectors when it sees fsl,mpic, as we're running out of vectors (on
>>>>> p4080 MSIs collided with the arbitrarily chosen timer vector, and on
>>>>> t4240 the normal internal interrupts alone go beyond 256).
>>>>> 
>>>>> We need to get the enhancements from our internal KVM MPIC back into QEMU.
>>>> 
>>>> Ok, so the quick fix for 1.2 would be to revert to the old compatible
>>>> name. Can we leave the 4-field interrupt numbers or do we need to
>>>> revert the whole patch?
>>> 
>>> In theory you shouldn't have 4-cell interrupt numbers without fsl,mpic,
>>> but I don't think it will actually break in Linux -- the extra cells
>>> should just be ignored.
>> 
>> So I suppose the best would be to revert the whole patch and simply
>> go back to the old format then, to make sure we don't introduce more
>> oddness for non-Linux guests (not that I'm aware we're capable of
>> running any, especially any that would use the dtb).
>> 
>> Once we have working large vectors in our MPIC emulation, we can
>> easily put the patch into place again and generate dt's that show an
>> fsl mpic.
> 
> Additionally, we should consider adding extra compatibles with the major
> QEMU version in them, so that QEMU-aware target code can work around
> QEMU limitations even if it's been fixed in a more recent QEMU.
> 
> I think this is what you were getting at in the e500 platform
> discussion, when you pointed at the PC versioning, but it's not about
> documenting semantics so much as identifying the actual implementation.

Yes, -M e500-1.2 should expose chrp,open-pic while -M e500 should expose fsl,mpic. We also need to make the MPIC code capabilities conditional here, so that large vectors are only supported when the machine requests them.


Alex
Scott Wood Aug. 9, 2012, 9:11 p.m. UTC | #7
On 08/09/2012 04:01 PM, Alexander Graf wrote:
> 
> On 09.08.2012, at 22:58, Scott Wood wrote:
> 
>> On 08/09/2012 03:52 PM, Alexander Graf wrote:
>>>
>>> On 09.08.2012, at 22:50, Scott Wood wrote:
>>>
>>>> On 08/09/2012 03:48 PM, Alexander Graf wrote:
>>>>>
>>>>> On 09.08.2012, at 00:40, Scott Wood wrote:
>>>>>
>>>>>> On 08/08/2012 04:16 PM, Alexander Graf wrote:
>>>>>>>
>>>>>>> On 24.06.2012, at 01:07, Alexander Graf wrote:
>>>>>>>
>>>>>>>> Due to popular demand, we're updating the way we generate the MPIC
>>>>>>>> node and interrupt lines based on what the current state of art is.
>>>>>>>>
>>>>>>>> Requested-by: Scott Wood <scottwood@freescale.com>
>>>>>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>>>>>
>>>>>>> Hey Scott,
>>>>>>>
>>>>>>> This patch breaks SMP for me. The reason for the breakage is that
>>>>>>> Linux does some things differently when it finds an fsl,mpic instead
>>>>>>> of a generic openpic. I have assembled logs between a working version
>>>>>>> (compatible openpic) and a broken version (compatible fsl,mpic) with
>>>>>>> guest and host debug turned on.
>>>>>>>
>>>>>>> Maybe you have an idea what's going wrong.
>>>>>>
>>>>>> IIRC QEMU is missing support for large vectors, which is probably
>>>>>> breaking IPIs.  A recent change to Linux has it assuming it can use
>>>>>> large vectors when it sees fsl,mpic, as we're running out of vectors (on
>>>>>> p4080 MSIs collided with the arbitrarily chosen timer vector, and on
>>>>>> t4240 the normal internal interrupts alone go beyond 256).
>>>>>>
>>>>>> We need to get the enhancements from our internal KVM MPIC back into QEMU.
>>>>>
>>>>> Ok, so the quick fix for 1.2 would be to revert to the old compatible
>>>>> name. Can we leave the 4-field interrupt numbers or do we need to
>>>>> revert the whole patch?
>>>>
>>>> In theory you shouldn't have 4-cell interrupt numbers without fsl,mpic,
>>>> but I don't think it will actually break in Linux -- the extra cells
>>>> should just be ignored.
>>>
>>> So I suppose the best would be to revert the whole patch and simply
>>> go back to the old format then, to make sure we don't introduce more
>>> oddness for non-Linux guests (not that I'm aware we're capable of
>>> running any, especially any that would use the dtb).
>>>
>>> Once we have working large vectors in our MPIC emulation, we can
>>> easily put the patch into place again and generate dt's that show an
>>> fsl mpic.
>>
>> Additionally, we should consider adding extra compatibles with the major
>> QEMU version in them, so that QEMU-aware target code can work around
>> QEMU limitations even if it's been fixed in a more recent QEMU.
>>
>> I think this is what you were getting at in the e500 platform
>> discussion, when you pointed at the PC versioning, but it's not about
>> documenting semantics so much as identifying the actual implementation.
> 
> Yes, -M e500-1.2 should expose chrp,open-pic while -M e500 should expose fsl,mpic.

We could do that too if the chrp,open-pic version actually makes it into
a release before we fix fsl,mpic (I don't know what the release schedule
is), but what I meant was that the device tree should have something like
compatible = "qemu,1.2-chrp-openpic", "chrp,open-pic"
or
compatible = "qemu,1.3-fsl-mpic", "fsl,mpic"

...so that we can run new kernels on old QEMUs, not just the other way
around.

> We also need to make the MPIC code capabilities conditional here, so
> that large vectors are only supported when the machine requests
> them.

Maybe, but the risk is minimal (target software would have to be setting
those bits to non-zero and expecting them to be ignored) and the
potential for making (more of) a mess of the code is high if we
conditionalize everything.

Are there any currently supported machines where real hardware doesn't
have large vectors?

-Scott
Alexander Graf Aug. 9, 2012, 9:19 p.m. UTC | #8
On 09.08.2012, at 23:11, Scott Wood wrote:

> On 08/09/2012 04:01 PM, Alexander Graf wrote:
>> 
>> On 09.08.2012, at 22:58, Scott Wood wrote:
>> 
>>> On 08/09/2012 03:52 PM, Alexander Graf wrote:
>>>> 
>>>> On 09.08.2012, at 22:50, Scott Wood wrote:
>>>> 
>>>>> On 08/09/2012 03:48 PM, Alexander Graf wrote:
>>>>>> 
>>>>>> On 09.08.2012, at 00:40, Scott Wood wrote:
>>>>>> 
>>>>>>> On 08/08/2012 04:16 PM, Alexander Graf wrote:
>>>>>>>> 
>>>>>>>> On 24.06.2012, at 01:07, Alexander Graf wrote:
>>>>>>>> 
>>>>>>>>> Due to popular demand, we're updating the way we generate the MPIC
>>>>>>>>> node and interrupt lines based on what the current state of art is.
>>>>>>>>> 
>>>>>>>>> Requested-by: Scott Wood <scottwood@freescale.com>
>>>>>>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>>>>>> 
>>>>>>>> Hey Scott,
>>>>>>>> 
>>>>>>>> This patch breaks SMP for me. The reason for the breakage is that
>>>>>>>> Linux does some things differently when it finds an fsl,mpic instead
>>>>>>>> of a generic openpic. I have assembled logs between a working version
>>>>>>>> (compatible openpic) and a broken version (compatible fsl,mpic) with
>>>>>>>> guest and host debug turned on.
>>>>>>>> 
>>>>>>>> Maybe you have an idea what's going wrong.
>>>>>>> 
>>>>>>> IIRC QEMU is missing support for large vectors, which is probably
>>>>>>> breaking IPIs.  A recent change to Linux has it assuming it can use
>>>>>>> large vectors when it sees fsl,mpic, as we're running out of vectors (on
>>>>>>> p4080 MSIs collided with the arbitrarily chosen timer vector, and on
>>>>>>> t4240 the normal internal interrupts alone go beyond 256).
>>>>>>> 
>>>>>>> We need to get the enhancements from our internal KVM MPIC back into QEMU.
>>>>>> 
>>>>>> Ok, so the quick fix for 1.2 would be to revert to the old compatible
>>>>>> name. Can we leave the 4-field interrupt numbers or do we need to
>>>>>> revert the whole patch?
>>>>> 
>>>>> In theory you shouldn't have 4-cell interrupt numbers without fsl,mpic,
>>>>> but I don't think it will actually break in Linux -- the extra cells
>>>>> should just be ignored.
>>>> 
>>>> So I suppose the best would be to revert the whole patch and simply
>>>> go back to the old format then, to make sure we don't introduce more
>>>> oddness for non-Linux guests (not that I'm aware we're capable of
>>>> running any, especially any that would use the dtb).
>>>> 
>>>> Once we have working large vectors in our MPIC emulation, we can
>>>> easily put the patch into place again and generate dt's that show an
>>>> fsl mpic.
>>> 
>>> Additionally, we should consider adding extra compatibles with the major
>>> QEMU version in them, so that QEMU-aware target code can work around
>>> QEMU limitations even if it's been fixed in a more recent QEMU.
>>> 
>>> I think this is what you were getting at in the e500 platform
>>> discussion, when you pointed at the PC versioning, but it's not about
>>> documenting semantics so much as identifying the actual implementation.
>> 
>> Yes, -M e500-1.2 should expose chrp,open-pic while -M e500 should expose fsl,mpic.
> 
> We could do that too if the chrp,open-pic version actually makes it into
> a release before we fix fsl,mpic (I don't know what the release schedule

We are past soft freeze, hard feature freeze is coming up next week.

> is), but what I meant was that the device tree should have something like
> compatible = "qemu,1.2-chrp-openpic", "chrp,open-pic"
> or
> compatible = "qemu,1.3-fsl-mpic", "fsl,mpic"
> 
> ...so that we can run new kernels on old QEMUs, not just the other way
> around.

Why would we bother?

> 
>> We also need to make the MPIC code capabilities conditional here, so
>> that large vectors are only supported when the machine requests
>> them.
> 
> Maybe, but the risk is minimal (target software would have to be setting
> those bits to non-zero and expecting them to be ignored) and the
> potential for making (more of) a mess of the code is high if we
> conditionalize everything.
> 
> Are there any currently supported machines where real hardware doesn't
> have large vectors?

I'm not sure what bamboo would support in real hw. Same for the frankenstein-U3 we expose.
I agree that risk looks low for now, but let's evaluate it when I see the patch that actually changes things.


Alex
Scott Wood Aug. 9, 2012, 9:28 p.m. UTC | #9
On 08/09/2012 04:19 PM, Alexander Graf wrote:
> 
> On 09.08.2012, at 23:11, Scott Wood wrote:
> 
>> On 08/09/2012 04:01 PM, Alexander Graf wrote:
>>>
>>> On 09.08.2012, at 22:58, Scott Wood wrote:
>>>> Additionally, we should consider adding extra compatibles with the major
>>>> QEMU version in them, so that QEMU-aware target code can work around
>>>> QEMU limitations even if it's been fixed in a more recent QEMU.
>>>>
>>>> I think this is what you were getting at in the e500 platform
>>>> discussion, when you pointed at the PC versioning, but it's not about
>>>> documenting semantics so much as identifying the actual implementation.
>>>
>>> Yes, -M e500-1.2 should expose chrp,open-pic while -M e500 should expose fsl,mpic.
>>
>> We could do that too if the chrp,open-pic version actually makes it into
>> a release before we fix fsl,mpic (I don't know what the release schedule
> 
> We are past soft freeze, hard feature freeze is coming up next week.
> 
>> is), but what I meant was that the device tree should have something like
>> compatible = "qemu,1.2-chrp-openpic", "chrp,open-pic"
>> or
>> compatible = "qemu,1.3-fsl-mpic", "fsl,mpic"
>>
>> ...so that we can run new kernels on old QEMUs, not just the other way
>> around.
> 
> Why would we bother?

Why not?  It's much easier than the other way around -- it's just an
extra string in the device tree.  It's up to someone who cares to put
relevant workarounds in the kernel.

Compare it to real hardware, where we expect new kernels to run on old
hardware but not the other way around, and where we expect to be able to
identify the specific hardware we're running on (there are version
registers in the MPIC for identifying real hardware version, but not
QEMU implementation version).

If we had this, we could have avoided needing to postpone fsl,mpic,
because I think the Linux patch using large vectors hasn't been released
yet, so we could have added a check for qemu,1.2-fsl-mpic and known that
it didn't include large vectors.  I guess it's still not too late...

It would also be useful for overriding SVR checks in drivers (for errata
and such), which can get particularly awkward if you mix some emulated
devices with some directly assigned devices (no one SVR will be right
for all of them, even if all the emulated devices faithfully emulate one
SoC's behavior).

-Scott
Alexander Graf Aug. 9, 2012, 9:36 p.m. UTC | #10
On 09.08.2012, at 23:28, Scott Wood wrote:

> On 08/09/2012 04:19 PM, Alexander Graf wrote:
>> 
>> On 09.08.2012, at 23:11, Scott Wood wrote:
>> 
>>> On 08/09/2012 04:01 PM, Alexander Graf wrote:
>>>> 
>>>> On 09.08.2012, at 22:58, Scott Wood wrote:
>>>>> Additionally, we should consider adding extra compatibles with the major
>>>>> QEMU version in them, so that QEMU-aware target code can work around
>>>>> QEMU limitations even if it's been fixed in a more recent QEMU.
>>>>> 
>>>>> I think this is what you were getting at in the e500 platform
>>>>> discussion, when you pointed at the PC versioning, but it's not about
>>>>> documenting semantics so much as identifying the actual implementation.
>>>> 
>>>> Yes, -M e500-1.2 should expose chrp,open-pic while -M e500 should expose fsl,mpic.
>>> 
>>> We could do that too if the chrp,open-pic version actually makes it into
>>> a release before we fix fsl,mpic (I don't know what the release schedule
>> 
>> We are past soft freeze, hard feature freeze is coming up next week.
>> 
>>> is), but what I meant was that the device tree should have something like
>>> compatible = "qemu,1.2-chrp-openpic", "chrp,open-pic"
>>> or
>>> compatible = "qemu,1.3-fsl-mpic", "fsl,mpic"
>>> 
>>> ...so that we can run new kernels on old QEMUs, not just the other way
>>> around.
>> 
>> Why would we bother?
> 
> Why not?  It's much easier than the other way around -- it's just an
> extra string in the device tree.  It's up to someone who cares to put
> relevant workarounds in the kernel.
> 
> Compare it to real hardware, where we expect new kernels to run on old
> hardware but not the other way around, and where we expect to be able to
> identify the specific hardware we're running on (there are version
> registers in the MPIC for identifying real hardware version, but not
> QEMU implementation version).
> 
> If we had this, we could have avoided needing to postpone fsl,mpic,
> because I think the Linux patch using large vectors hasn't been released
> yet, so we could have added a check for qemu,1.2-fsl-mpic and known that
> it didn't include large vectors.  I guess it's still not too late...

I'm not sure I like the idea. Usually, in the real hardware world, erratas happen on the board level, right? So we could have a compatible "qemu,1.2-e500" in the board compatible. But having one for every device sounds a bit too much to me.

Also, not implementing large vectors essentially mean we don't emulate an fsl,mpic well. That's fine, let's just not tell the guest we do. I don't think we should teach Linux about broken emulation. I'd rather see the emulation fixed.

> It would also be useful for overriding SVR checks in drivers (for errata
> and such), which can get particularly awkward if you mix some emulated
> devices with some directly assigned devices (no one SVR will be right
> for all of them, even if all the emulated devices faithfully emulate one
> SoC's behavior).

Hmm. Yes. Maybe it'd be good to just never check SVR but instead always rely on dt properties?


Alex
Scott Wood Aug. 9, 2012, 9:45 p.m. UTC | #11
On 08/09/2012 04:36 PM, Alexander Graf wrote:
> 
> On 09.08.2012, at 23:28, Scott Wood wrote:
> 
>> On 08/09/2012 04:19 PM, Alexander Graf wrote:
>>>
>>> On 09.08.2012, at 23:11, Scott Wood wrote:
>>>> ...so that we can run new kernels on old QEMUs, not just the other way
>>>> around.
>>>
>>> Why would we bother?
>>
>> Why not?  It's much easier than the other way around -- it's just an
>> extra string in the device tree.  It's up to someone who cares to put
>> relevant workarounds in the kernel.
>>
>> Compare it to real hardware, where we expect new kernels to run on old
>> hardware but not the other way around, and where we expect to be able to
>> identify the specific hardware we're running on (there are version
>> registers in the MPIC for identifying real hardware version, but not
>> QEMU implementation version).
>>
>> If we had this, we could have avoided needing to postpone fsl,mpic,
>> because I think the Linux patch using large vectors hasn't been released
>> yet, so we could have added a check for qemu,1.2-fsl-mpic and known that
>> it didn't include large vectors.  I guess it's still not too late...
> 
> I'm not sure I like the idea. Usually, in the real hardware world,
> erratas happen on the board level, right?

More often on a chip revision level.

> So we could have a
> compatible "qemu,1.2-e500" in the board compatible. But having one
> for every device sounds a bit too much to me.

But not every device is necessarily emulated.

> Also, not implementing large vectors essentially mean we don't
> emulate an fsl,mpic well. That's fine, let's just not tell the guest
> we do. I don't think we should teach Linux about broken emulation.
> I'd rather see the emulation fixed.

My point is even after we add large vector support, we probably aren't
100% emulating an FSL MPIC.

>> It would also be useful for overriding SVR checks in drivers (for errata
>> and such), which can get particularly awkward if you mix some emulated
>> devices with some directly assigned devices (no one SVR will be right
>> for all of them, even if all the emulated devices faithfully emulate one
>> SoC's behavior).
> 
> Hmm. Yes. Maybe it'd be good to just never check SVR but instead always rely on dt properties?

That assumes we know about the problem when the device tree is created
(we like to maintain compatibility with old trees), and that we maintain
the information accurately in all trees (which means u-boot fixups, and
compatibility is even more of an issue there, because some people don't
like upgrading u-boot in the field).  The only practical way to do that
would be to put the SVR in each device tree node, which seems like
overkill.  At least the QEMU compatibles only affect QEMU and target
code that specifically wants to improve its QEMU compatibility, rather
than wait for QEMU to improve.

-Scott
Alexander Graf Aug. 9, 2012, 9:48 p.m. UTC | #12
On 09.08.2012, at 23:45, Scott Wood wrote:

> On 08/09/2012 04:36 PM, Alexander Graf wrote:
>> 
>> On 09.08.2012, at 23:28, Scott Wood wrote:
>> 
>>> On 08/09/2012 04:19 PM, Alexander Graf wrote:
>>>> 
>>>> On 09.08.2012, at 23:11, Scott Wood wrote:
>>>>> ...so that we can run new kernels on old QEMUs, not just the other way
>>>>> around.
>>>> 
>>>> Why would we bother?
>>> 
>>> Why not?  It's much easier than the other way around -- it's just an
>>> extra string in the device tree.  It's up to someone who cares to put
>>> relevant workarounds in the kernel.
>>> 
>>> Compare it to real hardware, where we expect new kernels to run on old
>>> hardware but not the other way around, and where we expect to be able to
>>> identify the specific hardware we're running on (there are version
>>> registers in the MPIC for identifying real hardware version, but not
>>> QEMU implementation version).
>>> 
>>> If we had this, we could have avoided needing to postpone fsl,mpic,
>>> because I think the Linux patch using large vectors hasn't been released
>>> yet, so we could have added a check for qemu,1.2-fsl-mpic and known that
>>> it didn't include large vectors.  I guess it's still not too late...
>> 
>> I'm not sure I like the idea. Usually, in the real hardware world,
>> erratas happen on the board level, right?
> 
> More often on a chip revision level.
> 
>> So we could have a
>> compatible "qemu,1.2-e500" in the board compatible. But having one
>> for every device sounds a bit too much to me.
> 
> But not every device is necessarily emulated.
> 
>> Also, not implementing large vectors essentially mean we don't
>> emulate an fsl,mpic well. That's fine, let's just not tell the guest
>> we do. I don't think we should teach Linux about broken emulation.
>> I'd rather see the emulation fixed.
> 
> My point is even after we add large vector support, we probably aren't
> 100% emulating an FSL MPIC.
> 
>>> It would also be useful for overriding SVR checks in drivers (for errata
>>> and such), which can get particularly awkward if you mix some emulated
>>> devices with some directly assigned devices (no one SVR will be right
>>> for all of them, even if all the emulated devices faithfully emulate one
>>> SoC's behavior).
>> 
>> Hmm. Yes. Maybe it'd be good to just never check SVR but instead always rely on dt properties?
> 
> That assumes we know about the problem when the device tree is created
> (we like to maintain compatibility with old trees), and that we maintain
> the information accurately in all trees (which means u-boot fixups, and
> compatibility is even more of an issue there, because some people don't
> like upgrading u-boot in the field).  The only practical way to do that
> would be to put the SVR in each device tree node, which seems like
> overkill.  At least the QEMU compatibles only affect QEMU and target
> code that specifically wants to improve its QEMU compatibility, rather
> than wait for QEMU to improve.

Ah, you mean we should indicate "This is the QEMU emulated version of device x". I think I slowly see what you're getting at. In this case, the fix really is to get QEMU work properly, but I like the idea of exposing to the guest that a device is emulated by QEMU, and emulated by version X.


Alex
diff mbox

Patch

--- log.fails	2012-08-08 23:06:59.000000000 +0200
+++ log.works	2012-08-08 23:07:13.000000000 +0200
@@ -51,34 +51,34 @@ 
 mpic: Initializing for 256 sources
 openpic_cpu_write_internal: cpu 0 addr 0000000000000080 <= 0000000f
 mpic_timer_write: addr 0000000000000040 <= 00000001
-mpic_timer_write: addr 0000000000000030 <= 800907f3
-Set IDE 76 to 0x800007f3
+mpic_timer_write: addr 0000000000000030 <= 800900f3
+Set IDE 76 to 0x800000f3
 mpic_timer_write: addr 0000000000000080 <= 00000001
-mpic_timer_write: addr 0000000000000070 <= 800907f4
-Set IDE 77 to 0x800007f4
+mpic_timer_write: addr 0000000000000070 <= 800900f4
+Set IDE 77 to 0x800000f4
 mpic_timer_write: addr 00000000000000c0 <= 00000001
-mpic_timer_write: addr 00000000000000b0 <= 800907f5
-Set IDE 78 to 0x800007f5
+mpic_timer_write: addr 00000000000000b0 <= 800900f5
+Set IDE 78 to 0x800000f5
 mpic_timer_write: addr 0000000000000100 <= 00000001
-mpic_timer_write: addr 00000000000000f0 <= 800907f6
+mpic_timer_write: addr 00000000000000f0 <= 800900f6
 openpic_gbl_write: addr 00000000000010a0 <= 80000000
 openpic_update_irq: IRQ 92 is not pending
 Set IPVP 92 to 0x80000000 -> 0x80000000
 openpic_gbl_read: addr 00000000000010a0
 openpic_gbl_read: => 80000000
-openpic_gbl_write: addr 00000000000010a0 <= 800a07fb
+openpic_gbl_write: addr 00000000000010a0 <= 800a00fb
 openpic_update_irq: IRQ 92 is not pending
-Set IPVP 92 to 0x800a07fb -> 0x800a00fb
-openpic_gbl_write: addr 00000000000010b0 <= 800a07fc
+Set IPVP 92 to 0x800a00fb -> 0x800a00fb
+openpic_gbl_write: addr 00000000000010b0 <= 800a00fc
 openpic_update_irq: IRQ 93 is not pending
-Set IPVP 93 to 0x800a07fc -> 0x800a00fc
-openpic_gbl_write: addr 00000000000010c0 <= 800a07fd
+Set IPVP 93 to 0x800a00fc -> 0x800a00fc
+openpic_gbl_write: addr 00000000000010c0 <= 800a00fd
 openpic_update_irq: IRQ 94 is not pending
-Set IPVP 94 to 0x800a07fd -> 0x800a00fd
-openpic_gbl_write: addr 00000000000010d0 <= 800a07fe
+Set IPVP 94 to 0x800a00fd -> 0x800a00fd
+openpic_gbl_write: addr 00000000000010d0 <= 800a00fe
 openpic_update_irq: IRQ 95 is not pending
-Set IPVP 95 to 0x800a07fe -> 0x800a00fe
-MPIC flags: 9102
+Set IPVP 95 to 0x800a00fe -> 0x800a00fe
+MPIC flags: 1002
 mpic_src_ext_write: addr 0000000000000000 <= 80080000
 openpic_update_irq: IRQ 0 is not pending
 Set IPVP 0 to 0x80080000 -> 0x80080000
@@ -542,7 +542,7 @@ 
 mpic_src_msi_write: addr 00000000000000f0 <= 00000001
 Set IDE 91 to 0x00000001
 mpic_src_msi_write: addr 0000000000000100 <= 800800e8
-openpic_gbl_write: addr 00000000000010e0 <= 000007ff
+openpic_gbl_write: addr 00000000000010e0 <= 000000ff
 openpic_gbl_read: addr 0000000000001020
 openpic_gbl_read: => 00000000
 openpic_gbl_write: addr 0000000000001020 <= 20000000
@@ -554,31 +554,31 @@ 
 smp_mpic_probe()...
 nr_cpus: 2
 mpic: requesting IPIs...
-mpic: map virq 507, hwirq 0x7fb
+mpic: map virq 251, hwirq 0xfb
 mpic: mapping as IPI
- OpenPIC  : enable_ipi: 507 (ipi 0)
+ OpenPIC  : enable_ipi: 251 (ipi 0)
 openpic_gbl_read: addr 00000000000010a0
 openpic_gbl_read: => 800a00fb
 openpic_gbl_write: addr 00000000000010a0 <= 000a00fb
 openpic_update_irq: IRQ 92 is not pending
 Set IPVP 92 to 0x000a00fb -> 0x000a00fb
-mpic: map virq 508, hwirq 0x7fc
+mpic: map virq 252, hwirq 0xfc
 mpic: mapping as IPI
- OpenPIC  : enable_ipi: 508 (ipi 1)
+ OpenPIC  : enable_ipi: 252 (ipi 1)
 openpic_gbl_read: addr 00000000000010b0
 openpic_gbl_read: => 800a00fc
 openpic_gbl_write: addr 00000000000010b0 <= 000a00fc
 openpic_update_irq: IRQ 93 is not pending
 Set IPVP 93 to 0x000a00fc -> 0x000a00fc
-mpic: map virq 509, hwirq 0x7fd
+mpic: map virq 253, hwirq 0xfd
 mpic: mapping as IPI
- OpenPIC  : enable_ipi: 509 (ipi 2)
+ OpenPIC  : enable_ipi: 253 (ipi 2)
 openpic_gbl_read: addr 00000000000010c0
 openpic_gbl_read: => 800a00fd
 openpic_gbl_write: addr 00000000000010c0 <= 000a00fd
 openpic_update_irq: IRQ 94 is not pending
 Set IPVP 94 to 0x000a00fd -> 0x000a00fd
-mpic: map virq 510, hwirq 0x7fe
+mpic: map virq 254, hwirq 0xfe
 mpic: mapping as IPI
  OpenPIC  : setup_this_cpu(1)
 openpic_cpu_write_internal: cpu 1 addr 0000000000001080 <= 00000000
@@ -595,6 +595,11 @@ 
 PIAC: irq=93
 openpic_cpu_read_internal: => 000000fc
  OpenPIC  : get_one_irq(reg 0xa0): 252
+openpic_cpu_write_internal: cpu 1 addr 00000000000010b0 <= 00000000
+PEOI
+IRQ_check: irq 93 set ipvp_pr=10 pr=-1
+openpic_cpu_read_internal: cpu 1 addr 0000000000001090
+openpic_cpu_read_internal: => 00000001
  OpenPIC  : send_ipi(ipi_no: 1)
 openpic_cpu_write_internal: cpu 1 addr 0000000000001050 <= 00000001
 Set IDE 93 to 0x00000001