[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [Qemu-devel] [PATCH 2/4] macio: add dma_active to VMState
From: |
John Snow |
Subject: |
Re: [Qemu-ppc] [Qemu-devel] [PATCH 2/4] macio: add dma_active to VMStateDescription |
Date: |
Fri, 8 Jan 2016 15:55:28 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 01/06/2016 04:17 PM, Mark Cave-Ayland wrote:
> On 06/01/16 20:57, John Snow wrote:
>
>> On 01/06/2016 03:37 PM, Mark Cave-Ayland wrote:
>>> Make sure that we include the value of dma_active in the migration stream.
>>>
>>> Signed-off-by: Mark Cave-Ayland <address@hidden>
>>> ---
>>> hw/ide/macio.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/ide/macio.c b/hw/ide/macio.c
>>> index 560c071..695d4d2 100644
>>> --- a/hw/ide/macio.c
>>> +++ b/hw/ide/macio.c
>>> @@ -518,11 +518,12 @@ static const MemoryRegionOps pmac_ide_ops = {
>>>
>>> static const VMStateDescription vmstate_pmac = {
>>> .name = "ide",
>>> - .version_id = 3,
>>> + .version_id = 4,
>>> .minimum_version_id = 0,
>>> .fields = (VMStateField[]) {
>>> VMSTATE_IDE_BUS(bus, MACIOIDEState),
>>> VMSTATE_IDE_DRIVES(bus.ifs, MACIOIDEState),
>>> + VMSTATE_BOOL(dma_active, MACIOIDEState),
>>> VMSTATE_END_OF_LIST()
>>> }
>>> };
>>>
>>
>> Did you wind up ever observing this value to be non-zero when it was
>> written to the migration stream?
>>
>> I really did think that we should be able to assume this was always
>> false due to how migration will drain all outstanding AIO, but maybe I
>> am mistaken.
>
> I think this can happen because Darwin/MacOS sets the DBDMA processor
> running first *before* the IDE request is issued, compared to pretty
> much every other OS which issues the IDE request *first* which then in
> turn invokes the DMA engine (which is the general assumption in the QEMU
> IDE/DMA APIs).
>
> So there could be a window where the DBDMA is programmed and active but
> migration takes place before the corresponding IDE request has been
> issued (which is exactly the situation that this flag handles).
>
>
> ATB,
>
> Mark.
>
sadly that seems to be the case. ide_dbdma_start looks like it can yield
through DBDMA_kick, so there's time for things to go awry.
Acked-by: John Snow <address@hidden>
I had an off-list discussion with David Gilbert on how the migration
fields work here -- this will introduce a hard incompatibility between
pre-2.5 and post-2.5, which might be fine since Mac has never really
quite worked correctly anyway.
If you want to worry about compatibility, David advised me that a
conditional subsection might be appropriate:
since dma_active is /usually/ false, we can use this as a flag for
deciding to migrate it or not: i.e. if it's false, we skip the field and
the receiver assumes it's false in post_load, or if we migrate to an
older version, it never has to worry about it.
If it's true, you get a migration error that says the subsection wasn't
found, but you get to try to migrate again -- it's kind of a cheesy way
to say that you can't migrate to older versions while the DMA is active.
Future versions can accept the true boolean, though.
HTH
--js