On 04/11/14 14:02, Michael Tokarev wrote:
> Chris Boot updated his qemu from 1.7.0 to 1.7.1, and noticed that windows
> guests
> which was using virtio-scsi does not work anymore. Windows BSODs at
> boot with the following error:
>
>
> STOP: c0000221 Unknown Hard Error
> \StstenRiit\System32\ntdll.dll
>
> Collecting data for crash dump ...
> ...
>
> After reboot it offers to fix the error(s), apparently making the hdd image
> unusable even with older, previously working, versions. I can confirm this
> on my machine too, using windows 7 64bit (32bit win7 boots very very slow
> on virtio-scsi, probably windows 32bit driver is broken). Using windows
> drivers from virtio-win-0.1-74.iso.
>
>
> Bisecting between 1.7.0 and 1.7.1 was easy, and this is the first bad commit:
>
> commit 819ddf7d1fbcb74ecab885dc35fea741c6316b17
> Author: Paolo Bonzini <pbonz...@redhat.com>
> Date: Fri Feb 7 15:47:46 2014 +0100
>
> memory: fix limiting of translation at a page boundary
>
> Commit 360e607 (address_space_translate: do not cross page boundaries,
> 2014-01-30) broke MMIO accesses in cases where the section is shorter
> than the full register width. This can happen for example with the
> Bochs DISPI registers, which are 16 bits wide but have only a 1-byte
> long MemoryRegion (if you write to the "second byte" of the register
> your access is discarded; it doesn't write only to half of the register).
>
> Restrict the action of commit 360e607 to direct RAM accesses. This
> is enough for Xen, since MMIO will not go through the mapcache.
>
> Reported-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk>
> Cc: qemu-sta...@nongnu.org
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> Tested-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk>
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
> (cherry picked from commit a87f39543a9259f671c5413723311180ee2ad2a8)
> Signed-off-by: Michael Roth <mdr...@linux.vnet.ibm.com>
>
> Reverting this commit from 1.7.1 fixes the issue.
>
>
> More, the same issue exists on 2.0-tobe as well, but in this case, reverting
> the same commit from there -- a87f39543a9259f671c5413723311180ee2ad2a8 --
> does NOT fix the problem. I'm bisecting between 1.7.0 and 2.0 now.
>
> This is just a heads-up for now, dunno how important this use case is.
Disclaimer: I don't know what I'm talking about.
This hunk in 819ddf7d:
> @@ -295,6 +307,11 @@ MemoryRegion *address_space_translate(AddressSpace *as,
> hwaddr addr,
> as = iotlb.target_as;
> }
>
> + if (memory_access_is_direct(mr, is_write)) {
> + hwaddr page = ((addr & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE) - addr;
> + len = MIN(page, len);
> + }
> +
> *plen = len;
> *xlat = addr;
> return mr;
limits "len" at the *first* page boundary that is strictly above "addr". Is
that the intent? The commit subject says "at *a* page boundary".
Shouldn't it go like:
if (memory_access_is_direct(mr, is_write)) {
hwaddr page = (((addr + len) & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE) -
addr;
len = MIN(page, len);
}
This would drop only the trailing portion beyond the last page boundary covered.
Thanks,
Laszlo
