Re: [Qemu-devel] QEMU build broken
Il 10/05/2014 08:45, Brad Smith ha scritto: Having your feature in-tree is a privilege, not a right. You earn it by helping to maintain it. "it's not really maintained right now" means it has not been earning its keep. You're encouraged to remedy that. Huh? "my feature"? I have nothing to do with this. What kind of crazy is this? How to misdirect and not take responsibility for breaking something. If there wasn't sloppy irresponsible development in the first place it wouldn't be an issue. Brad, all this is doing, is convincing people that bsd-user is not worth keeping in the tree. It's a fact that in a million-line codebase not all patches can be tested by all people. Why don't you send a patch instead of whining? Paolo
Re: [Qemu-devel] Question about RAM migration flags
Peter Lieven wrote: >> Am 09.05.2014 um 11:43 schrieb "Dr. David Alan Gilbert" >> : >> >> * Peter Lieven (p...@kamp.de) wrote: >>> Hi, >>> >>> while working on ram migration and reading through the code I realized that >>> qemu does not stop loading a saved VM or rejecting an incoming migration >>> if there is a flag in the stream that it does not understand. An unknown >>> flag >>> is simply ignored. >>> >>> In the block migration code there is a catch at the end complaining about >>> unknown flags, but in RAM migration there isn't. >>> >>> Is this on purpose or an error? >> >> I think it's in error; the code doesn't have much checking. > > i will prepare a patch. Thanks.
Re: [Qemu-devel] QEMU build broken
On 10 May 2014 08:07, Paolo Bonzini wrote: > Why don't you send a patch? Or just test the one I sent yesterday: http://patchwork.ozlabs.org/patch/347443/ thanks -- PMM
Re: [Qemu-devel] [PATCH v2 1/3] linux-user: Support little-endian PPC64 in user mode.
On 10 May 2014 10:16, Doug Kwan wrote: > diff --git a/linux-user/uname.c b/linux-user/uname.c > index f5d4c66..cb1f9a3 100644 > --- a/linux-user/uname.c > +++ b/linux-user/uname.c > @@ -65,6 +65,12 @@ const char *cpu_to_uname_machine(void *cpu_env) > return "i586"; > } > return "i686"; > +#elif defined(TARGET_PPC64) > +#ifdef TARGET_WORDS_BIGENDIAN > +return UNAME_MACHINE; > +#else > +return UNAME_MACHINE "le"; > +#endif > #else > /* default is #define-d in each arch/ subdir */ > return UNAME_MACHINE; I think it would be nicer just to define UNAME_MACHINE correctly in linux-user/ppc/syscall.h. We should restrict the per-CPU ifdefs in uname.c to just those CPUs which need to pick the uname based on runtime CPU features. thanks -- PMM
Re: [Qemu-devel] [PATCH v2 3/3] Add a new user mode target for little-endian PPC64.
On 10 May 2014 10:16, Doug Kwan wrote: > Signed-off-by: Doug Kwan > --- > configure | 6 ++ > default-configs/ppc64le-linux-user.mak | 1 + > 2 files changed, 7 insertions(+) > create mode 100644 default-configs/ppc64le-linux-user.mak > > diff --git a/configure b/configure > index 46bc0c9..5f0926b 100755 > --- a/configure > +++ b/configure > @@ -4912,6 +4912,12 @@ case "$target_name" in > TARGET_ABI_DIR=ppc > gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml > power-spe.xml" >;; > + ppc64le) > +TARGET_ARCH=ppc64 > +TARGET_BASE_ARCH=ppc > +TARGET_ABI_DIR=ppc > +gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml > power-spe.xml" > + ;; >ppc64abi32) > TARGET_ARCH=ppc64 > TARGET_BASE_ARCH=ppc > diff --git a/default-configs/ppc64le-linux-user.mak > b/default-configs/ppc64le-linux-user.mak > new file mode 100644 > index 000..6948225 > --- /dev/null > +++ b/default-configs/ppc64le-linux-user.mak > @@ -0,0 +1 @@ > +# Default configuration for ppc64el-linux-user You forgot to change this "el" to "le". Otherwise Reviewed-by: Peter Maydell thanks -- PMM
Re: [Qemu-devel] [PATCH v2 1/3] linux-user: Support little-endian PPC64 in user mode.
On 10 May 2014 10:16, Doug Kwan wrote: > Look at ELF header to determin ABI version on PPC64. This is required typo: "determine". > for executing the first instruction correctly. Also print correct machine > name in uname() system call. thanks -- PMM
Re: [Qemu-devel] [PATCH v2 2/3] PPC: Allow little-endian user mode.
On 10 May 2014 10:16, Doug Kwan wrote: > This allow running PPC64 little-endian in user mode if target is configured > that way. In PPC64 LE user mode we set MSR.LE during initialization. > Overhaul handling of byteswapping in code generation and mem helpers. This looks pretty good to me (I'll let a ppc person do the detailed review). One thing I did spot: Outside target-ppc/, the other place we do memory accesses is linux-user/main.c:do_store_exclusive(). get_user_u32() and friends will honour TARGET_WORDS_BIGENDIAN, but the 128 bit store exclusive is done with two accesses. The system-emulation mode code handles the LE case by flipping around the register values, so I think the linux-user code will need to do a similar thing. thanks -- PMM
[Qemu-devel] [PATCH] migration: catch unknown flags in ram_load
if a saved vm has unknown flags in the memory data qemu
currently simply ignores this flag and continues which
yields in an unpredictable result.
this patch catches all unknown flags and
aborts the loading of the vm.
CC: qemu-sta...@nongnu.org
Signed-off-by: Peter Lieven
---
arch_init.c |7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 995f56d..582b716 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -1084,9 +1084,7 @@ static int ram_load(QEMUFile *f, void *opaque, int
version_id)
total_ram_bytes -= length;
}
-}
-
-if (flags & RAM_SAVE_FLAG_COMPRESS) {
+} else if (flags & RAM_SAVE_FLAG_COMPRESS) {
void *host;
uint8_t ch;
@@ -1121,6 +1119,9 @@ static int ram_load(QEMUFile *f, void *opaque, int
version_id)
}
} else if (flags & RAM_SAVE_FLAG_HOOK) {
ram_control_load_hook(f, flags);
+} else if (!(flags & RAM_SAVE_FLAG_EOS)) {
+ret = -EINVAL;
+goto done;
}
error = qemu_file_get_error(f);
if (error) {
--
1.7.9.5
[Qemu-devel] [PATCH] migration: cache memory region ram ptr
we currently look up the ram ptr for each single page. Cache
the pointer while we operate on the same block.
Signed-off-by: Peter Lieven
---
arch_init.c | 23 ---
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 582b716..ce338aa 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -594,13 +594,19 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
ram_bulk_stage = false;
}
} else {
+static uint8_t *ram_ptr;
int ret;
uint8_t *p;
bool send_async = true;
-int cont = (block == last_sent_block) ?
-RAM_SAVE_FLAG_CONTINUE : 0;
+int cont = 0;
-p = memory_region_get_ram_ptr(mr) + offset;
+if (block != last_sent_block) {
+ram_ptr = memory_region_get_ram_ptr(mr);
+} else {
+cont = RAM_SAVE_FLAG_CONTINUE;
+}
+
+p = ram_ptr + offset;
/* In doubt sent page as normal */
bytes_sent = -1;
@@ -990,16 +996,17 @@ static inline void *host_from_stream_offset(QEMUFile *f,
int flags)
{
static RAMBlock *block = NULL;
+static uint8_t *ram_ptr;
char id[256];
uint8_t len;
if (flags & RAM_SAVE_FLAG_CONTINUE) {
-if (!block) {
+if (!block || !ram_ptr) {
fprintf(stderr, "Ack, bad migration stream!\n");
return NULL;
}
-return memory_region_get_ram_ptr(block->mr) + offset;
+return ram_ptr + offset;
}
len = qemu_get_byte(f);
@@ -1007,8 +1014,10 @@ static inline void *host_from_stream_offset(QEMUFile *f,
id[len] = 0;
QTAILQ_FOREACH(block, &ram_list.blocks, next) {
-if (!strncmp(id, block->idstr, sizeof(id)))
-return memory_region_get_ram_ptr(block->mr) + offset;
+if (!strncmp(id, block->idstr, sizeof(id))) {
+ram_ptr = memory_region_get_ram_ptr(block->mr);
+return ram_ptr + offset;
+}
}
fprintf(stderr, "Can't find block %s!\n", id);
--
1.7.9.5
[Qemu-devel] [PATCH] linux-user/uname: Return correct uname string for x86_64
We were returning the incorrect uname string (with a hyphen, not an underscore) for x86_64. Fix this by removing the x86_64 special case, since the default "just use UNAME_MACHINE" behaviour suffices. This leaves cpu_to_uname_machine() special cases for only those architectures which need to vary the string based on runtime CPU features. Signed-off-by: Peter Maydell --- linux-user/uname.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/linux-user/uname.c b/linux-user/uname.c index f5d4c66..1e6560d 100644 --- a/linux-user/uname.c +++ b/linux-user/uname.c @@ -52,9 +52,7 @@ const char *cpu_to_uname_machine(void *cpu_env) /* earliest emulated CPU is ARMv5TE; qemu can emulate the 1026, but not its * Jazelle support */ return "armv5te" utsname_suffix; -#elif defined(TARGET_X86_64) -return "x86-64"; -#elif defined(TARGET_I386) +#elif defined(TARGET_I386) && !defined(TARGET_X86_64) /* see arch/x86/kernel/cpu/bugs.c: check_bugs(), 386, 486, 586, 686 */ CPUState *cpu = ENV_GET_CPU((CPUX86State *)cpu_env); int family = object_property_get_int(OBJECT(cpu), "family", NULL); -- 1.9.2
Re: [Qemu-devel] [PATCH] migration: cache memory region ram ptr
Hi,
The patch is correct. There is a small improved point.
>
> /* In doubt sent page as normal */
> bytes_sent = -1;
> @@ -990,16 +996,17 @@ static inline void *host_from_stream_offset(QEMUFile *f,
> int flags)
> {
> static RAMBlock *block = NULL;
RAMBlock *block = NULL;
> +static uint8_t *ram_ptr;
> char id[256];
> uint8_t len;
>
> if (flags & RAM_SAVE_FLAG_CONTINUE) {
> -if (!block) {
> +if (!block || !ram_ptr) {
if (!ram_ptr) {
Best regards
ChenLiang
> fprintf(stderr, "Ack, bad migration stream!\n");
> return NULL;
> }
>
> -return memory_region_get_ram_ptr(block->mr) + offset;
> +return ram_ptr + offset;
> }
>
> len = qemu_get_byte(f);
> @@ -1007,8 +1014,10 @@ static inline void *host_from_stream_offset(QEMUFile
> *f,
> id[len] = 0;
>
> QTAILQ_FOREACH(block, &ram_list.blocks, next) {
> -if (!strncmp(id, block->idstr, sizeof(id)))
> -return memory_region_get_ram_ptr(block->mr) + offset;
> +if (!strncmp(id, block->idstr, sizeof(id))) {
> +ram_ptr = memory_region_get_ram_ptr(block->mr);
> +return ram_ptr + offset;
> +}
> }
>
> fprintf(stderr, "Can't find block %s!\n", id);
> --
> 1.7.9.5
>
>
Re: [Qemu-devel] [Qemu-ppc] macio ide question/bug report
On Wed, 7 May 2014, Mark Cave-Ayland wrote: On 07/05/14 18:00, BALATON Zoltan wrote: On Wed, 7 May 2014, Mark Cave-Ayland wrote: I'm not sure if your problem related to s->lba == -1 should be solved just for macio or higher up in the block layer, but the block people will be on qemu-devel and not qemu-ppc so you should definitely CC the main list to get their feedback on this. Cc'd the devel list now, the original question can be found here: http://lists.nongnu.org/archive/html/qemu-ppc/2014-05/msg3.html I think it's handled by the block layer but the translation in macio breaks it and converts it to -4 which causes an error so probably it should only be solved for macio. But I hope Alex can look at it and tell for sure or maybe solve it. Okay. So in that case it could just be a macio bug. Those ISOs only boot for me with the default -M g3beige, but darwin is a huge culprit for these unaligned accesses. My point was that if you are making changes in this area, if you can still boot the darwin images then that would be a good test that any changes you make are working correctly :) Ah, OK. They boot for me without a -M parameter with my changes (to the point saying "available for installation:" and then empty as I did not specify a hard disk image) too, but I was testing with -M mac99 as that was what I changed. I think they still "work" as before. But... the block patchset link I sent you has the potential to fix this anyway, since if the unaligned accesses can be passed directly up to the block layer then in theory this problem should go away as this whole chunk of code should no longer be required. A good test for this would be to try reverting this patch: https://github.com/agraf/qemu/commit/063a333911f1bb09fcc1a4925c8ebf5c88fc2b63 which is Alex's original alignment fix. If you find with that with this patch reverted darwin boots as before, it means that the block alignment patches in core are working correctly and hopefully you may find that it fixes your MorphOS problem. But again, you really should get this on qemu-devel to get the definitive answer from the block guys. That patch would be 80fc95d8bdaf3392106b131a97ca701fd374489a in QEMU master. I've tried reverting it and Darwin still boots (without -M mac99) up to the point where it asks to install as before but I don't know how good a test is this as I'm not sure it does unaligned accesses up to this point. I see accesses like this: pmac_ide_transfer(ATAPI) lba=, buffer_index=30, len=930 pmac_ide_transfer(ATAPI) lba=0, buffer_index=0, len=930 pmac_ide_transfer(ATAPI) lba=0, buffer_index=0, len=800 pmac_ide_transfer(ATAPI) lba=1, buffer_index=0, len=800 pmac_ide_transfer(ATAPI) lba=1, buffer_index=0, len=800 pmac_ide_transfer(ATAPI) lba=2, buffer_index=0, len=800 pmac_ide_transfer(ATAPI) lba=2, buffer_index=0, len=800 pmac_ide_transfer(ATAPI) lba=0, buffer_index=0, len=2000 pmac_ide_transfer(ATAPI) lba=f2, buffer_index=0, len=800 pmac_ide_transfer(ATAPI) lba=f3, buffer_index=0, len=800 pmac_ide_transfer(ATAPI) lba=1ac, buffer_index=0, len=800 pmac_ide_transfer(ATAPI) lba=9ae, buffer_index=0, len=800 pmac_ide_transfer(ATAPI) lba=9af, buffer_index=0, len=400 pmac_ide_transfer(ATAPI) lba=9ae, buffer_index=0, len=400 pmac_ide_transfer(ATAPI) lba=9ae, buffer_index=400, len=400 pmac_ide_transfer(ATAPI) lba=9af, buffer_index=0, len=800 pmac_ide_transfer(ATAPI) lba=11ae, buffer_index=0, len=800 pmac_ide_transfer(ATAPI) lba=11af, buffer_index=0, len=1000 [...] pmac_ide_transfer(ATAPI) lba=3858e, buffer_index=0, len=2000 pmac_ide_transfer(ATAPI) lba=3856e, buffer_index=0, len=2000 pmac_ide_transfer(ATAPI) lba=38572, buffer_index=0, len=4000 pmac_ide_transfer(ATAPI) lba=38590, buffer_index=0, len=4000 pmac_ide_transfer(ATAPI) lba=38584, buffer_index=0, len=1000 pmac_ide_transfer(ATAPI) lba=38586, buffer_index=0, len=3000 pmac_ide_transfer(ATAPI) lba=3857e, buffer_index=0, len=3000 pmac_ide_transfer(ATAPI) lba=38566, buffer_index=0, len=4000 pmac_ide_transfer(ATAPI) lba=38550, buffer_index=0, len=1000 pmac_ide_transfer(ATAPI) lba=38552, buffer_index=0, len=4000 pmac_ide_transfer(ATAPI) lba=38536, buffer_index=0, len=4000 pmac_ide_transfer(ATAPI) lba=3852c, buffer_index=0, len=5000 pmac_ide_transfer(ATAPI) lba=385a6, buffer_index=0, len=4000 pmac_ide_transfer(ATAPI) lba=a724, buffer_index=0, len=2000 pmac_ide_transfer(ATAPI) lba=3854a, buffer_index=0, len=3000 pmac_ide_transfer(ATAPI) lba=1654, buffer_index=0, len=1000 pmac_ide_transfer(ATAPI) lba=3b1cc, buffer_index=0, len=8000 pmac_ide_transfer(ATAPI) lba=3b3a8, buffer_index=0, len=8000 pmac_ide_transfer(ATAPI) lba=3b3a0, buffer_index=0, len=4000 pmac_ide_transfer(ATAPI) lba=3b3a8, buffer_index=0, len=2000 pmac_ide_transfer(ATAPI) lba=38546, buffer_index=0, len=2000 pmac_ide_transfer(ATAPI) lba=3854a, buffer_index=0, len=8000 pmac_ide_transfer(ATAPI) lba=3b3d8, buffer_index=0, len=8000 pmac_ide_transfer(ATAPI) lba=3b3e8, buffer_index=0, len=7000 pmac_ide_transfe
Re: [Qemu-devel] [PATCH 7/8] hw/arm/stellaris: Correct handling of GPTM TAR register
On Fri, May 9, 2014 at 4:46 AM, Peter Maydell wrote:
> We don't implement very much of the GPTM TAR register, and what we
> do is wrong. The "are we in RT mode?" field is in s->config, not
> s->control. Correct this, use LOG_UNIMP rather than hw_error()
> for the cases we don't support, and avoid an unlabelled fallthrough
> that makes Coverity complain.
>
> Signed-off-by: Peter Maydell
> ---
> hw/arm/stellaris.c | 13 ++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
> index d6cc77b..487ee72 100644
> --- a/hw/arm/stellaris.c
> +++ b/hw/arm/stellaris.c
> @@ -185,12 +185,19 @@ static uint64_t gptm_read(void *opaque, hwaddr offset,
> case 0x44: /* TBPMR */
> return s->match_prescale[1];
> case 0x48: /* TAR */
> -if (s->control == 1)
> +if (s->config == 1) {
> return s->rtc;
> +}
> +qemu_log_mask(LOG_UNIMP,
> + "gptm_read of TAR but timer read not supported");
Should it perhaps be "GPTM read" to be more human?
gptm_read is the qemu fn name and more developer centric, but if thats
what your really going for the perhaps it is better done with %s
__func__ ?
Otherwise:
Reviewed-by: Peter Crosthwaite
Regards,
Peter
> +return 0;
> case 0x4c: /* TBR */
> -hw_error("TODO: Timer value read\n");
> +qemu_log_mask(LOG_UNIMP,
> + "gptm_read of TBR but timer read not supported");
> +return 0;
> default:
> -hw_error("gptm_read: Bad offset 0x%x\n", (int)offset);
> +qemu_log_mask(LOG_GUEST_ERROR,
> + "gptm_read: Bad offset 0x%x\n", (int)offset);
> return 0;
> }
> }
> --
> 1.9.2
>
>
Re: [Qemu-devel] [PATCH] migration: catch unknown flags in ram_load
> if a saved vm has unknown flags in the memory data qemu > currently simply ignores this flag and continues which > yields in an unpredictable result. > > this patch catches all unknown flags and > aborts the loading of the vm. > > CC: qemu-sta...@nongnu.org > Signed-off-by: Peter Lieven > --- > arch_init.c |7 --- > 1 file changed, 4 insertions(+), 3 deletions(-) > Reviewed-by: ChenLiang
[Qemu-devel] [PATCH] arch_init: Simplify code for load_xbzrle()
For xbzrle_decode_buffer(), when decoding contents will exceed writing
buffer, it will return -1, so need not check the return value whether
large than writing buffer.
And when failure occurs within load_xbzrle(), it always return -1
without any resources which need release.
So can remove the related checking statements, and also can remove 'rc'
and 'ret' local variables,
Signed-off-by: Chen Gang
---
arch_init.c | 14 --
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 60c975d..98ee5b6 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -908,7 +908,6 @@ static uint64_t ram_save_pending(QEMUFile *f, void *opaque,
uint64_t max_size)
static int load_xbzrle(QEMUFile *f, ram_addr_t addr, void *host)
{
-int ret, rc = 0;
unsigned int xh_len;
int xh_flags;
@@ -933,18 +932,13 @@ static int load_xbzrle(QEMUFile *f, ram_addr_t addr, void
*host)
qemu_get_buffer(f, xbzrle_decoded_buf, xh_len);
/* decode RLE */
-ret = xbzrle_decode_buffer(xbzrle_decoded_buf, xh_len, host,
- TARGET_PAGE_SIZE);
-if (ret == -1) {
+if (xbzrle_decode_buffer(xbzrle_decoded_buf, xh_len, host,
+ TARGET_PAGE_SIZE) == -1) {
fprintf(stderr, "Failed to load XBZRLE page - decode error!\n");
-rc = -1;
-} else if (ret > TARGET_PAGE_SIZE) {
-fprintf(stderr, "Failed to load XBZRLE page - size %d exceeds %d!\n",
-ret, TARGET_PAGE_SIZE);
-abort();
+return -1;
}
-return rc;
+return 0;
}
static inline void *host_from_stream_offset(QEMUFile *f,
--
1.7.11.7
Re: [Qemu-devel] [PATCH 8/8] hw/arm/omap_gpmc: Avoid buffer overrun filling prefetch FIFO
On Fri, May 9, 2014 at 4:46 AM, Peter Maydell wrote:
> In fill_prefetch_fifo(), if the device we are reading from is 16 bit,
> then we must not try to transfer an odd number of bytes into the FIFO.
> This could otherwise have resulted in our overrunning the prefetch.fifo
> array by one byte.
>
> Signed-off-by: Peter Maydell
Reviewed-by: Peter Crosthwaite
> ---
> Spotted by Coverity. I suspect Coverity is not smart enough
> to figure out that this change really does prevent the overrun,
> though :-(
> ---
> hw/misc/omap_gpmc.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/hw/misc/omap_gpmc.c b/hw/misc/omap_gpmc.c
> index 2047274..cddea24 100644
> --- a/hw/misc/omap_gpmc.c
> +++ b/hw/misc/omap_gpmc.c
> @@ -242,6 +242,10 @@ static void fill_prefetch_fifo(struct omap_gpmc_s *s)
> if (bytes > s->prefetch.count) {
> bytes = s->prefetch.count;
> }
> +if (is16bit) {
> +bytes &= ~1;
> +}
> +
> s->prefetch.count -= bytes;
> s->prefetch.fifopointer += bytes;
> fptr = 64 - s->prefetch.fifopointer;
> --
> 1.9.2
>
>
Re: [Qemu-devel] [PATCH] migration: cache memory region ram ptr
Am 10.05.2014 13:38, schrieb 陈梁:
> Hi,
> The patch is correct. There is a small improved point.
>> /* In doubt sent page as normal */
>> bytes_sent = -1;
>> @@ -990,16 +996,17 @@ static inline void *host_from_stream_offset(QEMUFile
>> *f,
>> int flags)
>> {
>> static RAMBlock *block = NULL;
>RAMBlock *block = NULL;
This one has to be static as well. Why do you think it should not?
Peter
>
>> +static uint8_t *ram_ptr;
>> char id[256];
>> uint8_t len;
>>
>> if (flags & RAM_SAVE_FLAG_CONTINUE) {
>> -if (!block) {
>> +if (!block || !ram_ptr) {
> if (!ram_ptr) {
>
> Best regards
> ChenLiang
>
>> fprintf(stderr, "Ack, bad migration stream!\n");
>> return NULL;
>> }
>>
>> -return memory_region_get_ram_ptr(block->mr) + offset;
>> +return ram_ptr + offset;
>> }
>>
>> len = qemu_get_byte(f);
>> @@ -1007,8 +1014,10 @@ static inline void *host_from_stream_offset(QEMUFile
>> *f,
>> id[len] = 0;
>>
>> QTAILQ_FOREACH(block, &ram_list.blocks, next) {
>> -if (!strncmp(id, block->idstr, sizeof(id)))
>> -return memory_region_get_ram_ptr(block->mr) + offset;
>> +if (!strncmp(id, block->idstr, sizeof(id))) {
>> +ram_ptr = memory_region_get_ram_ptr(block->mr);
>> +return ram_ptr + offset;
>> +}
>> }
>>
>> fprintf(stderr, "Can't find block %s!\n", id);
>> --
>> 1.7.9.5
>>
>>
Re: [Qemu-devel] [PATCH 3/4] block: replace fprintf(stderr, ...) with error_report() in block/
On Sat, May 10, 2014 at 9:55 AM, Le Tan wrote:
> Replace fprintf(stderr,...) with error_report() in files block/*, block.c,
> block-migration.c and blockdev.c. The trailing "\n"s of the @fmt argument
> have been removed because @fmt of error_report() should not contain newline.
>
> Signed-off-by: Le Tan
> ---
> block-migration.c |4 +-
> block.c|4 +-
> block/linux-aio.c |4 +-
> block/nbd-client.h |2 +-
> block/qcow2-cluster.c |4 +-
> block/qcow2-refcount.c | 114
>
> block/qcow2.c | 16 +++
> block/quorum.c |4 +-
> block/raw-posix.c |8 ++--
> block/raw-win32.c |6 +--
> block/ssh.c|4 +-
> block/vdi.c| 14 +++---
> block/vmdk.c | 21 -
> block/vpc.c|4 +-
> block/vvfat.c | 108 ++---
> blockdev.c |6 +--
> 16 files changed, 160 insertions(+), 163 deletions(-)
>
> diff --git a/block-migration.c b/block-migration.c
> index 56951e0..5bcf7c8 100644
> --- a/block-migration.c
> +++ b/block-migration.c
> @@ -790,7 +790,7 @@ static int block_load(QEMUFile *f, void *opaque, int
> version_id)
>
> bs = bdrv_find(device_name);
> if (!bs) {
> -fprintf(stderr, "Error unknown block device %s\n",
> +error_report("Error unknown block device %s",
> device_name);
> return -EINVAL;
> }
> @@ -833,7 +833,7 @@ static int block_load(QEMUFile *f, void *opaque, int
> version_id)
> (addr == 100) ? '\n' : '\r');
> fflush(stdout);
> } else if (!(flags & BLK_MIG_FLAG_EOS)) {
> -fprintf(stderr, "Unknown block migration flags: %#x\n", flags);
> +error_report("Unknown block migration flags: %#x", flags);
> return -EINVAL;
> }
> ret = qemu_file_get_error(f);
> diff --git a/block.c b/block.c
> index b749d31..7dc4acb 100644
> --- a/block.c
> +++ b/block.c
> @@ -2694,8 +2694,8 @@ static int bdrv_prwv_co(BlockDriverState *bs, int64_t
> offset,
> * if it has been enabled.
> */
> if (bs->io_limits_enabled) {
> -fprintf(stderr, "Disabling I/O throttling on '%s' due "
> -"to synchronous I/O.\n", bdrv_get_device_name(bs));
> +error_report("Disabling I/O throttling on '%s' due "
> + "to synchronous I/O.", bdrv_get_device_name(bs));
> bdrv_io_limits_disable(bs);
> }
>
> diff --git a/block/linux-aio.c b/block/linux-aio.c
> index 53434e2..b706a59 100644
> --- a/block/linux-aio.c
> +++ b/block/linux-aio.c
> @@ -162,8 +162,8 @@ BlockDriverAIOCB *laio_submit(BlockDriverState *bs, void
> *aio_ctx, int fd,
> break;
> /* Currently Linux kernel does not support other operations */
> default:
> -fprintf(stderr, "%s: invalid AIO request type 0x%x.\n",
> -__func__, type);
> +error_report("%s: invalid AIO request type 0x%x.",
> + __func__, type);
> goto out_free_aiocb;
> }
> io_set_eventfd(&laiocb->iocb, event_notifier_get_fd(&s->e));
> diff --git a/block/nbd-client.h b/block/nbd-client.h
> index f2a6337..74178f4 100644
> --- a/block/nbd-client.h
> +++ b/block/nbd-client.h
> @@ -9,7 +9,7 @@
>
> #if defined(DEBUG_NBD)
> #define logout(fmt, ...) \
> -fprintf(stderr, "nbd\t%-24s" fmt, __func__, ##__VA_ARGS__)
> +error_report("nbd\t%-24s" fmt, __func__, ##__VA_ARGS__)
So i'm not sure we want to convert debug printfery to error_report.
There's very good value in converting the printfs with user
visibility, but ones like this seem intended for developers only as
throwaway-output. My thinking is that this is a lower level output
than error_report. For instance, as a developer you may do something
to break your error API yet you still want your debug printfery.
Wouldn't matter in this location, but there may be other parts of the
tree where we don't want error_report relinace for debug
instrumentation and it just seems better to keep it consistent.
Thinking further afield, qemu_log may ultimately be the correct
mechanism for this instead (I think thats what I have been using for
new code recently anyway).
Thoughts from others?
Regards,
Peter
Re: [Qemu-devel] [PATCH 7/8] hw/arm/stellaris: Correct handling of GPTM TAR register
On 10 May 2014 13:33, Peter Crosthwaite wrote:
> On Fri, May 9, 2014 at 4:46 AM, Peter Maydell
> wrote:
>> We don't implement very much of the GPTM TAR register, and what we
>> do is wrong. The "are we in RT mode?" field is in s->config, not
>> s->control. Correct this, use LOG_UNIMP rather than hw_error()
>> for the cases we don't support, and avoid an unlabelled fallthrough
>> that makes Coverity complain.
>>
>> Signed-off-by: Peter Maydell
>> ---
>> hw/arm/stellaris.c | 13 ++---
>> 1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
>> index d6cc77b..487ee72 100644
>> --- a/hw/arm/stellaris.c
>> +++ b/hw/arm/stellaris.c
>> @@ -185,12 +185,19 @@ static uint64_t gptm_read(void *opaque, hwaddr offset,
>> case 0x44: /* TBPMR */
>> return s->match_prescale[1];
>> case 0x48: /* TAR */
>> -if (s->control == 1)
>> +if (s->config == 1) {
>> return s->rtc;
>> +}
>> +qemu_log_mask(LOG_UNIMP,
>> + "gptm_read of TAR but timer read not supported");
>
> Should it perhaps be "GPTM read" to be more human?
Good idea.
thanks
-- PMM
Re: [Qemu-devel] uvesafb doesn't work with seabios
On Fri, May 09, 2014 at 05:06:21PM +0200, Bernhard Walle wrote:
> Hello,
>
> I'm using QEmu 2.0.0 on a Linux host (x86-64) with a quite special
> target system that uses uvesafb ('video=uvesafb:1024x768-32'). I get
> following errors in the target system:
>
> uvesafb: Getting mode info block for mode 0x2 failed (eax=0x4f01, err=1)
> uvesafb: Getting mode info block for mode 0x3 failed (eax=0x4f01, err=1)
> uvesafb: Getting mode info block for mode 0x4 failed (eax=0x4f01, err=1)
> uvesafb: Getting mode info block for mode 0x5 failed (eax=0x4f01, err=1)
> uvesafb: Getting mode info block for mode 0x6 failed (eax=0x4f01, err=1)
> uvesafb: Getting mode info block for mode 0x7 failed (eax=0x4f01, err=1)
> uvesafb: Getting mode info block for mode 0xd failed (eax=0x4f01, err=1)
> uvesafb: Getting mode info block for mode 0xe failed (eax=0x4f01, err=1)
> uvesafb: Getting mode info block for mode 0xf failed (eax=0x4f01, err=1)
> uvesafb: Getting mode info block for mode 0x10 failed (eax=0x4f01, err=1)
> uvesafb: Getting mode info block for mode 0x11 failed (eax=0x4f01, err=1)
> uvesafb: Getting mode info block for mode 0x12 failed (eax=0x4f01, err=1)
> uvesafb: Getting mode info block for mode 0x13 failed (eax=0x4f01, err=1)
> uvesafb: Getting mode info block for mode 0x6a failed (eax=0x4f01, err=1)
> uvesafb: vbe_init() failed with -22
> uvesafb: probe of uvesafb.0 failed with error -22
>
> With QEmu 1.6, it worked. I bisected that down to following commit:
>
> commit 6eefccc0bb9c34051b1e21880fc3a1c1c8686edd
> Author: Gerd Hoffmann
> Date: Mon Dec 2 13:01:20 2013 +0100
>
> roms: update vgabios binaries
>
> And indeed, when I switch 'vgabios-stdvga.bin' to the old variant, it works.
>
> What can I do to track down the problem? I tried to build the svga bios
> inside the QEMU tree, but I didn't succeed. Building it in the current
> seabios git tree worked, but I got a different error...
Can you run the original SeaVGABIOS image and provide the output with
the following added to the qemu command line:
-chardev stdio,id=seabios -device isa-debugcon,iobase=0x402,chardev=seabios
Also, it looks like uvesafb can call x86emu. Older versions of x86emu
are known to not emulate some instructions properly, and it has caused
problems for SeaVGABIOS. The most recent version of SeaVGABIOS from
SeaBIOS git has additional checks for this - can you grab the seabios
git, set CONFIG_DEBUG_LEVEL to 8, build SeaVGABIOS (set
CONFIG_VGA_BOCHS), and post the logs from both Linux and QEMU with the
above options?
-Kevin
Re: [Qemu-devel] [PATCH] migration: cache memory region ram ptr
Il 10/05/2014 12:51, Peter Lieven ha scritto:
we currently look up the ram ptr for each single page. Cache
the pointer while we operate on the same block.
Why don't you instead cache the result in the MemoryRegion, so that
memory_region_get_ram_ptr becomes a simple, inline field access?
Paolo
Signed-off-by: Peter Lieven
---
arch_init.c | 23 ---
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 582b716..ce338aa 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -594,13 +594,19 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
ram_bulk_stage = false;
}
} else {
+static uint8_t *ram_ptr;
int ret;
uint8_t *p;
bool send_async = true;
-int cont = (block == last_sent_block) ?
-RAM_SAVE_FLAG_CONTINUE : 0;
+int cont = 0;
-p = memory_region_get_ram_ptr(mr) + offset;
+if (block != last_sent_block) {
+ram_ptr = memory_region_get_ram_ptr(mr);
+} else {
+cont = RAM_SAVE_FLAG_CONTINUE;
+}
+
+p = ram_ptr + offset;
/* In doubt sent page as normal */
bytes_sent = -1;
@@ -990,16 +996,17 @@ static inline void *host_from_stream_offset(QEMUFile *f,
int flags)
{
static RAMBlock *block = NULL;
+static uint8_t *ram_ptr;
char id[256];
uint8_t len;
if (flags & RAM_SAVE_FLAG_CONTINUE) {
-if (!block) {
+if (!block || !ram_ptr) {
fprintf(stderr, "Ack, bad migration stream!\n");
return NULL;
}
-return memory_region_get_ram_ptr(block->mr) + offset;
+return ram_ptr + offset;
}
len = qemu_get_byte(f);
@@ -1007,8 +1014,10 @@ static inline void *host_from_stream_offset(QEMUFile *f,
id[len] = 0;
QTAILQ_FOREACH(block, &ram_list.blocks, next) {
-if (!strncmp(id, block->idstr, sizeof(id)))
-return memory_region_get_ram_ptr(block->mr) + offset;
+if (!strncmp(id, block->idstr, sizeof(id))) {
+ram_ptr = memory_region_get_ram_ptr(block->mr);
+return ram_ptr + offset;
+}
}
fprintf(stderr, "Can't find block %s!\n", id);
Re: [Qemu-devel] [PATCH] migration: cache memory region ram ptr
You mean,
a) extent the MemoryRegion stuct with a pointer?
b) on the first call to memory_region_get_ram_ptr cache the result in the
struct?
Peter
Am 10.05.2014 17:33, schrieb Paolo Bonzini:
> Il 10/05/2014 12:51, Peter Lieven ha scritto:
>> we currently look up the ram ptr for each single page. Cache
>> the pointer while we operate on the same block.
>
> Why don't you instead cache the result in the MemoryRegion, so that
> memory_region_get_ram_ptr becomes a simple, inline field access?
>
> Paolo
>
>> Signed-off-by: Peter Lieven
>> ---
>> arch_init.c | 23 ---
>> 1 file changed, 16 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch_init.c b/arch_init.c
>> index 582b716..ce338aa 100644
>> --- a/arch_init.c
>> +++ b/arch_init.c
>> @@ -594,13 +594,19 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
>> ram_bulk_stage = false;
>> }
>> } else {
>> +static uint8_t *ram_ptr;
>> int ret;
>> uint8_t *p;
>> bool send_async = true;
>> -int cont = (block == last_sent_block) ?
>> -RAM_SAVE_FLAG_CONTINUE : 0;
>> +int cont = 0;
>>
>> -p = memory_region_get_ram_ptr(mr) + offset;
>> +if (block != last_sent_block) {
>> +ram_ptr = memory_region_get_ram_ptr(mr);
>> +} else {
>> +cont = RAM_SAVE_FLAG_CONTINUE;
>> +}
>> +
>> +p = ram_ptr + offset;
>>
>> /* In doubt sent page as normal */
>> bytes_sent = -1;
>> @@ -990,16 +996,17 @@ static inline void *host_from_stream_offset(QEMUFile
>> *f,
>> int flags)
>> {
>> static RAMBlock *block = NULL;
>> +static uint8_t *ram_ptr;
>> char id[256];
>> uint8_t len;
>>
>> if (flags & RAM_SAVE_FLAG_CONTINUE) {
>> -if (!block) {
>> +if (!block || !ram_ptr) {
>> fprintf(stderr, "Ack, bad migration stream!\n");
>> return NULL;
>> }
>>
>> -return memory_region_get_ram_ptr(block->mr) + offset;
>> +return ram_ptr + offset;
>> }
>>
>> len = qemu_get_byte(f);
>> @@ -1007,8 +1014,10 @@ static inline void *host_from_stream_offset(QEMUFile
>> *f,
>> id[len] = 0;
>>
>> QTAILQ_FOREACH(block, &ram_list.blocks, next) {
>> -if (!strncmp(id, block->idstr, sizeof(id)))
>> -return memory_region_get_ram_ptr(block->mr) + offset;
>> +if (!strncmp(id, block->idstr, sizeof(id))) {
>> +ram_ptr = memory_region_get_ram_ptr(block->mr);
>> +return ram_ptr + offset;
>> +}
>> }
>>
>> fprintf(stderr, "Can't find block %s!\n", id);
>>
>
Re: [Qemu-devel] [PATCH] migration: cache memory region ram ptr
Am 10.05.2014 17:33, schrieb Paolo Bonzini:
> Il 10/05/2014 12:51, Peter Lieven ha scritto:
>> we currently look up the ram ptr for each single page. Cache
>> the pointer while we operate on the same block.
>
> Why don't you instead cache the result in the MemoryRegion, so that
> memory_region_get_ram_ptr becomes a simple, inline field access?
This seems to work. Wondering if it has other side implications. Basic tests
like booting vServers and migration work. What about XEN?
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 1d55ad9..3003875 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -161,6 +161,7 @@ struct MemoryRegion {
unsigned ioeventfd_nb;
MemoryRegionIoeventfd *ioeventfds;
NotifierList iommu_notify;
+void *ram_ptr;
};
/**
diff --git a/memory.c b/memory.c
index 3f1df23..78d4032 100644
--- a/memory.c
+++ b/memory.c
@@ -862,6 +862,7 @@ void memory_region_init(MemoryRegion *mr,
mr->ioeventfd_nb = 0;
mr->ioeventfds = NULL;
mr->flush_coalesced_mmio = false;
+mr->ram_ptr = NULL;
}
static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
@@ -1249,7 +1250,12 @@ void *memory_region_get_ram_ptr(MemoryRegion *mr)
assert(mr->terminates);
-return qemu_get_ram_ptr(mr->ram_addr & TARGET_PAGE_MASK);
+if (mr->ram_ptr) {
+return mr->ram_ptr;
+}
+
+mr->ram_ptr = qemu_get_ram_ptr(mr->ram_addr & TARGET_PAGE_MASK);
+return mr->ram_ptr;
}
static void memory_region_update_coalesced_range_as(MemoryRegion *mr,
AddressSpace *as)
Peter
>
> Paolo
>
>> Signed-off-by: Peter Lieven
>> ---
>> arch_init.c | 23 ---
>> 1 file changed, 16 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch_init.c b/arch_init.c
>> index 582b716..ce338aa 100644
>> --- a/arch_init.c
>> +++ b/arch_init.c
>> @@ -594,13 +594,19 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
>> ram_bulk_stage = false;
>> }
>> } else {
>> +static uint8_t *ram_ptr;
>> int ret;
>> uint8_t *p;
>> bool send_async = true;
>> -int cont = (block == last_sent_block) ?
>> -RAM_SAVE_FLAG_CONTINUE : 0;
>> +int cont = 0;
>>
>> -p = memory_region_get_ram_ptr(mr) + offset;
>> +if (block != last_sent_block) {
>> +ram_ptr = memory_region_get_ram_ptr(mr);
>> +} else {
>> +cont = RAM_SAVE_FLAG_CONTINUE;
>> +}
>> +
>> +p = ram_ptr + offset;
>>
>> /* In doubt sent page as normal */
>> bytes_sent = -1;
>> @@ -990,16 +996,17 @@ static inline void *host_from_stream_offset(QEMUFile
>> *f,
>> int flags)
>> {
>> static RAMBlock *block = NULL;
>> +static uint8_t *ram_ptr;
>> char id[256];
>> uint8_t len;
>>
>> if (flags & RAM_SAVE_FLAG_CONTINUE) {
>> -if (!block) {
>> +if (!block || !ram_ptr) {
>> fprintf(stderr, "Ack, bad migration stream!\n");
>> return NULL;
>> }
>>
>> -return memory_region_get_ram_ptr(block->mr) + offset;
>> +return ram_ptr + offset;
>> }
>>
>> len = qemu_get_byte(f);
>> @@ -1007,8 +1014,10 @@ static inline void *host_from_stream_offset(QEMUFile
>> *f,
>> id[len] = 0;
>>
>> QTAILQ_FOREACH(block, &ram_list.blocks, next) {
>> -if (!strncmp(id, block->idstr, sizeof(id)))
>> -return memory_region_get_ram_ptr(block->mr) + offset;
>> +if (!strncmp(id, block->idstr, sizeof(id))) {
>> +ram_ptr = memory_region_get_ram_ptr(block->mr);
>> +return ram_ptr + offset;
>> +}
>> }
>>
>> fprintf(stderr, "Can't find block %s!\n", id);
>>
>
[Qemu-devel] [PATCH v3] snapshot: fixed bdrv_get_full_backing_filename can not get correct full_backing_filename
This patch fixed the following bug:
https://bugzilla.redhat.com/show_bug.cgi?id=1084302 .
path_combine can not calculate the correct full path name for backing_file.
Such as:
create a snapshot chain:
sn2->sn1->$BASE_IMG
backing file is : /home/wookpecker/img.qcow2
sn1 : /home/woodpecker/tmp/sn1
sn2 : /home/woodpecker/tmp/sn2
when create sn2, path_combine can not got a correct path for $BASE_IMG.
In this patch, will check the backing_filename is a symlink or not firstly,
then return the full(absolute) path via realpath.
Signed-off-by: Jun Li
---
block.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/block.c b/block.c
index b749d31..6674719 100644
--- a/block.c
+++ b/block.c
@@ -304,10 +304,26 @@ void path_combine(char *dest, int dest_size,
void bdrv_get_full_backing_filename(BlockDriverState *bs, char *dest, size_t
sz)
{
+struct stat sb;
+char *linkname;
+
if (bs->backing_file[0] == '\0' || path_has_protocol(bs->backing_file)) {
pstrcpy(dest, sz, bs->backing_file);
} else {
-path_combine(dest, sz, bs->filename, bs->backing_file);
+if (lstat(bs->backing_file, &sb) == -1) {
+perror("lstat");
+exit(EXIT_FAILURE);
+}
+
+/* Check linkname is a link or not */
+if (S_ISLNK(sb.st_mode)) {
+linkname = malloc(sb.st_size + 1);
+readlink(bs->backing_file, linkname, sb.st_size + 1);
+linkname[sb.st_size] = '\0';
+realpath(linkname, dest);
+} else {
+realpath(bs->backing_file, dest);
+}
}
}
--
1.8.3.1
[Qemu-devel] [PATCH v3] snapshot: fixed bdrv_get_full_backing_filename can not get correct full_backing_filename
From: Jun Li
This patch fixed the following bug:
https://bugzilla.redhat.com/show_bug.cgi?id=1084302 .
path_combine can not calculate the correct full path name for backing_file.
Such as:
create a snapshot chain:
sn2->sn1->$BASE_IMG
backing file is : /home/wookpecker/img.qcow2
sn1 : /home/woodpecker/tmp/sn1
sn2 : /home/woodpecker/tmp/sn2
when create sn2, path_combine can not got a correct path for $BASE_IMG.
In this patch, will check the backing_filename is a symlink or not firstly,
then return the full(absolute) path via realpath.
Signed-off-by: Jun Li
---
block.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/block.c b/block.c
index b749d31..6674719 100644
--- a/block.c
+++ b/block.c
@@ -304,10 +304,26 @@ void path_combine(char *dest, int dest_size,
void bdrv_get_full_backing_filename(BlockDriverState *bs, char *dest, size_t
sz)
{
+struct stat sb;
+char *linkname;
+
if (bs->backing_file[0] == '\0' || path_has_protocol(bs->backing_file)) {
pstrcpy(dest, sz, bs->backing_file);
} else {
-path_combine(dest, sz, bs->filename, bs->backing_file);
+if (lstat(bs->backing_file, &sb) == -1) {
+perror("lstat");
+exit(EXIT_FAILURE);
+}
+
+/* Check linkname is a link or not */
+if (S_ISLNK(sb.st_mode)) {
+linkname = malloc(sb.st_size + 1);
+readlink(bs->backing_file, linkname, sb.st_size + 1);
+linkname[sb.st_size] = '\0';
+realpath(linkname, dest);
+} else {
+realpath(bs->backing_file, dest);
+}
}
}
--
1.8.3.1
Re: [Qemu-devel] [PATCH] migration: cache memory region ram ptr
or even this:
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 1d55ad9..3003875 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -161,6 +161,7 @@ struct MemoryRegion {
unsigned ioeventfd_nb;
MemoryRegionIoeventfd *ioeventfds;
NotifierList iommu_notify;
+void *ram_ptr;
};
/**
diff --git a/memory.c b/memory.c
index 3f1df23..6c1e413 100644
--- a/memory.c
+++ b/memory.c
@@ -862,6 +862,7 @@ void memory_region_init(MemoryRegion *mr,
mr->ioeventfd_nb = 0;
mr->ioeventfds = NULL;
mr->flush_coalesced_mmio = false;
+mr->ram_ptr = NULL;
}
static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
@@ -1241,17 +1242,23 @@ void memory_region_reset_dirty(MemoryRegion *mr, hwaddr
addr,
cpu_physical_memory_reset_dirty(mr->ram_addr + addr, size, client);
}
-void *memory_region_get_ram_ptr(MemoryRegion *mr)
+static void *memory_region_get_ram_ptr2(MemoryRegion *mr)
{
if (mr->alias) {
return memory_region_get_ram_ptr(mr->alias) + mr->alias_offset;
}
-
assert(mr->terminates);
-
return qemu_get_ram_ptr(mr->ram_addr & TARGET_PAGE_MASK);
}
+inline void *memory_region_get_ram_ptr(MemoryRegion *mr)
+{
+if (!mr->ram_ptr) {
+mr->ram_ptr = memory_region_get_ram_ptr2(mr);
+}
+return mr->ram_ptr;
+}
+
static void memory_region_update_coalesced_range_as(MemoryRegion *mr,
AddressSpace *as)
{
FlatView *view;
Am 10.05.2014 18:32, schrieb Peter Lieven:
> Am 10.05.2014 17:33, schrieb Paolo Bonzini:
>> Il 10/05/2014 12:51, Peter Lieven ha scritto:
>>> we currently look up the ram ptr for each single page. Cache
>>> the pointer while we operate on the same block.
>> Why don't you instead cache the result in the MemoryRegion, so that
>> memory_region_get_ram_ptr becomes a simple, inline field access?
> This seems to work. Wondering if it has other side implications. Basic tests
> like booting vServers and migration work. What about XEN?
>
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 1d55ad9..3003875 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -161,6 +161,7 @@ struct MemoryRegion {
> unsigned ioeventfd_nb;
> MemoryRegionIoeventfd *ioeventfds;
> NotifierList iommu_notify;
> +void *ram_ptr;
> };
>
> /**
> diff --git a/memory.c b/memory.c
> index 3f1df23..78d4032 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -862,6 +862,7 @@ void memory_region_init(MemoryRegion *mr,
> mr->ioeventfd_nb = 0;
> mr->ioeventfds = NULL;
> mr->flush_coalesced_mmio = false;
> +mr->ram_ptr = NULL;
> }
>
> static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
> @@ -1249,7 +1250,12 @@ void *memory_region_get_ram_ptr(MemoryRegion *mr)
>
> assert(mr->terminates);
>
> -return qemu_get_ram_ptr(mr->ram_addr & TARGET_PAGE_MASK);
> +if (mr->ram_ptr) {
> +return mr->ram_ptr;
> +}
> +
> +mr->ram_ptr = qemu_get_ram_ptr(mr->ram_addr & TARGET_PAGE_MASK);
> +return mr->ram_ptr;
> }
>
> static void memory_region_update_coalesced_range_as(MemoryRegion *mr,
> AddressSpace *as)
>
>
> Peter
>
>> Paolo
>>
>>> Signed-off-by: Peter Lieven
>>> ---
>>> arch_init.c | 23 ---
>>> 1 file changed, 16 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/arch_init.c b/arch_init.c
>>> index 582b716..ce338aa 100644
>>> --- a/arch_init.c
>>> +++ b/arch_init.c
>>> @@ -594,13 +594,19 @@ static int ram_save_block(QEMUFile *f, bool
>>> last_stage)
>>> ram_bulk_stage = false;
>>> }
>>> } else {
>>> +static uint8_t *ram_ptr;
>>> int ret;
>>> uint8_t *p;
>>> bool send_async = true;
>>> -int cont = (block == last_sent_block) ?
>>> -RAM_SAVE_FLAG_CONTINUE : 0;
>>> +int cont = 0;
>>>
>>> -p = memory_region_get_ram_ptr(mr) + offset;
>>> +if (block != last_sent_block) {
>>> +ram_ptr = memory_region_get_ram_ptr(mr);
>>> +} else {
>>> +cont = RAM_SAVE_FLAG_CONTINUE;
>>> +}
>>> +
>>> +p = ram_ptr + offset;
>>>
>>> /* In doubt sent page as normal */
>>> bytes_sent = -1;
>>> @@ -990,16 +996,17 @@ static inline void *host_from_stream_offset(QEMUFile
>>> *f,
>>> int flags)
>>> {
>>> static RAMBlock *block = NULL;
>>> +static uint8_t *ram_ptr;
>>> char id[256];
>>> uint8_t len;
>>>
>>> if (flags & RAM_SAVE_FLAG_CONTINUE) {
>>> -if (!block) {
>>> +if (!block || !ram_ptr) {
>>> fprintf(stderr, "Ack, bad migration stream!\n");
>>> return NULL;
>>> }
>>>
>>> -return memory_region_get_ram_ptr(block->mr) + offset;
>>> +return ram_ptr + offset;
>>> }
>>>
>>> len = qemu_get_byte(f);
>>> @@ -1007,8 +1014,10 @@ static inline v
[Qemu-devel] [PATCH] vfio-pci: Quirk RTL8168 NIC
This device is ridiculous. It has two MMIO BARs, BAR4 and BAR2. BAR4
hosts the MSI-X table, so oviously it would be too easy to access it
directly, instead it creates a window register in BAR2 that, among
other things, provides access to the MSI-X table. This means MSI-X
doesn't work in the guest because the driver actually manages to
program the physical table. When interrupt remapping is present, the
device MSI will be blocked. The Linux driver doesn't make use of this
window, so apparently it's not required to make use of MSI-X. This
quirk makes the device work with the Windows driver that does use this
window for MSI-X, but I certainly cannot recommend this device for
assignment (the Windows 7 driver also constantly pokes PCI config
space).
Signed-off-by: Alex Williamson
---
hw/misc/vfio.c | 145
1 file changed, 145 insertions(+)
diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
index 9cf5b84..c3d2f7a 100644
--- a/hw/misc/vfio.c
+++ b/hw/misc/vfio.c
@@ -1668,6 +1668,150 @@ static void vfio_probe_ati_bar4_window_quirk(VFIODevice
*vdev, int nr)
vdev->host.function);
}
+#define PCI_VENDOR_ID_REALTEK 0x10ec
+
+/*
+ * RTL8168 devices have a backdoor that can access the MSI-X table. At BAR2
+ * offset 0x70 there is a dword data register, offset 0x74 is a dword address
+ * register. According to the Linux r8169 driver, the MSI-X table is addressed
+ * when the "type" portion of the address register is set to 0x1. This appears
+ * to be bits 16:30. Bit 31 is both a write indicator and some sort of
+ * "address latched" indicator. Bits 12:15 is a mask field, which we're
+ * going to ignore because we don't really know what it means and the MSI-X
+ * area always seems to be accessed with a full mask. Bits 0:11 is offset
+ * within the type.
+ *
+ * Example trace:
+ *
+ * Read from MSI-X table offset 0
+ * vfio: vfio_bar_write(:05:00.0:BAR2+0x74, 0x1f000, 4) // store read addr
+ * vfio: vfio_bar_read(:05:00.0:BAR2+0x74, 4) = 0x8001f000 // latch
+ * vfio: vfio_bar_read(:05:00.0:BAR2+0x70, 4) = 0xfee00398 // read data
+ *
+ * Write 0xfee0 to MSI-X table offset 0
+ * vfio: vfio_bar_write(:05:00.0:BAR2+0x70, 0xfee0, 4) // write data
+ * vfio: vfio_bar_write(:05:00.0:BAR2+0x74, 0x8001f000, 4) // do write
+ * vfio: vfio_bar_read(:05:00.0:BAR2+0x74, 4) = 0x1f000 // complete
+ */
+
+static uint64_t vfio_rtl8168_window_quirk_read(void *opaque,
+ hwaddr addr, unsigned size)
+{
+VFIOQuirk *quirk = opaque;
+VFIODevice *vdev = quirk->vdev;
+
+switch (addr) {
+case 4: /* address */
+if (quirk->data.flags) {
+DPRINTF("%s fake read(%04x:%02x:%02x.%xd)\n",
+memory_region_name(&quirk->mem), vdev->host.domain,
+vdev->host.bus, vdev->host.slot, vdev->host.function);
+
+return quirk->data.address_match ^ 0x1000U;
+}
+break;
+case 0: /* data */
+if (quirk->data.flags) {
+uint64_t val;
+
+DPRINTF("%s MSI-X table read(%04x:%02x:%02x.%xd)\n",
+memory_region_name(&quirk->mem), vdev->host.domain,
+vdev->host.bus, vdev->host.slot, vdev->host.function);
+
+if (!(vdev->pdev.cap_present & QEMU_PCI_CAP_MSIX)) {
+return 0;
+}
+
+io_mem_read(&vdev->pdev.msix_table_mmio,
+(hwaddr)(quirk->data.address_match & 0xfff),
+&val, size);
+return val;
+}
+}
+
+DPRINTF("%s direct read(%04x:%02x:%02x.%xd)\n",
+memory_region_name(&quirk->mem), vdev->host.domain,
+vdev->host.bus, vdev->host.slot, vdev->host.function);
+
+return vfio_bar_read(&vdev->bars[quirk->data.bar], addr + 0x70, size);
+}
+
+static void vfio_rtl8168_window_quirk_write(void *opaque, hwaddr addr,
+uint64_t data, unsigned size)
+{
+VFIOQuirk *quirk = opaque;
+VFIODevice *vdev = quirk->vdev;
+
+switch (addr) {
+case 4: /* address */
+if ((data & 0x7fff) == 0x1) {
+if (data & 0x1000U &&
+vdev->pdev.cap_present & QEMU_PCI_CAP_MSIX) {
+
+DPRINTF("%s MSI-X table write(%04x:%02x:%02x.%xd)\n",
+memory_region_name(&quirk->mem), vdev->host.domain,
+vdev->host.bus, vdev->host.slot, vdev->host.function);
+
+io_mem_write(&vdev->pdev.msix_table_mmio,
+ (hwaddr)(quirk->data.address_match & 0xfff),
+ data, size);
+}
+
+quirk->data.flags = 1;
+quirk->data.address_match = data;
+
+return;
+}
+quirk->data.flags = 0;
+break;
+case 0: /* data */
+quirk->data.address_mask = data;
+break;
Re: [Qemu-devel] QEMU build broken
On 10/05/14 3:07 AM, Paolo Bonzini wrote: Il 10/05/2014 08:45, Brad Smith ha scritto: Having your feature in-tree is a privilege, not a right. You earn it by helping to maintain it. "it's not really maintained right now" means it has not been earning its keep. You're encouraged to remedy that. Huh? "my feature"? I have nothing to do with this. What kind of crazy is this? How to misdirect and not take responsibility for breaking something. If there wasn't sloppy irresponsible development in the first place it wouldn't be an issue. Brad, all this is doing, is convincing people that bsd-user is not worth keeping in the tree. It's a fact that in a million-line codebase not all patches can be tested by all people. My posts have nothing to do with bsd-user. I don't give a shit about it. The real issue is the process and the fact that someone removed a constant from the configure script and didn't even grep the tree to see if it existed anywhere else. That is very sloppy. Why don't you send a patch instead of whining? Constantly trying to deflect from the real issues. -- This message has been scanned for viruses and dangerous content by MailScanner, and is believed to be clean.
[Qemu-devel] [PATCH v3] snapshot: fixed bdrv_get_full_backing_filename can not get correct full_backing_filename
This patch fixed the following bug:
https://bugzilla.redhat.com/show_bug.cgi?id=1084302 .
path_combine can not calculate the correct full path name for backing_file.
Such as:
create a snapshot chain:
sn2->sn1->$BASE_IMG
backing file is : /home/wookpecker/img.qcow2
sn1 : /home/woodpecker/tmp/sn1
sn2 : /home/woodpecker/tmp/sn2
when create sn2, path_combine can not got a correct path for $BASE_IMG.
In this patch, will check the backing_filename is a symlink or not firstly,
then return the full(absolute) path via realpath.
Signed-off-by: Jun Li
---
block.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/block.c b/block.c
index b749d31..6674719 100644
--- a/block.c
+++ b/block.c
@@ -304,10 +304,26 @@ void path_combine(char *dest, int dest_size,
void bdrv_get_full_backing_filename(BlockDriverState *bs, char *dest, size_t
sz)
{
+struct stat sb;
+char *linkname;
+
if (bs->backing_file[0] == '\0' || path_has_protocol(bs->backing_file)) {
pstrcpy(dest, sz, bs->backing_file);
} else {
-path_combine(dest, sz, bs->filename, bs->backing_file);
+if (lstat(bs->backing_file, &sb) == -1) {
+perror("lstat");
+exit(EXIT_FAILURE);
+}
+
+/* Check linkname is a link or not */
+if (S_ISLNK(sb.st_mode)) {
+linkname = malloc(sb.st_size + 1);
+readlink(bs->backing_file, linkname, sb.st_size + 1);
+linkname[sb.st_size] = '\0';
+realpath(linkname, dest);
+} else {
+realpath(bs->backing_file, dest);
+}
}
}
--
1.8.3.1
Re: [Qemu-devel] [PATCH v2] Add remove_boot_device_path() function for hot-unplug device
On 04/22/2014 05:21 PM, Marcel Apfelbaum wrote:
On Wed, 2014-04-16 at 22:20 +0800, Jun Li wrote:
Add remove_boot_device_path() function to remove bootindex when hot-unplug
a device. This patch fixed virtio-blk/virtio-net/scsi-disk/scsi-generic device.
So it has fixed bug1086603, ref:
https://bugzilla.redhat.com/show_bug.cgi?id=1086603
Make some changes based on Andreas's good suggestion.
Signed-off-by: Jun Li
---
hw/block/virtio-blk.c | 1 +
hw/net/virtio-net.c | 1 +
hw/scsi/scsi-disk.c | 6 --
hw/scsi/scsi-generic.c | 3 +++
include/sysemu/sysemu.h | 2 ++
vl.c| 16
6 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 8a568e5..ecdd266 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -752,6 +752,7 @@ static void virtio_blk_device_unrealize(DeviceState *dev,
Error **errp)
unregister_savevm(dev, "virtio-blk", s);
blockdev_mark_auto_del(s->bs);
virtio_cleanup(vdev);
+remove_boot_device_path(s->conf->bootindex, dev, "/disk@0,0");
}
static Property virtio_blk_properties[] = {
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 33bd233..520c029 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1633,6 +1633,7 @@ static void virtio_net_device_unrealize(DeviceState *dev,
Error **errp)
g_free(n->vqs);
qemu_del_nic(n->nic);
virtio_cleanup(vdev);
+remove_boot_device_path(n->nic_conf.bootindex, dev, "/ethernet-phy@0");
}
static void virtio_net_instance_init(Object *obj)
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 48a28ae..bb2176a 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2150,12 +2150,14 @@ static void scsi_disk_reset(DeviceState *dev)
s->tray_open = 0;
}
-static void scsi_destroy(SCSIDevice *dev)
+static void scsi_destroy(SCSIDevice *sdev)
{
-SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
+SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, sdev);
+DeviceState *dev = DEVICE(sdev);
scsi_device_purge_requests(&s->qdev, SENSE_CODE(NO_SENSE));
blockdev_mark_auto_del(s->qdev.conf.bs);
+remove_boot_device_path(s->qdev.conf.bootindex, dev, NULL);
}
static void scsi_disk_resize_cb(void *opaque)
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 8d92e0d..2531a44 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -388,8 +388,11 @@ static void scsi_generic_reset(DeviceState *dev)
static void scsi_destroy(SCSIDevice *s)
{
+DeviceState *dev = DEVICE(s);
+
scsi_device_purge_requests(s, SENSE_CODE(NO_SENSE));
blockdev_mark_auto_del(s->conf.bs);
+remove_boot_device_path(s->conf.bootindex, dev, NULL);
}
static int scsi_generic_initfn(SCSIDevice *s)
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index ba5c7f8..f7ad1e2 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -193,6 +193,8 @@ void rtc_change_mon_event(struct tm *tm);
void add_boot_device_path(int32_t bootindex, DeviceState *dev,
const char *suffix);
+void remove_boot_device_path(int32_t bootindex, DeviceState *dev,
+ const char *suffix);
char *get_boot_devices_list(size_t *size, bool ignore_suffixes);
DeviceState *get_boot_device(uint32_t position);
diff --git a/vl.c b/vl.c
index 9975e5a..1713c68 100644
--- a/vl.c
+++ b/vl.c
@@ -1184,6 +1184,22 @@ void add_boot_device_path(int32_t bootindex, DeviceState
*dev,
QTAILQ_INSERT_TAIL(&fw_boot_order, node, link);
}
+void remove_boot_device_path(int32_t bootindex, DeviceState *dev,
+ const char *suffix)
+{
+FWBootEntry *node, *next_node;
+
+if (bootindex == -1) {
+return;
+}
+
+QTAILQ_FOREACH_SAFE(node, &fw_boot_order, link, next_node)
+if (node->bootindex == bootindex) {
+QTAILQ_REMOVE(&fw_boot_order, node, link);
Hi Liu.
Sorry for the late response, but I was in vacation :) .
Your patch looks OK, I once wrote another one on the same issue
(it touched the same problem, but it was *completely* different).
Here is the *real* problem: fw_boot_order list is not queried
again on guest reboot, so 'touching' the list has no effect.
While your code is correct (as far as I can tell), it seems
that his place is after the above problem is solved.
No one is currently working on this, feel free
to take this challenge. I'll help however I can.
Hi Marcel,
Sorry, so later response. I got a heavy sick these days. I have see
these discussion emails between you and Paolo.
After guest reboot, seabios will got devices info from boards(this
boards be generated by qemu command line). Just as Paolo's explanations,
but I am not familiar with seabios. I will do some research about this.
Thank you very much.
BTW, My first name is Jun, my last name is Li, my English
