[Qemu-devel] [Bug 709584] Re: Fullscreen mode splits screen over monitor boundaries on dual-monitor system

[Launchpad Bug Tracker]

[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/709584

Title:
  Fullscreen mode splits screen over monitor boundaries on dual-monitor
  system

Status in QEMU:
  Expired

Bug description:
  I originally reported this for the Android SDK emulator, which is
  built on QEMU: http://code.google.com/p/android/issues/detail?id=14336

  Can confirm that this bug is present in stock QEMU shipped with Ubuntu 10.10:
  QEMU PC emulator version 0.12.5 (qemu-kvm-0.12.5), Copyright (c) 2003-2008 
Fabrice Bellard

  Steps to reproduce:
  - get a Linux machine with multiple-monitor setup (tested using NVidia 
proprietary driver on Ubuntu 10.10 x86_64, with two monitors side-by-side in 
'TwinView' configuration)
  - launch qemu
  - hit ctrl+alt+f to go into fullscreen mode

  Expected behavior:
  - emulator should take over one screen (either the first screen, largest 
screen, or screen the emulator is presently on)
  - emulator's display should center or scale to fit within available area of 
the taken-over screen

  Actual behavior:
  - emulator takes over both screens
  - emulator's display is split over the boundary between the two screens

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/709584/+subscriptions

[Qemu-devel] [Bug 899140] Re: Problem with Linux Kernel Traffic Control

[Launchpad Bug Tracker]

[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/899140

Title:
  Problem with Linux Kernel Traffic Control

Status in QEMU:
  Expired

Bug description:
  Hi,

  The last main versions of QEMU (0.14.1, 0.15 and 1.0) have an important 
problem when running on a Linux distribution which running itself a Traffic 
Control (TC) instance.
  Indeed, when TC is configured with a Token Bucket Filter (TBF) with a 
particular rate, the effective rate is very slower than the desired one.

  For instance, lets consider the following configuration :

  # tc qdisc add dev eth0 root tbf rate 20mbit burst 20k latency 50ms

  The effective rate will be about 100kbit/s ! (verified with iperf)
  I've encountered this problem on versions 0.14.1, 0.15 and 1.0 but not with 
the 0.14.0...
  In the 0.14.0, we have a rate of 19.2 mbit/s which is quiet normal.

  I've done the experimentation on several hosts :

  - Debian 32bit core i7, 4GB RAM
  - Debian 64bit core i7, 8GB RAM
  - 3 different high performance servers : Ubuntu 64 bits, 48 AMD Opteron, 
128GB of RAM

  The problem is always the same... The problem is also seen with a
  Class Based Queuing (CBQ) in TC.

  Thanks

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/899140/+subscriptions

Re: [Qemu-devel] [[RFC]PATCH:hw/sd:sd_init()] hw/sd : modified the sd_init() function

[Tejaswini Poluri]

Hii Paolo,
Waiting for your comments on this patch

Regards,
Tejaswini
On 28 Mar 2017 11:01 a.m., "Tejaswini Poluri" 
wrote:

>
>
> On Mon, Mar 27, 2017 at 4:43 PM, Stefan Hajnoczi 
> wrote:
>
>> On Mon, Mar 27, 2017 at 04:01:02PM +0530, Tejaswini wrote:
>> > From: Tejaswini Poluri 
>>
>> Please shorten the subject line: "[PATCH] hw/sd: simplify sd_init()
>> prototype"
>>
>> > @@ -573,16 +573,19 @@ SDState *sd_init(BlockBackend *blk, bool is_spi)
>> >  qdev_prop_set_drive(dev, "drive", blk, );
>> >  if (err) {
>> >  error_report("sd_init failed: %s", error_get_pretty(err));
>> > -return NULL;
>> > +return -1;
>> >  }
>> >  qdev_prop_set_bit(dev, "spi", is_spi);
>> >  object_property_set_bool(obj, true, "realized", );
>> >  if (err) {
>> >  error_report("sd_init failed: %s", error_get_pretty(err));
>> > -return NULL;
>> > +return -1;
>> >  }
>> > -
>> > -return SD_CARD(dev);
>> > +sd_state = SD_CARD(dev);
>>
>> The caller will not see the new value of sd_state.  In C arguments are
>> passed by value.  That means they are local variables inside the
>> function and do not affect the caller.
>>
>> The caller will call sd_init() along with passing SDState pointer as an
> argument like below
> if (sd_init(s->card, blk, false) < 0) .
> And the sd_init() function is modified to
> int sd_init(SDState *sd_state, BlockBackend *blk, bool is_spi)
> so that the caller gets the new value of SDstate updated.
> Looking forward for the comments of Paolo Bonzini to understand what more
> needs to be done as part of the task.
>
> I have CCed Paolo Bonzini, who posted this task.  Maybe he can explain
>> what he meant by "Include SDState by value instead of allocating it in
>> sd_init (hw/sd/)".
>>
>> > +if (!sd_state) {
>> > + return -1;
>> > + }
>>
>> QEMU use 4 space indentation.  Please do not use tabs.
>>
>
>
>
> --
> Regards,
> Tejaswini
>

Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 0/3] Enable MTTCG on PPC64

[luigi burdo]

Hi, on info is mttcg using an amouth of ram for cpu caching and translating 
operations like was did in past by emulators like virtualpc,realpc, bluelabel 
or softwindows?

in case of yes is possible increase it from the command line?

Thanks

Luigi

Re: [Qemu-devel] [PATCH for 2.10] slirp/smb: Replace constant strings by glib string

[Samuel Thibault]

Hello,

Applied to my tree, thanks!

Samuel

[Qemu-devel] [PATCH] alpha-user: wire epoll_create, epoll_ctl, epoll_wait

[Sergei Trofimovich]

Noticed when ran GHC on alpha:
$ qemu-alpha -L /usr/alpha-unknown-linux-gnu/ /tmp/a
qemu: Unsupported syscall: 407

linux-user/syscall.c does have 'epoll_create' wiring,
but under nondeprecated name.

Instead of defining both
TARGET_NR_sys_epoll_create
and
TARGET_NR_epoll_create
I've renamed former to later as old name is not used
anywhere else in qemu.

After this change GHC works fine under qemu-alpha:
$ ./alpha-linux-user/qemu-alpha -L /usr/alpha-unknown-linux-gnu/ /tmp/a
...

Cc: Peter Maydell 
Cc: Riku Voipio 
Cc: qemu-devel@nongnu.org
Signed-off-by: Sergei Trofimovich 
---
 linux-user/alpha/syscall_nr.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/linux-user/alpha/syscall_nr.h b/linux-user/alpha/syscall_nr.h
index 00e14bb6b3..e848154663 100644
--- a/linux-user/alpha/syscall_nr.h
+++ b/linux-user/alpha/syscall_nr.h
@@ -343,9 +343,9 @@
 #define TARGET_NR_io_cancel402
 #define TARGET_NR_exit_group   405
 #define TARGET_NR_lookup_dcookie   406
-#define TARGET_NR_sys_epoll_create 407
-#define TARGET_NR_sys_epoll_ctl408
-#define TARGET_NR_sys_epoll_wait   409
+#define TARGET_NR_epoll_create 407
+#define TARGET_NR_epoll_ctl408
+#define TARGET_NR_epoll_wait   409
 #define TARGET_NR_remap_file_pages 410
 #define TARGET_NR_set_tid_address  411
 #define TARGET_NR_restart_syscall  412
-- 
2.12.2

[Qemu-devel] [PATCH] hw/core: fix segmentation fault

[Suramya Shah]

Reproducer:
 $i386-softmmu/qemu-system-i386 -S -machine isapc,accel=tcg -device amd-iommu
Segmentation fault (core dumped)

Partial bt:
#0  bus_add_child (child=0x56d4e520, bus=0x0) at hw/core/qdev.c:88
#1  qdev_set_parent_bus (dev=0x56d4e520, bus=bus@entry=0x0)
at hw/core/qdev.c:119

Signed-off-by: Suramya Shah 
---
 hw/core/qdev.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 1e7fb33..07a211b 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -84,7 +84,11 @@ static void bus_add_child(BusState *bus, DeviceState *child)
 {
 char name[32];
 BusChild *kid = g_malloc0(sizeof(*kid));
-
+
+if (!bus) {
+error_report("bus not found ");
+exit(0);
+}
 kid->index = bus->max_index++;
 kid->child = child;
 object_ref(OBJECT(kid->child));
-- 
2.9.3

Re: [Qemu-devel] v2.8.1 configure / build fails - just to let you know :

[Jason Vas Dias]

Patch attached - removes '-c' from $CPP invocation in
  pc-bios/optionrom/Makefile
- $CPP was 'cpp' in my environment.
Maybe you should use $CC or $CXX with '-e' flag ? -
then no warning (error) would occur for '-c' .


qemu-v2.8.1-cpp_no-c.patch
Description: Binary data

Re: [Qemu-devel] v2.8.1 configure / build fails - just to let you know :

[Jason Vas Dias]

On 08/04/2017, Jason Vas Dias  wrote:
> Good day -
>
> After checking the v2.8.1 GIT tag into $SRC/qemu  and configuring in a
> separate $BLD/qemu directory :
>
>   $ $SRC/qemu/configure --prefix=/usr --libdir=/usr/lib64 \
>--sysconfdir=/etc --localstatedir=/var \
>--target-list=x86_64-linux-user,x86_64-softmmu,i386-linux-user,\
> i386-softmmu,aarch64-linux-user,aarch64-softmmu \
>2>&1 | tee configure.log
>
> & doing:
>
> $ make -j4 V=1 2>&1 | tee make.log
>
> The build fails for 2 reasons :
>
> 1. I had '-std=gnu11' in my $CFLAGS   , while CXXFLAGS contained
> '-std=gnu++11'
>(CC was set to 'gcc', while CXX was set to 'g++' - these are both
> from GCC 5.4.0
>)  ;
>however, the qemu build scripts incorrectly used $CFLAGS instead of
>$CXXFLAGS, so the build fails without '--disable-werror' because
>   '-std=gnu11' applies only to '--lang=c' compiles, not '--lang=c++'
> compiles -
>   there is no way to separately disable this warning (converted to an
> error).
>   Why is qemu's build scripts not using $CXXFLAGS instead of $CFLAGS here ?
>
> 2. With --disable-werror added to configure flags to overcome (1) , the
> build
> fails because 'cpp' gets a '-c' flag .
> No '-c' flag was in any $CFLAGS, $CXXFLAGS, or $CPPFLAGS setting -
> here are the flags settings I used for configure  -
> ( I actually ran 'c_env $SRC/qemu/configure ...') :
> $ c_env "bash -c 'declare -p CFLAGS CPPFLAGS CXXFLAGS'"
> declare -x CFLAGS="-std=gnu11 -march=x86-64 -mtune=native -O2 -g -fPIC
> -pipe"
> declare -x CPPFLAGS=""
> declare -x CXXFLAGS="-std=gnu++11 -march=x86-64 -mtune=native -O2 -g
> -fPIC -pipe"
>
> $ make -j4 V=1
> ...
> cpp -m32 -I/usr/os_src/qemu/tcg -I/usr/os_src/qemu/tcg/i386
> -I/usr/os_src/qemu/linux-headers
> -I/usr/build/linux/qemu-v2.8.1/linux-headers -I. -I/usr/os_src/qemu
> -I/usr/os_src/qemu/include -I/usr/os_src/qemu/pc-bios/optionrom -I.
> -I/usr/os_src/qemu -MMD -MP -MT kvmvapic.o -MF ./kvmvapic.d -c -o -
> /usr/os_src/qemu/pc-bios/optionrom/kvmvapic.S | as -32 -o kvmvapic.o
> cpp: fatal error: '-c' is not a valid option to the preprocessor
> compilation terminated.
>
> Now I have to dig out in which Makefile variable that erroneous '-c' is -
> nice!
>
> notes from the front ... just thought I should let you know!
>
> I don't think these are unreasonable options to be passing to the build.
> Maybe the build might succeed if I configured with no *FLAGS settings
> at all - but that is not the way autoconf systems are meant to work -
> they are meant to honor one's CFLAGS & CXXFLAGS settings,  however
> mistaken they may appear to be - that is the user's responsibility! - and
> use
> CFLAGS for C programs, and CXXFLAGS for C++ programs, IMHO .
>
> Thanks & Regards,
> Jason
>


This patch is necessary to enable build to complete :

diff --git a/pc-bios/optionrom/Makefile b/pc-bios/optionrom/Makefile
index fa53d9e..f894d5d 100644
--- a/pc-bios/optionrom/Makefile
+++ b/pc-bios/optionrom/Makefile
@@ -43,7 +43,7 @@ build-all: multiboot.bin linuxboot.bin
linuxboot_dma.bin kvmvapic.bin


 %.o: %.S
-   $(call quiet-command,$(CPP) $(QEMU_INCLUDES) $(QEMU_DGFLAGS)
-c -o - $< | $(AS) $(ASFLAGS) -o $@,"AS","$(TARGET_DIR)$@")
+   $(call quiet-command,$(CPP) $(QEMU_INCLUDES) $(QEMU_DGFLAGS)
-o - $< | $(AS) $(ASFLAGS) -o $@,"AS","$(TARGET_DIR)$@")

 %.img: %.o
$(call quiet-command,$(LD) $(LDFLAGS_NOPIE) -m
$(LD_I386_EMULATION) -T $(SRC_PATH)/pc-bios/optionrom/flat.lds -s -o
$@ $<,"BUILD","$(TARGET_DIR)$@")
[

[Qemu-devel] [PATCH v2] hw/net: convert "dma" property type from ptr to link

[Suramya Shah]

The lance device needs pointer to ISA DMA device to operate. But according to
qdev-properties.h, properties of pointer type should be avoided.
A link type property is a good substitution.

Changes since v1
 -changed the code in hw/sparc/sun4m.c which uses the device.

Signed-off-by: Suramya Shah 
---
 hw/net/lance.c   | 8 ++--
 hw/sparc/sun4m.c | 2 +-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/hw/net/lance.c b/hw/net/lance.c
index 573d724..3996b9c 100644
--- a/hw/net/lance.c
+++ b/hw/net/lance.c
@@ -44,6 +44,7 @@
 #include "pcnet.h"
 #include "trace.h"
 #include "sysemu/sysemu.h"
+#include "qapi/error.h"
 
 #define TYPE_LANCE "lance"
 #define SYSBUS_PCNET(obj) \
@@ -145,10 +146,14 @@ static void lance_instance_init(Object *obj)
 device_add_bootindex_property(obj, >conf.bootindex,
   "bootindex", "/ethernet-phy@0",
   DEVICE(obj), NULL);
+
+object_property_add_link(obj, "dma", TYPE_LANCE,
+ (Object **)>state.dma_opaque,
+ qdev_prop_allow_set_link_before_realize,
+ 0, _abort);
 }
 
 static Property lance_properties[] = {
-DEFINE_PROP_PTR("dma", SysBusPCNetState, state.dma_opaque),
 DEFINE_NIC_PROPERTIES(SysBusPCNetState, state.conf),
 DEFINE_PROP_END_OF_LIST(),
 };
@@ -164,7 +169,6 @@ static void lance_class_init(ObjectClass *klass, void *data)
 dc->reset = lance_reset;
 dc->vmsd = _lance;
 dc->props = lance_properties;
-/* Reason: pointer property "dma" */
 dc->cannot_instantiate_with_device_add_yet = true;
 }
 
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 873cd7d..9b3f5a5 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -336,7 +336,7 @@ static void lance_init(NICInfo *nd, hwaddr leaddr,
 
 dev = qdev_create(NULL, "lance");
 qdev_set_nic_properties(dev, nd);
-qdev_prop_set_ptr(dev, "dma", dma_opaque);
+object_property_set_link(OBJECT(dev), OBJECT(dma_opaque), "dma", NULL);
 qdev_init_nofail(dev);
 s = SYS_BUS_DEVICE(dev);
 sysbus_mmio_map(s, 0, leaddr);
-- 
2.9.3

[Qemu-devel] v2.8.1 configure / build fails - just to let you know :

[Jason Vas Dias]

Good day -

After checking the v2.8.1 GIT tag into $SRC/qemu  and configuring in a
separate $BLD/qemu directory :

  $ $SRC/qemu/configure --prefix=/usr --libdir=/usr/lib64 \
   --sysconfdir=/etc --localstatedir=/var \
   --target-list=x86_64-linux-user,x86_64-softmmu,i386-linux-user,\
i386-softmmu,aarch64-linux-user,aarch64-softmmu \
   2>&1 | tee configure.log

& doing:

$ make -j4 V=1 2>&1 | tee make.log

The build fails for 2 reasons :

1. I had '-std=gnu11' in my $CFLAGS   , while CXXFLAGS contained
'-std=gnu++11'
   (CC was set to 'gcc', while CXX was set to 'g++' - these are both
from GCC 5.4.0
   )  ;
   however, the qemu build scripts incorrectly used $CFLAGS instead of
   $CXXFLAGS, so the build fails without '--disable-werror' because
  '-std=gnu11' applies only to '--lang=c' compiles, not '--lang=c++' compiles -
  there is no way to separately disable this warning (converted to an error).
  Why is qemu's build scripts not using $CXXFLAGS instead of $CFLAGS here ?

2. With --disable-werror added to configure flags to overcome (1) , the build
fails because 'cpp' gets a '-c' flag .
No '-c' flag was in any $CFLAGS, $CXXFLAGS, or $CPPFLAGS setting -
here are the flags settings I used for configure  -
( I actually ran 'c_env $SRC/qemu/configure ...') :
$ c_env "bash -c 'declare -p CFLAGS CPPFLAGS CXXFLAGS'"
declare -x CFLAGS="-std=gnu11 -march=x86-64 -mtune=native -O2 -g -fPIC -pipe"
declare -x CPPFLAGS=""
declare -x CXXFLAGS="-std=gnu++11 -march=x86-64 -mtune=native -O2 -g
-fPIC -pipe"

$ make -j4 V=1
...
cpp -m32 -I/usr/os_src/qemu/tcg -I/usr/os_src/qemu/tcg/i386
-I/usr/os_src/qemu/linux-headers
-I/usr/build/linux/qemu-v2.8.1/linux-headers -I. -I/usr/os_src/qemu
-I/usr/os_src/qemu/include -I/usr/os_src/qemu/pc-bios/optionrom -I.
-I/usr/os_src/qemu -MMD -MP -MT kvmvapic.o -MF ./kvmvapic.d -c -o -
/usr/os_src/qemu/pc-bios/optionrom/kvmvapic.S | as -32 -o kvmvapic.o
cpp: fatal error: '-c' is not a valid option to the preprocessor
compilation terminated.

Now I have to dig out in which Makefile variable that erroneous '-c' is - nice!

notes from the front ... just thought I should let you know!

I don't think these are unreasonable options to be passing to the build.
Maybe the build might succeed if I configured with no *FLAGS settings
at all - but that is not the way autoconf systems are meant to work -
they are meant to honor one's CFLAGS & CXXFLAGS settings,  however
mistaken they may appear to be - that is the user's responsibility! - and use
CFLAGS for C programs, and CXXFLAGS for C++ programs, IMHO .

Thanks & Regards,
Jason

Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 0/3] Enable MTTCG on PPC64

[luigi burdo]

Hi i dint made much tests,

but for sure all is faster compared one thread only.

Fore sure tcg need to be optimized (in all emulated architectures) compared 
some old commercial emulators but all is better than before.

I will made more tests tomorrow and report.

ciao

Luigi


Do you have any timings? Did the guest run faster?

Re: [Qemu-devel] [PATCH v2] migration/block: use blk_pwrite_zeroes for each zero cluster

[858585 jemmy]

On Sat, Apr 8, 2017 at 12:52 PM, 858585 jemmy  wrote:
> On Fri, Apr 7, 2017 at 6:10 PM, Fam Zheng  wrote:
>> On Fri, 04/07 16:44, jemmy858...@gmail.com wrote:
>>> From: Lidong Chen 
>>>
>>> BLOCK_SIZE is (1 << 20), qcow2 cluster size is 65536 by default,
>>> this maybe cause the qcow2 file size is bigger after migration.
>>> This patch check each cluster, use blk_pwrite_zeroes for each
>>> zero cluster.
>>>
>>> Signed-off-by: Lidong Chen 
>>> ---
>>>  migration/block.c | 37 +++--
>>>  1 file changed, 35 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/migration/block.c b/migration/block.c
>>> index 7734ff7..c32e046 100644
>>> --- a/migration/block.c
>>> +++ b/migration/block.c
>>> @@ -885,6 +885,11 @@ static int block_load(QEMUFile *f, void *opaque, int 
>>> version_id)
>>>  int64_t total_sectors = 0;
>>>  int nr_sectors;
>>>  int ret;
>>> +int i;
>>> +int64_t addr_offset;
>>> +uint8_t *buf_offset;
>>
>> Poor variable names, they are not offset, maybe "cur_addr" and "cur_buf"? And
>> they can be moved to the loop block below.
> ok, i will change.
>
>>
>>> +BlockDriverInfo bdi;
>>> +int cluster_size;
>>>
>>>  do {
>>>  addr = qemu_get_be64(f);
>>> @@ -934,8 +939,36 @@ static int block_load(QEMUFile *f, void *opaque, int 
>>> version_id)
>>>  } else {
>>>  buf = g_malloc(BLOCK_SIZE);
>>>  qemu_get_buffer(f, buf, BLOCK_SIZE);
>>> -ret = blk_pwrite(blk, addr * BDRV_SECTOR_SIZE, buf,
>>> - nr_sectors * BDRV_SECTOR_SIZE, 0);
>>> +
>>> +ret = bdrv_get_info(blk_bs(blk), );
>>> +cluster_size = bdi.cluster_size;
>>> +
>>> +if (ret == 0 && cluster_size > 0 &&
>>> +cluster_size < BLOCK_SIZE &&
>>
>> I think cluster_size == BLOCK_SIZE should work too.
> This case the (flags & BLK_MIG_FLAG_ZERO_BLOCK) should be true,
> and will invoke blk_pwrite_zeroes before apply this patch.
> but maybe the source qemu maybe not enabled zero flag.
> so i think cluster_size <= BLOCK_SIZE is ok.
>
>>
>>> +BLOCK_SIZE % cluster_size == 0) {
>>> +for (i = 0; i < BLOCK_SIZE / cluster_size; i++) {
>>> +addr_offset = addr * BDRV_SECTOR_SIZE
>>> ++ i * cluster_size;
>>> +buf_offset = buf + i * cluster_size;
>>> +
>>> +if (buffer_is_zero(buf_offset, cluster_size)) {
>>> +ret = blk_pwrite_zeroes(blk, addr_offset,
>>> +cluster_size,
>>> +BDRV_REQ_MAY_UNMAP);
>>> +} else {
>>> + ret = blk_pwrite(blk, addr_offset, buf_offset,
>>> +  cluster_size, 0);
>>> +}
>>> +
>>> +if (ret < 0) {
>>> +g_free(buf);
>>> +return ret;
>>> +}
>>> +}
>>> +} else {
>>> +ret = blk_pwrite(blk, addr * BDRV_SECTOR_SIZE, buf,
>>> + nr_sectors * BDRV_SECTOR_SIZE, 0);
>>> +}
>>>  g_free(buf);
>>>  }
>>>
>>> --
>>> 1.8.3.1
>>>
>>
>> Is it possible use (source) cluster size as the transfer chunk size, instead 
>> of
>> BDRV_SECTORS_PER_DIRTY_CHUNK? Then the existing BLK_MIG_FLAG_ZERO_BLOCK logic
>> can help and you don't need to send zero bytes on the wire. This may still 
>> not
>> be optimal if dest has larger cluster, but it should cover the common use 
>> case
>> well.
>
> yes, i also think BDRV_SECTORS_PER_DIRTY_CHUNK is too large.
> This have two disadvantage:
> 1. it will cause the dest qcow2 file size is bigger after migration.
> 2. it will cause transfer not necessary data, and maybe cause the
> migration can't be successful.
>
> in my production environment, some vm only write 2MB/s, the dirty
> block migrate speed is 70MB/s.
> but it still migration timeout.
>
> but if we change the size of BDRV_SECTORS_PER_DIRTY_CHUNK, it will
> break the protocol.
> the old version qemu will not be able to migrate to new version qemu.
> there are not information about the length about the migration buffer.
>
> so i think we should add new flags to indicate that there are an
> additional byte about the length
> of migration buffer. i will send another patch later, and test the result.

Hi Fam:
Do we need consider the circumstances than migrate from new qemu version
to old qemu version?

>
> this patch is also valuable, there are many old version qemu in my
> production environment.
> and will be benefit with this patch.
>
>>
>> 

Re: [Qemu-devel] [PATCH v3] migration/block:limit the time used for block migration

[858585 jemmy]

On Fri, Apr 7, 2017 at 7:34 PM, Stefan Hajnoczi  wrote:
> On Fri, Apr 07, 2017 at 09:30:33AM +0800, 858585 jemmy wrote:
>> On Thu, Apr 6, 2017 at 10:02 PM, Stefan Hajnoczi  wrote:
>> > On Wed, Apr 05, 2017 at 05:27:58PM +0800, jemmy858...@gmail.com wrote:
>> >
>> > A proper solution is to refactor the synchronous code to make it
>> > asynchronous.  This might require invoking the system call from a
>> > thread pool worker.
>> >
>>
>> yes, i agree with you, but this is a big change.
>> I will try to find how to optimize this code, maybe need a long time.
>>
>> this patch is not a perfect solution, but can alleviate the problem.
>
> Let's try to understand the problem fully first.
>

when migrate the vm with high speed, i find vnc response slowly sometime.
not only vnc response slowly, virsh console aslo response slowly sometime.
and the guest os block io performance is also reduce.

the bug can be reproduce by this command:
virsh migrate-setspeed 165cf436-312f-47e7-90f2-f8aa63f34893 900
virsh migrate --live 165cf436-312f-47e7-90f2-f8aa63f34893
--copy-storage-inc qemu+ssh://10.59.163.38/system

and --copy-storage-all have no problem.
virsh migrate --live 165cf436-312f-47e7-90f2-f8aa63f34893
--copy-storage-all qemu+ssh://10.59.163.38/system

compare the difference between --copy-storage-inc and
--copy-storage-all. i find out the reason is
mig_save_device_bulk invoke bdrv_is_allocated, but bdrv_is_allocated
is synchronous and maybe wait
for a long time.

i write this code to measure the time used by  brdrv_is_allocated()

 279 static int max_time = 0;
 280 int tmp;

 288 clock_gettime(CLOCK_MONOTONIC_RAW, );
 289 ret = bdrv_is_allocated(blk_bs(bb), cur_sector,
 290 MAX_IS_ALLOCATED_SEARCH, _sectors);
 291 clock_gettime(CLOCK_MONOTONIC_RAW, );
 292
 293
 294 tmp =  (ts2.tv_sec - ts1.tv_sec)*10L
 295+ (ts2.tv_nsec - ts1.tv_nsec);
 296 if (tmp > max_time) {
 297max_time=tmp;
 298fprintf(stderr, "max_time is %d\n", max_time);
 299 }

the test result is below:

 max_time is 37014
 max_time is 1075534
 max_time is 17180913
 max_time is 28586762
 max_time is 49563584
 max_time is 103085447
 max_time is 110836833
 max_time is 120331438

bdrv_is_allocated is called after qemu_mutex_lock_iothread.
and the main thread is also call qemu_mutex_lock_iothread.
so cause the main thread maybe wait for a long time.

   if (bmds->shared_base) {
qemu_mutex_lock_iothread();
aio_context_acquire(blk_get_aio_context(bb));
/* Skip unallocated sectors; intentionally treats failure as
 * an allocated sector */
while (cur_sector < total_sectors &&
   !bdrv_is_allocated(blk_bs(bb), cur_sector,
  MAX_IS_ALLOCATED_SEARCH, _sectors)) {
cur_sector += nr_sectors;
}
aio_context_release(blk_get_aio_context(bb));
qemu_mutex_unlock_iothread();
}

#0  0x7f107322f264 in __lll_lock_wait () from /lib64/libpthread.so.0
#1  0x7f107322a508 in _L_lock_854 () from /lib64/libpthread.so.0
#2  0x7f107322a3d7 in pthread_mutex_lock () from /lib64/libpthread.so.0
#3  0x00949ecb in qemu_mutex_lock (mutex=0xfc51a0) at
util/qemu-thread-posix.c:60
#4  0x00459e58 in qemu_mutex_lock_iothread () at /root/qemu/cpus.c:1516
#5  0x00945322 in os_host_main_loop_wait (timeout=28911939) at
util/main-loop.c:258
#6  0x009453f2 in main_loop_wait (nonblocking=0) at util/main-loop.c:517
#7  0x005c76b4 in main_loop () at vl.c:1898
#8  0x005ceb77 in main (argc=49, argv=0x7fff921182b8,
envp=0x7fff92118448) at vl.c:4709



