[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/2] via-ide: Also emulate non 100% native mode
From: |
Mark Cave-Ayland |
Subject: |
Re: [PATCH 2/2] via-ide: Also emulate non 100% native mode |
Date: |
Fri, 6 Mar 2020 18:44:29 +0000 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
On 06/03/2020 12:40, BALATON Zoltan wrote:
> On Fri, 6 Mar 2020, Mark Cave-Ayland wrote:
>> On 06/03/2020 00:21, BALATON Zoltan wrote:
>>> On Fri, 6 Mar 2020, BALATON Zoltan wrote:
>>>> On Thu, 5 Mar 2020, Mark Cave-Ayland wrote:
>>>>> On 04/03/2020 22:33, BALATON Zoltan wrote:
>>>>> another possibility: PCI configuration space register 0x3d (Interrupt
>>>>> pin) is
>>>>> documented as having value 0 == Legacy IRQ routing which should be the
>>>>> initial
>>>>> value
>>>>> on reset, but QEMU incorrectly sets it to 1 which indicates PCI IRQ
>>>>> routing.
>>>>
>>>> The VT8231 docs say this should always read 1 but may be this is somehow
>>>> set to 0
>>>> on the Pegasos2. What does that mean? Should we use this value instead of
>>>> the
>>>> feature bit to force using legacy interrupts? We'd still need a property
>>>> in via-ide
>>>> to set this reg or is it possible to set it from board code overriding the
>>>> default
>>>> after device is created? That would allow to drop patch 1. I can try this.
>>>
>>> This seemed like it could simplify patches a bit but it does not work.
>>> Setting this
>>> reg to 0 breaks Linux and MorphOS which then think the device does not have
>>> an
>>> interrupt at all and fail as before waiting for the irq. So we still need
>>> the feature
>>> bit, cant use this reg to force legacy interrupts. I've spent considerable
>>> time
>>> testing different OSes before I've ended up with this patch series I've
>>> submitted and
>>> I could not find a simpler way that works with everything.
>>
>> I appreciate that testing these things can take a lot of time, but what is
>> important
>> thing to ask here is whether these hacks are attempting to work around
>> something in
>> QEMU that doesn't match the hardware specification, and to me it feels that
>> this is
>> what is happening here.
>
> It may be we need to work around some incomplete modelling of devices in
> QEMU, e.g.
> we only model the native mode of these IDE interfaces so anything involving
> legacy
> mode is out of scope. To also emulate legacy mode we'd need changing common
> ISA code
> and maybe PIC code as well. As those parts are also used by other more
> commonly used
> machine models I'd avoid breaking those and rather implement it confined to
> these
> machines that are not yet finished or complete anyway than try to change all
> dependent devices that would need even more testing. These "hacks" could be
> cleaned
> up later and this would not be the only hack in QEMU, I don't have time to fix
> everything and it's unreasonable to demand it I think. I'd suggest to take
> this patch
> as it is now and if you don't like it you can submit patches that clean it up
> the way
> you think is correct or submit an alternative patch now that shows how do you
> think
> it can be done in a cleaner way because I don't see it and don't have more
> time for
> it now.
>
>> Obviously this thread has become quite long (and even I'm struggling to find
>> previous
>> discussions) but here is my summary below:
>>
>> - I don't think the patch in its current form is the right way to do this.
>> Instead of
>> adding a feature bit to fudge the existing IRQ routing when the existing IRQ
>> routing
>> is wrong, let's fix the existing IRQ routing instead.
>
> I think that would involve changing parts which could break other machines so
> I'd
> rather go with a featute bit only affecting pegasos2 and fulonge2 than touch
> i8259 or
> ISA emulation basing that on some guess how the real chip may be implemented.
> Is it
> possible to implement what you propose without changing common IDE, ISA and
> PIC
> emulation only in via-ide and fulong2e code?
>
>> - There is no mention of "non-100%" native mode in the 8231 or 686B
>> datasheet: this
>> is simply a term used within the Linux patches. The controller is either in
>> native
>> mode, or legacy mode. It may be that guests are making use of some undefined
>> behaviour here.
>
> Yes, this is a Linux term and Linux also uses a feature bit to enable this
> workaround. If that's good enough for Linux why isn't it good enough for you?
>
>> - The code that uses the value of PCI_INTERRUPT_LINE in via-ide is incorrect
>> (as your
>> patch comment points out, some guests ignore it anyway).
>
> You're misunderstanding the comment. The via_ide_config_read function is
> needed to
> restore value in interrupt line that common PCI reset code deletes. Linux
> depends on
> this value to be the same as on real hardware so this is needed to work
> around QEMU
> and Linux pecularities.
>
> I've tried using PCI_INTERRUPT_PIN in place of the feature bit but setting
> that to 0
> breaks Linux and MorphOS on pegasos2 because these apparently expect this to
> be set
> to 1 corresponding to native mode. (Firmware only sets native mode enable
> bits in
> prog-if but datasheet says this reg should be 1 by default and other PCI docs
> say 0
> here means no interrupt used so maybe that's why Linux and MorphOS don't like
> it.)
>
>> - There is different behaviour here between the 8231 and 686B in this area,
>> despite
>> having the same vendor/device id.
>>
>>
>> The first thing you need to fix is the PCI_INTERRUPT_LINE part; I would
>> start by
>> removing the existing code and instead expose a qdev named gpio "legacy-irq"
>> and
>> wiring that up to your interrupt controller. Note you'd have to do the same
>> for
>> fulong2e, but that is reasonably trivial.
>
> Please go ahead and do it but if you don't submit an alternative patch before
> the
> freeze I'd ask John and Peter to make a judgement here and tell if my series
> is
> acceptable or not in its current form and if it is then please merge it and
> leave
> clean ups for subsequent patches. This is blocking my further patches to
> implement
> pegasos2 emulation.
I believe I've answered this in detail in my previous email, so I suggest we
keep the
discussion there so it's all in one place.
ATB,
Mark.
- Re: [PATCH 2/2] via-ide: Also emulate non 100% native mode, (continued)
- Re: [PATCH 2/2] via-ide: Also emulate non 100% native mode, BALATON Zoltan, 2020/03/02
- Re: [PATCH 2/2] via-ide: Also emulate non 100% native mode, Mark Cave-Ayland, 2020/03/03
- Re: [PATCH 2/2] via-ide: Also emulate non 100% native mode, BALATON Zoltan, 2020/03/03
- Re: [PATCH 2/2] via-ide: Also emulate non 100% native mode, Mark Cave-Ayland, 2020/03/04
- Re: [PATCH 2/2] via-ide: Also emulate non 100% native mode, BALATON Zoltan, 2020/03/04
- Re: [PATCH 2/2] via-ide: Also emulate non 100% native mode, Mark Cave-Ayland, 2020/03/05
- Re: [PATCH 2/2] via-ide: Also emulate non 100% native mode, BALATON Zoltan, 2020/03/05
- Re: [PATCH 2/2] via-ide: Also emulate non 100% native mode, BALATON Zoltan, 2020/03/05
- Re: [PATCH 2/2] via-ide: Also emulate non 100% native mode, Mark Cave-Ayland, 2020/03/06
- Re: [PATCH 2/2] via-ide: Also emulate non 100% native mode, BALATON Zoltan, 2020/03/06
- Re: [PATCH 2/2] via-ide: Also emulate non 100% native mode,
Mark Cave-Ayland <=
- Re: [PATCH 2/2] via-ide: Also emulate non 100% native mode, Mark Cave-Ayland, 2020/03/06
- Re: [PATCH 2/2] via-ide: Also emulate non 100% native mode, BALATON Zoltan, 2020/03/06
- Re: [PATCH 2/2] via-ide: Also emulate non 100% native mode, Mark Cave-Ayland, 2020/03/06
- Re: [PATCH 2/2] via-ide: Also emulate non 100% native mode, BALATON Zoltan, 2020/03/06
- Re: [PATCH 2/2] via-ide: Also emulate non 100% native mode, Mark Cave-Ayland, 2020/03/06
- Re: [PATCH 2/2] via-ide: Also emulate non 100% native mode, BALATON Zoltan, 2020/03/06
- Re: [PATCH 2/2] via-ide: Also emulate non 100% native mode, Mark Cave-Ayland, 2020/03/07
- Re: [PATCH 2/2] via-ide: Also emulate non 100% native mode, BALATON Zoltan, 2020/03/07
- Re: [PATCH 2/2] via-ide: Also emulate non 100% native mode, Artyom Tarasenko, 2020/03/06
Re: [PATCH 2/2] via-ide: Also emulate non 100% native mode, BALATON Zoltan, 2020/03/01