Re: [Qemu-devel] [PATCH 3/3] ide/via: Implement and use native PCI IDE mode

[John Snow]

On 1/25/19 7:25 AM, BALATON Zoltan wrote:
> On Thu, 24 Jan 2019, BALATON Zoltan wrote:
>> On Wed, 23 Jan 2019, John Snow wrote:
>>> I guess this is technically an external change in behavior... I have no
>>> real read on if this will break anything for anyone, or if anyone was
>>> even using it.
>>
>> Currently nothing but the mips_fulong2e board seems to use this device
>> and I don't think there's anything even on that board that would
>> depend on (or would work with) legacy only IDE mode of this device but
>> I could not find a test image to try. That board seems to be quite
>> unfinished, I've tried to get it in better shape but haven't succeeded
>> yet. So I don't think this change in the IDE device would break
>> anything for anyone as it does not seem to be usable yet but I plan to
>> use this IDE part in subsequent patches and PCI native mode works better.
>>
>>> Any thoughts on why it's okay to switch?
>>
>> As said above it's unlikely anyone would depend on it now and all
>> drivers I know about prefer native mode anyway so likely it would work
>> better after this change. If not and it turns out someone is using the
>> current behaviour I can look at implementing switching between the two
>> modes but that would be more difficult (the ISA ports would need to be
>> mapped and unmapped based on bits in PCI_PROG_INTERFACE but there's no
>> API to do this currently, ide/core.c only has ide_init_ioport to add
>> mapping but not the counterpart to remove it; this could be
>> implemented but unless found to be needed it probably does not worth
>> it). So I suggest to switch based on that this is a quite unused part
>> that's not likely to break anything and if it still found to break
>> something I'll look at fixing it or it could be reverted, but I don't
>> want to spend time now on something that's not actually used.
> 
> I'd appreciated if this could be included now based on the above as I
> have some pathces in the works that will need this, unless there's a
> strong reason to not apply it.
> 
> Thank you,
> BALATON Zoltan

I'll stage it. If anyone disagrees we can roll it back before release if
we are breaking legacy behavior, but I really strongly suspect nobody is
relying on this in any kind of production environment, so I'll take the
risk to just merge the improvement.

--js