> Stefan

Re: [Qemu-devel] [PATCH v3] migration/block:limit the time used for block migration

[Paolo Bonzini]

On 07/04/2017 19:33, Stefan Hajnoczi wrote:
> The migration thread is holding the QEMU global mutex, the AioContext,
> and the qcow2 s->lock while the L2 table is read from disk.
> 
> The QEMU global mutex is needed for block layer operations that touch
> the global drives list.  bdrv_is_allocated() can be called without the
> global mutex.

Hi Stefan,

only virtio-blk and virtio-scsi take the AioContext lock (because they
support dataplane).  For block migration to work with devices such as
IDE, it needs to take the iothread lock too.  I think there's a comment
about this in migration/block.c.

However, this will hopefully be fixed in 2.10 by making the block layer
thread safe.

Paolo

Re: [Qemu-devel] [PATCH 09/10] blockjob: reorganize block_job_completed_txn_abort

[Paolo Bonzini]

On 08/04/2017 09:15, John Snow wrote:
> 
> 
> On 03/23/2017 01:39 PM, Paolo Bonzini wrote:
>> This splits the part that touches job states from the part that invokes
>> callbacks.  It will be a bit simpler to understand once job states will
>> be protected by a different mutex than the AioContext lock.
>>
> 
> Maybe easier to review then, too :)

The idea is that the "touching job states" part (block_job_cancel_async)
doesn't release the mutex, while the "invokes callbacks" functions
(block_job_finish_sync, block_job_completed_single) part does.  It is
much simpler to understand if all block_job_cancel_asyncs are done first.

I should once more split code movement from changes, but I think this
patch is still reviewable on its own.  All the "introduce block job
mutex" does in block_job_completed_txn_abort is remove all
aio_context_acquire and release calls.

Paolo

>> Signed-off-by: Paolo Bonzini 
>> ---
>>  blockjob.c | 165 
>> -
>>  1 file changed, 88 insertions(+), 77 deletions(-)
>>
>> diff --git a/blockjob.c b/blockjob.c
>> index 093962b..3fa2885 100644
>> --- a/blockjob.c
>> +++ b/blockjob.c
>> @@ -76,6 +76,39 @@ BlockJob *block_job_get(const char *id)
>>  return NULL;
>>  }
>>  
>> +BlockJobTxn *block_job_txn_new(void)
>> +{
>> +BlockJobTxn *txn = g_new0(BlockJobTxn, 1);
>> +QLIST_INIT(>jobs);
>> +txn->refcnt = 1;
>> +return txn;
>> +}
>> +
>> +static void block_job_txn_ref(BlockJobTxn *txn)
>> +{
>> +txn->refcnt++;
>> +}
>> +
>> +void block_job_txn_unref(BlockJobTxn *txn)
>> +{
>> +if (txn && --txn->refcnt == 0) {
>> +g_free(txn);
>> +}
>> +}
>> +
>> +void block_job_txn_add_job(BlockJobTxn *txn, BlockJob *job)
>> +{
>> +if (!txn) {
>> +return;
>> +}
>> +
>> +assert(!job->txn);
>> +job->txn = txn;
>> +
>> +QLIST_INSERT_HEAD(>jobs, job, txn_list);
>> +block_job_txn_ref(txn);
>> +}
>> +
>>  static void block_job_pause(BlockJob *job)
>>  {
>>  job->pause_count++;
>> @@ -336,6 +369,8 @@ void block_job_start(BlockJob *job)
>>  
>>  static void block_job_completed_single(BlockJob *job)
>>  {
>> +assert(job->completed);
>> +
>>  if (!job->ret) {
>>  if (job->driver->commit) {
>>  job->driver->commit(job);
>> @@ -376,14 +411,49 @@ static void block_job_completed_single(BlockJob *job)
>>  static void block_job_cancel_async(BlockJob *job)
>>  {
>>  job->cancelled = true;
>> -block_job_iostatus_reset(job);
>> +if (!job->completed) {
>> +block_job_iostatus_reset(job);
>> +}
>> +}
>> +
>> +static int block_job_finish_sync(BlockJob *job,
>> + void (*finish)(BlockJob *, Error **errp),
>> + Error **errp)
>> +{
>> +Error *local_err = NULL;
>> +int ret;
>> +
>> +assert(blk_bs(job->blk)->job == job);
>> +
>> +block_job_ref(job);
>> +
>> +if (finish) {
>> +finish(job, _err);
>> +}
>> +if (local_err) {
>> +error_propagate(errp, local_err);
>> +block_job_unref(job);
>> +return -EBUSY;
>> +}
>> +/* block_job_drain calls block_job_enter, and it should be enough to
>> + * induce progress until the job completes or moves to the main thread.
>> +*/
>> +while (!job->deferred_to_main_loop && !job->completed) {
>> +block_job_drain(job);
>> +}
>> +while (!job->completed) {
>> +aio_poll(qemu_get_aio_context(), true);
>> +}
>> +ret = (job->cancelled && job->ret == 0) ? -ECANCELED : job->ret;
>> +block_job_unref(job);
>> +return ret;
>>  }
>>  
>>  static void block_job_completed_txn_abort(BlockJob *job)
>>  {
>>  AioContext *ctx;
>>  BlockJobTxn *txn = job->txn;
>> -BlockJob *other_job, *next;
>> +BlockJob *other_job;
>>  
>>  if (txn->aborting) {
>>  /*
>> @@ -392,29 +462,34 @@ static void block_job_completed_txn_abort(BlockJob 
>> *job)
>>  return;
>>  }
>>  txn->aborting = true;
>> +block_job_txn_ref(txn);
>> +
>>  /* We are the first failed job. Cancel other jobs. */
>>  QLIST_FOREACH(other_job, >jobs, txn_list) {
>>  ctx = blk_get_aio_context(other_job->blk);
>>  aio_context_acquire(ctx);
>>  }
>> +
>> +/* Other jobs are "effectively" cancelled by us, set the status for
>> + * them; this job, however, may or may not be cancelled, depending
>> + * on the caller, so leave it. */
>>  QLIST_FOREACH(other_job, >jobs, txn_list) {
>> -if (other_job == job || other_job->completed) {
>> -/* Other jobs are "effectively" cancelled by us, set the status 
>> for
>> - * them; this job, however, may or may not be cancelled, 
>> depending
>> - * on the caller, so leave it. */
>> -if (other_job != job) {
>> -block_job_cancel_async(other_job);
>> -}
>> -continue;
>> + 

Re: [Qemu-devel] [PATCH 10/10] blockjob: use deferred_to_main_loop to indicate the coroutine has ended

[Paolo Bonzini]

On 08/04/2017 09:32, John Snow wrote:
> 
> 
> On 03/23/2017 01:39 PM, Paolo Bonzini wrote:
>> All block jobs are using block_job_defer_to_main_loop as the final
>> step just before the coroutine terminates.  At this point,
>> block_job_enter should do nothing, but currently it restarts
>> the freed coroutine.
>>
>> Now, the job->co states should probably be changed to an enum
>> (e.g. BEFORE_START, STARTED, YIELDED, COMPLETED) subsuming
> 
> Yes, I'd love to formalize the FSM for jobs.
> 
>> block_job_started, job->deferred_to_main_loop and job->busy.
>> For now, this patch eliminates the problematic reenter by
>> removing the reset of job->deferred_to_main_loop (which served
>> no purpose, as far as I could see) and checking the flag in
>> block_job_enter.
> 
> Not sure -- the original commit 794f01414 makes it seem like it should
> stay so that the correct AIO context can be acquired. Probably a race as
> jobs don't often stay long once they've deferred to the main loop, but I
> think the reset is harmless as you say.

You're right.  The difference is that now we pretty much expect the
deferred part to be at the end of the job:

commit bae8196d9f97916de6323e70e3e374362ee16ec4
Author: Paolo Bonzini 
Date:   Thu Oct 27 12:48:50 2016 +0200

blockjob: introduce .drain callback for jobs

This is required to decouple block jobs from running in an
AioContext.  With multiqueue block devices, a BlockDriverState
does not really belong to a single AioContext.

The solution is to first wait until all I/O operations are
complete; then loop in the main thread for the block job to
complete entirely.

so I'll improve the commit message to point to both 794f01414 and this one.

Paolo
>>
>> Signed-off-by: Paolo Bonzini 
>> ---
>>  blockjob.c   | 10 --
>>  include/block/blockjob_int.h |  3 ++-
>>  2 files changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/blockjob.c b/blockjob.c
>> index 3fa2885..2d80dae 100644
>> --- a/blockjob.c
>> +++ b/blockjob.c
>> @@ -750,7 +750,14 @@ void block_job_resume_all(void)
>>  
>>  void block_job_enter(BlockJob *job)
>>  {
>> -if (job->co && !job->busy) {
>> +if (!block_job_started(job)) {
>> +return;
>> +}
>> +if (job->deferred_to_main_loop) {
>> +return;
>> +}
>> +
>> +if (!job->busy) {
>>  qemu_coroutine_enter(job->co);
>>  }
>>  }
>> @@ -874,7 +881,6 @@ static void block_job_defer_to_main_loop_bh(void *opaque)
>>  aio_context_acquire(aio_context);
>>  }
>>  
>> -data->job->deferred_to_main_loop = false;
>>  data->fn(data->job, data->opaque);
>>  
>>  if (aio_context != data->aio_context) {
>> diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
>> index 97ffc43..4d287ba 100644
>> --- a/include/block/blockjob_int.h
>> +++ b/include/block/blockjob_int.h
>> @@ -227,7 +227,8 @@ typedef void BlockJobDeferToMainLoopFn(BlockJob *job, 
>> void *opaque);
>>   * @fn: The function to run in the main loop
>>   * @opaque: The opaque value that is passed to @fn
>>   *
>> - * Execute a given function in the main loop with the BlockDriverState
>> + * This function must be called by the main job coroutine just before it
>> + * returns.  @fn is executed in the main loop with the BlockDriverState
>>   * AioContext acquired.  Block jobs must call bdrv_unref(), bdrv_close(), 
>> and
>>   * anything that uses bdrv_drain_all() in the main loop.
>>   *
>>
> 
> Reviewed-by: John Snow 
> 

Re: [Qemu-devel] [PATCH 08/10] blockjob: introduce block_job_cancel_async

[Paolo Bonzini]

On 08/04/2017 09:13, John Snow wrote:
> 
> 
> On 03/23/2017 01:39 PM, Paolo Bonzini wrote:
>> Signed-off-by: Paolo Bonzini 
> 
> What was the bad design that required you to fix the previous test? :)

It was actually patch 9 that had the bug, I'll reorder to match the
commit message.

Paolo

>> ---
>>  blockjob.c | 11 ---
>>  1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/blockjob.c b/blockjob.c
>> index c9cb5b1..093962b 100644
>> --- a/blockjob.c
>> +++ b/blockjob.c
>> @@ -373,6 +373,12 @@ static void block_job_completed_single(BlockJob *job)
>>  block_job_unref(job);
>>  }
>>  
>> +static void block_job_cancel_async(BlockJob *job)
>> +{
>> +job->cancelled = true;
>> +block_job_iostatus_reset(job);
>> +}
>> +
>>  static void block_job_completed_txn_abort(BlockJob *job)
>>  {
>>  AioContext *ctx;
>> @@ -397,7 +403,7 @@ static void block_job_completed_txn_abort(BlockJob *job)
>>   * them; this job, however, may or may not be cancelled, 
>> depending
>>   * on the caller, so leave it. */
>>  if (other_job != job) {
>> -other_job->cancelled = true;
>> +block_job_cancel_async(other_job);
> 
> Adds an ioreset here, which I think is probably fine...
> 
>>  }
>>  continue;
>>  }
>> @@ -489,8 +495,7 @@ void block_job_user_resume(BlockJob *job)
>>  void block_job_cancel(BlockJob *job)
>>  {
>>  if (block_job_started(job)) {
>> -job->cancelled = true;
>> -block_job_iostatus_reset(job);
>> +block_job_cancel_async(job);
>>  block_job_enter(job);
>>  } else {
>>  block_job_completed(job, -ECANCELED);
>>
> 
> Reviewed-by: John Snow 
> 

Re: [Qemu-devel] [PATCH 05/10] blockjob: separate monitor and blockjob APIs

[Paolo Bonzini]

On 08/04/2017 08:03, John Snow wrote:
> Looks clean, though it may be useful to do a few more things;
> 
> - Demarcate what you think is the monitor API in this file

It's already there:

+/*
+ * API for block job drivers and the block layer.
+ */
+

where everything before is for the monitor.

> - Organize blockjob.h to match to serve as a useful reference.

Hmm, yes.

Paolo

Re: [Qemu-devel] [PATCH 1/4] pam:refactor PAM related code

[Paolo Bonzini]

On 08/04/2017 08:45, Anthony Xu wrote:
> split PAM SMRAM functions in piix.c
> create mch_init_pam in q35.c

Could you further move

MemoryRegion *ram_memory;
MemoryRegion *system_memory;
MemoryRegion *pci_address_space;
PAMMemoryRegion pam_regions[13];

from the northbridge devices up to PCMachineState, and move the common
code for PIIX and Q35 to hw/pci-host/pam.c?

It looks like you can define a better API than what pam.c currently
provides:

void pc_init_pam(PCMachineState *pc_machine, DeviceState *d);
void pc_update_pam(PCMachineState *pc_machine, uint8_t *pam_config);

or even remove "DeviceState *d" because the owner of the memory regions
can be the PCMachineState.

Thanks,

Paolo

> Signed-off-by: Anthony Xu 
> ---
>  hw/pci-host/piix.c | 58 
> ++
>  hw/pci-host/q35.c  | 23 +-
>  2 files changed, 55 insertions(+), 26 deletions(-)
> 
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index f9218aa..ff4e8b5 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -138,16 +138,11 @@ static int pci_slot_get_pirq(PCIDevice *pci_dev, int 
> pci_intx)
>  return (pci_intx + slot_addend) & 3;
>  }
>  
> -static void i440fx_update_memory_mappings(PCII440FXState *d)
> +static void i440fx_update_smram(PCII440FXState *d)
>  {
> -int i;
>  PCIDevice *pd = PCI_DEVICE(d);
>  
>  memory_region_transaction_begin();
> -for (i = 0; i < 13; i++) {
> -pam_update(>pam_regions[i], i,
> -   pd->config[I440FX_PAM + ((i + 1) / 2)]);
> -}
>  memory_region_set_enabled(>smram_region,
>!(pd->config[I440FX_SMRAM] & SMRAM_D_OPEN));
>  memory_region_set_enabled(>smram,
> @@ -155,6 +150,39 @@ static void i440fx_update_memory_mappings(PCII440FXState 
> *d)
>  memory_region_transaction_commit();
>  }
>  
> +static void i440fx_update_pam(PCII440FXState *d)
> +{
> +int i;
> +PCIDevice *pd = PCI_DEVICE(d);
> +memory_region_transaction_begin();
> +pam_update(>pam_regions[0], 0,
> +   pd->config[I440FX_PAM]);
> +for (i = 1; i < 13; i++) {
> +pam_update(>pam_regions[i], i,
> +   pd->config[I440FX_PAM + ((i + 1) / 2)]);
> +}
> +memory_region_transaction_commit();
> +}
> +
> +static void i440fx_update_memory_mappings(PCII440FXState *d)
> +{
> +i440fx_update_pam(d);
> +i440fx_update_smram(d);
> +}
> +
> +
> +static void i440fx_init_pam(PCII440FXState *d)
> +{
> +int i;
> +init_pam(DEVICE(d), d->ram_memory, d->system_memory,
> + d->pci_address_space, >pam_regions[0],
> + PAM_BIOS_BASE,  PAM_BIOS_SIZE);
> +for (i = 0; i < 12; ++i) {
> +init_pam(DEVICE(d), d->ram_memory, d->system_memory,
> + d->pci_address_space, >pam_regions[i + 1],
> + PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE, PAM_EXPAN_SIZE);
> +}
> +}
>  
>  static void i440fx_write_config(PCIDevice *dev,
>  uint32_t address, uint32_t val, int len)
> @@ -163,9 +191,12 @@ static void i440fx_write_config(PCIDevice *dev,
>  
>  /* XXX: implement SMRAM.D_LOCK */
>  pci_default_write_config(dev, address, val, len);
> -if (ranges_overlap(address, len, I440FX_PAM, I440FX_PAM_SIZE) ||
> -range_covers_byte(address, len, I440FX_SMRAM)) {
> -i440fx_update_memory_mappings(d);
> +if (ranges_overlap(address, len, I440FX_PAM, I440FX_PAM_SIZE)) {
> +i440fx_update_pam(d);
> +}
> +
> +if (range_covers_byte(address, len, I440FX_SMRAM)) {
> +i440fx_update_smram(d);
>  }
>  }
>  
> @@ -335,7 +366,6 @@ PCIBus *i440fx_init(const char *host_type, const char 
> *pci_type,
>  PCIHostState *s;
>  PIIX3State *piix3;
>  PCII440FXState *f;
> -unsigned i;
>  I440FXState *i440fx;
>  
>  dev = qdev_create(NULL, host_type);
> @@ -378,13 +408,7 @@ PCIBus *i440fx_init(const char *host_type, const char 
> *pci_type,
>  object_property_add_const_link(qdev_get_machine(), "smram",
> OBJECT(>smram), _abort);
>  
> -init_pam(dev, f->ram_memory, f->system_memory, f->pci_address_space,
> - >pam_regions[0], PAM_BIOS_BASE, PAM_BIOS_SIZE);
> -for (i = 0; i < 12; ++i) {
> -init_pam(dev, f->ram_memory, f->system_memory, f->pci_address_space,
> - >pam_regions[i+1], PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE,
> - PAM_EXPAN_SIZE);
> -}
> +i440fx_init_pam(f);
>  
>  /* Xen supports additional interrupt routes from the PCI devices to
>   * the IOAPIC: the four pins of each PCI device on the bus are also
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index 344f77b..8866357 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -460,9 +460,21 @@ static void mch_reset(DeviceState *qdev)
>  mch_update(mch);
>  }
>  
> -static void 

Re: [Qemu-devel] [PATCH 4/4] pam: setup pc.bios

[Paolo Bonzini]

On 08/04/2017 08:45, Anthony Xu wrote:
> -if (!isapc_ram_fw) {
> -memory_region_set_readonly(bios, true);
> +if (PC_MACHINE(current_machine)->pam) {
> +/* if PAM is disabled, set it as readwrite */
> +if (!isapc_ram_fw) {
> +memory_region_set_readonly(bios, true);
> +}
>  }

I think this is wrong, the high copy should remain read-only or pflash
stops working when you remove PAM.

The comment only explains the "what" but not the "why" and the "why" is
not in the commit message.  See also here:

> +if (PC_MACHINE(current_machine)->pam) {
> +memory_region_add_subregion_overlap(rom_memory,
> +0x10 - isa_bios_size,
> +isa_bios,
> +1);
> +if (!isapc_ram_fw) {
> +memory_region_set_readonly(isa_bios, true);
> +}
> +} else {
> +/* if PAM is disabed, add isa-bios to system memory region */
> +memory_region_add_subregion_overlap(system_memory,
>  0x10 - isa_bios_size,
>  isa_bios,
>  1);

Thanks,

Paolo

[Qemu-devel] [PATCH] raspi: Add Raspberry Pi 1 support

[Omar Rizwan]

Signed-off-by: Omar Rizwan 
---
 hw/arm/Makefile.objs |   2 +-
 hw/arm/bcm2835.c | 119 +++
 hw/arm/bcm2835_peripherals.c |   1 +
 hw/arm/raspi.c   |  47 ++---
 include/hw/arm/bcm2835.h |  31 +++
 5 files changed, 191 insertions(+), 9 deletions(-)
 create mode 100644 hw/arm/bcm2835.c
 create mode 100644 include/hw/arm/bcm2835.h

diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index 4c5c4ee..77ef47f 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -11,7 +11,7 @@ obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o 
pxa2xx_pic.o
 obj-$(CONFIG_DIGIC) += digic.o
 obj-y += omap1.o omap2.o strongarm.o
 obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o
-obj-$(CONFIG_RASPI) += bcm2835_peripherals.o bcm2836.o raspi.o
+obj-$(CONFIG_RASPI) += bcm2835_peripherals.o bcm2835.o bcm2836.o raspi.o
 obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o
 obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp.o xlnx-ep108.o
 obj-$(CONFIG_FSL_IMX25) += fsl-imx25.o imx25_pdk.o
diff --git a/hw/arm/bcm2835.c b/hw/arm/bcm2835.c
new file mode 100644
index 000..b19b86c
--- /dev/null
+++ b/hw/arm/bcm2835.c
@@ -0,0 +1,119 @@
+/*
+ * Raspberry Pi emulation (c) 2012 Gregory Estrade
+ * Upstreaming code cleanup [including bcm2835_*] (c) 2013 Jan Petrous
+ *
+ * Raspberry Pi 2 emulation and refactoring Copyright (c) 2015, Microsoft
+ * Written by Andrew Baumann
+ *
+ * Rebase onto master (c) 2017 Omar Rizwan
+ *
+ * This code is licensed under the GNU GPLv2 and later.
+ */
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "hw/arm/bcm2835.h"
+#include "hw/arm/raspi_platform.h"
+#include "hw/sysbus.h"
+#include "exec/address-spaces.h"
+
+/* Peripheral base address seen by the CPU */
+#define BCM2835_PERI_BASE   0x2000
+
+static void bcm2835_init(Object *obj)
+{
+BCM2835State *s = BCM2835(obj);
+
+object_initialize(>cpu, sizeof(s->cpu), "arm1176-" TYPE_ARM_CPU);
+object_property_add_child(obj, "cpu", OBJECT(>cpu), _abort);
+
+object_initialize(>peripherals, sizeof(s->peripherals),
+  TYPE_BCM2835_PERIPHERALS);
+object_property_add_child(obj, "peripherals", OBJECT(>peripherals),
+  _abort);
+object_property_add_alias(obj, "board-rev", OBJECT(>peripherals),
+  "board-rev", _abort);
+object_property_add_alias(obj, "vcram-size", OBJECT(>peripherals),
+  "vcram-size", _abort);
+qdev_set_parent_bus(DEVICE(>peripherals), sysbus_get_default());
+}
+
+static void bcm2835_realize(DeviceState *dev, Error **errp)
+{
+BCM2835State *s = BCM2835(dev);
+Object *obj;
+Error *err = NULL;
+
+/* common peripherals from bcm2835 */
+obj = object_property_get_link(OBJECT(dev), "ram", );
+if (obj == NULL) {
+error_setg(errp, "%s: required ram link not found: %s",
+   __func__, error_get_pretty(err));
+return;
+}
+
+object_property_add_const_link(OBJECT(>peripherals), "ram", obj, );
+if (err) {
+error_propagate(errp, err);
+return;
+}
+
+object_property_set_bool(OBJECT(>peripherals), true, "realized", );
+if (err) {
+error_propagate(errp, err);
+return;
+}
+
+object_property_add_alias(OBJECT(s), "sd-bus", OBJECT(>peripherals),
+  "sd-bus", );
+if (err) {
+error_propagate(errp, err);
+return;
+}
+
+sysbus_mmio_map_overlap(SYS_BUS_DEVICE(>peripherals), 0,
+BCM2835_PERI_BASE, 1);
+
+sysbus_mmio_map_overlap(SYS_BUS_DEVICE(>peripherals), 1,
+0x4000, 1);
+
+object_property_set_bool(OBJECT(>cpu), true, "realized", );
+if (err) {
+error_report_err(err);
+exit(1);
+}
+
+sysbus_connect_irq(SYS_BUS_DEVICE(>peripherals), 0,
+   qdev_get_gpio_in(DEVICE(>cpu), ARM_CPU_IRQ));
+sysbus_connect_irq(SYS_BUS_DEVICE(>peripherals), 1,
+   qdev_get_gpio_in(DEVICE(>cpu), ARM_CPU_FIQ));
+
+}
+
+static void bcm2835_class_init(ObjectClass *oc, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(oc);
+
+dc->realize = bcm2835_realize;
+
+/*
+ * Reason: creates an ARM CPU, thus use after free(), see
+ * arm_cpu_class_init()
+ */
+dc->cannot_destroy_with_object_finalize_yet = true;
+}
+
+static const TypeInfo bcm2835_type_info = {
+.name = TYPE_BCM2835,
+.parent = TYPE_SYS_BUS_DEVICE,
+.instance_size = sizeof(BCM2835State),
+.instance_init = bcm2835_init,
+.class_init = bcm2835_class_init,
+};
+
+static void bcm2835_register_types(void)
+{
+type_register_static(_type_info);
+}
+
+type_init(bcm2835_register_types)
diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 369ef1e..a6cd54a 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ 

Re: [Qemu-devel] [PATCH] raspi: Add Raspberry Pi 1 support

[Omar Rizwan]

Hi Andrew --

On Fri, Apr 7, 2017 at 10:57 PM Andrew Baumann
 wrote:
>
> Hi Omar,
>
> > From: Omar Rizwan [mailto:omar.riz...@gmail.com]
> > Sent: Friday, 7 April 2017 22:43
>
> Did you do any testing of this? One of the reasons I never got around to 
> upstreaming this was that I couldn't get Raspbian working reliably (there was 
> some problem with stalled DMA reads from the MMC controller), and I didn't 
> have the cycles to debug it further. Also, IIRC Raspbian on pi1 depended on 
> the system timer devices, so it might not make sense to have this board-level 
> support without adding that first.


I did test Raspbian on the raspi machine and maybe encountered the
same issue you did (or, more likely, a systimer related issue);
it would just hang in a loop with program counter at 0xC000A000.

But my use for this is bare-metal code, which works OK.
(Is there some standard that the machine should run Linux before
getting upstreamed?)

I'd like to upstream the system timer next, anyway.

>
> > --- a/hw/arm/bcm2835_peripherals.c
> > +++ b/hw/arm/bcm2835_peripherals.c
> > @@ -34,6 +34,7 @@ static void bcm2835_peripherals_init(Object *obj)
> >  /* Internal memory region for peripheral bus addresses (not exported) 
> > */
> >  memory_region_init(>gpu_bus_mr, obj, "bcm2835-gpu", (uint64_t)1 <<
> > 32);
> >  object_property_add_child(obj, "gpu-bus", OBJECT(>gpu_bus_mr),
> > NULL);
> > +sysbus_init_mmio(SYS_BUS_DEVICE(s), >gpu_bus_mr);
> >
> >  /* Internal memory region for request/response communication with
> >   * mailbox-addressable peripherals (not exported)
>
> This line looks like it might have snuck in by accident?


Do you recall why that line was removed? bcm2835_realize fails without
it, because the second sysbus_mmio_map_overlap call

sysbus_mmio_map_overlap(SYS_BUS_DEVICE(>peripherals), 1,
0x4000, 1);

fails this assertion in sysbus.c, in sysbus_mmio_map_common:

assert(n >= 0 && n < dev->num_mmio);

I can't easily find documentation for that 0x4000 memory mapping,
which I figured I'd keep in; should it just be removed from bcm2835?
Am I misunderstanding something?

>
>
> Andrew


thanks,
Omar