RE: Any example command lines for the RaspberryPi Models

[Andrew Baumann]

> From: Alex Bennée 
>
> Someone was asking on IRC earlier and I realised we don't give any examples
> in the documentation of how to attach block devices. I found a reference in
> an out-of-tree repo that mentioned -sd which seems right but I assume you
> must be able to use -device sd-card as well?
> 
> Could we document some of the common boot examples in
> docs/system/arm/raspi.rst please?

All the examples I have used -sd. Here's an invocation that worked back in 
2016/17 (sorry I know this is not terribly helpful):

  qemu-system-arm -M raspi2 -kernel raspbian-boot/kernel7.img -sd
  2015-09-24-raspbian-jessie.vhd -append "rw earlyprintk loglevel=8
  console=ttyAMA0 root=/dev/mmcblk0p2 rootwait" -serial stdio

Unfortunately I have not been keeping track of raspi support in QEMU. I'll send 
a patch to remove myself from MAINTAINERS.

Andrew

[PATCH] MAINTAINERS: Remove myself (for raspi).

[Andrew Baumann]

Signed-off-by: Andrew Baumann 
---
 MAINTAINERS | 1 -
 1 file changed, 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index e4b3a4bcdf..3baa83dfc9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -818,7 +818,6 @@ F: docs/system/arm/palm.rst

 Raspberry Pi
 M: Peter Maydell 
-R: Andrew Baumann 
 R: Philippe Mathieu-Daudé 
 L: qemu-...@nongnu.org
 S: Odd Fixes
--
2.25.1

Re: [Qemu-devel] RFC raspberry pi sd-card reset

[Andrew Baumann via Qemu-devel]

Hi,



(Sorry for top-posting, just wanted to give you some quick context.)



The Pi-specific quirk here is that there are two different SD controllers on 
the board, both accessing the same card, where only one can be used at a time. 
IIRC Clement Deschamps added this reparenting logic to accomplish that when he 
implemented the second SD controller. I can’t give you a concrete suggestion, 
but “initialize the platform with the sd-card at the right initial place” is 
not really viable given that the right place changes depending on GPIO 
programming by the guest.



Andrew




From: Damien Hedde 
Sent: Wednesday, July 31, 2019 7:21:02 AM
To: QEMU Developers 
Cc: Peter Maydell ; Andrew Baumann 
; f4...@amsat.org ; qemu-arm 

Subject: RFC raspberry pi sd-card reset

Hi,

Concerning the reset on the raspi2/3 machine, I ran into an issue with
the sd-card.

Here follows a subset of the qbus/qdev tree of the raspi2&3 machine as
it is initialized:
 + System
   * bcm2835_gpio
 + sd-bus
   * sd-card
   * bcm2835-sdhost
 + bcm2835-sdhost-bus
   * generic-sdhci
 + sdhci-bus

raspi_init does 2 things:
 + it creates the soc
 + then it explicitly creates the sd-card on the bcm2835_gpio's sd-bus

As it happens, the reset moves the sd-card on another bus: the
sdhci-bus. More precisely the bcm2835_gpio reset method does it (the
sd-card can be mapped on bcm2835-sdhost-bus or sdhci-bus depending on
bcm2835_gpio registers, reset configuration corresponds to the sdhci-bus).

Reset call order is the following (it follows children-before-parent order):
 1 sd-card
 2 sd-bus
 3 bcm2835_gpio -> move the sd-card
 4 bcm2835-sdhost-bus
 5 bcm2835-sdhost
 6 sd-card  (again)
 7 sdhci-bus
 8 generic-sdhci

In the end, the sd-card is reset twice, which is no big-deal in itself.
But it only depends on the bcm2835_gpio tree being reset before the
generic-sdhci (probably depends on the machine creation code).

I checked and it seems this is the only platform where such things
happen during master reset.

IMO this is a bit hazardous because reset is based on the qdev/qbus
tree (and with the multi-phase I'm working on, even if it still works,
it's worse).
I'm not sure what we should do to avoid this (and if there's is
something to be done).

The easiest solution would be to initialize the platform with the
sd-card at the right initial place (I do not really understand why it is
created in the bcm2835_gpio in the first place since we move it just
after). But it won't solve the issue if a reset is done afterwards.

Or maybe we could move the sd-card on the proper bus in a machine
reset code so that it's on the right place when we do the sysbus tree
reset just after.

What do you think ?

--
Damien

Re: [Qemu-devel] [PATCH] Added periodic IRQ support for bcm2836_control local timer

[Andrew Baumann via Qemu-devel]

> From: bzt 
> Sent: Wednesday, 27 February 2019 03:54
> 
> I'd like to add more drivers for the bcm283[567] too, and this question goes 
> to
> Andrew Baumann mostly. I've seen implemented those missing drivers in his
> repo, which aren't in the qemu mainline yet. I don't want to reinvent the 
> wheel,
> so I'm willing to fix those classes to get them right, but I'd like to know 
> what's
> wrong with them in the first place.

Nothing's wrong with them per se, but when I was working on upstreaming the 
raspi board I prioritised the device support needed to boot Linux and Windows 
on raspi2. The remaining devices were always planned for eventual inclusion, 
but just fell off the end of my TODO list. They never went through code review 
on qemu-devel, so would probably need some work to bring them up to modern qemu 
APIs and standards.

> I'm interested in fixing the
> TYPE_BCM2835_POWER and TYPE_BCM2835_ST drivers, which I know many
> hoppy OS developers lack and would improve the raspi emulation experience
> remarkably.

Sounds good to me! 

> It would be still a long way to boot a raspbian image, but still, a
> small step towards that goal at least.

That's interesting. Raspbian did seem to be working (on pi2) when I last worked 
on this. Perhaps it now depends on these devices, but didn't before?

Cheers,
Andrew

Re: [Qemu-devel] [v3 PATCH] hw/arm/bcm2835_peripherals: add bcm283x sp804-alike timer

[Andrew Baumann via Qemu-devel]

Hi Mark,

> From: Mark 
> Sent: Sunday, 10 February 2019 08:58
> Subject: [v3 PATCH] hw/arm/bcm2835_peripherals: add bcm283x sp804-alike
> timer

Thanks for the patch. The integration with bcm2835_peripherals looks good, but 
hopefully one of the QEMU maintainers can give you a review of the timer 
implementation as I'm really short on time at the moment.

If you didn't already see it, you may want to compare with the implementation 
here, which originates from Gregory Estrade:
https://github.com/0xabu/qemu/blob/raspi/hw/timer/bcm2835_timer.c
... sadly I never got around to merging it upstream with the rest of the raspi 
emulation. ☹

Cheers,
Andrew

Re: [Qemu-devel] [PATCH for 3.1 v2 1/7] MAINTAINERS: Add an entry for the Raspberry Pi machines

[Andrew Baumann via Qemu-devel]

> From: Peter Maydell 
> Sent: Monday, 5 November 2018 09:58
> 
> On 5 November 2018 at 17:06, Andrew Baumann
>  wrote:
> > From: Peter Maydell 
> >> Andrew, as the original submitter of the Raspberry Pi code would you
> >> like to be listed as a maintainer for it?
> >
> > Thanks for asking; probably not -- I'm happy to review the odd patch
> > if my input would be helpful, but I haven't worked on it for years,
> > and I don't have spare cycles, so I'd just be a bottleneck.
> 
> Sure, no problem. Would you like to be listed as a "reviewer"
> (means people will CC you on patches, basically), or not at all?

Sure, reviewer is fine with me.

Andrew

Re: [Qemu-devel] [PATCH for 3.1 v2 1/7] MAINTAINERS: Add an entry for the Raspberry Pi machines

[Andrew Baumann via Qemu-devel]

> From: Peter Maydell 
> Sent: Monday, 5 November 2018 07:26
> 
> On 2 November 2018 at 00:12, Philippe Mathieu-Daudé 
> wrote:
> > So far the Raspi machines use the BCM2836 SoC which includes a BCM2835
> > for the peripherals.
> >
> > Signed-off-by: Philippe Mathieu-Daudé 
> > ---
> >  MAINTAINERS | 6 ++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS index f2360efe3e..c41ea5ed3f
> > 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -683,6 +683,12 @@ F: hw/arm/nrf51_soc.c
> >  F: hw/arm/microbit.c
> >  F: include/hw/arm/nrf51_soc.h
> >
> > +Raspberry Pi
> > +L: qemu-...@nongnu.org
> > +S: Odd Fixes
> > +F: hw/*/bcm283*
> > +F: include/hw/*/bcm283*
> > +
> 
> We should also have F:hw/arm/raspi.c
> 
> Andrew, as the original submitter of the Raspberry Pi code would you like to
> be listed as a maintainer for it?

Thanks for asking; probably not -- I'm happy to review the odd patch if my 
input would be helpful, but I haven't worked on it for years, and I don't have 
spare cycles, so I'd just be a bottleneck.

Andrew

Re: [Qemu-devel] crashes with win2008 host

[Andrew Baumann via Qemu-devel]

Thanks for digging into this Fred, and sorry -- it seems Andrey and I both 
missed that subtlety with TryAcquireSRWLockExclusive when we first made this 
change.



On the other hand, since these OSes are a decade old (mainstream support ended; 
will drop out of extended support in just over a year from now), and you are 
the first person to report a problem in 1.5 years since the patch went in, I 
wonder if we could “fix” the problem by updating the required OS versions for 
QEMU on Windows to Windows 7 / Server 2008 R2...



Paolo, do you have any opinion? Note that backing out the patch isn’t a great 
idea either – it fixed a deadlock.



Andrew




From: KONRAD Frederic 
Sent: Friday, September 14, 2018 4:17:36 AM
To: Andrew Baumann; Andrey Shedel
Cc: Paolo Bonzini; QEMU Developers
Subject: Re: [Qemu-devel] crashes with win2008 host

Ok finally got it,

The SRWL API seems to be available since Vista and Server 2008
except the 'TryAcquireSRWLockExclusive' function which is
available starting Seven and Server 2008 *R2*. Hence the error
message.

So basically that means QEMU is not compatible with version older
than Seven or 2008 R2.

Cheers,
Fred

Le 09/14/2018 à 10:29 AM, KONRAD Frederic a écrit :
>
>
> Le 09/13/2018 à 07:29 PM, Andrew Baumann a écrit :
>> Does this crash always happen at startup? Is it deterministic?
>>
>
> Hi Andrew,
>
> Thanks for your reactivity! Yes it crashes all the time..
>
>>
>>
>> c135 is STATUS_DLL_NOT_FOUND. I suspect ntdll is trying to
>> demand-load another DLL to provide that API, and it is missing
>> or corrupt on your Windows installation
>
> Actually that helps! Compiling in debug mode gave me this
> errors.. But it wasn't the error I was chasing..
>
> The initial error I was chasing is the following:
>gdb: unknown target exception 0xc139 at 0x77636698
>
> When I execute it in graphic mode I get the following issue:
>
>The procedure entry point TryAcquireSRWLockExclusive could not
>be located in the dynamic link library KERNEL32.dll.
>
> It seems that there was the same issue raised with MySQL on
> Windows server 2008:
>https://forums.mysql.com/read.php?11,642417,642417
>
> Is there anything I can do appart swaping back locally to the CS?
>
> Thanks,
> Fred
>
>>
>>
>>
>> BTW, you’ll probably get a better stack trace from a native
>> debugger (windbg, etc.) in this scenario.
>>
>>
>>
>> Cheers,
>>
>> Andrew
>>
>>
>>
>>
>>
>> 
>> From: KONRAD Frederic 
>> Sent: Thursday, September 13, 2018 10:02:56 AM
>> To: Andrey Shedel
>> Cc: Andrew Baumann; Paolo Bonzini; QEMU Developers
>> Subject: crashes with win2008 host
>>
>> Hi Andrey,
>>
>> I've strange crashes since this commit: (yes its old)
>>
>> commit 12f8def0e02232d7c6416ad9b66640f973c531d1
>> Author: Andrey Shedel 
>> Date:   Fri Mar 24 15:01:41 2017 -0700
>>
>>   win32: replace custom mutex and condition variable with
>>  native primitives
>>
>> Basically it just crashes.. (exception 0xc135) like this:
>>
>> (gdb) run
>> Starting program: C:\home\konrad\temp\qemu-system-sparc --version
>> [New Thread 5324.0xdf8]
>> gdb: unknown target exception 0xc135 at 0x77636698
>> gdb: unknown target exception 0xc135 at 0x77636698
>>
>> Program received signal ?, Unknown signal.
>> 0x77636698 in ntdll!RtlRaiseStatus ()
>>  from C:\Windows\system32\ntdll.dll
>> (gdb) bt
>> #0  0x77636698 in ntdll!RtlRaiseStatus ()
>>  from C:\Windows\system32\ntdll.dll
>> #1  0x775dcbf7 in ntdll!LdrGetProcedureAddress ()
>>  from C:\Windows\system32\ntdll.dll
>> #2  0x775a536e in ntdll!LdrInitializeThunk ()
>>  from C:\Windows\system32\ntdll.dll
>> #3  0x in ?? ()
>> Backtrace stopped: previous frame inner to this frame (corrupt
>> stack?)
>> (gdb)
>>
>> Sorry the backtrace is not really helpful..
>>
>> I can reproduce the same behavior with v3.0.0.. and only with
>> the Windows 2008 server host..
>>
>> If I partially revert the patch, eg: using CriticalSection
>> instead of SRWL it seems to work.. But I don't understand why
>> because SRWL should be supported on 2008 Server..
>>
>> Here is the change I did (which is wrongly making qemu_mutex
>> recursive for now):
>>
>> diff --git a/include/qemu/thread-win32.h
>> b/include/qemu/thread-win32.h
>> index d668d789b4..b335687604 100644
>> --- a/inclu

Re: [Qemu-devel] crashes with win2008 host

[Andrew Baumann via Qemu-devel]

Does this crash always happen at startup? Is it deterministic?



c135 is STATUS_DLL_NOT_FOUND. I suspect ntdll is trying to demand-load 
another DLL to provide that API, and it is missing or corrupt on your Windows 
installation.



BTW, you’ll probably get a better stack trace from a native debugger (windbg, 
etc.) in this scenario.



Cheers,

Andrew






From: KONRAD Frederic 
Sent: Thursday, September 13, 2018 10:02:56 AM
To: Andrey Shedel
Cc: Andrew Baumann; Paolo Bonzini; QEMU Developers
Subject: crashes with win2008 host

Hi Andrey,

I've strange crashes since this commit: (yes its old)

commit 12f8def0e02232d7c6416ad9b66640f973c531d1
Author: Andrey Shedel 
Date:   Fri Mar 24 15:01:41 2017 -0700

 win32: replace custom mutex and condition variable with
native primitives

Basically it just crashes.. (exception 0xc135) like this:

(gdb) run
Starting program: C:\home\konrad\temp\qemu-system-sparc --version
[New Thread 5324.0xdf8]
gdb: unknown target exception 0xc135 at 0x77636698
gdb: unknown target exception 0xc135 at 0x77636698

Program received signal ?, Unknown signal.
0x77636698 in ntdll!RtlRaiseStatus ()
from C:\Windows\system32\ntdll.dll
(gdb) bt
#0  0x77636698 in ntdll!RtlRaiseStatus ()
from C:\Windows\system32\ntdll.dll
#1  0x775dcbf7 in ntdll!LdrGetProcedureAddress ()
from C:\Windows\system32\ntdll.dll
#2  0x775a536e in ntdll!LdrInitializeThunk ()
from C:\Windows\system32\ntdll.dll
#3  0x in ?? ()
Backtrace stopped: previous frame inner to this frame (corrupt
stack?)
(gdb)

Sorry the backtrace is not really helpful..

I can reproduce the same behavior with v3.0.0.. and only with
the Windows 2008 server host..

If I partially revert the patch, eg: using CriticalSection
instead of SRWL it seems to work.. But I don't understand why
because SRWL should be supported on 2008 Server..

Here is the change I did (which is wrongly making qemu_mutex
recursive for now):

diff --git a/include/qemu/thread-win32.h
b/include/qemu/thread-win32.h
index d668d789b4..b335687604 100644
--- a/include/qemu/thread-win32.h
+++ b/include/qemu/thread-win32.h
@@ -4,7 +4,8 @@
  #include 

  struct QemuMutex {
-SRWLOCK lock;
+CRITICAL_SECTION lock;
+LONG owner;
  #ifdef CONFIG_DEBUG_MUTEX
  const char *file;
  int line;
diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c
index b303188a36..09ce4fd957 100644
--- a/util/qemu-thread-win32.c
+++ b/util/qemu-thread-win32.c
@@ -45,7 +45,7 @@ static void error_exit(int err, const char *msg)

  void qemu_mutex_init(QemuMutex *mutex)
  {
-InitializeSRWLock(>lock);
+InitializeCriticalSection(>lock);
  qemu_mutex_post_init(mutex);
  }

@@ -53,14 +53,14 @@ void qemu_mutex_destroy(QemuMutex *mutex)
  {
  assert(mutex->initialized);
  mutex->initialized = false;
-InitializeSRWLock(>lock);
+DeleteCriticalSection(>lock);
  }

  void qemu_mutex_lock_impl(QemuMutex *mutex, const char *file,
const int line)
  {
  assert(mutex->initialized);
  qemu_mutex_pre_lock(mutex, file, line);
-AcquireSRWLockExclusive(>lock);
+EnterCriticalSection(>lock);
  qemu_mutex_post_lock(mutex, file, line);
  }

@@ -69,7 +69,7 @@ int qemu_mutex_trylock_impl(QemuMutex *mutex,
const char *file, const int line)
  int owned;

  assert(mutex->initialized);
-owned = TryAcquireSRWLockExclusive(>lock);
+owned = TryEnterCriticalSection(>lock);there
  if (owned) {
  qemu_mutex_post_lock(mutex, file, line);
  return 0;
@@ -81,7 +81,7 @@ void qemu_mutex_unlock_impl(QemuMutex *mutex,
const char *file, const int line)
  {
  assert(mutex->initialized);
  qemu_mutex_pre_unlock(mutex, file, line);
-ReleaseSRWLockExclusive(>lock);
+LeaveCriticalSection(>lock);
  }

  void qemu_rec_mutex_init(QemuRecMutex *mutex)
@@ -141,11 +141,12 @@ void qemu_cond_broadcast(QemuCond *cond)
  WakeAllConditionVariable(>var);
  }

-void qemu_cond_wait_impl(QemuCond *cond, QemuMutex *mutex, const
char *file, const int line)
+void qemu_cond_wait_impl(QemuCond *cond, QemuMutex *mutex, const
char *file,
+ const int line)
  {
  assert(cond->initialized);
  qemu_mutex_pre_unlock(mutex, file, line);
-SleepConditionVariableSRW(>var, >lock,
INFINITE, 0);
+SleepConditionVariableCS(>var, >lock, INFINITE);
  qemu_mutex_post_lock(mutex, file, line);
  }

--
2.16.2

Do you have any idea of what's happening?

Regards,
Fred

Re: [Qemu-devel] [PATCH 8/9] hw/arm/bcm2836: Hardcode correct CPU type

[Andrew Baumann via Qemu-devel]

> From: Qemu-devel  bounces+andrew.baumann=microsoft@nongnu.org> On Behalf Of Peter
> Maydell
> Sent: Tuesday, 13 March 2018 08:35
> 
> Now we have separate types for BCM2386 and BCM2387, we might as well
> just hard-code the CPU type they use rather than having it passed
> through as an object property. This then lets us put the initialization
> of the CPU object in init rather than realize.
> 
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
> ---
>  hw/arm/bcm2836.c | 22 +-
>  hw/arm/raspi.c   |  2 --
>  2 files changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
> index 7140257c98..12f75b55a7 100644
> --- a/hw/arm/bcm2836.c
> +++ b/hw/arm/bcm2836.c
> @@ -25,16 +25,19 @@
> 
>  struct BCM283XInfo {
>  const char *name;
> +const char *cpu_type;
>  int clusterid;
>  };
> 
>  static const BCM283XInfo bcm283x_socs[] = {
>  {
>  .name = TYPE_BCM2836,
> +.cpu_type = ARM_CPU_TYPE_NAME("cortex-a15"),

At some point I remember seeing a patch to change this to cortex-a7. Is there a 
reason we didn't make that change?

(Background: the real Pi2 has an A7. When I first implemented the machine model 
there was no A7 emulation in QEMU, so I used the A15 which was the closest 
equivalent.)

>  .clusterid = 0xf,
>  },
>  {
>  .name = TYPE_BCM2837,
> +.cpu_type = ARM_CPU_TYPE_NAME("cortex-a53"),
>  .clusterid = 0x0,
>  },
>  };
> @@ -42,6 +45,16 @@ static const BCM283XInfo bcm283x_socs[] = {
>  static void bcm2836_init(Object *obj)
>  {
>  BCM283XState *s = BCM283X(obj);
> +BCM283XClass *bc = BCM283X_GET_CLASS(obj);
> +const BCM283XInfo *info = bc->info;
> +int n;
> +
> +for (n = 0; n < BCM283X_NCPUS; n++) {
> +object_initialize(>cpus[n], sizeof(s->cpus[n]),
> +  info->cpu_type);
> +object_property_add_child(obj, "cpu[*]", OBJECT(>cpus[n]),
> +  _abort);
> +}
> 
>  object_initialize(>control, sizeof(s->control), TYPE_BCM2836_CONTROL);
>  object_property_add_child(obj, "control", OBJECT(>control), NULL);
> @@ -69,14 +82,6 @@ static void bcm2836_realize(DeviceState *dev, Error
> **errp)
> 
>  /* common peripherals from bcm2835 */
> 
> -obj = OBJECT(dev);
> -for (n = 0; n < BCM283X_NCPUS; n++) {
> -object_initialize(>cpus[n], sizeof(s->cpus[n]),
> -  s->cpu_type);
> -object_property_add_child(obj, "cpu[*]", OBJECT(>cpus[n]),
> -  _abort);
> -}
> -
>  obj = object_property_get_link(OBJECT(dev), "ram", );
>  if (obj == NULL) {
>  error_setg(errp, "%s: required ram link not found: %s",
> @@ -168,7 +173,6 @@ static void bcm2836_realize(DeviceState *dev, Error
> **errp)
>  }
> 
>  static Property bcm2836_props[] = {
> -DEFINE_PROP_STRING("cpu-type", BCM283XState, cpu_type),
>  DEFINE_PROP_UINT32("enabled-cpus", BCM283XState, enabled_cpus,
> BCM283X_NCPUS),
>  DEFINE_PROP_END_OF_LIST()
> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
> index f588720138..ae15997669 100644
> --- a/hw/arm/raspi.c
> +++ b/hw/arm/raspi.c
> @@ -150,8 +150,6 @@ static void raspi_init(MachineState *machine, int
> version)
>  /* Setup the SOC */
>  object_property_add_const_link(OBJECT(>soc), "ram", OBJECT(>ram),
> _abort);
> -object_property_set_str(OBJECT(>soc), machine->cpu_type, "cpu-type",
> -        _abort);
>  object_property_set_int(OBJECT(>soc), smp_cpus, "enabled-cpus",
>  _abort);
>  int board_rev = version == 3 ? 0xa02082 : 0xa21041;

What about the default_cpu_type field of MachineClass set in 
raspi[23]_machine_init? That seems irrelevant now... Also, if anyone cares (I 
don't), we also just lost the ability to override the CPU type of a raspi 
model. 

Otherwise,
Reviewed-by: Andrew Baumann <andrew.baum...@microsoft.com>

Cheers,
Andrew

Re: [Qemu-devel] [PATCH 5/9] hw/arm/bcm2836: Rename bcm2836 type/struct to bcm283x

[Andrew Baumann via Qemu-devel]

@ -32,7 +32,7 @@
>  static const int raspi_boardid[] = {[1] = 0xc42, [2] = 0xc43, [3] = 0xc44};
> 
>  typedef struct RasPiState {
> -BCM2836State soc;
> +BCM283XState soc;
>  MemoryRegion ram;
>  } RasPiState;
> 
> @@ -136,7 +136,7 @@ static void raspi_init(MachineState *machine, int
> version)
>  BusState *bus;
>  DeviceState *carddev;
> 
> -object_initialize(>soc, sizeof(s->soc), TYPE_BCM2836);
> +object_initialize(>soc, sizeof(s->soc), TYPE_BCM283X);
>  object_property_add_child(OBJECT(machine), "soc", OBJECT(>soc),
>_abort);
> 
> @@ -189,9 +189,9 @@ static void raspi2_machine_init(MachineClass *mc)
>  mc->no_floppy = 1;
>  mc->no_cdrom = 1;
>  mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a15");
> -mc->max_cpus = BCM2836_NCPUS;
> -mc->min_cpus = BCM2836_NCPUS;
> -mc->default_cpus = BCM2836_NCPUS;
> +mc->max_cpus = BCM283X_NCPUS;
> +mc->min_cpus = BCM283X_NCPUS;
> +mc->default_cpus = BCM283X_NCPUS;
>  mc->default_ram_size = 1024 * 1024 * 1024;
>  mc->ignore_memory_transaction_failures = true;
>  };
> @@ -212,9 +212,9 @@ static void raspi3_machine_init(MachineClass *mc)
>  mc->no_floppy = 1;
>  mc->no_cdrom = 1;
>  mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a53");
> -mc->max_cpus = BCM2836_NCPUS;
> -mc->min_cpus = BCM2836_NCPUS;
> -mc->default_cpus = BCM2836_NCPUS;
> +mc->max_cpus = BCM283X_NCPUS;
> +mc->min_cpus = BCM283X_NCPUS;
> +mc->default_cpus = BCM283X_NCPUS;
>  mc->default_ram_size = 1024 * 1024 * 1024;
>  }
>  DEFINE_MACHINE("raspi3", raspi3_machine_init)

Reviewed-by: Andrew Baumann <andrew.baum...@microsoft.com>

Re: [Qemu-devel] [PATCH 6/9] hw/arm/bcm2836: Create proper bcm2837 device

[Andrew Baumann via Qemu-devel]

> From: Peter Maydell 
> Sent: Tuesday, 13 March 2018 08:35
> 
> The bcm2837 is pretty similar to the bcm2836, but it does have
> some differences. Notably, the MPIDR affinity aff1 values it
> sets for the CPUs are 0x0, rather than the 0xf that the bcm2836
> uses, and if this is wrong Linux will not boot.
> 
> Rather than trying to have one device with properties that
> configure it differently for the two cases, create two
> separate QOM devices for the two SoCs. We use the same approach
> as hw/arm/aspeed_soc.c and share code and have a data table
> that might differ per-SoC. For the moment the two types don't
> actually have different behaviour.
> 
> Signed-off-by: Peter Maydell 
> ---
>  include/hw/arm/bcm2836.h | 19 +++
>  hw/arm/bcm2836.c | 37 -
>  hw/arm/raspi.c   |  3 ++-
>  3 files changed, 53 insertions(+), 6 deletions(-)

I haven't tried to understand enough of QOM to review this one... (It looks 
sane to me though.)

Andrew

Re: [Qemu-devel] [PATCH 4/9] hw/arm/bcm2386: Fix parent type of bcm2386

[Andrew Baumann via Qemu-devel]

> From: Qemu-devel  bounces+andrew.baumann=microsoft@nongnu.org> On Behalf Of Peter
> Maydell
> Sent: Tuesday, 13 March 2018 08:35
> 
> The TypeInfo and state struct for bcm2386 disagree about what the
> parent class is -- the TypeInfo says it's TYPE_SYS_BUS_DEVICE,
> but the BCM2386State struct only defines the parent_obj field
> as DeviceState. This would have caused problems if anything
> actually tried to treat the object as a TYPE_SYS_BUS_DEVICE.
> Fix the TypeInfo to use TYPE_DEVICE as the parent, since we don't
> need any of the additional functionality TYPE_SYS_BUS_DEVICE
> provides.
> 
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
> ---
> I noticed this when I tried to make the type into one which
> has its own class struct, because we hit the assert that the
> child's class struct had better be bigger than the parent's.
> ---
>  hw/arm/bcm2836.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
> index 40e8b25a46..9266f27c14 100644
> --- a/hw/arm/bcm2836.c
> +++ b/hw/arm/bcm2836.c
> @@ -165,7 +165,7 @@ static void bcm2836_class_init(ObjectClass *oc, void
> *data)
> 
>  static const TypeInfo bcm2836_type_info = {
>  .name = TYPE_BCM2836,
> -.parent = TYPE_SYS_BUS_DEVICE,
> +.parent = TYPE_DEVICE,
>  .instance_size = sizeof(BCM2836State),
>  .instance_init = bcm2836_init,
>  .class_init = bcm2836_class_init,

Reviewed-by: Andrew Baumann <andrew.baum...@microsoft.com>

Thanks for catching this -- it looks like bcm2835.c (which never got merged) 
has the same bug, and I copied it along.

Re: [Qemu-devel] [PATCH 1/9] hw/arm/raspi: Don't do board-setup or secure-boot for raspi3

[Andrew Baumann via Qemu-devel]

> From: Qemu-devel  bounces+andrew.baumann=microsoft@nongnu.org> On Behalf Of Peter
> Maydell
> Sent: Tuesday, 13 March 2018 08:35
> 
> For the rpi1 and 2 we want to boot the Linux kernel via some
> custom setup code that makes sure that the SMC instruction
> acts as a no-op, because it's used for cache maintenance.
> The rpi3 boots AArch64 kernels, which don't need SMC for
> cache maintenance and always expect to be booted non-secure.
> Don't fill in the aarch32-specific parts of the binfo struct.
> 
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
> ---
>  hw/arm/raspi.c | 17 +
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
> index a37881433c..1ac0737149 100644
> --- a/hw/arm/raspi.c
> +++ b/hw/arm/raspi.c
> @@ -82,10 +82,19 @@ static void setup_boot(MachineState *machine, int
> version, size_t ram_size)
>  binfo.board_id = raspi_boardid[version];
>  binfo.ram_size = ram_size;
>  binfo.nb_cpus = smp_cpus;
> -binfo.board_setup_addr = BOARDSETUP_ADDR;
> -binfo.write_board_setup = write_board_setup;
> -binfo.secure_board_setup = true;
> -binfo.secure_boot = true;
> +
> +if (version <= 2) {
> +/* The rpi1 and 2 require some custom setup code to run in Secure
> + * mode before booting a kernel (to set up the SMC vectors so
> + * that we get a no-op SMC; this is used by Linux to call the
> + * firmware for some cache maintenance operations.
> + * The rpi3 doesn't need this.
> + */
> +binfo.board_setup_addr = BOARDSETUP_ADDR;
> +binfo.write_board_setup = write_board_setup;
> +binfo.secure_board_setup = true;
> +    binfo.secure_boot = true;
> +}
> 
>  /* Pi2 and Pi3 requires SMP setup */
>  if (version >= 2) {

Reviewed-by: Andrew Baumann <andrew.baum...@microsoft.com>

Re: [Qemu-devel] [PATCH 7/9] hw/arm/bcm2836: Use correct affinity values for BCM2837

[Andrew Baumann via Qemu-devel]

> From: Peter Maydell <peter.mayd...@linaro.org>
> Sent: Tuesday, 13 March 2018 08:35
> 
> The BCM2837 sets the Aff1 field of the MPIDR affinity values for the
> CPUs to 0, whereas the BCM2836 uses 0xf. Set this correctly, as it
> is required for Linux to boot.
> 
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
> ---
>  hw/arm/bcm2836.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
> index 07d2705f96..7140257c98 100644
> --- a/hw/arm/bcm2836.c
> +++ b/hw/arm/bcm2836.c
> @@ -25,14 +25,17 @@
> 
>  struct BCM283XInfo {
>  const char *name;
> +int clusterid;
>  };
> 
>  static const BCM283XInfo bcm283x_socs[] = {
>  {
>  .name = TYPE_BCM2836,
> +.clusterid = 0xf,
>  },
>  {
>  .name = TYPE_BCM2837,
> +.clusterid = 0x0,
>  },
>  };
> 
> @@ -58,6 +61,8 @@ static void bcm2836_init(Object *obj)
>  static void bcm2836_realize(DeviceState *dev, Error **errp)
>  {
>  BCM283XState *s = BCM283X(dev);
> +BCM283XClass *bc = BCM283X_GET_CLASS(dev);
> +const BCM283XInfo *info = bc->info;
>  Object *obj;
>  Error *err = NULL;
>  int n;
> @@ -119,7 +124,7 @@ static void bcm2836_realize(DeviceState *dev, Error
> **errp)
>  /* Mirror bcm2836, which has clusterid set to 0xf

The first line of this comment should probably move or just be deleted. It's 
not relevant here.

>   * TODO: this should be converted to a property of ARM_CPU
>   */
> -s->cpus[n].mp_affinity = 0xF00 | n;
> +s->cpus[n].mp_affinity = (info->clusterid << 8) | n;
> 
>  /* set periphbase/CBAR value for CPU-local registers */
>  object_property_set_int(OBJECT(>cpus[n]),

Reviewed-by: Andrew Baumann <andrew.baum...@microsoft.com>

Re: [Qemu-devel] [PATCH v3 33/42] hw/arm/bcm2835_peripherals: implement SDHCI Spec v3

[Andrew Baumann via Qemu-devel]

> From: Philippe Mathieu-Daudé [mailto:philippe.mathieu.da...@gmail.com]
> Sent: Friday, 29 December 2017 09:49
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> 
> Note, the bcm2835 seems to have 1KB minimum blocksize, however the
> current
> model is implemented with 512B.
> 
> Can someone with access to the datasheets verify?

The public BCM2835 peripherals datasheet says:
  "The EMMC module restricts the maximum block size to the size of the internal 
data FIFO
  which is 1k bytes."
... so I think your patch is correct.

Andrew

Re: [Qemu-devel] [PATCH] BCM2837 and machine raspi3

[Andrew Baumann via Qemu-devel]

> From: bzt bzt [mailto:bztem...@gmail.com]
> Sent: Tuesday, 28 November 2017 03:27

> Yes, I agree. I've provided a parameterised version on Oct 24, which does not
> have a separate bcm2837 implementation. Is that patch ok?

That patch was moving in the right direction, but I think there were two 
problems with it. First, the right way to pass a parameter is via a property 
field (as Peter explained). Second, IMO the parameter should be the CPU model 
string to instantiate and not the Pi version number.

> Or should I wait
> for Alistair's patch (https://lists.gnu.org/archive/html/qemu-devel/2017-
> 10/msg04153.html)?

Alistair's patch doesn't actually help you as far as I can see, since it 
doesn't change what CPU bcm2836 instantiates. I think it just means that if the 
user specifies a different CPU type they get an error rather than being 
silently ignored.

Andrew

Re: [Qemu-devel] [PATCH] BCM2837 and machine raspi3

[Andrew Baumann via Qemu-devel]

> From: Peter Maydell [mailto:peter.mayd...@linaro.org]
> Sent: Saturday, 25 November 2017 10:05

> On 25 November 2017 at 16:43, bzt bzt  wrote:
> > Dear Andrew,
> >
> > A month passed, and the maintainers didn't gave a damn about raspi3
> support
> > in qemu. Any ideas?
> 
> Sorry this patch fell through the cracks, but it looked to me
> to be in the state "patch got code review comments, waiting for
> submitter to resend a new version" (from about the point Konrad
> said "split this up, it's too big", which I agree with.)
> QEMU gets a lot of patches every day, so our code process relies
> on the submitter to prod things if what they're interested in
> seems to have been accidentally ignored:
> https://wiki.qemu.org/Contribute/SubmitAPatch#If_your_patch_seems_to_hav
> e_been_ignored
> 
> FWIW, as far as I'm concerned Andrew pretty much is the maintainer
> as far as the raspi boards are concerned, so as far as those
> bits of code goes I'm happy to take his opinion on it.

In that case, IIRC my high-level suggestion was to either parameterise bcm2836 
to take a CPU model string, or else move the CPU creation out of bcm2836.c into 
the board file. From what I've understood thus far about pi3, it does not seem 
necessary to have a separate bcm2837 implementation. I suspect at that point 
the patch would be small enough that it didn't require further splitting.

> PS: if you send a new patch, send it as a new top level
> thread -- if you send it as a reply/followup to the
> first patch it's liable to not be noticed.

Please also CC me on the new patch as I am not very reliable at monitoring 
qemu-devel.

Thanks!
Andrew

[Qemu-devel] [Bug 1717708] Re: QEMU aarch64 can't run Windows ARM64 iso's

[Andrew Baumann]

@Laszlo, I got the binaries from someone else, but they from a parallel
build system... nothing particularly special: some preprocessor rules to
build against the newer WDK, and also it was just those three drivers
(netkvm, vioinput, viostor). I imagine you can get the same result by
tweaking the VS-based build (sln/vcxproj files), but haven't tried.
Sorry that's not terribly helpful.

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

Title:
  QEMU aarch64 can't run Windows ARM64 iso's

Status in QEMU:
  New

Bug description:
  Hi,
  recently Windows ARM64 ISOs have been posted on the internet..
  just checked with latest QEMU 2.10 release from 
  https://qemu.weilnetz.de/w64/qemu-w64-setup-20170830.exe 
  "h:\qemu\qemu-system-aarch64.exe" -boot d -cdrom 
h:\iso\16353.1000.170825-1423.RS_PRERELEASE_CLIENTPRO_OEMRET_ARM64FRE_ES-ES.ISO 
-m 2048 -cpu cortex-a57 -smp 1 -machine virt
  seems no video output..
  checked various machine options for example versatilepb (says guest has not 
initialized the guest)..

  so don't know if it's a QEMU bug or lacking feature but can support
  running Windows ARM64 builds (would be nice if you can add a
  Snapdragon835 machine type which is which first machines will be
  running..)

  
  note running a Windows x64 ISO with similar parameters works (removing -cpu 
and -machine as not needed)

  thanks..

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

[Qemu-devel] [Bug 1717708] Re: QEMU aarch64 can't run Windows ARM64 iso's

[Andrew Baumann]

I just noticed this bug via qemu-devel. For the record, recent QEMU
(2.11-rc1 and later) can run recent arm64 Windows images using -M virt
and the virtio storage/input/networking drivers in the guest. I was
using older UEFI build that still includes the VGA framebuffer support.
(I only just learned via this bug that it had been removed, but as
PeterM pointed out this isn't the right place to discuss it.)

Here's a sample invocation:
qemu-system-aarch64 -M virt -cpu cortex-a57 -m 4G -smp 4 -bios 
QEMU_EFI-arm64.fd \
  -serial stdio -device VGA -device virtio-keyboard-pci -device 
virtio-mouse-pci \
  -netdev user,id=net0 -device 
virtio-net-pci,disable-legacy=on,netdev=net0,mac=00:11:22:33:44:55,addr=08 \
  -drive if=none,file=myimage.vhdx,id=hd0 -device virtio-blk-pci,drive=hd0

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

Title:
  QEMU aarch64 can't run Windows ARM64 iso's

Status in QEMU:
  New

Bug description:
  Hi,
  recently Windows ARM64 ISOs have been posted on the internet..
  just checked with latest QEMU 2.10 release from 
  https://qemu.weilnetz.de/w64/qemu-w64-setup-20170830.exe 
  "h:\qemu\qemu-system-aarch64.exe" -boot d -cdrom 
h:\iso\16353.1000.170825-1423.RS_PRERELEASE_CLIENTPRO_OEMRET_ARM64FRE_ES-ES.ISO 
-m 2048 -cpu cortex-a57 -smp 1 -machine virt
  seems no video output..
  checked various machine options for example versatilepb (says guest has not 
initialized the guest)..

  so don't know if it's a QEMU bug or lacking feature but can support
  running Windows ARM64 builds (would be nice if you can add a
  Snapdragon835 machine type which is which first machines will be
  running..)

  
  note running a Windows x64 ISO with similar parameters works (removing -cpu 
and -machine as not needed)

  thanks..

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

Re: [Qemu-devel] [PATCH v2 00/43] Windbg supporting

[Andrew Baumann via Qemu-devel]

> From: Ladi Prosek [mailto:lpro...@redhat.com]
> Sent: Tuesday, 7 November 2017 00:11
> 
> On Mon, Nov 6, 2017 at 7:41 PM, Andrew Baumann
> <andrew.baum...@microsoft.com> wrote:
> > Hi,
> >
> > I just noticed this thread, and the problem sounded very familiar...
> >
> >> From: Ladi Prosek
> >> Sent: Monday, 6 November 2017 07:16
> > [...]
> >> FS base passes all the checks in windbg_on_load() as the guest kernel
> >> loads and it returns true. QEMU then sends some data over the pipe.
> >> Windbg doesn't print anything, it's still showing:
> >>
> >>   Opened \\.\pipe\win7_dbg
> >>   Waiting to reconnect...
> >>
> >> Is this expected? In regular remote kernel debugging, windbg produces
> >> a bunch of output about the target state when it attaches.
> >
> > Just a guess, but I suspect the problem you're seeing may be due to the
> combination of windbg's tight timing requirements for the serial debug
> protocol and qemu's polling-based implementation of named pipe IO. I ran
> into this a year or two ago when bringing up raspberry pi... I could never
> reliably connect windbg to the emulated serial port using a named pipe. I
> believe the problem is that the named pipe driver relies on receiving input
> through win_chr_pipe_poll(), which is called too infrequently to satisfy
> windbg's needs (it has a rather short timeout before it gives up and
> retransmits). The correct fix is to use blocking IO for the named pipes inside
> qemu, but I know from experience that win32 is awful in this respect -- qemu
> wants a single unified blocking call to wait for IO on any combination of
> named pipes, sockets, events, etc. and Windows appears to lack this, at least
> at the level of poll/select.
> 
> Thanks, I don't usually set up windbg remote debugging over named
> pipes so this indeed sounded plausible.
> 
> My Windows runs in a QEMU/KVM VM so to eliminate the pipe I have set
> up a virtual COM port like so:
> 
> -serial tcp:127.0.0.1:8889,server,nowait
> 
> and the nested QEMU ran with:
> 
> -windbg tcp:10.0.2.2:8889
> 
> where 10.0.2.2 is the address assigned to the host with user
> networking. Windbg then used the default COM1 to connect to the
> target. I routinely use COMx for remote debugging between VMs with a
> TCP connection on the host side acting as a null-modem cable and never
> had any timing related issues.
> 
> However, the symptoms were exactly the same with this set up.

Ok, then I agree this is probably not the same issue.

Andrew

Re: [Qemu-devel] [PATCH v2 00/43] Windbg supporting

[Andrew Baumann via Qemu-devel]

Hi,

I just noticed this thread, and the problem sounded very familiar...

> From: Ladi Prosek
> Sent: Monday, 6 November 2017 07:16
[...]
> FS base passes all the checks in windbg_on_load() as the guest kernel
> loads and it returns true. QEMU then sends some data over the pipe.
> Windbg doesn't print anything, it's still showing:
> 
>   Opened \\.\pipe\win7_dbg
>   Waiting to reconnect...
> 
> Is this expected? In regular remote kernel debugging, windbg produces
> a bunch of output about the target state when it attaches.

Just a guess, but I suspect the problem you're seeing may be due to the 
combination of windbg's tight timing requirements for the serial debug protocol 
and qemu's polling-based implementation of named pipe IO. I ran into this a 
year or two ago when bringing up raspberry pi... I could never reliably connect 
windbg to the emulated serial port using a named pipe. I believe the problem is 
that the named pipe driver relies on receiving input through 
win_chr_pipe_poll(), which is called too infrequently to satisfy windbg's needs 
(it has a rather short timeout before it gives up and retransmits). The correct 
fix is to use blocking IO for the named pipes inside qemu, but I know from 
experience that win32 is awful in this respect -- qemu wants a single unified 
blocking call to wait for IO on any combination of named pipes, sockets, 
events, etc. and Windows appears to lack this, at least at the level of 
poll/select.

My workaround at the time was to kludge up a proxy between sockets and named 
pipes, so the connection was:
qemu <-socket-> proxy <-named pipe-> windbg

(The proxy was a fairly flaky python script, but it was good enough to tide me 
over until I could get kdnet working. I can probably dig it out if you're 
interested.)

Cheers,
Andrew

[Qemu-devel] [PATCH v4] arm: implement cache/shareability attribute bits for PAR registers

[Andrew Baumann via Qemu-devel]

On a successful address translation instruction, PAR is supposed to
contain cacheability and shareability attributes determined by the
translation. We previously returned 0 for these bits (in line with the
general strategy of ignoring caches and memory attributes), but some
guest OSes may depend on them.

This patch collects the attribute bits in the page-table walk, and
updates PAR with the correct attributes for all LPAE translations.
Short descriptor formats still return 0 for these bits, as in the
prior implementation.

Signed-off-by: Andrew Baumann <andrew.baum...@microsoft.com>
---
v2:
 * return attrs via out parameter from get_phys_addr, rather than MemTxAttrs
 * move MAIR lookup/index inline, since it turned out to be simple
 * implement attributes for stage 2 translations
 * combine attributes from stages 1 and 2 when required

v3:
 * implement S2 allocation hints and check for cache-disabled
 * fix stage 2 shareability bits
 * fix combined allocation hints (always use stage 1 hints)
 * remove LOG_UNIMP message

v4:
 * fix hihint shift buglet in convert_stage2_attrs
 * remove TODO comment (what was there is complete)
 * mention relevant pseudocode procedures in comments

Thanks,
Andrew

 target/arm/helper.c | 178 +++-
 1 file changed, 164 insertions(+), 14 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 96113fe989..f61fb3ef68 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -19,17 +19,23 @@
 #define ARM_CPU_FREQ 10 /* FIXME: 1 GHz, should be configurable */
 
 #ifndef CONFIG_USER_ONLY
+/* Cacheability and shareability attributes for a memory access */
+typedef struct ARMCacheAttrs {
+unsigned int attrs:8; /* as in the MAIR register encoding */
+unsigned int shareability:2; /* as in the SH field of the VMSAv8-64 PTEs */
+} ARMCacheAttrs;
+
 static bool get_phys_addr(CPUARMState *env, target_ulong address,
   MMUAccessType access_type, ARMMMUIdx mmu_idx,
   hwaddr *phys_ptr, MemTxAttrs *attrs, int *prot,
   target_ulong *page_size, uint32_t *fsr,
-  ARMMMUFaultInfo *fi);
+  ARMMMUFaultInfo *fi, ARMCacheAttrs *cacheattrs);
 
 static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
MMUAccessType access_type, ARMMMUIdx mmu_idx,
hwaddr *phys_ptr, MemTxAttrs *txattrs, int 
*prot,
target_ulong *page_size_ptr, uint32_t *fsr,
-   ARMMMUFaultInfo *fi);
+   ARMMMUFaultInfo *fi, ARMCacheAttrs *cacheattrs);
 
 /* Security attributes for an address, as returned by v8m_security_lookup. */
 typedef struct V8M_SAttributes {
@@ -2159,9 +2165,10 @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t 
value,
 uint64_t par64;
 MemTxAttrs attrs = {};
 ARMMMUFaultInfo fi = {};
+ARMCacheAttrs cacheattrs = {};
 
-ret = get_phys_addr(env, value, access_type, mmu_idx,
-_addr, , , _size, , );
+ret = get_phys_addr(env, value, access_type, mmu_idx, _addr, ,
+, _size, , , );
 if (extended_addresses_enabled(env)) {
 /* fsr is a DFSR/IFSR value for the long descriptor
  * translation table format, but with WnR always clear.
@@ -2173,7 +2180,8 @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t 
value,
 if (!attrs.secure) {
 par64 |= (1 << 9); /* NS */
 }
-/* We don't set the ATTR or SH fields in the PAR. */
+par64 |= (uint64_t)cacheattrs.attrs << 56; /* ATTR */
+par64 |= cacheattrs.shareability << 7; /* SH */
 } else {
 par64 |= 1; /* F */
 par64 |= (fsr & 0x3f) << 1; /* FS */
@@ -6925,7 +6933,7 @@ static bool v7m_read_half_insn(ARMCPU *cpu, ARMMMUIdx 
mmu_idx,
 return false;
 }
 if (get_phys_addr(env, addr, MMU_INST_FETCH, mmu_idx,
-  , , , _size, , )) {
+  , , , _size, , , NULL)) {
 /* the MPU lookup failed */
 env->v7m.cfsr[env->v7m.secure] |= R_V7M_CFSR_IACCVIOL_MASK;
 armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_MEM, env->v7m.secure);
@@ -8207,7 +8215,7 @@ static hwaddr S1_ptw_translate(CPUARMState *env, 
ARMMMUIdx mmu_idx,
 int ret;
 
 ret = get_phys_addr_lpae(env, addr, 0, ARMMMUIdx_S2NS, ,
- , , , fsr, fi);
+ , , , fsr, fi, NULL);
 if (ret) {
 fi->s2addr = addr;
 fi->stage2 = true;
@@ -8608,11 +8616,41 @@ static bool check_s2_mmu_setup(ARMCPU *cpu, bool 
is_aa64, int level,
 return true;
 }
 
+/* Translate from the 4-bit stage 2 representation of
+ * memory attributes (without cac

Re: [Qemu-devel] [PATCH v3] arm: implement cache/shareability attribute bits for PAR registers

[Andrew Baumann via Qemu-devel]

> From: Andrew Baumann
> Sent: Tuesday, 31 October 2017 21:02
[...]
> +static uint8_t convert_stage2_attrs(CPUARMState *env, uint8_t s2attrs)
> +{
> +uint8_t hiattr = extract32(s2attrs, 2, 2);
> +uint8_t loattr = extract32(s2attrs, 0, 2);
> +uint8_t hihint = 0, lohint = 0;
> +
> +if (hiattr != 0) { /* normal memory */
> +/* TODO: to faithfully emulate S2CacheDisabled() for the case
> + * when EL2 is aarch32, we should check HCR2 (not HCR_EL2), but
> + * qemu doesn't presently model this register.
> + */
> +if ((env->cp15.hcr_el2 & HCR_CD) != 0) { /* cache disabled */
> +hiattr = loattr = 1; /* non-cacheable */
> +} else {
> +if (hiattr != 1) { /* Write-through or write-back */
> +hihint = 3; /* RW allocate */
> +}
> +if (loattr != 1) { /* Write-through or write-back */
> +lohint = 3; /* RW allocate */
> +}
> +}
> +}
> +
> +return (hiattr << 6) | (hihint << 2) | (loattr << 2) | lohint;
> +}

Right after sending this, I noticed a dumb copy-paste bug: the above should be 
hihint << 4.
I'm obviously going a bit too fast, sorry.

Andrew

[Qemu-devel] [PATCH v3] arm: implement cache/shareability attribute bits for PAR registers

[Andrew Baumann via Qemu-devel]

On a successful address translation instruction, PAR is supposed to
contain cacheability and shareability attributes determined by the
translation. We previously returned 0 for these bits (in line with the
general strategy of ignoring caches and memory attributes), but some
guest OSes may depend on them.

This patch collects the attribute bits in the page-table walk, and
updates PAR with the correct attributes for all LPAE translations.
Short descriptor formats still return 0 for these bits, as in the
prior implementation.

Signed-off-by: Andrew Baumann <andrew.baum...@microsoft.com>
---
v2:
 * return attrs via out parameter from get_phys_addr, rather than MemTxAttrs
 * move MAIR lookup/index inline, since it turned out to be simple
 * implement attributes for stage 2 translations
 * combine attributes from stages 1 and 2 when required

v3:
 * implement S2 allocation hints and check for cache-disabled
 * fix stage 2 shareability bits
 * fix combined allocation hints (always use stage 1 hints)
 * remove LOG_UNIMP message

There's unfortunately one new wrinkle in this revision: the spec for
S2ConvertAttrsHints() calls S2CacheDisabled() and returns non-cached if the
cache is disabled. I assumed that we ought to emulate this. However, it wasn't
clear to me what to make of the "if ELUsingAArch32(EL2)" branch -- we have a
cp15.hcr_el2, but no HCR2 register. I just left a TODO comment, but it's quite
likely I've missed something.

Thanks,
Andrew

 target/arm/helper.c | 180 
 1 file changed, 166 insertions(+), 14 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 96113fe989..81fe9283ea 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -19,17 +19,23 @@
 #define ARM_CPU_FREQ 10 /* FIXME: 1 GHz, should be configurable */
 
 #ifndef CONFIG_USER_ONLY
+/* Cacheability and shareability attributes for a memory access */
+typedef struct ARMCacheAttrs {
+unsigned int attrs:8; /* as in the MAIR register encoding */
+unsigned int shareability:2; /* as in the SH field of the VMSAv8-64 PTEs */
+} ARMCacheAttrs;
+
 static bool get_phys_addr(CPUARMState *env, target_ulong address,
   MMUAccessType access_type, ARMMMUIdx mmu_idx,
   hwaddr *phys_ptr, MemTxAttrs *attrs, int *prot,
   target_ulong *page_size, uint32_t *fsr,
-  ARMMMUFaultInfo *fi);
+  ARMMMUFaultInfo *fi, ARMCacheAttrs *cacheattrs);
 
 static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
MMUAccessType access_type, ARMMMUIdx mmu_idx,
hwaddr *phys_ptr, MemTxAttrs *txattrs, int 
*prot,
target_ulong *page_size_ptr, uint32_t *fsr,
-   ARMMMUFaultInfo *fi);
+   ARMMMUFaultInfo *fi, ARMCacheAttrs *cacheattrs);
 
 /* Security attributes for an address, as returned by v8m_security_lookup. */
 typedef struct V8M_SAttributes {
@@ -2159,9 +2165,10 @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t 
value,
 uint64_t par64;
 MemTxAttrs attrs = {};
 ARMMMUFaultInfo fi = {};
+ARMCacheAttrs cacheattrs = {};
 
-ret = get_phys_addr(env, value, access_type, mmu_idx,
-_addr, , , _size, , );
+ret = get_phys_addr(env, value, access_type, mmu_idx, _addr, ,
+, _size, , , );
 if (extended_addresses_enabled(env)) {
 /* fsr is a DFSR/IFSR value for the long descriptor
  * translation table format, but with WnR always clear.
@@ -2173,7 +2180,8 @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t 
value,
 if (!attrs.secure) {
 par64 |= (1 << 9); /* NS */
 }
-/* We don't set the ATTR or SH fields in the PAR. */
+par64 |= (uint64_t)cacheattrs.attrs << 56; /* ATTR */
+par64 |= cacheattrs.shareability << 7; /* SH */
 } else {
 par64 |= 1; /* F */
 par64 |= (fsr & 0x3f) << 1; /* FS */
@@ -6925,7 +6933,7 @@ static bool v7m_read_half_insn(ARMCPU *cpu, ARMMMUIdx 
mmu_idx,
 return false;
 }
 if (get_phys_addr(env, addr, MMU_INST_FETCH, mmu_idx,
-  , , , _size, , )) {
+  , , , _size, , , NULL)) {
 /* the MPU lookup failed */
 env->v7m.cfsr[env->v7m.secure] |= R_V7M_CFSR_IACCVIOL_MASK;
 armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_MEM, env->v7m.secure);
@@ -8207,7 +8215,7 @@ static hwaddr S1_ptw_translate(CPUARMState *env, 
ARMMMUIdx mmu_idx,
 int ret;
 
 ret = get_phys_addr_lpae(env, addr, 0, ARMMMUIdx_S2NS, ,
- , , , fsr, fi);
+ , , , fsr, fi, NULL);
 if (ret) {
 fi->

Re: [Qemu-devel] [PATCH v2] arm: implement cache/shareability attribute bits for PAR registers

[Andrew Baumann via Qemu-devel]

> From: Peter Maydell [mailto:peter.mayd...@linaro.org]
> Sent: Tuesday, 31 October 2017 17:53
> 
> On 31 October 2017 at 01:23, Andrew Baumann
> <andrew.baum...@microsoft.com> wrote:
> >> From: Peter Maydell [mailto:peter.mayd...@linaro.org]
> >> Hi. This is looking pretty good, but I have a few comments below,
> >> and we're pretty much at the softfreeze date (KVM Forum last week
> >> meant I didn't get much code review done, unfortunately). Would
> >> you be too sad if this missed 2.11 ?
> >
> > No worries. It would be nice to have a stable release that we can tell 
> > people
> supports arm64 Windows, but I recognise that this is a non-trivial change, so 
> if
> we have to wait for 2.12 to get that, then fair enough.
> 
> Is this the only missing piece, then? If we squint a bit we can
> call it a bug fix as long as we can get it in early in the softfreeze
> rather than later...

It is the last piece, and I'll try to get you a v3 soon, but this week it's my 
turn to be travelling, so it may take a day or two. I'll understand if we don't 
get it in -- not a show-stopper.

> >> The v8A ARM ARM pseudocode CombineS1S2AttrHints() says that the hint
> >> bits always come from s1 regardless of whose attrs won.
> >
> > Aha, I was wondering about this. Thanks for the pointer to the pseudocode...
> it isn't referenced anywhere in the relevant section! It's reassuring to see 
> that,
> aside from the hints (where the English was ambiguous IIRC), I seem to have
> got the rest of the translation correct.
> 
> I didn't find anything in the English text about hints at all.
> (I'll have to go and have another look.)

I didn't find any mention of it. That's pretty ambiguous IMO :)

Andrew

Re: [Qemu-devel] [PATCH v2] arm: implement cache/shareability attribute bits for PAR registers

[Andrew Baumann via Qemu-devel]

> From: Peter Maydell [mailto:peter.mayd...@linaro.org]
> Sent: Tuesday, 31 October 2017 03:25
> 
> On 20 October 2017 at 22:49, Andrew Baumann
> <andrew.baum...@microsoft.com> wrote:
> > On a successful address translation instruction, PAR is supposed to
> > contain cacheability and shareability attributes determined by the
> > translation. We previously returned 0 for these bits (in line with the
> > general strategy of ignoring caches and memory attributes), but some
> > guest OSes may depend on them.
> >
> > This patch collects the attribute bits in the page-table walk, and
> > updates PAR with the correct attributes for all LPAE
> > translations. Short descriptor formats still return 0 for these bits,
> > as in the prior implementation, but now log an unimplemented message.
> >
> > Signed-off-by: Andrew Baumann <andrew.baum...@microsoft.com>
> > ---
> > v2:
> >  * return attrs via out parameter from get_phys_addr, rather than
> MemTxAttrs
> >  * move MAIR lookup/index inline, since it turned out to be simple
> >  * implement attributes for stage 2 translations
> >  * combine attributes from stages 1 and 2 when required
> 
> Hi. This is looking pretty good, but I have a few comments below,
> and we're pretty much at the softfreeze date (KVM Forum last week
> meant I didn't get much code review done, unfortunately). Would
> you be too sad if this missed 2.11 ?

No worries. It would be nice to have a stable release that we can tell people 
supports arm64 Windows, but I recognise that this is a non-trivial change, so 
if we have to wait for 2.12 to get that, then fair enough.

> > Attributes for short PTE formats remain unimplemented; there's a
> LOG_UNIMP for
> > this case, but it's likely to be noisy for guests that trigger it -- do we 
> > need
> > a one-shot mechanism for the log statement?
> 
> I think we should just drop that LOG_UNIMP.

Ok.

> > @@ -8929,6 +8939,28 @@ static bool get_phys_addr_lpae(CPUARMState
> *env, target_ulong address,
> >   */
> >  txattrs->secure = false;
> >  }
> > +
> > +if (cacheattrs != NULL) {
> > +if (mmu_idx == ARMMMUIdx_S2NS) {
> > +/* Translate from the 4-bit stage 2 representation of
> > + * memory attributes (without cache-allocation hints) to
> > + * the 8-bit representation of the stage 1 MAIR registers
> > + * (which includes allocation hints).
> > + */
> > +uint8_t memattr = extract32(attrs, 0, 4);
> > +cacheattrs->attrs = (extract32(memattr, 2, 2) << 4)
> > +  | (extract32(memattr, 0, 2) << 2);
> 
> Pseudocode S2ConvertAttrsHints() specifies some hint bit defaults
> (no-allocate for NC; RW-allocate for WT or WB) -- do we want to
> follow that?

Thanks for the pointer. Yes, I think we do.

> 
> > +cacheattrs->shareability = extract32(attrs, 4, 2);
> 
> Are you sure this is the right bit offset for the shareability bits?
> I think 4,2 is the S2AP (access) bits, and the SH bits are in 6,2, same
> as for stage 1 descriptors.

You're right. I was convinced it differed, but I don't recall why. Thanks for 
catching this.

> > +} else {
> > +/* Index into MAIR registers for cache attributes */
> > +uint8_t attrindx = extract32(attrs, 0, 3);
> > +uint64_t mair = env->cp15.mair_el[regime_el(env, mmu_idx)];
> > +assert(attrindx <= 7);
> > +cacheattrs->attrs = extract64(mair, attrindx * 8, 8);
> > +cacheattrs->shareability = extract32(attrs, 6, 2);
> > +}
> > +}
> > +
> >  *phys_ptr = descaddr;
> >  *page_size_ptr = page_size;
> >  return false;
> > @@ -9490,6 +9522,89 @@ static bool
> get_phys_addr_pmsav5(CPUARMState *env, uint32_t address,
> >  return false;
> >  }
> >
> > +/* Combine either inner or outer cacheability attributes for normal
> > + * memory, according to table D4-42 of ARM DDI 0487B.b (the ARMv8
> ARM).
> > + *
> > + * NB: only stage 1 includes allocation hints (RW bits), leading to
> > + * some asymmetry.
> > + */
> > +static uint8_t combine_cacheattr_nibble(uint8_t s1, uint8_t s2)
> > +{
> > +if (s1 == 4 || s2 == 4) {
> > +/* non-cacheable has precedence */
> > +return 4;
> > +} else if (extract32(s1, 2, 2) == 0 || extract32(s1, 2, 2) == 2) {
> > +/* stage 1 write-through takes precedence */
> > +return s1;
> > +} els

Re: [Qemu-devel] [PATCH] BCM2837 and machine raspi3

[Andrew Baumann via Qemu-devel]

> From: bzt bzt [mailto:bztem...@gmail.com]
> Sent: Tuesday, 24 October 2017 02:54
> On Mon, Oct 23, 2017 at 6:34 PM, Andrew Baumann wrote:
>   Are there any changes from bcm2836 other than the CPU model?
> 
> 
>   Duplicating the whole file just to have a different CPU seems like a bad
> idea to me. I would suggest parameterising the CPU model in bcm2836 (and
> maybe noting in header comments etc. that it can also model 2837), and then
> instantiating it from raspi.c with the correct CPU type depending on which
> machine is being used. That will also reduce the size of the patch 
> significantly,
> which will make it easier to get it reviewed.
> 
> 
> 
> For now, there's no more difference in the emulation than the CPU, but
> BCM2837 has a lot more to offer (40 bit address space, wifi, bluetooth etc.). 
> So
> sooner or later it will need it's own source file to support all of that 
> without
> getting the code messy. I have asked Peter whether it does worth writing
> future proof code, or keep the change at the bare minimum, but had no answer
> yet.

I see. The address space size sounds like it would affect the SoC (although is 
there really 40 bits of usable physical address space beyond the core?). If 
it's like pi2, however, the wifi and BT are both behind USB, so that would be 
handled more naturally in the board model (raspi.c) than the SoC.

> Well, bcm2837 is backward compatible with the bcm2836, but has a lot more
> registers and a different boot up process (different address and no atags). 
> The
> common functionality is already in bcm2835_peripherals, and the MMIO
> address change from bcm2835 to bcm2836 has already been added by you.
> What's different is a lot more devices, which imho would be unwise to add to
> bcm2835 or either to bcm2836. So to be future proof I've created a separated
> file, but you are right at the current level of emulation it's not necessary.

Right, but as I wrote above if those devices are behind USB that's not part of 
the SoC model, and belongs in the board logic. If "more registers" just refers 
to the CPU, then that's irrelevant. If there are really more devices / device 
registers on the system bus, then that calls for a deeper change in the SoC 
model and perhaps a new implementation.

>   Similarly, this looks like a clone of raspi2_init. I think it would be 
> better
> to have one function, parametrised if needed.
> 
> 
> 
> For now, yes. But the extra devices bcm2837 have will need extra 
> initialization,
> and I though it would be clearer to separate in advance. Again, I've asked 
> Peter
> about this, but had no response. As I see having a parametrised single 
> function
> is simplier for now, but could lead to a messy code later when all the SoC
> features will be added. But I'm ain't no expert on qemu source, this is my 
> first
> patch, so I'm open to suggestions! :-)

I'm more open to the need for evolution in the machine init logic (raspi.c), so 
splitting there makes more sense to me than in bcm2836/7.

I see you just posted another patch. FWIW, this isn't quite what I was 
proposing -- rather than have bcm2736.c take a version number that is 2 or 3, I 
would just pass it the CPU model to instantiate. But at this point I think it's 
best waiting for one of the Qemu maintainers to chime in and see what they 
prefer.

Andrew

Re: [Qemu-devel] [PATCH] BCM2837 and machine raspi3

[Andrew Baumann via Qemu-devel]

> From: Qemu-devel On Behalf Of bzt bzt
> Sent: Sunday, 22 October 2017 06:21
> 
> I've added support for "-M raspi3" to qemu. This is my first patch, I hope
> it's okay. The github repo is here: https://github.com/bztsrc/qemu-raspi3
> in case my patch does not work for some reason.

Thanks for the patch!

[...]

> Subject: [PATCH] BCM2837 and machine raspi3

*add

[...]

> --- /dev/null
> +++ b/hw/arm/bcm2837.c
> @@ -0,0 +1,179 @@
> +/*
> + * Raspberry Pi emulation (c) 2012 Gregory Estrade
> + * Upstreaming code cleanup [including bcm2835_*] (c) 2013 Jan Petrous
> + *
> + * Rasperry Pi 2 emulation and refactoring Copyright (c) 2015, Microsoft
> + * Written by Andrew Baumann
> + *
> + * Raspberry Pi 3 emulation 2017 by bzt
> + *
> + * This code is licensed under the GNU GPLv2 and later.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "qemu-common.h"
> +#include "cpu.h"
> +#include "hw/arm/bcm2836.h"
> +#include "hw/arm/bcm2837.h"
> +#include "hw/arm/raspi_platform.h"
> +#include "hw/sysbus.h"
> +#include "exec/address-spaces.h"
> +
> +/* According to
> https://www.raspberrypi.org/documentation/hardware/raspberrypi/bcm2837/
> README.md
> + * The underlying architecture of the BCM2837 is identical to the BCM2836.
> The only significant
> + * difference is the replacement of the ARMv7 quad core cluster with a
> quad-core ARM Cortex A53
> + * (ARMv8) cluster. So we use cortex-a53- here. */

Are there any changes from bcm2836 other than the CPU model?

Duplicating the whole file just to have a different CPU seems like a bad idea 
to me. I would suggest parameterising the CPU model in bcm2836 (and maybe 
noting in header comments etc. that it can also model 2837), and then 
instantiating it from raspi.c with the correct CPU type depending on which 
machine is being used. That will also reduce the size of the patch 
significantly, which will make it easier to get it reviewed.

(Alternatively, if there are minor but non-trivial differences between 2836 and 
2837 other than the CPU model, you may want to create something like 
bcm2835_peripherals that contains all the functionality common to both. But I 
suspect that isn't the case here.)

> --- a/hw/arm/raspi.c
> +++ b/hw/arm/raspi.c
[...]
> @@ -171,3 +177,62 @@ static void raspi2_machine_init(MachineClass *mc)
>  mc->ignore_memory_transaction_failures = true;
>  };
>  DEFINE_MACHINE("raspi2", raspi2_machine_init)
> +
> +static void raspi3_init(MachineState *machine)
> +{
> +RasPiState *s = g_new0(RasPiState, 1);
> +uint32_t vcram_size;
> +DriveInfo *di;
> +BlockBackend *blk;
> +BusState *bus;
> +DeviceState *carddev;
> +
> +object_initialize(>soc, sizeof(s->soc), TYPE_BCM2837);
> +object_property_add_child(OBJECT(machine), "soc", OBJECT(>soc),
> +  _abort);
> +
> +/* Allocate and map RAM */
> +memory_region_allocate_system_memory(>ram, OBJECT(machine),
> "ram",
> + machine->ram_size);
> +/* FIXME: Remove when we have custom CPU address space support */
> +memory_region_add_subregion_overlap(get_system_memory(), 0, 
> >ram,
> 0);
> +
> +/* Setup the SOC */
> +object_property_add_const_link(OBJECT(>soc), "ram", OBJECT(
> >ram),
> +   _abort);
> +object_property_set_int(OBJECT(>soc), smp_cpus, "enabled-cpus",
> +_abort);
> +object_property_set_int(OBJECT(>soc), 0xa02082, "board-rev",
> +_abort);
> +object_property_set_bool(OBJECT(>soc), true, "realized",
> _abort);
> +
> +/* Create and plug in the SD cards */
> +di = drive_get_next(IF_SD);
> +blk = di ? blk_by_legacy_dinfo(di) : NULL;
> +bus = qdev_get_child_bus(DEVICE(>soc), "sd-bus");
> +if (bus == NULL) {
> +error_report("No SD bus found in SOC object");
> +exit(1);
> +}
> +carddev = qdev_create(bus, TYPE_SD_CARD);
> +qdev_prop_set_drive(carddev, "drive", blk, _fatal);
> +object_property_set_bool(OBJECT(carddev), true, "realized",
> _fatal);
> +
> +vcram_size = object_property_get_uint(OBJECT(>soc), "vcram-size",
> +  _abort);
> +setup_boot(machine, 3, machine->ram_size - vcram_size);
> +}

Similarly, this looks like a clone of raspi2_init. I think it would be better 
to have one function, parametrised if needed.

Regards,
Andrew

[Qemu-devel] [PATCH v2] arm: implement cache/shareability attribute bits for PAR registers

[Andrew Baumann via Qemu-devel]

On a successful address translation instruction, PAR is supposed to
contain cacheability and shareability attributes determined by the
translation. We previously returned 0 for these bits (in line with the
general strategy of ignoring caches and memory attributes), but some
guest OSes may depend on them.

This patch collects the attribute bits in the page-table walk, and
updates PAR with the correct attributes for all LPAE
translations. Short descriptor formats still return 0 for these bits,
as in the prior implementation, but now log an unimplemented message.

Signed-off-by: Andrew Baumann <andrew.baum...@microsoft.com>
---
v2:
 * return attrs via out parameter from get_phys_addr, rather than MemTxAttrs
 * move MAIR lookup/index inline, since it turned out to be simple
 * implement attributes for stage 2 translations
 * combine attributes from stages 1 and 2 when required

Attributes for short PTE formats remain unimplemented; there's a LOG_UNIMP for
this case, but it's likely to be noisy for guests that trigger it -- do we need
a one-shot mechanism for the log statement?

Cheers,
Andrew

 target/arm/helper.c | 153 +++-
 1 file changed, 139 insertions(+), 14 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 96113fe989..60cdc7ce1f 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -19,17 +19,23 @@
 #define ARM_CPU_FREQ 10 /* FIXME: 1 GHz, should be configurable */
 
 #ifndef CONFIG_USER_ONLY
+/* Cacheability and shareability attributes for a memory access */
+typedef struct ARMCacheAttrs {
+unsigned int attrs:8; /* as in the MAIR register encoding */
+unsigned int shareability:2; /* as in the SH field of the VMSAv8-64 PTEs */
+} ARMCacheAttrs;
+
 static bool get_phys_addr(CPUARMState *env, target_ulong address,
   MMUAccessType access_type, ARMMMUIdx mmu_idx,
   hwaddr *phys_ptr, MemTxAttrs *attrs, int *prot,
   target_ulong *page_size, uint32_t *fsr,
-  ARMMMUFaultInfo *fi);
+  ARMMMUFaultInfo *fi, ARMCacheAttrs *cacheattrs);
 
 static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
MMUAccessType access_type, ARMMMUIdx mmu_idx,
hwaddr *phys_ptr, MemTxAttrs *txattrs, int 
*prot,
target_ulong *page_size_ptr, uint32_t *fsr,
-   ARMMMUFaultInfo *fi);
+   ARMMMUFaultInfo *fi, ARMCacheAttrs *cacheattrs);
 
 /* Security attributes for an address, as returned by v8m_security_lookup. */
 typedef struct V8M_SAttributes {
@@ -2159,9 +2165,10 @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t 
value,
 uint64_t par64;
 MemTxAttrs attrs = {};
 ARMMMUFaultInfo fi = {};
+ARMCacheAttrs cacheattrs = {};
 
-ret = get_phys_addr(env, value, access_type, mmu_idx,
-_addr, , , _size, , );
+ret = get_phys_addr(env, value, access_type, mmu_idx, _addr, ,
+, _size, , , );
 if (extended_addresses_enabled(env)) {
 /* fsr is a DFSR/IFSR value for the long descriptor
  * translation table format, but with WnR always clear.
@@ -2173,7 +2180,8 @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t 
value,
 if (!attrs.secure) {
 par64 |= (1 << 9); /* NS */
 }
-/* We don't set the ATTR or SH fields in the PAR. */
+par64 |= (uint64_t)cacheattrs.attrs << 56; /* ATTR */
+par64 |= cacheattrs.shareability << 7; /* SH */
 } else {
 par64 |= 1; /* F */
 par64 |= (fsr & 0x3f) << 1; /* FS */
@@ -2189,6 +2197,8 @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t 
value,
  */
 if (!ret) {
 /* We do not set any attribute bits in the PAR */
+qemu_log_mask(LOG_UNIMP, "ats_write: cache/shareability attributes"
+  " for short PTEs not implemented in PAR\n");
 if (page_size == (1 << 24)
 && arm_feature(env, ARM_FEATURE_V7)) {
 par64 = (phys_addr & 0xff00) | (1 << 1);
@@ -6925,7 +6935,7 @@ static bool v7m_read_half_insn(ARMCPU *cpu, ARMMMUIdx 
mmu_idx,
 return false;
 }
 if (get_phys_addr(env, addr, MMU_INST_FETCH, mmu_idx,
-  , , , _size, , )) {
+  , , , _size, , , NULL)) {
 /* the MPU lookup failed */
 env->v7m.cfsr[env->v7m.secure] |= R_V7M_CFSR_IACCVIOL_MASK;
 armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_MEM, env->v7m.secure);
@@ -8207,7 +8217,7 @@ static hwaddr S1_ptw_translate(CPUARMState *env, 
ARMMMUIdx mmu_idx,
 int ret;
 
 ret = get_phys

Re: [Qemu-devel] [RFC PATCH] arm: implement cache/shareability attribute bits for PAR registers

[Andrew Baumann via Qemu-devel]

> From: Peter Maydell [mailto:peter.mayd...@linaro.org]
> Sent: Thursday, 19 October 2017 09:51
> 
> On 18 October 2017 at 01:16, Andrew Baumann
> <andrew.baum...@microsoft.com> wrote:
> Hi; thanks for this patch. Looks like you forgot to add your
> signed-off-by line.

Thanks for the review! 

I didn’t sign the patch, because it wasn't ready to go in, and my (limited) 
understanding of the protocol is that that's one of the things you're 
certifying in a signature.

> > 1. Is adding these entirely ARM-specific fields to the generic
> >MemTxAttrs bitfield the right approach, or would you prefer a new
> >return field from get_phys_addr() and friends?
> 
> MemTxAttrs is for attributes of a memory transaction as they
> go out on a bus (conceptually speaking). While some of the
> cacheability info does go out on a hardware AXI bus, it
> obviously isn't formatted as an index into the MAIR registers,
> since those are entirely CPU internal. I'm not sure the
> shareability stuff goes outside the CPU at all, but I haven't
> looked closely at the AXI spec.
> 
> While you could wade into the complexities of figuring out what
> attributes we want to expose to devices via the MemTxAttrs fields,
> I would suggest keeping things internal to the CPU implementation,
> by adding another parameter to get_phys_addr() 

Ok, will do.

> > 2. Is it acceptable to implement this piecemeal for the special LPAE
> >case we care about, and defer the messy special cases for later? (I
> >hope so!)
> 
> Depends whether I look at the other parts and decide on your
> behalf that they're easy ;-)

I think I will take a stab at the stage 2 translations, but grovelling through 
the details of the various short PTE formats and their remapping mechanisms 
wasn't my idea of a fun time :)

> > 3. Is my implementation of mair0 and mair1 (with the XXX comments)
> >sane? I suspect it's not. I had a hard time mapping what the ARM
> >ARM describes (e.g. in the pseudocode for AArch32.S1AttrDecode and
> >AArch64.S1AttrDecode) to the union interpreting mair_el[0] and
> >mair_el[1].
> 
> Where the AArch64.S1AttrDecode() pseudocode says MAIR[] this is
> MAIR[S1TranslationRegime()], which in QEMU terms is
> env->cp15.mair_el[regime_el(env, mmu_idx)].
> 
> For AArch32 things are a little trickier. NS MAIR0/MAIR1 are always
> in env->cp15.mair0_ns and env->cp15.mair1_ns (and if you happen to
> want the combined 64 bits MAIR1:MAIR0 then env->cp15.mair_el[1]
> will give you that). But where Secure MAIR0/MAIR1 live depends
> on whether EL3 is AArch32 or AArch64. If EL3 is AArch32 then the
> MAIR0/MAIR1 are really banked registers, and the secure copies
> are in env->cp15.mair0_s and env->cp15.mair1_s (with the MAIR1:MAIR0
> combo being in env->cp15.mair_el[3])). But if EL3 is AArch64 then
> these registers are architecturally not banked, and a secure EL1
> will be using env->cp15.mair0_ns and mair1_ns just like NS EL1.
> 
> Helpfully, regime_el() abstracts away most of this, so if all
> you want is a MAIR1:MAIR0 pair you can just say
>  env->cp15.mair_el[regime_el(env, mmu_idx)];
> the same as for AArch64.
> 
> We don't implement AArch32 EL2 yet, so the datastructure isn't
> quite right for it yet, but HMAIR1:HMAIR0 is in mair_el[2]
> (and the 32-bit part of the union should have uint32_t for
> hmair0 and hmair1 where it currently has _unused_mair_1).

Got it. Thanks.

[...]
> > @@ -2173,7 +2224,9 @@ static uint64_t do_ats_write(CPUARMState *env,
> uint64_t value,
> >  if (!attrs.secure) {
> >  par64 |= (1 << 9); /* NS */
> >  }
> > -/* We don't set the ATTR or SH fields in the PAR. */
> > +par64 |= (uint64_t)get_memattrs(env, mmu_idx,
> > +attrs.arm_attrindx) << 56; 
> > /*ATTR*/
> 
> I think this is the wrong place to try to do the lookup from
> AttrIndex[] pagetable bits to memory attributes via the MAIR*,
> because not every kind of page table does memory attributes
> that way. In particular for short descriptors with TEX remap
> disabled, the attributes are directly in the page descriptors,
> and this is also true for stage 2 translation tables.
> 
> So you want the page table walk functions to directly fill in
> the information about cacheability and shareability, including
> doing lookups in MAIR registers (or PRRR/NMRR registers, which
> QEMU stores in the same CPUState fields, but they have a different
> format.)

I agree. FWIW, the original reason for doing it here was that I didn't want to 
slow down the normal page-table walk with additional logic to compute the 
attributes, which isn't necessary for a TLB fill. If we're adding extra out 
parameters to the walker, then I can make those optional and skip computing 
them when the parameter is NULL.

Andrew

[Qemu-devel] [RFC PATCH] arm: implement cache/shareability attribute bits for PAR registers

[Andrew Baumann via Qemu-devel]

On a successful address translation instruction, PAR is supposed to
contain cacheability and shareability attributes determined by the
translation. Previous versions of QEMU returned 0 for these bits (in
line with the general strategy of ignoring caches and memory
attributes), but some guests may depend on them.

This patch collects the attribute bits in the page-table work, and
updates PAR with the correct attributes after translation by the MAIR
registers, but only in the restricted case of an LPAE descriptor with
no second-stage translation. Other cases log an unimplemented message,
and return 0 in these bits as in the prior implementation.
---
This is my second attempt at implementing enough of the PAR attribute
bits to be able to boot Windows on aarch64. I'd appreciate some
feedback/guidance on the following:

1. Is adding these entirely ARM-specific fields to the generic
   MemTxAttrs bitfield the right approach, or would you prefer a new
   return field from get_phys_addr() and friends?

2. Is it acceptable to implement this piecemeal for the special LPAE
   case we care about, and defer the messy special cases for later? (I
   hope so!)

3. Is my implementation of mair0 and mair1 (with the XXX comments)
   sane? I suspect it's not. I had a hard time mapping what the ARM
   ARM describes (e.g. in the pseudocode for AArch32.S1AttrDecode and
   AArch64.S1AttrDecode) to the union interpreting mair_el[0] and
   mair_el[1].

Thanks,
Andrew


 include/exec/memattrs.h |  3 +++
 target/arm/helper.c | 57 -
 2 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/include/exec/memattrs.h b/include/exec/memattrs.h
index d4a1642098..3f4e1d7f0d 100644
--- a/include/exec/memattrs.h
+++ b/include/exec/memattrs.h
@@ -37,6 +37,9 @@ typedef struct MemTxAttrs {
 unsigned int user:1;
 /* Requester ID (for MSI for example) */
 unsigned int requester_id:16;
+/* ARM: memory cacheability and shareability attributes */
+unsigned int arm_attrindx:3;
+unsigned int arm_shareability:2;
 } MemTxAttrs;
 
 /* Bus masters which don't specify any attributes will get this,
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 96113fe989..1dc1834929 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -19,6 +19,9 @@
 #define ARM_CPU_FREQ 10 /* FIXME: 1 GHz, should be configurable */
 
 #ifndef CONFIG_USER_ONLY
+static inline bool regime_using_lpae_format(CPUARMState *env,
+ARMMMUIdx mmu_idx);
+
 static bool get_phys_addr(CPUARMState *env, target_ulong address,
   MMUAccessType access_type, ARMMMUIdx mmu_idx,
   hwaddr *phys_ptr, MemTxAttrs *attrs, int *prot,
@@ -2148,6 +2151,54 @@ static CPAccessResult ats_access(CPUARMState *env, const 
ARMCPRegInfo *ri,
 return CP_ACCESS_OK;
 }
 
+static uint8_t get_memattrs(CPUARMState *env, ARMMMUIdx mmu_idx,
+uint8_t attrindx)
+{
+uint64_t mair;
+
+if (!regime_using_lpae_format(env, mmu_idx)) {
+qemu_log_mask(LOG_UNIMP,
+  "arm: short PTE memory attributes not implemented");
+return 0; /* keep consistency with the prior implementation */
+}
+
+switch (mmu_idx) {
+case ARMMMUIdx_S1E2:
+mair = env->cp15.mair_el[2];
+break;
+
+case ARMMMUIdx_S1E3:
+mair = env->cp15.mair_el[3];
+break;
+
+case ARMMMUIdx_S1SE0:
+case ARMMMUIdx_S1SE1:
+/* XXX: is this correct? */
+mair = (uint64_t)env->cp15.mair1_s << 32 | env->cp15.mair0_s;
+break;
+
+case ARMMMUIdx_S1NSE0:
+case ARMMMUIdx_S1NSE1:
+/* XXX: is this correct? */
+mair = (uint64_t)env->cp15.mair1_ns << 32 | env->cp15.mair0_ns;
+break;
+
+case ARMMMUIdx_S12NSE0:
+case ARMMMUIdx_S12NSE1:
+case ARMMMUIdx_S2NS:
+/* these cases (involving stage 2 attribute combining) are also NYI */
+qemu_log_mask(LOG_UNIMP,
+  "arm: stage 2 memory attributes not implemented");
+return 0;
+
+default:
+abort(); /* M-profile code shouldn't reach here */
+}
+
+assert(attrindx <= 7);
+return extract64(mair, attrindx * 8, 8);
+}
+
 static uint64_t do_ats_write(CPUARMState *env, uint64_t value,
  MMUAccessType access_type, ARMMMUIdx mmu_idx)
 {
@@ -2173,7 +2224,9 @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t 
value,
 if (!attrs.secure) {
 par64 |= (1 << 9); /* NS */
 }
-/* We don't set the ATTR or SH fields in the PAR. */
+par64 |= (uint64_t)get_memattrs(env, mmu_idx,
+attrs.arm_attrindx) << 56; /*ATTR*/
+par64 |= attrs.arm_shareability << 7; /* SH */
 } else {
 par64 |= 1; /* F */
 par64 |= (fsr & 0x3f) << 1; /* FS */
@@ -8929,6 

Re: [Qemu-devel] [PATCH] notdirty_mem_write: implement 8-byte accesses

[Andrew Baumann via Qemu-devel]

> From: Andrew Baumann
> Sent: Friday, 13 October 2017 11:19
> 
> Aligned 8-byte memory writes by a 64-bit target on a 64-bit host should
> always turn into atomic 8-byte writes on the host, however if we missed
> in the softmmu, and the TLB line was marked as not dirty, then we
> would end up tearing the 8-byte write into two 4-byte writes in
> access_with_adjusted_size().
> 
> Signed-off-by: Andrew Baumann <andrew.baum...@microsoft.com>
> ---
> This manifested as a race in lock-free synchronisation with an aarch64
> Windows guest on an x86-64 host (with multithreaded TCG).
> 
>  exec.c | 13 +
>  1 file changed, 13 insertions(+)

By the way, I noticed that watch_mem_ops are also 4-byte only. I suspect the 
same fix may be needed there?

Andrew

[Qemu-devel] [PATCH] notdirty_mem_write: implement 8-byte accesses

[Andrew Baumann via Qemu-devel]

Aligned 8-byte memory writes by a 64-bit target on a 64-bit host should
always turn into atomic 8-byte writes on the host, however if we missed
in the softmmu, and the TLB line was marked as not dirty, then we
would end up tearing the 8-byte write into two 4-byte writes in
access_with_adjusted_size().

Signed-off-by: Andrew Baumann <andrew.baum...@microsoft.com>
---
This manifested as a race in lock-free synchronisation with an aarch64
Windows guest on an x86-64 host (with multithreaded TCG).

 exec.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/exec.c b/exec.c
index 6378714a2b..7c591a9b75 100644
--- a/exec.c
+++ b/exec.c
@@ -2348,6 +2348,9 @@ static void notdirty_mem_write(void *opaque, hwaddr 
ram_addr,
 case 4:
 stl_p(qemu_map_ram_ptr(NULL, ram_addr), val);
 break;
+case 8:
+stq_p(qemu_map_ram_ptr(NULL, ram_addr), val);
+break;
 default:
 abort();
 }
@@ -2378,6 +2381,16 @@ static const MemoryRegionOps notdirty_mem_ops = {
 .write = notdirty_mem_write,
 .valid.accepts = notdirty_mem_accepts,
 .endianness = DEVICE_NATIVE_ENDIAN,
+.valid = {
+.min_access_size = 1,
+.max_access_size = 8,
+.unaligned = false,
+},
+.impl = {
+.min_access_size = 1,
+.max_access_size = 8,
+.unaligned = false,
+},
 };
 
 /* Generate a debug exception if a watchpoint has been hit.  */
-- 
2.14.2

Re: [Qemu-devel] Torn read/write possible on aarch64/x86-64 MTTCG?

[Andrew Baumann via Qemu-devel]

> From: Richard Henderson [mailto:rth7...@gmail.com] On Behalf Of Richard
> Henderson
> Sent: Monday, 24 July 2017 15:03
> 
> On 07/24/2017 02:23 PM, Emilio G. Cota wrote:
> > (Adding some Cc's)
> >
> > On Mon, Jul 24, 2017 at 19:05:33 +, Andrew Baumann via Qemu-devel
> wrote:
> >> Hi all,
> >>
> >> I'm trying to track down what appears to be a translation bug in either
> >> the aarch64 target or x86_64 TCG (in multithreaded mode). The
> symptoms
> 
> I assume this is really x86_64 and not i686 as host.

Yes.

> >> are entirely consistent with a torn read/write -- that is, a 64-bit
> >> load or store that was translated to two 32-bit loads and stores --
> >> but that's obviously not what happens in the common path through the
> >> translation for this code, so I'm wondering: are there any cases in
> >> which qemu will split a 64-bit memory access into two 32-bit accesses?
> >
> > That would be a bug in MTTCG.
> >
> >> The code: Guest CPU A writes a 64-bit value to an aligned memory
> >> location that was previously 0, using a regular store; e.g.:
> >>f934 str x20,[x1]
> >>
> >> Guest CPU B (who is busy-waiting) reads a value from the same location:
> >>f9400280 ldr x0,[x20]
> >>
> >> The symptom: CPU B loads a value that is neither NULL nor the value
> >> written. Instead, x0 gets only the low 32-bits of the value written
> >> (high bits are all zero). By the time this value is dereferenced (a
> >> few instructions later) and the exception handlers run, the memory
> >> location from which it was loaded has the correct 64-bit value with
> >> a non-zero upper half.
> >>
> >> Obviously on a real ARM memory barriers are critical, and indeed
> >> the code has such barriers in it, but I'm assuming that any possible
> >> mistranslation of the barriers is irrelevant because for a 64-bit load
> >> and a 64-bit store you should get all or nothing. Other clues that may
> >> be relevant: the code is _near_ a LDREX/STREX pair (the busy-waiting
> >> is used to resolve a race when updating another variable), and the
> >> busy-wait loop has a yield instruction in it (although those appear
> >> to be no-ops with MTTCG).
> >
> > This might have to do with how ldrex/strex is emulated; are you relying
> > on the exclusive pair detecting ABA? If so, your code won't work in
> > QEMU since it uses cmpxchg to emulate ldrex/strex.
> 
> ABA problem is nothing to do with tearing.  And cmpxchg will definitely not
> create tearing problems.

In fact, the ldrex/strex are there implementing a cmpxchg. :)

> I don't know how we would manage 64-bit tearing on a 64-bit host, at least
> for
> the aarch64 guest, for which I believe we have a good emulation.
> 
> > - Pin the QEMU-MTTCG process to a single CPU. Can you repro then?
> 
> A good suggestion.

Thanks for the suggestions. I've been running this configuration all day, and 
haven't seen a repro yet in hundreds of attempts. I'll report if that changes.

The problem is that this test isn't very conclusive... I don't know how often 
the VCPU threads will context switch, but I suspect it's pretty rare relative 
to the ability to expose races when they run directly on different host cores. 
And if the race really doesn't exist when running on a single core, that to me 
suggests a hardware problem, which is even less likely.

> > - Force the emulation of cmpxchg via EXCP_ATOMIC with:
> >
> > diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
> > index 87f673e..771effe5 100644
> > --- a/tcg/tcg-op.c
> > +++ b/tcg/tcg-op.c
> > @@ -2856,7 +2856,7 @@ void tcg_gen_atomic_cmpxchg_i64(TCGv_i64
> retv, TCGv addr, TCGv_i64 cmpv,
> >   }
> >   tcg_temp_free_i64(t1);
> >   } else if ((memop & MO_SIZE) == MO_64) {
> > -#ifdef CONFIG_ATOMIC64
> > +#if 0
> 
> I suspect this will simply alter the timing.  However, give it a go by all 
> means.

I haven't tried this yet. If it behaves as you describe, it seems likely to 
make the race harder to hit.

> If there's a test case that you can share, that would be awesome.

I wish we could :(

> Especially if you can prod it to happen with a standalone minimal binary.  
> With
> luck you can reproduce via aarch64-linux-user too, and simply signal an error
> via branch to __builtin_trap.

Initial attempts to repro this with a tight loop failed, but I'll take another 
stab at producing a standalone test program that we can share.

Andrew

[Qemu-devel] Torn read/write possible on aarch64/x86-64 MTTCG?

[Andrew Baumann via Qemu-devel]

Hi all,

I'm trying to track down what appears to be a translation bug in either the 
aarch64 target or x86_64 TCG (in multithreaded mode). The symptoms are entirely 
consistent with a torn read/write -- that is, a 64-bit load or store that was 
translated to two 32-bit loads and stores -- but that's obviously not what 
happens in the common path through the translation for this code, so I'm 
wondering: are there any cases in which qemu will split a 64-bit memory access 
into two 32-bit accesses?

The code:
Guest CPU A writes a 64-bit value to an aligned memory location that was 
previously 0, using a regular store; e.g.:
f934 str x20,[x1]

Guest CPU B (who is busy-waiting) reads a value from the same location:
f9400280 ldr x0,[x20]

The symptom:
CPU B loads a value that is neither NULL nor the value written. Instead, x0 
gets only the low 32-bits of the value written (high bits are all zero). By the 
time this value is dereferenced (a few instructions later) and the exception 
handlers run, the memory location from which it was loaded has the correct 
64-bit value with a non-zero upper half.

Obviously on a real ARM memory barriers are critical, and indeed the code has 
such barriers in it, but I'm assuming that any possible mistranslation of the 
barriers is irrelevant because for a 64-bit load and a 64-bit store you should 
get all or nothing. Other clues that may be relevant: the code is _near_ a 
LDREX/STREX pair (the busy-waiting is used to resolve a race when updating 
another variable), and the busy-wait loop has a yield instruction in it 
(although those appear to be no-ops with MTTCG).

The bug repros more easily with more guest VCPUs, and more load on the host 
(i.e. more context switching to expose the race). It doesn't repro for the 
single-threaded TCG. Unfortunately it's hard to get detailed trace information, 
because the bug only repros roughly every one in 40 attempts, and it's a long 
way into the guest OS boot before it arises.

I'm not yet 100% convinced this is a qemu bug -- the obvious path through the 
translator for those instructions does 64-bit memory accesses on the host -- 
but at the same time, it has never been seen outside qemu, and after staring 
long and hard at the guest code, we're pretty sure it's correct. It's also 
extremely unlikely to be a wild write, given that it occurs on a wide variety 
of guest call-stacks, and the memory is later inconsistent with what was loaded.

Any clues or debugging suggestions appreciated!

Thanks,
Andrew

Re: [Qemu-devel] [PATCH 2/2] file-posix: Do runtime check for ofd lock API

[Andrew Baumann via Qemu-devel]

> From: Fam Zheng [mailto:f...@redhat.com]
> Sent: Friday, 21 July 2017 3:21
> 
> It is reported that on Windows Subsystem for Linux, ofd operations fail
> with -EINVAL. In other words, QEMU binary built with system headers that
> exports F_OFD_SETLK doesn't necessarily run in an environment that
> actually supports it:
> 
> $ qemu-system-aarch64 ... -drive file=test.vhdx,if=none,id=hd0 \
> -device virtio-blk-pci,drive=hd0
> qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to unlock
> byte 100
> qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to unlock
> byte 100
> qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to lock
> byte 100
> 
> Let's do a runtime check to cope with that.
> 
> Reported-by: Andrew Baumann <andrew.baum...@microsoft.com>
> Signed-off-by: Fam Zheng <f...@redhat.com>
> ---
>  block/file-posix.c | 19 ---
>  1 file changed, 8 insertions(+), 11 deletions(-)

If it helps:
Tested-By: Andrew Baumann <andrew.baum...@microsoft.com>

Thanks!
Andrew

Re: [Qemu-devel] Disable image locking for snapshot drive?

[Andrew Baumann via Qemu-devel]

> From: Fam Zheng [mailto:f...@redhat.com]
> Sent: Wednesday, 19 July 2017 23:53
> 
> On Tue, 07/18 16:19, Andrew Baumann wrote:
> > > From: Eric Blake [mailto:ebl...@redhat.com]
> > > Sent: Tuesday, 18 July 2017 8:07
> > > On 07/17/2017 07:33 PM, John Snow wrote:
> > > > On 07/17/2017 07:30 PM, Andrew Baumann via Qemu-devel wrote:
> > > >> I'm running a recent Linux build of qemu on Windows Subsystem for
> Linux
> > > (WSL) which doesn't appear to implement file locking:
> > > >>
> > > >> $ qemu-system-aarch64 ... -drive file=test.vhdx,if=none,id=hd0 -
> device
> > > virtio-blk-pci,drive=hd0
> > > >> qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to
> > > unlock byte 100
> > >
> > > Does WSL implement fcntl(F_SETLK) but not fcntl(F_OFD_SETLK)?
> >
> > Yes, this appears to be the case (there's also one report that it's broken):
> > https://github.com/Microsoft/BashOnWindows/issues/1927
> 
> What does fcntl(F_OFD_SETLK) return? If it is -ENOTSUP, we can probably
> detect
> that and disable locking. Can you try the patch pasted in the end?

It returns -EINVAL. Your patch works, if I change the error code. Maybe testing 
for either ENOTSUP or EINVAL, and only allowing the fallback if the user didn't 
force locking on, would be a reasonable compromise?

Cheers,
Andrew

> ---
> 
> diff --git a/block/file-posix.c b/block/file-posix.c
> index cfbb236f6f..0be5bbbd53 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -493,6 +493,12 @@ static int raw_open_common(BlockDriverState *bs,
> QDict *options,
>  }
>  s->fd = fd;
> 
> +if (s->use_lock) {
> +int ret0 = qemu_unlock_fd(fd, 0, 0);
> +if (ret0 == -ENOTSUP) {
> +s->use_lock = false;
> +}
> +}
>  s->lock_fd = -1;
>  if (s->use_lock) {
>  fd = qemu_open(filename, s->open_flags);

Re: [Qemu-devel] Disable image locking for snapshot drive?

[Andrew Baumann via Qemu-devel]

> From: Eric Blake [mailto:ebl...@redhat.com]
> Sent: Tuesday, 18 July 2017 8:07
> On 07/17/2017 07:33 PM, John Snow wrote:
> > On 07/17/2017 07:30 PM, Andrew Baumann via Qemu-devel wrote:
> >> I'm running a recent Linux build of qemu on Windows Subsystem for Linux
> (WSL) which doesn't appear to implement file locking:
> >>
> >> $ qemu-system-aarch64 ... -drive file=test.vhdx,if=none,id=hd0 -device
> virtio-blk-pci,drive=hd0
> >> qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to
> unlock byte 100
> 
> Does WSL implement fcntl(F_SETLK) but not fcntl(F_OFD_SETLK)?

Yes, this appears to be the case (there's also one report that it's broken):
https://github.com/Microsoft/BashOnWindows/issues/1927

> We
> already have code in place for compiling when F_OFD_SETLK is not
> supported (which makes lock=auto do nothing, and issues a warning that
> F_SETLK locks may be ineffective when locks are explicitly requested),
> do we need to just expand that code into a runtime test of whether
> F_OFD_SETLK appears to be unsupported?

That would be a nice fix, and it would avoid the need for yet another flag. On 
the other hand, WSL is aiming for ABI compatibility, so they should get around 
to implementing F_OFD_SETLK et al eventually.

Even if this were fixed in QEMU or implemented in WSL, shouldn't there to be a 
way to turn snapshot file locking off on a per-drive basis?

Andrew 

Re: [Qemu-devel] Disable image locking for snapshot drive?

[Andrew Baumann via Qemu-devel]

> From: John Snow [mailto:js...@redhat.com]
> Sent: Monday, 17 July 2017 17:34
> On 07/17/2017 07:30 PM, Andrew Baumann via Qemu-devel wrote:
> > Hi all,
> >
> > I'm running a recent Linux build of qemu on Windows Subsystem for Linux
> (WSL) which doesn't appear to implement file locking:
> >
> > $ qemu-system-aarch64 ... -drive file=test.vhdx,if=none,id=hd0 -device
> virtio-blk-pci,drive=hd0
> > qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to
> unlock byte 100
> > qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to
> unlock byte 100
> > qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to lock
> byte 100
> >
> > That's no big deal; I can switch it off:
> >
> > $ qemu-system-aarch64 ... -drive
> file=test.vhdx,if=none,file.locking=off,id=hd0 ...
> > (all good)
> >
> > But how can I do the same for a snapshot drive?
> >
> > $ qemu-system-aarch64 ... -drive
> file=test.vhdx,if=none,file.locking=off,id=hd0 -snapshot ...
> > qemu-system-aarch64: -drive
> file=test.vhdx,if=none,file.locking=off,id=hd0: Failed to unlock byte 100
> > qemu-system-aarch64: -drive
> file=test.vhdx,if=none,file.locking=off,id=hd0: Failed to unlock byte 100
> > qemu-system-aarch64: -drive
> file=test.vhdx,if=none,file.locking=off,id=hd0: Could not create temporary
> overlay '/var/tmp/vl.o83dxn': Failed to lock byte 100
> >
> > (I also tried the snapshot=on drive option with similar results.)
> >
> > Thanks,
> > Andrew
> >
> 
> Looks like the shorthand "-snapshot" doesn't let you specify any further
> options, which is a bummer.
> 
> You may need to do something a little more manual, and create your own
> temporary overlay, and launch QEMU pointing to that overlay instead.

Can you give me some more clues what this might look like?

> That sounds like a bit of a hassle.
> 
> Can we compile locking support out of QEMU instead for this platform? Or
> is there a runtime option for disabling it globally?

The compile target is just Linux, so at best it would need to be a runtime 
choice based on platform detection, and I doubt that is a good idea (WSL may 
well get around to implementing this syscall in the future). I agree a runtime 
command-line flag to disable it globally would be ideal, but from a quick look 
at the code it doesn't seem to exist at present.

Thanks,
Andrew

[Qemu-devel] Disable image locking for snapshot drive?

[Andrew Baumann via Qemu-devel]

Hi all,

I'm running a recent Linux build of qemu on Windows Subsystem for Linux (WSL) 
which doesn't appear to implement file locking:

$ qemu-system-aarch64 ... -drive file=test.vhdx,if=none,id=hd0 -device 
virtio-blk-pci,drive=hd0
qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to unlock 
byte 100
qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to unlock 
byte 100
qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to lock byte 
100

That's no big deal; I can switch it off:

$ qemu-system-aarch64 ... -drive file=test.vhdx,if=none,file.locking=off,id=hd0 
...
(all good)

But how can I do the same for a snapshot drive?

$ qemu-system-aarch64 ... -drive file=test.vhdx,if=none,file.locking=off,id=hd0 
-snapshot ...
qemu-system-aarch64: -drive file=test.vhdx,if=none,file.locking=off,id=hd0: 
Failed to unlock byte 100
qemu-system-aarch64: -drive file=test.vhdx,if=none,file.locking=off,id=hd0: 
Failed to unlock byte 100
qemu-system-aarch64: -drive file=test.vhdx,if=none,file.locking=off,id=hd0: 
Could not create temporary overlay '/var/tmp/vl.o83dxn': Failed to lock byte 100

(I also tried the snapshot=on drive option with similar results.)

Thanks,
Andrew

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

[Andrew Baumann via Qemu-devel]

> From: Andrew Baumann
> Sent: Monday, 10 April 2017 10:05
> > From: Omar Rizwan [mailto:omar.riz...@gmail.com]
> > Sent: Friday, 7 April 2017 23:13

> > 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?
> 
> The various mapping aliases are intended to implement the bus addresses from
> S1.2.3 / figure on page 5 of this document:
> https://www.raspberrypi.org/wp-content/uploads/2012/02/BCM2835-ARM-
> Peripherals.pdf
> 
> In ARM CPU physical addresses, the RAM is mapped at 0, and the peripherals at
> 0x2000
> 
> In VC CPU bus addresses (which get used for DMA), RAM is aliased four times,
> at 0 0x4000 0x8000 and 0xC000. Peripherals are also aliased
> four times at a fixed offset of 0x3e00 from the base of the RAM alias,
> which is how you get the 0x7e00 peripheral base (0x4000 +
> 0x3e00).
> 
> This is implemented as follows: bcm2835_peripherals implements/exports one
> memory region "peri_mr" which contains just the peripherals. It also builds a
> private MR for the GPU ("VC CPU" in the spec) bus addresses, "gpu_mr", and
> aliases RAM into it four times along with the peripheral MR at
> BCM2835_VC_PERI_BASE (0x7e00). Its parent, device, either bcm2835 or
> bcm2836, maps the peripheral MR into the default sysbus mmio bus (i.e., the
> CPU's physical address space) at the relevant CPU physical base address
> (0x2000 on pi1, 0x3F00 on pi2).
> 
> Now, as to why the mapping call fails: I'm confused where you saw that call
> with the 0x4000 address. The code in my tree for bcm2835.c is:
> 
> /* Peripheral base address seen by the CPU */
> #define BCM2835_PERI_BASE   0x2000
> ...
> sysbus_mmio_map_overlap(SYS_BUS_DEVICE(>peripherals), 0,
> BCM2835_PERI_BASE, 1);

Ok, I just looked at your patch, and it looks like you added the 0x4000 
mapping. It's definitely not in my tree 
(https://github.com/0xabu/qemu/blob/raspi/hw/arm/bcm2835.c). I would suggest 
taking it back out again and sanity-checking your patch against the code in my 
tree for any other spurious changes that may have snuck in...

Andrew

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

[Andrew Baumann via Qemu-devel]

> From: Omar Rizwan [mailto:omar.riz...@gmail.com]
> Sent: Friday, 7 April 2017 23:13
> On Fri, Apr 7, 2017 at 10:57 PM Andrew Baumann
> <andrew.baum...@microsoft.com> wrote:
> > > 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?)

No, I don't know of such a standard, but it's probably worth noting the 
limitations in the commit message. And it might even be worth putting the 
systimers first...

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

Sounds good.

> > > --- 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?

The various mapping aliases are intended to implement the bus addresses from 
S1.2.3 / figure on page 5 of this document:
https://www.raspberrypi.org/wp-content/uploads/2012/02/BCM2835-ARM-Peripherals.pdf

In ARM CPU physical addresses, the RAM is mapped at 0, and the peripherals at 
0x2000

In VC CPU bus addresses (which get used for DMA), RAM is aliased four times, at 
0 0x4000 0x8000 and 0xC000. Peripherals are also aliased four times 
at a fixed offset of 0x3e00 from the base of the RAM alias, which is how 
you get the 0x7e00 peripheral base (0x4000 + 0x3e00).

This is implemented as follows: bcm2835_peripherals implements/exports one 
memory region "peri_mr" which contains just the peripherals. It also builds a 
private MR for the GPU ("VC CPU" in the spec) bus addresses, "gpu_mr", and 
aliases RAM into it four times along with the peripheral MR at 
BCM2835_VC_PERI_BASE (0x7e00). Its parent, device, either bcm2835 or 
bcm2836, maps the peripheral MR into the default sysbus mmio bus (i.e., the 
CPU's physical address space) at the relevant CPU physical base address 
(0x2000 on pi1, 0x3F00 on pi2).

Now, as to why the mapping call fails: I'm confused where you saw that call 
with the 0x4000 address. The code in my tree for bcm2835.c is:

/* Peripheral base address seen by the CPU */
#define BCM2835_PERI_BASE   0x2000
...
sysbus_mmio_map_overlap(SYS_BUS_DEVICE(>peripherals), 0,
BCM2835_PERI_BASE, 1);

Also, note that this is mapping the peripherals MR, not the the gpu_bus_mr 
which is never used with any sysbus APIs, so I'm still not clear on why we need 
to call sysbus_init_mmio on it. It's entirely possible my code is broken, 
and/or that assert wasn't there when I tested it, but you might have to dig a 
bit deeper to convince me that what you're doing is also sane :)

BTW, I suspect nobody else will want to review this until after 2.9.

Andrew

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

[Andrew Baumann via Qemu-devel]

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.

> --- 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?

Andrew

Re: [Qemu-devel] [PATCH] win32: replace custom mutex and condition variable with native primitives

[Andrew Baumann via Qemu-devel]

> From: Cornelia Huck [mailto:cornelia.h...@de.ibm.com]
> Sent: Monday, 3 April 2017 7:20
> 
> On Fri, 24 Mar 2017 15:01:41 -0700
> Andrew Baumann <andrew.baum...@microsoft.com> wrote:
> 
> > From: Andrey Shedel <ashe...@microsoft.com>
> >
> > The multithreaded TCG implementation exposed deadlocks in the win32
> > condition variables: as implemented, qemu_cond_broadcast waited on
> > receivers, whereas the pthreads API it was intended to emulate does
> > not. This was causing a deadlock because broadcast was called while
> > holding the IO lock, as well as all possible waiters blocked on the
> > same lock.
> >
> > This patch replaces all the custom synchronisation code for mutexes
> > and condition variables with native Windows primitives (SRWlocks and
> > condition variables) with the same semantics as their POSIX
> > equivalents. To enable that, it requires a Windows Vista or newer host
> > OS.
> >
> > [AB: edited commit message]
> > Signed-off-by: Andrew Baumann <andrew.baum...@microsoft.com>
> > ---
> > The major implication of this patch is that it drops support for
> > pre-Vista versions of Windows. However, those OSes are past their end
> > of life, and other OSS projects have dropped support. e.g.; the last
> > Cygwin release supporting XP was in Jun 2016. It doesn't seem like a
> > good tradeoff to invest effort in fixing broken code needed to support
> > them, so hopefully this isn't too controversial.
> 
> Just a quick question:
> 
> I noticed that this breaks builds on an old fc20-based installation I
> run some mingw cross builds on (getting an undefined reference to
> `_imp__TryAcquireSRWLockExclusive@4'). I should probably take that as a
> trigger to update that installation ;), but wanted to confirm that this
> is expected.

Yes, please try updating your mingw headers. I've done x64 cross-builds from 
Ubuntu 16.04 (WSL) using mingw-w64 4.0.4.

Andrew

Re: [Qemu-devel] [PATCH for 2.9?] tap-win32: don't abort in tap_enable(); enables -netdev tap

[Andrew Baumann via Qemu-devel]

> From: Jason Wang [mailto:jasow...@redhat.com]
> Sent: Tuesday, 28 March 2017 19:39
> 
> On 2017年03月29日 02:55, Andrew Baumann wrote:
> >> From: Stefan Weil [mailto:s...@weilnetz.de]
> >> Sent: Tuesday, 28 March 2017 11:28
> >> Am 25.03.2017 um 00:46 schrieb Andrew Baumann:
> >>> The docs generally steer users away from using the legacy -net
> >>> parameter, however on win32 attempting to enable a tap device using
> >>> -netdev tap fails at an abort() in tap_enable(). Removing the abort()s
> >>> seems to be enough to get everything working, so do that.
> >>>
> >>> Signed-off-by: Andrew Baumann <andrew.baum...@microsoft.com>
[...]
> >> Jason, what is the use of tap_enable, tap_disable?
> 
> It should be only used when we want to enable and disable a specific
> queue of a multiqueue supported tap.
> 
> >>   Is it fine
> >> to simply do nothing on Windows here?
> 
> Unless windows support multiqueue tap, we should keep the assert here.
> 
> > I was also hoping for a review -- I'm no expert on this stuff either, but my
> quick reading of those code paths is that they issue ioctls to enable/disable
> packet reception on the underlying tap device. As win32 TAP is implemented,
> that is already enabled from start of day.
> >
> > It's possible this patch still does not permit dynamic reconfiguration of 
> > tap
> devices (e.g. from the monitor console). However, it does work with the -
> netdev tap option on the command-line.
> >
> >> And is this something for QEMU‌ 2.9 (I added question to subject line)?
> > Ideally, yes. If not, -netdev tap will continue to blow up in the abort as 
> > it does
> today...
> >
> > Andrew
> 
> Yes, so the problem is we should prevent tap_enable() and tap_disable()
> from being called if multiqueue is disabled.
> 
> I believe the following patch can fix this issue, could you give a try
> on this?
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index c321680..7d091c9 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -510,6 +510,10 @@ static int peer_attach(VirtIONet *n, int index)
>   return 0;
>   }
> 
> +if (n->max_queues == 1) {
> +return 0;
> +}
> +
>   return tap_enable(nc->peer);
>   }
> 

Yep, this works. Thanks!

Andrew

Re: [Qemu-devel] [PATCH v3 for-2.9?] virtio: fix vring_align() on 64-bit windows

[Andrew Baumann via Qemu-devel]

> From: Eric Blake [mailto:ebl...@redhat.com]
> Sent: Tuesday, 28 March 2017 11:52
> 
> On 03/28/2017 01:38 PM, Stefan Weil wrote:
> > Am 25.03.2017 um 00:19 schrieb Andrew Baumann:
> >> long is 32-bits on 64-bit windows, which caused the top half of the
> >> address to be truncated; this patch changes it to use the
> >> QEMU_ALIGN_UP macro which does not suffer the same problem
> >>
> >> Signed-off-by: Andrew Baumann <andrew.baum...@microsoft.com>
> >> Reviewed-by: Eric Blake <ebl...@redhat.com>
> >> ---
> 
> > Eric added "for-2.9" to the subject line of v2, but now it was
> > missing again for v3.
> >
> > Is this needed for 2.9?
> 
> Yes, it's a correctness bug that avoids miscompilation on 64-bit targets
> where long is 32 bits (which, at the moment, is really just Windows).

I agree, this should be in 2.9. I dropped the tag by accident.

> > I wonder why I never before noticed
> > a problem or got a bug report for this issue.
> 
> Probably because so few people are testing on native Windows, and it
> doesn't affect other platforms.

In addition to that, you only notice it on virtio devices mapped above the 
32-bit limit...

Andrew

Re: [Qemu-devel] [PATCH for 2.9?] tap-win32: don't abort in tap_enable(); enables -netdev tap

[Andrew Baumann via Qemu-devel]

> From: Stefan Weil [mailto:s...@weilnetz.de]
> Sent: Tuesday, 28 March 2017 11:28

> Am 25.03.2017 um 00:46 schrieb Andrew Baumann:
> > The docs generally steer users away from using the legacy -net
> > parameter, however on win32 attempting to enable a tap device using
> > -netdev tap fails at an abort() in tap_enable(). Removing the abort()s
> > seems to be enough to get everything working, so do that.
> >
> > Signed-off-by: Andrew Baumann <andrew.baum...@microsoft.com>
> > ---
> >  net/tap-win32.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/tap-win32.c b/net/tap-win32.c
> > index 662f9b6..3620843 100644
> > --- a/net/tap-win32.c
> > +++ b/net/tap-win32.c
> > @@ -811,10 +811,10 @@ int net_init_tap(const Netdev *netdev, const char
> *name,
> >
> >  int tap_enable(NetClientState *nc)
> >  {
> > -abort();
> > +return 0;
> >  }
> >
> >  int tap_disable(NetClientState *nc)
> >  {
> > -abort();
> > +return 0;
> >  }
> 
> As I never worked with TAP on Windows, I cannot say much to this fix.
> 
> Jason, what is the use of tap_enable, tap_disable? Is it fine
> to simply do nothing on Windows here?

I was also hoping for a review -- I'm no expert on this stuff either, but my 
quick reading of those code paths is that they issue ioctls to enable/disable 
packet reception on the underlying tap device. As win32 TAP is implemented, 
that is already enabled from start of day.

It's possible this patch still does not permit dynamic reconfiguration of tap 
devices (e.g. from the monitor console). However, it does work with the -netdev 
tap option on the command-line.

> And is this something for QEMU‌ 2.9 (I added question to subject line)?

Ideally, yes. If not, -netdev tap will continue to blow up in the abort as it 
does today...

Andrew

Re: [Qemu-devel] qemu-devel mailing list vs DMARC and microsoft.com's p=reject policy

[Andrew Baumann via Qemu-devel]

> From: Peter Maydell [mailto:peter.mayd...@linaro.org]
> Sent: Tuesday, 28 March 2017 10:36
> 
> Hi; it's been pointed out to me that we have a problem with qemu-devel
> unsubscribing people because of DMARC. Specifically:
>  * microsoft.com publishes a DMARC policy that has p=reject
>  * some subscribers use mail systems that honour this and send bounces
>for non-verifying emails from those domains
>  * the mailing list software (mailman) modifies emails that pass through
>it, among other things adding the "[qemu-devel]" subject tag, in
>a way that means that signatures no longer verify
>  * bounces back to mailman as a result of mailing list postings from
>microsoft.com people can then cause people to be unintentionally
>unsubscribed

Ugh. I agree, this is painful. Sorry :(

>  (1) I could set dmarc_moderation_action=Reject
>* this means nobody can subscribe if they've set their dmarc policy
>  to reject (the "if you don't believe in mailing lists we don't
>  believe in you" policy).

Clarification: if I've understood correctly, this means nobody can *post*, 
right? The problem arises when I post to the list, not when I subscribed (and 
qemu-devel doesn't require subscription to post).

>* there is a certain purity to this option, in that it is pushing
>  the costs of this unhelpful mail config back on the organisations
>  which have chosen it; on the other hand I'm reluctant to make
>  life harder for people who are contributing to the project
>  and who typically don't have much say over corporate email config.

That would be an uphill battle, I'm sure... in practice, it's more likely to 
simply discourage contributions from folks like me.

>  (2) I could reconfigure mailman to try to not rewrite anything that
>  we think is likely to be signed (in particular not the body or the
>  subject)
>* this means dropping the [qemu-devel] tag from the subject, which I'm
>  a bit reluctant to do (it seems likely at least some readers are
>  filtering on it, and personally I quite like it)
>* if anybody DKIM-signs the Sender: header we're stuck anyway
>  (3) I could set dmarc_moderation_action to Munge From, which means that
>  those senders who have a p=reject policy will get their mails
>  rewritten to have a From="Whoever (via the list) "
>  and their actual email in the Reply-to:
>* if anybody's mail client doesn't honour Reply-to: then what they
>  think is a personal reply will go to the list by accident
>  (4) I could do nothing, and hope that we don't get so many of these
>  that they actually result in unsubscriptions
>* in any case emails won't end up going through to some recipients,
>  so this isn't much of an option anyway
>  (5) I could set the bounce processing config to be (much) less aggressive
>* this seems like a bad idea
>* in any case people whose systems honour DMARC still wouldn't get
>  mails from the p=reject senders
> 
> I don't really like any of these choices.
> 
> For the moment I have picked option (3), but I'm open to argument
> that we should pick something else.

Option 3 is a fine one from my perspective (I could also live with 2). This 
email will hopefully help you test whether it's effective.

Andrew

[Qemu-devel] [PATCH] tap-win32: don't abort in tap_enable(); enables -netdev tap

[Andrew Baumann]

The docs generally steer users away from using the legacy -net
parameter, however on win32 attempting to enable a tap device using
-netdev tap fails at an abort() in tap_enable(). Removing the abort()s
seems to be enough to get everything working, so do that.

Signed-off-by: Andrew Baumann <andrew.baum...@microsoft.com>
---
 net/tap-win32.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/tap-win32.c b/net/tap-win32.c
index 662f9b6..3620843 100644
--- a/net/tap-win32.c
+++ b/net/tap-win32.c
@@ -811,10 +811,10 @@ int net_init_tap(const Netdev *netdev, const char *name,
 
 int tap_enable(NetClientState *nc)
 {
-abort();
+return 0;
 }
 
 int tap_disable(NetClientState *nc)
 {
-abort();
+return 0;
 }
-- 
2.8.3

Re: [Qemu-devel] [PATCH for-2.9 v2] virtio: fix vring_align() on 64-bit win32 platforms

[Andrew Baumann]

> From: Eric Blake [mailto:ebl...@redhat.com]
> Sent: Friday, 24 March 2017 15:20

> So is 'win32' really a 64-bit platform, or should the subject be '64-bit
> windows'?

Sigh. Yes, ok, it's a little silly if you put it like that. I guess I was 
thinking of the _WIN32 ifdef and the files named *win32.

I'll send you a v3...

Andrew

[Qemu-devel] [PATCH v3] virtio: fix vring_align() on 64-bit windows

[Andrew Baumann]

long is 32-bits on 64-bit windows, which caused the top half of the
address to be truncated; this patch changes it to use the
QEMU_ALIGN_UP macro which does not suffer the same problem

Signed-off-by: Andrew Baumann <andrew.baum...@microsoft.com>
Reviewed-by: Eric Blake <ebl...@redhat.com>
---
 include/hw/virtio/virtio.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 15efcf2..7b6edba 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -34,7 +34,7 @@ struct VirtQueue;
 static inline hwaddr vring_align(hwaddr addr,
  unsigned long align)
 {
-return (addr + align - 1) & ~(align - 1);
+return QEMU_ALIGN_UP(addr, align);
 }
 
 typedef struct VirtQueue VirtQueue;
-- 
2.8.3

Re: [Qemu-devel] [PATCH] win32: replace custom mutex and condition variable with native primitives

[Andrew Baumann]

> From: Paolo Bonzini [mailto:pbonz...@redhat.com]
> Sent: Friday, 24 March 2017 15:29

> > From: Andrey Shedel <ashe...@microsoft.com>
> >
> > The multithreaded TCG implementation exposed deadlocks in the win32
> > condition variables: as implemented, qemu_cond_broadcast waited on
> > receivers, whereas the pthreads API it was intended to emulate does
> > not. This was causing a deadlock because broadcast was called while
> > holding the IO lock, as well as all possible waiters blocked on the
> > same lock.
> >
> > This patch replaces all the custom synchronisation code for mutexes
> > and condition variables with native Windows primitives (SRWlocks and
> > condition variables) with the same semantics as their POSIX
> > equivalents. To enable that, it requires a Windows Vista or newer host
> > OS.
> >
> > [AB: edited commit message]
> > Signed-off-by: Andrew Baumann <andrew.baum...@microsoft.com>
> 
> Oops, just a nit but an important one: there should be a
> Signed-off-by for Andrey as well.

Oops, my fault, since I took his code and prepared the patch submission. We can 
resend with the signoff, but perhaps I should wait for a review?

Andrew

[Qemu-devel] [PATCH] win32: replace custom mutex and condition variable with native primitives

[Andrew Baumann]

From: Andrey Shedel <ashe...@microsoft.com>

The multithreaded TCG implementation exposed deadlocks in the win32
condition variables: as implemented, qemu_cond_broadcast waited on
receivers, whereas the pthreads API it was intended to emulate does
not. This was causing a deadlock because broadcast was called while
holding the IO lock, as well as all possible waiters blocked on the
same lock.

This patch replaces all the custom synchronisation code for mutexes
and condition variables with native Windows primitives (SRWlocks and
condition variables) with the same semantics as their POSIX
equivalents. To enable that, it requires a Windows Vista or newer host
OS.

[AB: edited commit message]
Signed-off-by: Andrew Baumann <andrew.baum...@microsoft.com>
---
The major implication of this patch is that it drops support for
pre-Vista versions of Windows. However, those OSes are past their end
of life, and other OSS projects have dropped support. e.g.; the last
Cygwin release supporting XP was in Jun 2016. It doesn't seem like a
good tradeoff to invest effort in fixing broken code needed to support
them, so hopefully this isn't too controversial.

 include/qemu/thread-win32.h |   7 +--
 util/qemu-thread-win32.c| 136 +---
 2 files changed, 17 insertions(+), 126 deletions(-)

diff --git a/include/qemu/thread-win32.h b/include/qemu/thread-win32.h
index 5fb6541..4c4a261 100644
--- a/include/qemu/thread-win32.h
+++ b/include/qemu/thread-win32.h
@@ -4,8 +4,7 @@
 #include 
 
 struct QemuMutex {
-CRITICAL_SECTION lock;
-LONG owner;
+SRWLOCK lock;
 };
 
 typedef struct QemuRecMutex QemuRecMutex;
@@ -19,9 +18,7 @@ int qemu_rec_mutex_trylock(QemuRecMutex *mutex);
 void qemu_rec_mutex_unlock(QemuRecMutex *mutex);
 
 struct QemuCond {
-LONG waiters, target;
-HANDLE sema;
-HANDLE continue_event;
+CONDITION_VARIABLE var;
 };
 
 struct QemuSemaphore {
diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c
index 29c3e4d..59befd5 100644
--- a/util/qemu-thread-win32.c
+++ b/util/qemu-thread-win32.c
@@ -10,6 +10,11 @@
  * See the COPYING file in the top-level directory.
  *
  */
+
+#ifndef _WIN32_WINNT
+#define _WIN32_WINNT 0x0600
+#endif
+
 #include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "qemu/thread.h"
@@ -39,44 +44,30 @@ static void error_exit(int err, const char *msg)
 
 void qemu_mutex_init(QemuMutex *mutex)
 {
-mutex->owner = 0;
-InitializeCriticalSection(>lock);
+InitializeSRWLock(>lock);
 }
 
 void qemu_mutex_destroy(QemuMutex *mutex)
 {
-assert(mutex->owner == 0);
-DeleteCriticalSection(>lock);
+InitializeSRWLock(>lock);
 }
 
 void qemu_mutex_lock(QemuMutex *mutex)
 {
-EnterCriticalSection(>lock);
-
-/* Win32 CRITICAL_SECTIONs are recursive.  Assert that we're not
- * using them as such.
- */
-assert(mutex->owner == 0);
-mutex->owner = GetCurrentThreadId();
+AcquireSRWLockExclusive(>lock);
 }
 
 int qemu_mutex_trylock(QemuMutex *mutex)
 {
 int owned;
 
-owned = TryEnterCriticalSection(>lock);
-if (owned) {
-assert(mutex->owner == 0);
-mutex->owner = GetCurrentThreadId();
-}
+owned = TryAcquireSRWLockExclusive(>lock);
 return !owned;
 }
 
 void qemu_mutex_unlock(QemuMutex *mutex)
 {
-assert(mutex->owner == GetCurrentThreadId());
-mutex->owner = 0;
-LeaveCriticalSection(>lock);
+ReleaseSRWLockExclusive(>lock);
 }
 
 void qemu_rec_mutex_init(QemuRecMutex *mutex)
@@ -107,124 +98,27 @@ void qemu_rec_mutex_unlock(QemuRecMutex *mutex)
 void qemu_cond_init(QemuCond *cond)
 {
 memset(cond, 0, sizeof(*cond));
-
-cond->sema = CreateSemaphore(NULL, 0, LONG_MAX, NULL);
-if (!cond->sema) {
-error_exit(GetLastError(), __func__);
-}
-cond->continue_event = CreateEvent(NULL,/* security */
-   FALSE,   /* auto-reset */
-   FALSE,   /* not signaled */
-   NULL);   /* name */
-if (!cond->continue_event) {
-error_exit(GetLastError(), __func__);
-}
+InitializeConditionVariable(>var);
 }
 
 void qemu_cond_destroy(QemuCond *cond)
 {
-BOOL result;
-result = CloseHandle(cond->continue_event);
-if (!result) {
-error_exit(GetLastError(), __func__);
-}
-cond->continue_event = 0;
-result = CloseHandle(cond->sema);
-if (!result) {
-error_exit(GetLastError(), __func__);
-}
-cond->sema = 0;
+InitializeConditionVariable(>var);
 }
 
 void qemu_cond_signal(QemuCond *cond)
 {
-DWORD result;
-
-/*
- * Signal only when there are waiters.  cond->waiters is
- * incremented by pthread_cond_wait under the external lock,
- * so we are safe about that.
- */
-if (cond->waiters == 0) {
-retu

[Qemu-devel] [PATCH v2] virtio: fix vring_align() on 64-bit win32 platforms

[Andrew Baumann]

long is 32-bits on win32, which caused the top half of the address to
be truncated; this patch changes it to use the QEMU_ALIGN_UP macro
which does not suffer the same problem

Signed-off-by: Andrew Baumann <andrew.baum...@microsoft.com>
---
 include/hw/virtio/virtio.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 15efcf2..7b6edba 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -34,7 +34,7 @@ struct VirtQueue;
 static inline hwaddr vring_align(hwaddr addr,
  unsigned long align)
 {
-return (addr + align - 1) & ~(align - 1);
+return QEMU_ALIGN_UP(addr, align);
 }
 
 typedef struct VirtQueue VirtQueue;
-- 
2.8.3

Re: [Qemu-devel] [PATCH] virtio: fix vring_align() on 64-bit win32 platforms

[Andrew Baumann]

> From: Eric Blake [mailto:ebl...@redhat.com]
> Sent: Tuesday, 21 March 2017 15:52
> 
> On 03/21/2017 05:31 PM, Andrew Baumann wrote:
> > "long" is 32-bits on win32, but we need to promote it to a 64-bit hwaddr
> > before negating, or else the top half of the address is truncated
> >
> > Signed-off-by: Andrew Baumann <andrew.baum...@microsoft.com>
> > ---
> >  include/hw/virtio/virtio.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> > index 15efcf2..a0a8543 100644
> > --- a/include/hw/virtio/virtio.h
> > +++ b/include/hw/virtio/virtio.h
> > @@ -34,7 +34,7 @@ struct VirtQueue;
> >  static inline hwaddr vring_align(hwaddr addr,
> >   unsigned long align)
> >  {
> > -return (addr + align - 1) & ~(align - 1);
> > +return (addr + align - 1) & ~(hwaddr)(align - 1);
> 
> Why not just use the QEMU_ALIGN_DOWN macro, instead of open-coding it?

Well, this code is aligning up, but yes the ALIGN_UP macro looks like it should 
also avoid the type promotion problem. This patch is just the 
minimally-invasive change after discovering the bug.

Let me know if you want me to spin another patch with the macro.

Andrew

[Qemu-devel] [PATCH] virtio: fix vring_align() on 64-bit win32 platforms

[Andrew Baumann]

"long" is 32-bits on win32, but we need to promote it to a 64-bit hwaddr
before negating, or else the top half of the address is truncated

Signed-off-by: Andrew Baumann <andrew.baum...@microsoft.com>
---
 include/hw/virtio/virtio.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 15efcf2..a0a8543 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -34,7 +34,7 @@ struct VirtQueue;
 static inline hwaddr vring_align(hwaddr addr,
  unsigned long align)
 {
-return (addr + align - 1) & ~(align - 1);
+return (addr + align - 1) & ~(hwaddr)(align - 1);
 }
 
 typedef struct VirtQueue VirtQueue;
-- 
2.7.4

Re: [Qemu-devel] [PATCH 0/2] misc aarch64 fixes for Windows

[Andrew Baumann]

> From: no-re...@patchew.org [mailto:no-re...@patchew.org]
> Sent: Tuesday, 28 February 2017 14:13
> 
> Hi,
> 
> This series failed build test on s390x host. Please find the details below.
[..]
> /var/tmp/patchew-tester-tmp-64adnjjr/src/target/arm/helper.c:931:23:
> error: ‘pmreg_access_ccntr’ defined but not used [-Werror=unused-
> function]
>  static CPAccessResult pmreg_access_ccntr(CPUARMState *env,
>^~
> /var/tmp/patchew-tester-tmp-64adnjjr/src/target/arm/helper.c:917:23:
> error: ‘pmreg_access_selr’ defined but not used [-Werror=unused-function]
>  static CPAccessResult pmreg_access_selr(CPUARMState *env,
>^

Oops, these need to be protected with #ifndef CONFIG_USER_ONLY... will be fixed 
in v2, but I'll wait for any other feedback.

Andrew

[Qemu-devel] [PATCH 0/2] misc aarch64 fixes for Windows

[Andrew Baumann]

Hi all,

I've been experimenting with running arm64 Windows on qemu's aarch64
virt target. There were a couple of fixes needed in the core ISA
emulation to enable this -- I'd appreciate feedback on whether this is
(a) the best way to implement these and (b) suitable for inclusion in
mainline. (I realise this is too late for 2.9, but would be happy to
have any feedback nevertheless.)

I've smoke-tested that these don't break Linux by following:
https://www.bennee.com/~alex/blog/2014/05/09/running-linux-in-qemus-aarch64-system-emulation-mode/
... however, that is fairly old; I'd also be happy to try a more
recent kernel if someone could point me to a bootable image.

Cheers,
Andrew

Andrew Baumann (2):
  target/arm: implement armv8 PMUSERENR (user-mode enable bits)
  target/arm: lie more convincingly about memory attributes in PAR_EL1

 target/arm/helper.c | 87 +++--
 1 file changed, 78 insertions(+), 9 deletions(-)

-- 
2.8.3

[Qemu-devel] [PATCH 1/2] target/arm: implement armv8 PMUSERENR (user-mode enable bits)

[Andrew Baumann]

In armv8, this register implements more than a single bit, with
fine-grained enables for read access to event counters, cycles
counters, and write access to the software increment. This change
implements those checks using custom access functions for the relevant
registers.

Signed-off-by: Andrew Baumann <andrew.baum...@microsoft.com>
---
 target/arm/helper.c | 79 +++--
 1 file changed, 71 insertions(+), 8 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 3f4211b..760092a 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -885,7 +885,7 @@ static CPAccessResult pmreg_access(CPUARMState *env, const 
ARMCPRegInfo *ri,
  */
 int el = arm_current_el(env);
 
-if (el == 0 && !env->cp15.c9_pmuserenr) {
+if (el == 0 && !(env->cp15.c9_pmuserenr & 1)) {
 return CP_ACCESS_TRAP;
 }
 if (el < 2 && (env->cp15.mdcr_el2 & MDCR_TPM)
@@ -899,6 +899,65 @@ static CPAccessResult pmreg_access(CPUARMState *env, const 
ARMCPRegInfo *ri,
 return CP_ACCESS_OK;
 }
 
+static CPAccessResult pmreg_access_xevcntr(CPUARMState *env,
+   const ARMCPRegInfo *ri,
+   bool isread)
+{
+/* ER: event counter read trap control */
+if (arm_feature(env, ARM_FEATURE_V8)
+&& arm_current_el(env) == 0
+&& (env->cp15.c9_pmuserenr & (1 << 3)) != 0
+&& isread) {
+return CP_ACCESS_OK;
+}
+
+return pmreg_access(env, ri, isread);
+}
+
+static CPAccessResult pmreg_access_selr(CPUARMState *env,
+const ARMCPRegInfo *ri,
+bool isread)
+{
+/* ER: event counter read trap control */
+if (arm_feature(env, ARM_FEATURE_V8)
+&& arm_current_el(env) == 0
+&& (env->cp15.c9_pmuserenr & (1 << 3)) != 0) {
+return CP_ACCESS_OK;
+}
+
+return pmreg_access(env, ri, isread);
+}
+
+static CPAccessResult pmreg_access_ccntr(CPUARMState *env,
+ const ARMCPRegInfo *ri,
+ bool isread)
+{
+/* CR: cycle counter read trap control */
+if (arm_feature(env, ARM_FEATURE_V8)
+&& arm_current_el(env) == 0
+&& (env->cp15.c9_pmuserenr & (1 << 2)) != 0
+&& isread) {
+return CP_ACCESS_OK;
+}
+
+return pmreg_access(env, ri, isread);
+}
+
+static CPAccessResult pmreg_access_swinc(CPUARMState *env,
+ const ARMCPRegInfo *ri,
+ bool isread)
+{
+/* SW: software increment write trap control */
+if (arm_feature(env, ARM_FEATURE_V8)
+&& arm_current_el(env) == 0
+&& (env->cp15.c9_pmuserenr & (1 << 1)) != 0
+&& !isread) {
+return CP_ACCESS_OK;
+}
+
+return pmreg_access(env, ri, isread);
+}
+
 #ifndef CONFIG_USER_ONLY
 
 static inline bool arm_ccnt_enabled(CPUARMState *env)
@@ -1068,7 +1127,11 @@ static uint64_t pmxevtyper_read(CPUARMState *env, const 
ARMCPRegInfo *ri)
 static void pmuserenr_write(CPUARMState *env, const ARMCPRegInfo *ri,
 uint64_t value)
 {
-env->cp15.c9_pmuserenr = value & 1;
+if (arm_feature(env, ARM_FEATURE_V8)) {
+env->cp15.c9_pmuserenr = value & 0xf;
+} else {
+env->cp15.c9_pmuserenr = value & 1;
+}
 }
 
 static void pmintenset_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -1212,25 +1275,25 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
   .raw_writefn = raw_write },
 /* Unimplemented so WI. */
 { .name = "PMSWINC", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 4,
-  .access = PL0_W, .accessfn = pmreg_access, .type = ARM_CP_NOP },
+  .access = PL0_W, .accessfn = pmreg_access_swinc, .type = ARM_CP_NOP },
 #ifndef CONFIG_USER_ONLY
 { .name = "PMSELR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 5,
   .access = PL0_RW, .type = ARM_CP_ALIAS,
   .fieldoffset = offsetoflow32(CPUARMState, cp15.c9_pmselr),
-  .accessfn = pmreg_access, .writefn = pmselr_write,
+  .accessfn = pmreg_access_selr, .writefn = pmselr_write,
   .raw_writefn = raw_write},
 { .name = "PMSELR_EL0", .state = ARM_CP_STATE_AA64,
   .opc0 = 3, .opc1 = 3, .crn = 9, .crm = 12, .opc2 = 5,
-  .access = PL0_RW, .accessfn = pmreg_access,
+  .access = PL0_RW, .accessfn = pmreg_access_selr,
   .fieldoffset = offsetof(CPUARMState, cp15.c9_pmselr),
   .writefn = pmselr_write, .raw_writefn = raw_write, },
 { .name = "PMCCNTR", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 0,
   .access = PL0_RW, .resetvalue = 0, .type = A

[Qemu-devel] [PATCH 2/2] target/arm: lie more convincingly about memory attributes in PAR_EL1

[Andrew Baumann]

On a successful long-descriptor translation, PAR_EL1 bits 56:63 are
expected to report the memory attributes of the page. However, the
page table walker (get_phys_addr()) does not currently retrieve these
attributes. Rather than leaving these bits clear (which implies
uncacheable device memory), this change sets them to 0xff, which
corresponds to write-back cached normal memory.

Signed-off-by: Andrew Baumann <andrew.baum...@microsoft.com>
---
In my (biased!) opinion, this is a better lie to tell for an emulated
environment. It also happens to un-break the Windows boot process, and
enable an NT kernel optimisation (DC ZVA for page zeroing).

 target/arm/helper.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 760092a..b858d6d 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -2159,7 +2159,13 @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t 
value,
 if (!attrs.secure) {
 par64 |= (1 << 9); /* NS */
 }
-/* We don't set the ATTR or SH fields in the PAR. */
+/* ATTR bits are all set, which implies:
+ *   normal memory
+ *   outer write-back non-transient, read/write-allocate
+ *   inner write-back non-transient, read/write-allocate
+ */
+par64 |= ((uint64_t)0xff << 56); /* ATTR */
+/* We don't set the SH field in the PAR. */
 } else {
 par64 |= 1; /* F */
 par64 |= (fsr & 0x3f) << 1; /* FS */
-- 
2.8.3

Re: [Qemu-devel] [PATCH v2 3/3] bcm2835: add sdhost and gpio controllers

[Andrew Baumann]

Hi,

> From: Clement Deschamps [mailto:clement.descha...@antfield.fr]
> Sent: Wednesday, 22 February 2017 3:24
> Subject: [PATCH v2 3/3] bcm2835: add sdhost and gpio controllers
> 
> This adds the bcm2835_sdhost and bcm2835_gpio to the BCM2835 platform.
> 
> The bcm2835_gpio has a link to both the sdhci and sdhost controllers for
> supporting the alternate function of GPIOs 48-53 (SD controller selection)
> 
> Signed-off-by: Clement Deschamps <clement.descha...@antfield.fr>
[...]
> +/* SDHOST */
> +object_property_set_bool(OBJECT(>sdhost), true, "realized", );
> +if (err) {
> +error_propagate(errp, err);
> +return;
> +}
> +
> +memory_region_add_subregion(>peri_mr, MMCI0_OFFSET,
> +sysbus_mmio_get_region(SYS_BUS_DEVICE(>sdhost), 0));
> +sysbus_connect_irq(SYS_BUS_DEVICE(>sdhost), 0,
> +qdev_get_gpio_in_named(DEVICE(>ic), BCM2835_IC_GPU_IRQ,
> +   INTERRUPT_SDIO));
> +object_property_add_alias(OBJECT(s), "sd-bus-2", OBJECT(>sdhost),
> +  "sd-bus", );

Is this alias still meaningful / needed, or is it a relic from the previous 
version? Right now it doesn't appear to be used, and I'm thinking that if 
someone did try to use it (e.g. at the board level by connecting an SD card) 
then the new GPIO logic would allow swapping the two SD cards between the two 
controllers, which doesn't sound like a faithful recreation of the hardware.

Otherwise,
Reviewed-by: Andrew Baumann <andrew.baum...@microsoft.com>

Cheers,
Andrew

Re: [Qemu-devel] [PATCH 2/2] bcm2835: add bcm2835_sdhost to bcm2835 platform

[Andrew Baumann]

Hi Clement,

I think Peter already gave you a great high-level suggestion, but FWIW I 
spotted one minor niggle here:

> diff --git a/hw/sd/Makefile.objs b/hw/sd/Makefile.objs
> index 31c83308f2..c2b7664264 100644
> --- a/hw/sd/Makefile.objs
> +++ b/hw/sd/Makefile.objs
> @@ -6,3 +6,4 @@ common-obj-$(CONFIG_SDHCI) += sdhci.o
>  obj-$(CONFIG_MILKYMIST) += milkymist-memcard.o
>  obj-$(CONFIG_OMAP) += omap_mmc.o
>  obj-$(CONFIG_PXA2XX) += pxa2xx_mmci.o
> +obj-$(CONFIG_RASPI) += bcm2835_sdhost.o

I think the Makefile addition should be part of the other patch (1/2) in the 
series.

Andrew

Re: [Qemu-devel] Call for GSoC 2017 mentors & project ideas

[Andrew Baumann]

> From: Peter Maydell [mailto:peter.mayd...@linaro.org]
> Sent: Thursday, 9 February 2017 14:02
> On 9 February 2017 at 20:46, John Snow <js...@redhat.com> wrote:
> > On 02/09/2017 07:46 AM, Peter Maydell wrote:
> >> Slightly late, but I had an idea last night: would "get
> >> Raspbian booting on our raspi2 board model" be a good
> >> project? Would involve some mix of bug fixing, cleaning
> >> up and adding device models from the raspi2 tree on github,
> >> and implementing missing devices. Bit of a "how long is a
> >> piece of string" project, but on the other hand breaks
> >> down easily into small parts that can all go upstream
> >> individually. Would suit student who likes debugging :-)
> 
> > Who was working on this most recently? There was someone submitting
> > patches pretty frequently for the raspi2 board within the last year,
> > wasn't there?
> 
> That was Andrew Baumann, but I think his use case was
> getting Windows 10 to boot on it, which exercises
> different bits of the hardware.

That's right, but I thought we also had Raspbian working after Peter C's 
implementation of SETEND. (I wouldn't be surprised if it had regressed, 
however.)

I've dropped the ball on this, and don't forsee finding time for it in the 
immediate future, but if anyone wants to work on it there are still some 
outstanding bits of emulation lurking in my github tree (github.com/0xabu/qemu) 
mostly from the original implementation by Gregory Estrade: pi1 model, system 
timers, stub power management control, and USB controller. The last one (USB) 
is essential to having a usable device, but also sadly dubious in terms of code 
quality and completeness.

Cheers,
Andrew

Re: [Qemu-devel] [PATCH 26/37] sd: free timer

[Andrew Baumann]

> From: Marc-André Lureau [mailto:marcandre.lur...@gmail.com]
> Sent: Thursday, 21 July 2016 4:15
> Hi Andrew,
> 
> Since you introduced the timer, could you review this patch?
> 
> thanks
> 
> 
> -- Forwarded message --
> From:  <marcandre.lur...@redhat.com>
> Date: Tue, Jul 19, 2016 at 12:54 PM
> Subject: [Qemu-devel] [PATCH 26/37] sd: free timer
> To: qemu-devel@nongnu.org
> Cc: Marc-André Lureau <marcandre.lur...@redhat.com>
> 
> 
> From: Marc-André Lureau <marcandre.lur...@redhat.com>
> 
> Free the timer allocated in instance_init.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> ---
>  hw/sd/sd.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 87c6dc1..8e88e83 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1876,6 +1876,14 @@ static void sd_instance_init(Object *obj)
>  sd->ocr_power_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
> sd_ocr_powerup, sd);  }
> 
> +static void sd_instance_finalize(Object *obj) {
> +SDState *sd = SD_CARD(obj);
> +
> +timer_del(sd->ocr_power_timer);
> +timer_free(sd->ocr_power_timer);
> +}
> +
>  static void sd_realize(DeviceState *dev, Error **errp)  {
>  SDState *sd = SD_CARD(dev);
> @@ -1927,6 +1935,7 @@ static const TypeInfo sd_info = {
>  .class_size = sizeof(SDCardClass),
>  .class_init = sd_class_init,
>  .instance_init = sd_instance_init,
> +.instance_finalize = sd_instance_finalize,
>  };
> 
>  static void sd_register_types(void)

Thanks for the fix. This was based on some other timer code I found in the tree 
that was evidently also leaky (I don't remember where unfortunately).

One thing: are you sure it is safe to call timer_del() again if the timer may 
already have been deleted? It looks that way from the implementation, but the 
header comment isn't explicit.

Otherwise,
Reviewed-by: Andrew Baumann <andrew.baum...@microsoft.com>

Cheers,
Andrew

[Qemu-devel] Shouldn't cortex-a15 enable ARM_FEATURE_EL2?

[Andrew Baumann]

Hi Peter,

I'm trying to use the MRS/MSR banked register instructions you recently 
implemented, but found that they raised an undefined instruction exception on 
the cortex-a15 CPU model. This seems to be caused by the check in 
msr_banked_access_decode(), which looks for ARM_FEATURE_V8 or ARM_FEATURE_EL2.

The quick kludge below worked for me, but I don't have high confidence in its 
correctness -- the CPU supports the virtualisation extensions, but I've no idea 
whether the rest of qemu is consistent with enabling that feature. I guess you 
have a better idea?

--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -1132,6 +1132,7 @@ static void cortex_a15_initfn(Object *obj)
 set_feature(>env, ARM_FEATURE_DUMMY_C15_REGS);
 set_feature(>env, ARM_FEATURE_CBAR_RO);
 set_feature(>env, ARM_FEATURE_LPAE);
+set_feature(>env, ARM_FEATURE_EL2);
 set_feature(>env, ARM_FEATURE_EL3);
 cpu->kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A15;
 cpu->midr = 0x412fc0f1;

Cheers,
Andrew

Re: [Qemu-devel] [Qemu-arm] [PATCH] bcm2835_property: use cached values when querying framebuffer

[Andrew Baumann]

> From: Gerd Hoffmann [mailto:kra...@redhat.com]
> Sent: Friday, 22 April 2016 09:46
>
> Alternatively we can just go for a later boot loader stage, i.e. put a u-boot 
> build
> for rpi2 to pc-bios/ (we already have one for ppc there) and run that by 
> default.
> 
> Our sdcard emulation seems to have problems though:
> 
>U-Boot 2016.05-rc2 (Apr 22 2016 - 09:11:45 +0200)
> 
>DRAM:  960 MiB
>RPI 2 Model B (0xa21041)
>MMC:   
> 
> Recent linux kernels have trouble talking to the mmc too.

I know :( I haven't looked at the more recent kernels, but it could be the same 
issue I spent a while debugging fruitlessly on raspi1 -- it looked like Linux 
was issuing a large MMC read for N blocks, but only programming the DMA 
controller for fewer (

Re: [Qemu-devel] [Qemu-arm] [PATCH] bcm2835_property: use cached values when querying framebuffer

[Andrew Baumann]

Hi all,

> From: Peter Crosthwaite [mailto:crosthwaitepe...@gmail.com]
> Sent: Friday, 22 April 2016 09:18
> 
> On Thu, Apr 21, 2016 at 9:06 AM, Stephen Warren
>  wrote:
> > On 04/21/2016 08:07 AM, Sylvain Garrigues wrote:
> >>
> >> Le 21 avr. 2016 à 15:42, Peter Maydell  a
> >> écrit
> >> :
> >>>
> >>>
> >>> There may be something we can do here to make FreeBSD's life easier,
> >>> but we definitely can't do it on the eve of a release.
> >>
> >>
> >> I didn’t know it was release day, my timing is not perfect then,
> >> sorry about that, I didn’t intend to put stress on you guys today.
> >> Like you mentioned, the Linux boot protocol doesn’t mandate any
> >> loading address, hence the possibility to set it on the command line.
> >> It would benefit not only to FreeBSD (which is strictly Linux boot
> >> ABI compliant BTW - that is how I found the qemu bootloader bug and
> >> fixed it in
> >> b4850e5) but all other OS.
> >> On the real hardware Raspberry Pi, there is the kernel_address
> >> firmware feature which enable to set the kernel load address. Would
> >> be neat to have it *someday* in qemu for any board if it is not too hard to
> implement.
> >
> >
> > It would indeed be nice if qemu for the Pi implemented the exact same
> > bootloader setup as real HW does. That is, loading boot images from
> > the FAT partition on the SD card, parsing config.txt, etc.
> >
> > Ideally as was mentioned earlier this would be done by simply
> > executing the existing bootloader under emulation, rather than
> > building all that code into qemu. However, in the Pi case, the
> > bootloader runs on the VideoCore (a separate non-ARM CPU), so isn't
> > (and likely won't be since IIUC it isn't fully documented) emulated by
> > qemu. by the time the ARM CPU runs, everything (kernel, DTB/ATAGS, ARM
> > boot stub, ...) is already loaded into RAM, the display is already
> > probed over HDMI and the controller scanning out a dummy image, etc.
> >
> > So I think if that were to be supported, it'd have to be coded into
> > qemu. Is that something that could happen, or would such patches not
> > fit qemu's model well?
> 
> I made half a start on this project but had to shelve it.
> 
> The hard part is the FAT filesystem. The basic approach I started on was to 
> link
> in the relevant bits of the GNU mtools so QEMU can poke around in a FAT
> filesystem hosted on a block device. Then just mount the SD card and emulate
> the boot logic of the VideoCore bootloader.
> This amount of new code should actually be pretty small, as the FS driver is
> handled by mtools and the SDHCI can be shorted by just having this alternate
> bootloader go direct to the block device (fish the blockdev out from the SD
> card).

You got further on this than I did. I considered a couple of options, of 
varying complexity/compatibility:

 0. The status quo: we support -kernel (for Linux images) and -bios (e.g. 
Windows), but otherwise all the options that can be set in config.txt are 
treated as the defaults. For example, -bios images are always loaded at 0x8000. 
Additionally, framebuffer parameters can be set directly from the command line 
(e.g. Windows wants -global bcm2835-fb.pixo=0).
 1. More pi-specific parameters in the line of the framebuffer parameters 
above, to control things like the image load address, and maybe to enable 
trustzone boot akin to config.txt's kernel-old=1.
 2. Code to parse config.txt and set the parameters above.
 3. Code to emulate the bootloader entirely, pulling config.txt and all the 
relevant boot bits out of an SD image (this is what Peter describes above).
 4. An ARM-based reimplementation of the bootloader, running under emulation.

I discarded even considering option 3 because I assumed you wouldn't want a FAT 
implementation linked into qemu. And I'm not inclined to add ini-parsing code, 
so my gut feeling was to go a little further on option 1 and add parameters to 
the raspi machines (I assume this is possible?) for basic things like the load 
address, and rely on users to translate config.txt settings into command-line 
parameters.

Andrew

Re: [Qemu-devel] [PATCH] bcm2835_property: use cached values when querying framebuffer

[Andrew Baumann]

> From: Sylvain Garrigues [mailto:sylv...@sylvaingarrigues.com]
> Sent: Friday, 22 April 2016 13:27
> 
> Le 22 avr. 2016 à 13:22, Andrew Baumann
> <andrew.baum...@microsoft.com> a écrit :
> >> +stl_le_phys(>dma_as, value + 16, tmp_xres * tmp_yres
> >> + * (tmp_bpp >> 3));
> >
> > Personal style nit: I prefer * 8 rather than >> 3, because it's more 
> > immediately
> obvious what you're computing, a trivial optimisation for the compiler, and in
> this specific example wouldn't need the brackets to ensure the correct 
> operator
> precedence.
> >
> > But in any case,
> > Reviewed-by: Andrew Baumann <andrew.baum...@microsoft.com>
> 
> 
> Thanks! It is actually / 8, but I used >> 3 to remain consistent with what I 
> read
> in hw/display/bcm2835_fb.c. Feel free to adapt.

Duh, yes sorry it's obviously /8. The _fb code was probably like that in the 
original version.

But I won't be merging this patch (hopefully Peter can do that?), so it's 
probably best if you make the tweak and resend with my Reviewed-by.

Andrew

Re: [Qemu-devel] [PATCH] bcm2835_property: use cached values when querying framebuffer

[Andrew Baumann]

> From: Qemu-devel [mailto:qemu-devel-
> bounces+andrew.baumann=microsoft@nongnu.org] On Behalf Of Sylvain
> Garrigues
> Sent: Thursday, 21 April 2016 12:42
> 
> As the framebuffer settings are copied into the result message before it is
> reconfigured, inconsistent behavior can happen when, for instance, you set
> with a sinle message the width, height, and depth, and ask at the same time to
> allocate the buffer and get the pitch and the size.
> 
> In this case, the reported pitch and size would be incorrect as they were
> computed with the initial values of width, height and depth, not the ones the
> client requested.
> 
> Signed-off-by: Sylvain Garrigues <sylv...@sylvaingarrigues.com>
> ---
>  hw/misc/bcm2835_property.c | 31 ++-
>  1 file changed, 22 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/misc/bcm2835_property.c b/hw/misc/bcm2835_property.c
> index 530411f..0102144 100644
> --- a/hw/misc/bcm2835_property.c
> +++ b/hw/misc/bcm2835_property.c
> @@ -21,6 +21,7 @@ static void
> bcm2835_property_mbox_push(BCM2835PropertyState *s, uint32_t value)
>  int n;
>  uint32_t offset, length, color;
>  uint32_t xres, yres, xoffset, yoffset, bpp, pixo, alpha;
> +uint32_t tmp_xres, tmp_yres, tmp_xoffset, tmp_yoffset, tmp_bpp,
> + tmp_pixo, tmp_alpha;
>  uint32_t *newxres = NULL, *newyres = NULL, *newxoffset = NULL,
>  *newyoffset = NULL, *newbpp = NULL, *newpixo = NULL, *newalpha =
> NULL;
> 
> @@ -139,7 +140,10 @@ static void
> bcm2835_property_mbox_push(BCM2835PropertyState *s, uint32_t value)
> 
>  case 0x00040001: /* Allocate buffer */
>  stl_le_phys(>dma_as, value + 12, s->fbdev->base);
> -stl_le_phys(>dma_as, value + 16, s->fbdev->size);
> +tmp_xres = newxres != NULL ? *newxres : s->fbdev->xres;
> +tmp_yres = newyres != NULL ? *newyres : s->fbdev->yres;
> +tmp_bpp = newbpp != NULL ? *newbpp : s->fbdev->bpp;
> +stl_le_phys(>dma_as, value + 16, tmp_xres * tmp_yres *
> + (tmp_bpp >> 3));

Personal style nit: I prefer * 8 rather than >> 3, because it's more 
immediately obvious what you're computing, a trivial optimisation for the 
compiler, and in this specific example wouldn't need the brackets to ensure the 
correct operator precedence.

But in any case,
Reviewed-by: Andrew Baumann <andrew.baum...@microsoft.com>

Thanks,
Andrew

Re: [Qemu-devel] [PATCH] hw/arm/bcm2836: Wire up CPU timer interrupts correctly

[Andrew Baumann]

> From: Peter Maydell [mailto:peter.mayd...@linaro.org]
> Sent: Thursday, 17 March 2016 3:33 AM
> 
> Wire up the CPU timer interrupts in the right order, with the
> nonsecure physical timer on cntpnsirq, the hyp timer on cnthpirq,
> and the secure physical timer on cntpsirq. (We did get the
> virt timer right, at least.)
> 
> Reported-by: Antonio Huete Jiménez <tuxi...@quantumachine.net>
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>

Reviewed-by: Andrew Baumann <andrew.baum...@microsoft.com>

Thanks Peter!
Andrew

Re: [Qemu-devel] Timer interrupts for -M raspi2

[Andrew Baumann]

Hi Antonio,

> From: Antonio Huete Jiménez [mailto:tuxi...@quantumachine.net]
> Sent: Wednesday, 16 March 2016 3:40 PM
> 
> Hi,
> 
> I am experiencing what I think it's an issue with -M raspi2 and
> interrupts in a baremetal application.
> 
> According to this document
> (https://www.raspberrypi.org/documentation/hardware/raspberrypi/bcm2
> 836/QA7_rev3.4.pdf), and if I understood it correctly, you can enable timer
> interrupts for core0 by setting bit 0 at 0x4040 and you can trigger them
> by using the ARM Generic
> Timer.
> 
> With this procedure I can reliably trigger that timer interrupt in the
> real hardware by setting CNTP_CTL and CNTP_TVAL but on qemu it doesn't
> seem to be triggered.
> 
> Can somebody please tell me what I might be doing wrong?

I don't think you're doing anything wrong; we just don't model this timer 
hardware yet (neither Linux nor Windows needs it). If you want to take a stab 
at adding it, the relevant hardware emulation is hw/intc/bcm2836_control.c.

If you're willing to use different timer sources, then I suggest looking at the 
ARM per-core timers. I also have Gregory's emulation code for the other bcm2835 
timers in my private github, and I hope to submit to upstream qemu after the 
current freeze, since it is needed for pi1 Linux support.

Cheers,
Andrew

Re: [Qemu-devel] Timer interrupts for -M raspi2

[Andrew Baumann]

Hi Antonio,

> From: Antonio Huete Jiménez [mailto:tuxi...@quantumachine.net]
> Sent: Wednesday, 16 March 2016 4:24 PM
> 
> Hi Andrew,
> 
> I thought the timer that was not implemented was the local timer
> (located at 0x4034) and that the core timers interrupt control
> registers starting at 0x4040 were the per-core timers.

Oh, sorry, you're right; I replied too quickly.

Yes, that should work. Bits 0 and 3 are wired up to what qemu refers to as 
GTIMER_PHYS and GTIMER_VIRT respectively. (The other two timers aren't 
currently connected; I can't remember if that's because they weren't modelled 
by core QEMU when I was implementing the device model, or just because I wasn't 
sure how to route them and never came back to fix it.)

If you are seeing interrupts on real hardware and not on qemu, then it may be 
because the board performs some additional setup that you are relying on. I'm 
hardly an expert on ARM, but I would imagine you need to also need to setup the 
timer's control and count registers (using mcr/mrc) to get it ticking. The 
write to 0x4040 simply enables the interrupt.

> Can you please point me to the documentation about this ARM per-core
> timers?

Here's a starting point to some docs (just from a quick search):
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0438c/BGBBIJCB.html

Cheers,
Andrew 

> Andrew Baumann <andrew.baum...@microsoft.com> escribió:
> 
> > Hi Antonio,
> >
> >> From: Antonio Huete Jiménez [mailto:tuxi...@quantumachine.net]
> >> Sent: Wednesday, 16 March 2016 3:40 PM
> >>
> >> Hi,
> >>
> >> I am experiencing what I think it's an issue with -M raspi2 and
> >> interrupts in a baremetal application.
> >>
> >> According to this document
> >>
> (https://www.raspberrypi.org/documentation/hardware/raspberrypi/bcm2
> >> 836/QA7_rev3.4.pdf), and if I understood it correctly, you can enable
> timer
> >> interrupts for core0 by setting bit 0 at 0x4040 and you can trigger
> them
> >> by using the ARM Generic
> >> Timer.
> >>
> >> With this procedure I can reliably trigger that timer interrupt in the
> >> real hardware by setting CNTP_CTL and CNTP_TVAL but on qemu it
> doesn't
> >> seem to be triggered.
> >>
> >> Can somebody please tell me what I might be doing wrong?
> >
> > I don't think you're doing anything wrong; we just don't model this
> > timer hardware yet (neither Linux nor Windows needs it). If you want
> > to take a stab at adding it, the relevant hardware emulation is
> > hw/intc/bcm2836_control.c.
> >
> > If you're willing to use different timer sources, then I suggest
> > looking at the ARM per-core timers. I also have Gregory's emulation
> > code for the other bcm2835 timers in my private github, and I hope
> > to submit to upstream qemu after the current freeze, since it is
> > needed for pi1 Linux support.
> >
> > Cheers,
> > Andrew
> 

Re: [Qemu-devel] Ubuntu 14.04 LTS Arm for Raspberry - QEMU Support

[Andrew Baumann]

Hi Vincenzo,

> From: Vincenzo Calabrò [mailto:muawi...@moosth.net] 
> Sent: Wednesday, 16 March 2016 12:39 PM
[...]
> 1. I'm using the "official" arm ubuntu 14.04 
> (https://wiki.ubuntu.com/ARM/RaspberryPi) - you can download the image from 
> that link and follow the instruction to flash a SD card yes I'm using the 
> SD card instead of the image on the host. The reason is I want to emulate the 
> RPi with its real SD card.

Ok. Using the SD card should be fine, but I have never tested the Ubuntu image 
before, just an (older) version of Raspbian.

> 2. after copying the kernel7.img and the bcm2709-rpi-2-b.dtb file into a 
> "boot" folder, I use the following command with the lastest (up to one day 
> ago) qemu:
> qemu-system-arm \
> -kernel boot/kernel7.img \
> -dtb boot/bcm2709-rpi-2-b.dtb \
> -M raspi2 -m 256 \

Why are you emulating a 256MB pi2? Does such a configuration exist? I thought 
they all had 1GB of RAM.

> -no-reboot -serial stdio \
> -append "rw earlyprintk loglevel=8 console=ttyAMA0,115200 
> dwc_otg.lpm_enable=0 root=/dev/mmcblk0p2 panic=1 rootfstype=ext4" \
> -hda /dev/sdb \
> -net nic -net 
> user,net=http://10.0.0.0/8,host=10.0.0.1,hostfwd=tcp:127.0.0.1:-10.0.0.2:22

[...]
> [    5.109543] bcm_power: Broadcom power driver
> [    5.111766] bcm_power_open() -> 0
> [    5.112687] bcm_power_request(0, 8)

This is a known (albeit only as of Peter's email to me slightly earlier today 
:) issue, since we don't model the power management unit in mainline qemu.

There is a trivial emulation for it in the raspi branch here:
https://github.com/0xabu/qemu ... I would be curious to know whether you fare 
any better with this version?

Cheers,
Andrew

Re: [Qemu-devel] [PATCH v3 0/5] Raspberry Pi framebuffer, DMA and Windows support

[Andrew Baumann]

> From: Peter Maydell [mailto:peter.mayd...@linaro.org]
> Sent: Wednesday, 16 March 2016 9:54 AM
> 
> On 8 March 2016 at 20:05, Andrew Baumann
> <andrew.baum...@microsoft.com> wrote:
> > This patch series adds support for the AUX (second UART), framebuffer
> > and DMA controller on Raspberry Pi 2, and enables booting Windows on
> > this device. As with the previous series, it is heavily based on the
> > original (out of tree) work of Gregory Estrade, Stefan Weil and others
> > to support Raspberry Pi 1.
> >
> > After this series, it is possible to boot Windows by following the
> > instructions at https://github.com/0xabu/qemu/wiki. You also boot
> > Raspbian to the GUI using a command such as:
> >
> >   qemu-system-arm -M raspi2 -kernel raspbian-boot/kernel7.img -sd
> >   2015-09-24-raspbian-jessie.img -append "rw earlyprintk loglevel=8
> >   console=ttyAMA0 dwc_otg.lpm_enable=0 root=/dev/mmcblk0p2
> rootwait"
> >   -dtb raspbian-boot/bcm2709-rpi-2-b.dtb -serial stdio
> 
> So I tried something like this, and the kernel gives a WARNING with
> a backtrace, and then hangs:
> 
> [5.413943] VFP support v0.3: implementor 41 architecture 4 part 30
> variant f rev 0
> [5.976888] pinctrl core: initialized pinctrl subsystem
> [6.171197] NET: Registered protocol family 16
> [6.286799] DMA: preallocated 4096 KiB pool for atomic coherent allocations
> [6.366889] cpuidle: using governor ladder
> [6.391665] cpuidle: using governor menu
> [6.401604] bcm2709.uart_clock = 300
> [6.478063] [ cut here ]
> [6.481052] WARNING: CPU: 0 PID: 1 at
> /build/buildd/linux-3.18.0/arch/arm/mach-bcm2709/armctrl.c:148
> armctrl_xlate+0x188/0x274()
> [6.484474] Modules linked in:
> [6.489783] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
> 3.18.0-20-rpi2 #21-Ubuntu
> [6.502781] [<80016ab0>] (unwind_backtrace) from [<800129d0>]
> (show_stack+0x10/0x14)
> [6.506355] [<800129d0>] (show_stack) from [<805fee10>]
> (dump_stack+0x9c/0xd4)
> [6.508946] [<805fee10>] (dump_stack) from [<80026ed8>]
> (warn_slowpath_common+0x70/0x8c)
> [6.511251] [<80026ed8>] (warn_slowpath_common) from [<80026f90>]
> (warn_slowpath_null+0x1c/0x24)
> [6.512797] [<80026f90>] (warn_slowpath_null) from [<80022044>]
> (armctrl_xlate+0x188/0x274)
> [6.514558] [<80022044>] (armctrl_xlate) from [<8007399c>]
> (irq_create_of_mapping+0x64/0x110)
> [6.517173] [<8007399c>] (irq_create_of_mapping) from [<804c4ef4>]
> (irq_of_parse_and_map+0x24/0x2c)
> [6.518741] [<804c4ef4>] (irq_of_parse_and_map) from [<804c4f14>]
> (of_irq_to_resource+0x18/0xb8)
> [6.520209] [<804c4f14>] (of_irq_to_resource) from [<804c4ff0>]
> (of_irq_to_resource_table+0x3c/0x54)
> [6.523598] [<804c4ff0>] (of_irq_to_resource_table) from
> [<804c25b8>] (of_device_alloc+0xd8/0x180)
> [6.528838] [<804c25b8>] (of_device_alloc) from [<804c26a8>]
> (of_platform_device_create_pdata+0x48/0x98)
> [6.535352] [<804c26a8>] (of_platform_device_create_pdata) from
> [<804c27f0>] (of_platform_bus_create+0xec/0x3ac)
> [6.539440] [<804c27f0>] (of_platform_bus_create) from [<804c2860>]
> (of_platform_bus_create+0x15c/0x3ac)
> [6.541217] [<804c2860>] (of_platform_bus_create) from [<804c2c28>]
> (of_platform_populate+0x5c/0xa0)
> [6.542770] [<804c2c28>] (of_platform_populate) from [<8088cad8>]
> (bcm2709_init+0x64/0x3fc)
> [6.546211] [<8088cad8>] (bcm2709_init) from [<8070>]
> (customize_machine+0x20/0x40)
> [6.547711] [<8070>] (customize_machine) from [<800088bc>]
> (do_one_initcall+0xd8/0x208)
> [6.549200] [<800088bc>] (do_one_initcall) from [<80885ef4>]
> (kernel_init_freeable+0x1fc/0x29c)
> [6.551771] [<80885ef4>] (kernel_init_freeable) from [<805f9298>]
> (kernel_init+0x8/0xf0)
> [6.553319] [<805f9298>] (kernel_init) from [<8000efe8>]
> (ret_from_fork+0x14/0x2c)
> [6.556670] ---[ end trace 088ba587f0a009cc ]---
> [6.649948] No ATAGs?
> [6.658029] hw-breakpoint: found 5 (+1 reserved) breakpoint and 4
> watchpoint registers.
> [6.660393] hw-breakpoint: maximum watchpoint size is 8 bytes.
> [6.664206] mailbox: Broadcom VideoCore Mailbox driver
> [6.693912] bcm2708_vcio: mailbox at f300b880
> [6.703745] bcm_power: Broadcom power driver
> [6.707642] bcm_power_open() -> 0
> [6.708671] bcm_power_request(0, 8)
> 
> Does that look familiar? (My first guess is just that it wants som

Re: [Qemu-devel] [PATCH] Include setjmp.h in qemu/osdep.h (bug fix for w64)

[Andrew Baumann]

> From: Stefan Weil [mailto:s...@weilnetz.de]
> Sent: Friday, 11 March 2016 10:32 PM
> 
> setjmp must be declared before sysemu/os-win32.h
> because it is redefined there for 64 bit Windows.
> 
> Signed-off-by: Stefan Weil <s...@weilnetz.de>
> ---

Tested-by: Andrew Baumann <andrew.baum...@microsoft.com>

Thanks Stefan,
Andrew

Re: [Qemu-devel] [PATCH v2 00/18] Multiple fixes & improvements to QIOChannel & Win32

[Andrew Baumann]

Hi folks,

> From: Paolo Bonzini [mailto:pbonz...@redhat.com]
> Sent: Thursday, 10 March 2016 9:37 AM
> 
> On 10/03/2016 18:26, Daniel P. Berrange wrote:
> > This series started out as an attempt to fix the Win32 problems
> > identified by Andrew Baumann
> >
> >https://lists.gnu.org/archive/html/qemu-devel/2016-03/msg01343.html
> >
> > It turned into a significantly larger cleanup of some chardev
> > and osdep win32 portability code.
[...]

Sorry for chiming in a bit late here. I've tested these patches (the complete 
set, not individually), and they do appear to fix my immediate problem: socket 
char devices now work again. So thank you!

However, I'm now seeing a problem I don't believe we had before: very slow 
responses to GDB commands. From looking at a packet capture (using a localhost 
tcp socket between qemu and my gdb client), it seems that a couple of 
operations will go through just fine, and then there is a 1 second delay 
between my client's request and qemu's response. After fiddling with poll 
timeouts, it became clear that we were noticing the socket events when waking 
up from the poll, but the events themselves were still not waking us. It turns 
out that we were not calling WSAEventSelect on the accept path. At least, the 
following patch fixed the problem for me:

diff --git a/qemu-char.c b/qemu-char.c
index 3bf30b5..c1be622 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3047,6 +3047,7 @@ static gboolean tcp_chr_accept(QIOChannel *channel,
 return TRUE;
 }

+qio_channel_set_blocking(QIO_CHANNEL(sioc), false, NULL);
 tcp_chr_new_client(chr, sioc);

 object_unref(OBJECT(sioc));

However, I'd note that both callers of tcp_chr_new_client() make the same call 
to set blocking to false immediately before calling tcp_chr_new_client(). 
Furthermore, the doc comment for qio_channel_set_blocking() appears to suggest 
that non-blocking mode is the default. If that's true, maybe you don't even 
want to rely on the caller explicitly setting blocking to false?

Cheers,
Andrew

[Qemu-devel] [PATCH v3 4/5] bcm2835_property: implement framebuffer control/configuration properties

[Andrew Baumann]

From: Grégory ESTRADE <gregory.estr...@gmail.com>

The property channel driver now interfaces with the framebuffer device
to query and set framebuffer parameters. As a result of this, the "get
ARM RAM size" query now correctly returns the video RAM base address
(not total RAM size), and the ram-size property is no longer relevant
here.

Signed-off-by: Grégory ESTRADE <gregory.estr...@gmail.com>
[AB: cleanup/refactoring for upstream submission]
Signed-off-by: Andrew Baumann <andrew.baum...@microsoft.com>
Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
---

Notes:
v2:
 * avoid ldl/stl_phys
 * move code to increase default pi2 memory size from preceding patch here
   (it was incorrect without the property channel implementation changes)

 hw/arm/bcm2835_peripherals.c   |   8 +--
 hw/arm/raspi.c |   7 +-
 hw/misc/bcm2835_property.c | 139 -
 include/hw/misc/bcm2835_property.h |   5 +-
 4 files changed, 144 insertions(+), 15 deletions(-)

diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index c2fe6b7..4d74a18 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -79,6 +79,8 @@ static void bcm2835_peripherals_init(Object *obj)
   "board-rev", _abort);
 qdev_set_parent_bus(DEVICE(>property), sysbus_get_default());
 
+object_property_add_const_link(OBJECT(>property), "fb",
+   OBJECT(>fb), _abort);
 object_property_add_const_link(OBJECT(>property), "dma-mr",
OBJECT(>gpu_bus_mr), _abort);
 
@@ -211,12 +213,6 @@ static void bcm2835_peripherals_realize(DeviceState *dev, 
Error **errp)
qdev_get_gpio_in(DEVICE(>mboxes), MBOX_CHAN_FB));
 
 /* Property channel */
-object_property_set_int(OBJECT(>property), ram_size, "ram-size", );
-if (err) {
-error_propagate(errp, err);
-return;
-}
-
 object_property_set_bool(OBJECT(>property), true, "realized", );
 if (err) {
 error_propagate(errp, err);
diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index 5498209..83fe809 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -164,11 +164,6 @@ static void raspi2_machine_init(MachineClass *mc)
 mc->no_floppy = 1;
 mc->no_cdrom = 1;
 mc->max_cpus = BCM2836_NCPUS;
-
-/* XXX: Temporary restriction in RAM size from the full 1GB. Since
- * we do not yet support the framebuffer / GPU, we need to limit
- * RAM usable by the OS to sit below the peripherals.
- */
-mc->default_ram_size = 0x3F00; /* BCM2836_PERI_BASE */
+mc->default_ram_size = 1024 * 1024 * 1024;
 };
 DEFINE_MACHINE("raspi2", raspi2_machine_init)
diff --git a/hw/misc/bcm2835_property.c b/hw/misc/bcm2835_property.c
index 41fbbe3..15dcc02 100644
--- a/hw/misc/bcm2835_property.c
+++ b/hw/misc/bcm2835_property.c
@@ -17,6 +17,11 @@ static void bcm2835_property_mbox_push(BCM2835PropertyState 
*s, uint32_t value)
 uint32_t tot_len;
 size_t resplen;
 uint32_t tmp;
+int n;
+uint32_t offset, length, color;
+uint32_t xres, yres, xoffset, yoffset, bpp, pixo, alpha;
+uint32_t *newxres = NULL, *newyres = NULL, *newxoffset = NULL,
+*newyoffset = NULL, *newbpp = NULL, *newpixo = NULL, *newalpha = NULL;
 
 value &= ~0xf;
 
@@ -60,7 +65,14 @@ static void bcm2835_property_mbox_push(BCM2835PropertyState 
*s, uint32_t value)
 /* base */
 stl_le_phys(>dma_as, value + 12, 0);
 /* size */
-stl_le_phys(>dma_as, value + 16, s->ram_size);
+stl_le_phys(>dma_as, value + 16, s->fbdev->vcram_base);
+resplen = 8;
+break;
+case 0x00010006: /* Get VC memory */
+/* base */
+stl_le_phys(>dma_as, value + 12, s->fbdev->vcram_base);
+/* size */
+stl_le_phys(>dma_as, value + 16, s->fbdev->vcram_size);
 resplen = 8;
 break;
 case 0x00028001: /* Set power state */
@@ -122,6 +134,114 @@ static void 
bcm2835_property_mbox_push(BCM2835PropertyState *s, uint32_t value)
 resplen = 8;
 break;
 
+/* Frame buffer */
+
+case 0x00040001: /* Allocate buffer */
+stl_le_phys(>dma_as, value + 12, s->fbdev->base);
+stl_le_phys(>dma_as, value + 16, s->fbdev->size);
+resplen = 8;
+break;
+case 0x00048001: /* Release buffer */
+resplen = 0;
+break;
+case 0x00040002: /* Blank screen */
+resplen = 4;
+break;
+case 0x00040003: /* Get display width/height */
+case 0x00040004:
+stl_le_phys(>dma_as, value + 12, s-&

[Qemu-devel] [PATCH v3 5/5] bcm2835_dma: add emulation of Raspberry Pi DMA controller

[Andrew Baumann]

From: Grégory ESTRADE <gregory.estr...@gmail.com>

At present, all DMA transfers complete inline (so a looping descriptor
queue will lock up the device). We also do not model pause/abort,
arbitrarion/priority, or debug features.

Signed-off-by: Grégory ESTRADE <gregory.estr...@gmail.com>
[AB: implement 2D mode, cleanup/refactoring for upstream submission]
Signed-off-by: Andrew Baumann <andrew.baum...@microsoft.com>
Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
---

Notes:
v2:
 * avoid ldl_phys/stl_phys
 * compute address of channel structure only after asserting its validity
 * correctly implement per-channel reset and abort bits
 * set channel paused bit when completing DMA

 hw/arm/bcm2835_peripherals.c |  26 +++
 hw/dma/Makefile.objs |   1 +
 hw/dma/bcm2835_dma.c | 408 +++
 include/hw/arm/bcm2835_peripherals.h |   2 +
 include/hw/dma/bcm2835_dma.h |  47 
 5 files changed, 484 insertions(+)
 create mode 100644 hw/dma/bcm2835_dma.c
 create mode 100644 include/hw/dma/bcm2835_dma.h

diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 4d74a18..8099a8a 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -88,6 +88,14 @@ static void bcm2835_peripherals_init(Object *obj)
 object_initialize(>sdhci, sizeof(s->sdhci), TYPE_SYSBUS_SDHCI);
 object_property_add_child(obj, "sdhci", OBJECT(>sdhci), NULL);
 qdev_set_parent_bus(DEVICE(>sdhci), sysbus_get_default());
+
+/* DMA Channels */
+object_initialize(>dma, sizeof(s->dma), TYPE_BCM2835_DMA);
+object_property_add_child(obj, "dma", OBJECT(>dma), NULL);
+qdev_set_parent_bus(DEVICE(>dma), sysbus_get_default());
+
+object_property_add_const_link(OBJECT(>dma), "dma-mr",
+   OBJECT(>gpu_bus_mr), _abort);
 }
 
 static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
@@ -258,6 +266,24 @@ static void bcm2835_peripherals_realize(DeviceState *dev, 
Error **errp)
 return;
 }
 
+/* DMA Channels */
+object_property_set_bool(OBJECT(>dma), true, "realized", );
+if (err) {
+error_propagate(errp, err);
+return;
+}
+
+memory_region_add_subregion(>peri_mr, DMA_OFFSET,
+sysbus_mmio_get_region(SYS_BUS_DEVICE(>dma), 0));
+memory_region_add_subregion(>peri_mr, DMA15_OFFSET,
+sysbus_mmio_get_region(SYS_BUS_DEVICE(>dma), 1));
+
+for (n = 0; n <= 12; n++) {
+sysbus_connect_irq(SYS_BUS_DEVICE(>dma), n,
+   qdev_get_gpio_in_named(DEVICE(>ic),
+  BCM2835_IC_GPU_IRQ,
+  INTERRUPT_DMA0 + n));
+}
 }
 
 static void bcm2835_peripherals_class_init(ObjectClass *oc, void *data)
diff --git a/hw/dma/Makefile.objs b/hw/dma/Makefile.objs
index 0e65ed0..a1abbcf 100644
--- a/hw/dma/Makefile.objs
+++ b/hw/dma/Makefile.objs
@@ -11,3 +11,4 @@ common-obj-$(CONFIG_SUN4M) += sun4m_iommu.o
 
 obj-$(CONFIG_OMAP) += omap_dma.o soc_dma.o
 obj-$(CONFIG_PXA2XX) += pxa2xx_dma.o
+obj-$(CONFIG_RASPI) += bcm2835_dma.o
diff --git a/hw/dma/bcm2835_dma.c b/hw/dma/bcm2835_dma.c
new file mode 100644
index 000..c7ce4e4
--- /dev/null
+++ b/hw/dma/bcm2835_dma.c
@@ -0,0 +1,408 @@
+/*
+ * Raspberry Pi emulation (c) 2012 Gregory Estrade
+ * This code is licensed under the GNU GPLv2 and later.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/dma/bcm2835_dma.h"
+
+/* DMA CS Control and Status bits */
+#define BCM2708_DMA_ACTIVE  (1 << 0)
+#define BCM2708_DMA_END (1 << 1) /* GE */
+#define BCM2708_DMA_INT (1 << 2)
+#define BCM2708_DMA_ISPAUSED(1 << 4)  /* Pause requested or not active */
+#define BCM2708_DMA_ISHELD  (1 << 5)  /* Is held by DREQ flow control */
+#define BCM2708_DMA_ERR (1 << 8)
+#define BCM2708_DMA_ABORT   (1 << 30) /* stop current CB, go to next, WO */
+#define BCM2708_DMA_RESET   (1 << 31) /* WO, self clearing */
+
+/* DMA control block "info" field bits */
+#define BCM2708_DMA_INT_EN  (1 << 0)
+#define BCM2708_DMA_TDMODE  (1 << 1)
+#define BCM2708_DMA_WAIT_RESP   (1 << 3)
+#define BCM2708_DMA_D_INC   (1 << 4)
+#define BCM2708_DMA_D_WIDTH (1 << 5)
+#define BCM2708_DMA_D_DREQ  (1 << 6)
+#define BCM2708_DMA_D_IGNORE(1 << 7)
+#define BCM2708_DMA_S_INC   (1 << 8)
+#define BCM2708_DMA_S_WIDTH (1 << 9)
+#define BCM2708_DMA_S_DREQ  (1 << 10)
+#define BCM2708_DMA_S_IGNORE(1 << 11)
+
+/* Register offsets */
+#define BCM2708_DMA_CS  0x00 /* Control and Status */
+#define BCM2708_DMA_ADDR0x04 /* Control block address 

[Qemu-devel] [PATCH v3 3/5] bcm2835_fb: add framebuffer device for Raspberry Pi

[Andrew Baumann]

From: Grégory ESTRADE <gregory.estr...@gmail.com>

The framebuffer occupies the upper portion of memory (64MiB by
default), but it can only be controlled/configured via a system
mailbox or property channel (to be added by a subsequent patch).

Signed-off-by: Grégory ESTRADE <gregory.estr...@gmail.com>
[AB: added Windows (BGR) support and cleanup/refactoring for upstream 
submission]
Signed-off-by: Andrew Baumann <andrew.baum...@microsoft.com>
Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
---

Notes:
v2:
 * avoid ldl_phys
 * move code to increase default pi2 memory size back to the final patch

 hw/arm/bcm2835_peripherals.c |  38 +++-
 hw/arm/bcm2836.c |   2 +
 hw/arm/raspi.c   |   5 +-
 hw/display/Makefile.objs |   1 +
 hw/display/bcm2835_fb.c  | 424 +++
 include/hw/arm/bcm2835_peripherals.h |   2 +
 include/hw/display/bcm2835_fb.h  |  47 
 7 files changed, 517 insertions(+), 2 deletions(-)
 create mode 100644 hw/display/bcm2835_fb.c
 create mode 100644 include/hw/display/bcm2835_fb.h

diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index d6453cc..c2fe6b7 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -62,6 +62,16 @@ static void bcm2835_peripherals_init(Object *obj)
 object_property_add_const_link(OBJECT(>mboxes), "mbox-mr",
OBJECT(>mbox_mr), _abort);
 
+/* Framebuffer */
+object_initialize(>fb, sizeof(s->fb), TYPE_BCM2835_FB);
+object_property_add_child(obj, "fb", OBJECT(>fb), NULL);
+object_property_add_alias(obj, "vcram-size", OBJECT(>fb), "vcram-size",
+  _abort);
+qdev_set_parent_bus(DEVICE(>fb), sysbus_get_default());
+
+object_property_add_const_link(OBJECT(>fb), "dma-mr",
+   OBJECT(>gpu_bus_mr), _abort);
+
 /* Property channel */
 object_initialize(>property, sizeof(s->property), 
TYPE_BCM2835_PROPERTY);
 object_property_add_child(obj, "property", OBJECT(>property), NULL);
@@ -84,7 +94,7 @@ static void bcm2835_peripherals_realize(DeviceState *dev, 
Error **errp)
 Object *obj;
 MemoryRegion *ram;
 Error *err = NULL;
-uint32_t ram_size;
+uint32_t ram_size, vcram_size;
 CharDriverState *chr;
 int n;
 
@@ -174,6 +184,32 @@ static void bcm2835_peripherals_realize(DeviceState *dev, 
Error **errp)
 qdev_get_gpio_in_named(DEVICE(>ic), BCM2835_IC_ARM_IRQ,
INTERRUPT_ARM_MAILBOX));
 
+/* Framebuffer */
+vcram_size = (uint32_t)object_property_get_int(OBJECT(s), "vcram-size",
+   );
+if (err) {
+error_propagate(errp, err);
+return;
+}
+
+object_property_set_int(OBJECT(>fb), ram_size - vcram_size,
+"vcram-base", );
+if (err) {
+error_propagate(errp, err);
+return;
+}
+
+object_property_set_bool(OBJECT(>fb), true, "realized", );
+if (err) {
+error_propagate(errp, err);
+return;
+}
+
+memory_region_add_subregion(>mbox_mr, MBOX_CHAN_FB << 
MBOX_AS_CHAN_SHIFT,
+sysbus_mmio_get_region(SYS_BUS_DEVICE(>fb), 0));
+sysbus_connect_irq(SYS_BUS_DEVICE(>fb), 0,
+   qdev_get_gpio_in(DEVICE(>mboxes), MBOX_CHAN_FB));
+
 /* Property channel */
 object_property_set_int(OBJECT(>property), ram_size, "ram-size", );
 if (err) {
diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
index 0321439..89a6b35 100644
--- a/hw/arm/bcm2836.c
+++ b/hw/arm/bcm2836.c
@@ -42,6 +42,8 @@ static void bcm2836_init(Object *obj)
   _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());
 }
 
diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index 6582279..5498209 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -113,6 +113,7 @@ static void setup_boot(MachineState *machine, int version, 
size_t ram_size)
 static void raspi2_init(MachineState *machine)
 {
 RasPiState *s = g_new0(RasPiState, 1);
+uint32_t vcram_size;
 DriveInfo *di;
 BlockBackend *blk;
 BusState *bus;
@@ -149,7 +150,9 @@ static void raspi2_init(MachineState *machine)
 qdev_prop_set_drive(carddev, "drive", blk, _fatal);
 object_property_set_bool(OBJECT(carddev), true, "realized", _fatal);
 
-setup_boot(machine, 

[Qemu-devel] [PATCH v3 2/5] bcm2835_aux: add emulation of BCM2835 AUX (aka UART1) block

[Andrew Baumann]

At present only the core UART functions (data path for tx/rx) are
implemented, which is enough for UEFI to boot. The following
features/registers are unimplemented:
  * Line/modem control
  * Scratch register
  * Extra control
  * Baudrate
  * SPI interfaces

Signed-off-by: Andrew Baumann <andrew.baum...@microsoft.com>
Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
---

Notes:
v3:
 * use qemu_char_get_next_serial() to configure the chardev prop in
   bcm2835_peripherals, rather than reaching into serial_hds[] directly

v2:
 * document unimplemented features, log unimplemented register accesses
 * model read path as 8-bit
 * drop incorrect event (break detection) functionality
 * implement AUX_IRQ register
 * model interrupt enables as a uint8 rather than 2 bools
 * corrected bugs in implementation of IIR bits 0-2
 * use chardev prop rather than qemu_char_get_next_serial(); the
   soc-level (bcm2835_peripherals) handling is still a little messy
   however, because uart0 is a pl011 device which still calls
   qemu_char_get_next_serial()

 hw/arm/bcm2835_peripherals.c |  32 
 hw/char/Makefile.objs|   1 +
 hw/char/bcm2835_aux.c| 316 +++
 include/hw/arm/bcm2835_peripherals.h |   2 +
 include/hw/char/bcm2835_aux.h|  33 
 5 files changed, 384 insertions(+)
 create mode 100644 hw/char/bcm2835_aux.c
 create mode 100644 include/hw/char/bcm2835_aux.h

diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 6ce9cd1..d6453cc 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -12,6 +12,7 @@
 #include "hw/arm/bcm2835_peripherals.h"
 #include "hw/misc/bcm2835_mbox_defs.h"
 #include "hw/arm/raspi_platform.h"
+#include "sysemu/char.h"
 
 /* Peripheral base address on the VC (GPU) system bus */
 #define BCM2835_VC_PERI_BASE 0x7e00
@@ -48,6 +49,11 @@ static void bcm2835_peripherals_init(Object *obj)
 object_property_add_child(obj, "uart0", OBJECT(s->uart0), NULL);
 qdev_set_parent_bus(DEVICE(s->uart0), sysbus_get_default());
 
+/* AUX / UART1 */
+object_initialize(>aux, sizeof(s->aux), TYPE_BCM2835_AUX);
+object_property_add_child(obj, "aux", OBJECT(>aux), NULL);
+qdev_set_parent_bus(DEVICE(>aux), sysbus_get_default());
+
 /* Mailboxes */
 object_initialize(>mboxes, sizeof(s->mboxes), TYPE_BCM2835_MBOX);
 object_property_add_child(obj, "mbox", OBJECT(>mboxes), NULL);
@@ -79,6 +85,7 @@ static void bcm2835_peripherals_realize(DeviceState *dev, 
Error **errp)
 MemoryRegion *ram;
 Error *err = NULL;
 uint32_t ram_size;
+CharDriverState *chr;
 int n;
 
 obj = object_property_get_link(OBJECT(dev), "ram", );
@@ -131,6 +138,29 @@ static void bcm2835_peripherals_realize(DeviceState *dev, 
Error **errp)
 qdev_get_gpio_in_named(DEVICE(>ic), BCM2835_IC_GPU_IRQ,
INTERRUPT_UART));
 
+/* AUX / UART1 */
+/* TODO: don't call qemu_char_get_next_serial() here, instead set
+ * chardev properties for each uart at the board level, once pl011
+ * (uart0) has been updated to avoid qemu_char_get_next_serial()
+ */
+chr = qemu_char_get_next_serial();
+if (chr == NULL) {
+chr = qemu_chr_new("bcm2835.uart1", "null", NULL);
+}
+qdev_prop_set_chr(DEVICE(>aux), "chardev", chr);
+
+object_property_set_bool(OBJECT(>aux), true, "realized", );
+if (err) {
+error_propagate(errp, err);
+return;
+}
+
+memory_region_add_subregion(>peri_mr, UART1_OFFSET,
+sysbus_mmio_get_region(SYS_BUS_DEVICE(>aux), 0));
+sysbus_connect_irq(SYS_BUS_DEVICE(>aux), 0,
+qdev_get_gpio_in_named(DEVICE(>ic), BCM2835_IC_GPU_IRQ,
+   INTERRUPT_AUX));
+
 /* Mailboxes */
 object_property_set_bool(OBJECT(>mboxes), true, "realized", );
 if (err) {
@@ -203,6 +233,8 @@ static void bcm2835_peripherals_class_init(ObjectClass *oc, 
void *data)
 DeviceClass *dc = DEVICE_CLASS(oc);
 
 dc->realize = bcm2835_peripherals_realize;
+/* Reason: realize() method uses qemu_char_get_next_serial() */
+dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo bcm2835_peripherals_type_info = {
diff --git a/hw/char/Makefile.objs b/hw/char/Makefile.objs
index 5931cc8..69a553c 100644
--- a/hw/char/Makefile.objs
+++ b/hw/char/Makefile.objs
@@ -16,6 +16,7 @@ obj-$(CONFIG_SH4) += sh_serial.o
 obj-$(CONFIG_PSERIES) += spapr_vty.o
 obj-$(CONFIG_DIGIC) += digic-uart.o
 obj-$(CONFIG_STM32F2XX_USART) += stm32f2xx_usart.o
+obj-$(CONFIG_RASPI) += bcm2835_aux.o
 
 common-obj-$(CONFIG_ETRAXFS) += etraxfs_ser.o
 common-obj-$(CONFIG_ISA_DEBUG) += debu

[Qemu-devel] [PATCH v3 0/5] Raspberry Pi framebuffer, DMA and Windows support

[Andrew Baumann]

This patch series adds support for the AUX (second UART), framebuffer
and DMA controller on Raspberry Pi 2, and enables booting Windows on
this device. As with the previous series, it is heavily based on the
original (out of tree) work of Gregory Estrade, Stefan Weil and others
to support Raspberry Pi 1.

After this series, it is possible to boot Windows by following the
instructions at https://github.com/0xabu/qemu/wiki. You also boot
Raspbian to the GUI using a command such as:

  qemu-system-arm -M raspi2 -kernel raspbian-boot/kernel7.img -sd
  2015-09-24-raspbian-jessie.img -append "rw earlyprintk loglevel=8
  console=ttyAMA0 dwc_otg.lpm_enable=0 root=/dev/mmcblk0p2 rootwait"
  -dtb raspbian-boot/bcm2709-rpi-2-b.dtb -serial stdio

I plan to add USB, and remaining timers / system devices in
future patch series, along with support for pi1 (bcm2835). In the
meantime, the complete code is available at https://github.com/0xabu/qemu

v2:
 * added DMA controller
 * revised per PMM's review feedback
 * rebased on top of the patch for fixing ldl_phys/stl_phys in raspi
   devices, presently in target-arm.next

v3:
 * tweaked instantiation of uart1 char device
 * identified Gregory as primary author of framebuffer and DMA emulation

Cheers,
Andrew

Andrew Baumann (2):
  bcm2835_peripherals: enable sdhci pending-insert quirk for raspberry
pi
  bcm2835_aux: add emulation of BCM2835 AUX (aka UART1) block

Grégory ESTRADE (3):
  bcm2835_fb: add framebuffer device for Raspberry Pi
  bcm2835_property: implement framebuffer control/configuration
properties
  bcm2835_dma: add emulation of Raspberry Pi DMA controller

 hw/arm/bcm2835_peripherals.c | 103 -
 hw/arm/bcm2836.c |   2 +
 hw/arm/raspi.c   |  12 +-
 hw/char/Makefile.objs|   1 +
 hw/char/bcm2835_aux.c| 316 ++
 hw/display/Makefile.objs |   1 +
 hw/display/bcm2835_fb.c  | 424 +++
 hw/dma/Makefile.objs |   1 +
 hw/dma/bcm2835_dma.c | 408 +
 hw/misc/bcm2835_property.c   | 139 +++-
 include/hw/arm/bcm2835_peripherals.h |   6 +
 include/hw/char/bcm2835_aux.h|  33 +++
 include/hw/display/bcm2835_fb.h  |  47 
 include/hw/dma/bcm2835_dma.h |  47 
 include/hw/misc/bcm2835_property.h   |   5 +-
 15 files changed, 1532 insertions(+), 13 deletions(-)
 create mode 100644 hw/char/bcm2835_aux.c
 create mode 100644 hw/display/bcm2835_fb.c
 create mode 100644 hw/dma/bcm2835_dma.c
 create mode 100644 include/hw/char/bcm2835_aux.h
 create mode 100644 include/hw/display/bcm2835_fb.h
 create mode 100644 include/hw/dma/bcm2835_dma.h

-- 
2.7.0

[Qemu-devel] [PATCH v3 1/5] bcm2835_peripherals: enable sdhci pending-insert quirk for raspberry pi

[Andrew Baumann]

Signed-off-by: Andrew Baumann <andrew.baum...@microsoft.com>
Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
---
 hw/arm/bcm2835_peripherals.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 6d66fa0..6ce9cd1 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -171,6 +171,13 @@ static void bcm2835_peripherals_realize(DeviceState *dev, 
Error **errp)
 return;
 }
 
+object_property_set_bool(OBJECT(>sdhci), true, "pending-insert-quirk",
+ );
+if (err) {
+error_propagate(errp, err);
+return;
+}
+
 object_property_set_bool(OBJECT(>sdhci), true, "realized", );
 if (err) {
 error_propagate(errp, err);
-- 
2.7.0

Re: [Qemu-devel] [PATCH v2 5/5] bcm2835_dma: add emulation of Raspberry Pi DMA controller

[Andrew Baumann]

> From: Peter Maydell [mailto:peter.mayd...@linaro.org]
> Sent: Monday, 7 March 2016 8:37 PM
> 
> On 4 March 2016 at 01:24, Andrew Baumann
> <andrew.baum...@microsoft.com> wrote:
> > At present, all DMA transfers complete inline (so a looping descriptor
> > queue will lock up the device). We also do not model pause/abort,
> > arbitrarion/priority, or debug features.
> >
> > Signed-off-by: Andrew Baumann <andrew.baum...@microsoft.com>
> > ---
> 
> Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
> 
> Can you add Gregory's Signed-off-by (and authorship from?) to whichever
> patches are appropriate when you resend (as per discussion on the
> earlier version of this patch)?

Yes, happily. But what's the best way to capture this when the code we're 
committing is a mixture of code from Gregory and Stefan (via a manual patch 
into Stefan's ar7 repo, so much of that history is lost to me) that I've then 
extended and refactored? I know what I added (e.g. aux), but for the files with 
joint authorship (fb, dma) should we just put Gregory as the author on those 
patches with a signed-off-by from me? Can I also add a signed-off-by on behalf 
of Gregory, or does he need to do it?

Forgive the newbie questions, but I'm new to this process and want to get it 
right :)

Note that we didn't do this for the previous patch series. Mea culpa. If 
there's a good way to go back and correct history, I'm happy to help.

Thanks,
Andrew

Re: [Qemu-devel] [PATCH] win32: fix socket_error() to work with Mingw64

[Andrew Baumann]

> From: Daniel P. Berrange [mailto:berra...@redhat.com]
> Sent: Monday, 7 March 2016 3:29 AM
> 
> Historically QEMU has had a socket_error() macro that was
> defined to map to WSASocketError(). The os-win32.h header
> file would define errno constants that mapped to the
> WSA error constants. This worked fine with Mingw32 since
> its header files never defined any errno values, nor did
> it even provide an errno.h.  So callers of socket_error()
> could match on traditional E constants and it would
> all "just work".
> 
> With Mingw64 though, things work rather differently. First
> there is an errno.h file which defines all the traditional
> errno constants you'd expect from a UNIX platform. There
> is then a winerror.h which defined the WSA error constants.
> Crucially the WSAE errno values in winerror.h do not
> match the E errno values in error.h.
> 
> If QEMU had only imported winerror.h it would still work,
> but the qemu/osdep.h file unconditionally imports errno.h.
> So callers of socket_error() will get now WSAE values
> back and compare them to the Exxx constants. This will
> always fail silently at runtime.
> 
> To solve this QEMU needs to stop assuming the WSAE
> constant values match the Exxx constant values. Thus the
> socket_error() macro is turned into a small function that
> re-maps WSAE values into Exxx.
> 
> Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
> ---
> 
> NB, I've not actually done anything other that compile
> test this so far. I'll be doing a runtime test once I
> get my windows VM working with QEMU builds agian...

If it helps, this works for me.

Tested-by: Andrew Baumann <andrew.baum...@microsoft.com>

(It doesn't fix the watch/accept problem, obviously, but at least we can listen 
now.)

Thanks,
Andrew

Re: [Qemu-devel] broken socket events on win32 qemu

[Andrew Baumann]

> From: Daniel P. Berrange [mailto:berra...@redhat.com]
> Sent: Monday, 7 March 2016 2:19 AM
> 
> On Mon, Mar 07, 2016 at 07:23:12AM +0000, Andrew Baumann wrote:
> > BTW, The same change introduces another problem on win32: server
> sockets
> > like the GDB example above fail on getpeername() with "Unable to query
> >  remote socket address: Unknown error". This seems to be caused by a
> > definition of ENOTCONN that is not WSAENOTCONN. I'm still trying to
> > figure out why that is, and how to best fix it.
> 
> Can you say how you are building QEMU ? Are you using mingw to do a cross
> compile for Win32, or something else ?

I'm building natively using an unhealthy mixture of mingw-w64 and gcc installed 
from win-builds, 32-bit tools from a native mingw/msys installation, and 
probably some Cygwin stuff that leaked into my PATH as well. (It's a setup that 
evolved, not one I'd advise replicating! :)

> Looking at my local Mingw64 install, the errno definitions look potentially
> problematic· The QEMU  socket_error() method is quite crude - it simply
> expands to WSAGetLastError(), so any code calling it is assuming that the
> WSAEx constants match the E constants. QEMU has a header which
> sets up such a mapping, but it only does so conditionally. eg in
> include/sysemu/os-win32.h
> 
>   #ifndef ENOTCONN
>   # define ENOTCONN WSAENOTCONN
>   #endif
> 
> The current versions of mingw64 I have installed though has a winerror.h
> which defines
> 
>   #define WSABASEERR 1
>   #define WSAENOTCONN (WSABASEERR + 57)
> 
> And a separate  errno.h that defines
> 
>   #ifndef ENOTCONN
>   #define ENOTCONN 126
>   #endif
> 
> This obviously does not match the WSAENOTCONN value
> 
> So my guess would be that QEMU is pulling in the mingw64 errno.h values
> and so QEMU's own  os-win32.h hack is not getting activated.

Yes, I’m seeing the same thing. I will test your patch, but it looks like the 
right thing to do for the socket errors.

Cheers,
Andrew

[Qemu-devel] broken socket events on win32 qemu

[Andrew Baumann]

Hi Daniel,

This commit ("char: convert from GIOChannel to QIOChannel"):
https://github.com/qemu/qemu/commit/9894dc0cdcc397ee5b26370bc53da6d360a363c2
... appears to have broken socket events for character devices on Win32. For 
example, I can no longer connect to a GDB stub (started with: "-gdb 
tcp:127.0.0.1:1234"), since tcp_chr_accept is never called.

Without having looked very closely at the code, I suspect the problem may be 
that we've lost the special-case treatment of socket handles as distinct from 
file descriptors on Win32 (they are different namespaces, and different APIs 
are needed). The previous version of qemu-char.c special-cased sockets in 
io_channel_from_socket():

-#ifdef _WIN32
-chan = g_io_channel_win32_new_socket(fd);
-#else
-chan = g_io_channel_unix_new(fd);
-#endif

... but I don't see anything equivalent in io/channel-socket.c. Am I looking in 
the wrong place?

BTW, The same change introduces another problem on win32: server sockets like 
the GDB example above fail on getpeername() with "Unable to query remote socket 
address: Unknown error". This seems to be caused by a definition of ENOTCONN 
that is not WSAENOTCONN. I'm still trying to figure out why that is, and how to 
best fix it.

Regards,
Andrew

[Qemu-devel] [PATCH v2 5/5] bcm2835_dma: add emulation of Raspberry Pi DMA controller

[Andrew Baumann]

At present, all DMA transfers complete inline (so a looping descriptor
queue will lock up the device). We also do not model pause/abort,
arbitrarion/priority, or debug features.

Signed-off-by: Andrew Baumann <andrew.baum...@microsoft.com>
---

Notes:
v2:
 * avoid ldl_phys/stl_phys
 * compute address of channel structure only after asserting its validity
 * correctly implement per-channel reset and abort bits
 * set channel paused bit when completing DMA

 hw/arm/bcm2835_peripherals.c |  26 +++
 hw/dma/Makefile.objs |   1 +
 hw/dma/bcm2835_dma.c | 408 +++
 include/hw/arm/bcm2835_peripherals.h |   2 +
 include/hw/dma/bcm2835_dma.h |  47 
 5 files changed, 484 insertions(+)
 create mode 100644 hw/dma/bcm2835_dma.c
 create mode 100644 include/hw/dma/bcm2835_dma.h

diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 9b9de99..eeb4934 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -89,6 +89,14 @@ static void bcm2835_peripherals_init(Object *obj)
 object_initialize(>sdhci, sizeof(s->sdhci), TYPE_SYSBUS_SDHCI);
 object_property_add_child(obj, "sdhci", OBJECT(>sdhci), NULL);
 qdev_set_parent_bus(DEVICE(>sdhci), sysbus_get_default());
+
+/* DMA Channels */
+object_initialize(>dma, sizeof(s->dma), TYPE_BCM2835_DMA);
+object_property_add_child(obj, "dma", OBJECT(>dma), NULL);
+qdev_set_parent_bus(DEVICE(>dma), sysbus_get_default());
+
+object_property_add_const_link(OBJECT(>dma), "dma-mr",
+   OBJECT(>gpu_bus_mr), _abort);
 }
 
 static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
@@ -257,6 +265,24 @@ static void bcm2835_peripherals_realize(DeviceState *dev, 
Error **errp)
 return;
 }
 
+/* DMA Channels */
+object_property_set_bool(OBJECT(>dma), true, "realized", );
+if (err) {
+error_propagate(errp, err);
+return;
+}
+
+memory_region_add_subregion(>peri_mr, DMA_OFFSET,
+sysbus_mmio_get_region(SYS_BUS_DEVICE(>dma), 0));
+memory_region_add_subregion(>peri_mr, DMA15_OFFSET,
+sysbus_mmio_get_region(SYS_BUS_DEVICE(>dma), 1));
+
+for (n = 0; n <= 12; n++) {
+sysbus_connect_irq(SYS_BUS_DEVICE(>dma), n,
+   qdev_get_gpio_in_named(DEVICE(>ic),
+  BCM2835_IC_GPU_IRQ,
+  INTERRUPT_DMA0 + n));
+}
 }
 
 static void bcm2835_peripherals_class_init(ObjectClass *oc, void *data)
diff --git a/hw/dma/Makefile.objs b/hw/dma/Makefile.objs
index 0e65ed0..a1abbcf 100644
--- a/hw/dma/Makefile.objs
+++ b/hw/dma/Makefile.objs
@@ -11,3 +11,4 @@ common-obj-$(CONFIG_SUN4M) += sun4m_iommu.o
 
 obj-$(CONFIG_OMAP) += omap_dma.o soc_dma.o
 obj-$(CONFIG_PXA2XX) += pxa2xx_dma.o
+obj-$(CONFIG_RASPI) += bcm2835_dma.o
diff --git a/hw/dma/bcm2835_dma.c b/hw/dma/bcm2835_dma.c
new file mode 100644
index 000..c7ce4e4
--- /dev/null
+++ b/hw/dma/bcm2835_dma.c
@@ -0,0 +1,408 @@
+/*
+ * Raspberry Pi emulation (c) 2012 Gregory Estrade
+ * This code is licensed under the GNU GPLv2 and later.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/dma/bcm2835_dma.h"
+
+/* DMA CS Control and Status bits */
+#define BCM2708_DMA_ACTIVE  (1 << 0)
+#define BCM2708_DMA_END (1 << 1) /* GE */
+#define BCM2708_DMA_INT (1 << 2)
+#define BCM2708_DMA_ISPAUSED(1 << 4)  /* Pause requested or not active */
+#define BCM2708_DMA_ISHELD  (1 << 5)  /* Is held by DREQ flow control */
+#define BCM2708_DMA_ERR (1 << 8)
+#define BCM2708_DMA_ABORT   (1 << 30) /* stop current CB, go to next, WO */
+#define BCM2708_DMA_RESET   (1 << 31) /* WO, self clearing */
+
+/* DMA control block "info" field bits */
+#define BCM2708_DMA_INT_EN  (1 << 0)
+#define BCM2708_DMA_TDMODE  (1 << 1)
+#define BCM2708_DMA_WAIT_RESP   (1 << 3)
+#define BCM2708_DMA_D_INC   (1 << 4)
+#define BCM2708_DMA_D_WIDTH (1 << 5)
+#define BCM2708_DMA_D_DREQ  (1 << 6)
+#define BCM2708_DMA_D_IGNORE(1 << 7)
+#define BCM2708_DMA_S_INC   (1 << 8)
+#define BCM2708_DMA_S_WIDTH (1 << 9)
+#define BCM2708_DMA_S_DREQ  (1 << 10)
+#define BCM2708_DMA_S_IGNORE(1 << 11)
+
+/* Register offsets */
+#define BCM2708_DMA_CS  0x00 /* Control and Status */
+#define BCM2708_DMA_ADDR0x04 /* Control block address */
+/* the current control block appears in the following registers - read only */
+#define BCM2708_DMA_INFO0x08
+#define BCM2708_DMA_SOURCE_AD   0x0c
+#define BCM2708_DMA_DEST_AD 0x10
+#define BCM2708_DMA_TXFR_LEN0x14
+#define B

[Qemu-devel] [PATCH v2 4/5] bcm2835_property: implement framebuffer control/configuration properties

[Andrew Baumann]

The property channel driver now interfaces with the framebuffer device
to query and set framebuffer parameters. As a result of this, the "get
ARM RAM size" query now correctly returns the video RAM base address
(not total RAM size), and the ram-size property is no longer relevant
here.

Signed-off-by: Andrew Baumann <andrew.baum...@microsoft.com>
---

Notes:
v2:
 * avoid ldl/stl_phys
 * move code to increase default pi2 memory size from preceding patch here
   (it was incorrect without the property channel implementation changes)

 hw/arm/bcm2835_peripherals.c   |   8 +--
 hw/arm/raspi.c |   7 +-
 hw/misc/bcm2835_property.c | 139 -
 include/hw/misc/bcm2835_property.h |   5 +-
 4 files changed, 144 insertions(+), 15 deletions(-)

diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 552611a..9b9de99 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -80,6 +80,8 @@ static void bcm2835_peripherals_init(Object *obj)
   "board-rev", _abort);
 qdev_set_parent_bus(DEVICE(>property), sysbus_get_default());
 
+object_property_add_const_link(OBJECT(>property), "fb",
+   OBJECT(>fb), _abort);
 object_property_add_const_link(OBJECT(>property), "dma-mr",
OBJECT(>gpu_bus_mr), _abort);
 
@@ -210,12 +212,6 @@ static void bcm2835_peripherals_realize(DeviceState *dev, 
Error **errp)
qdev_get_gpio_in(DEVICE(>mboxes), MBOX_CHAN_FB));
 
 /* Property channel */
-object_property_set_int(OBJECT(>property), ram_size, "ram-size", );
-if (err) {
-error_propagate(errp, err);
-return;
-}
-
 object_property_set_bool(OBJECT(>property), true, "realized", );
 if (err) {
 error_propagate(errp, err);
diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index 5498209..83fe809 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -164,11 +164,6 @@ static void raspi2_machine_init(MachineClass *mc)
 mc->no_floppy = 1;
 mc->no_cdrom = 1;
 mc->max_cpus = BCM2836_NCPUS;
-
-/* XXX: Temporary restriction in RAM size from the full 1GB. Since
- * we do not yet support the framebuffer / GPU, we need to limit
- * RAM usable by the OS to sit below the peripherals.
- */
-mc->default_ram_size = 0x3F00; /* BCM2836_PERI_BASE */
+mc->default_ram_size = 1024 * 1024 * 1024;
 };
 DEFINE_MACHINE("raspi2", raspi2_machine_init)
diff --git a/hw/misc/bcm2835_property.c b/hw/misc/bcm2835_property.c
index 41fbbe3..15dcc02 100644
--- a/hw/misc/bcm2835_property.c
+++ b/hw/misc/bcm2835_property.c
@@ -17,6 +17,11 @@ static void bcm2835_property_mbox_push(BCM2835PropertyState 
*s, uint32_t value)
 uint32_t tot_len;
 size_t resplen;
 uint32_t tmp;
+int n;
+uint32_t offset, length, color;
+uint32_t xres, yres, xoffset, yoffset, bpp, pixo, alpha;
+uint32_t *newxres = NULL, *newyres = NULL, *newxoffset = NULL,
+*newyoffset = NULL, *newbpp = NULL, *newpixo = NULL, *newalpha = NULL;
 
 value &= ~0xf;
 
@@ -60,7 +65,14 @@ static void bcm2835_property_mbox_push(BCM2835PropertyState 
*s, uint32_t value)
 /* base */
 stl_le_phys(>dma_as, value + 12, 0);
 /* size */
-stl_le_phys(>dma_as, value + 16, s->ram_size);
+stl_le_phys(>dma_as, value + 16, s->fbdev->vcram_base);
+resplen = 8;
+break;
+case 0x00010006: /* Get VC memory */
+/* base */
+stl_le_phys(>dma_as, value + 12, s->fbdev->vcram_base);
+/* size */
+stl_le_phys(>dma_as, value + 16, s->fbdev->vcram_size);
 resplen = 8;
 break;
 case 0x00028001: /* Set power state */
@@ -122,6 +134,114 @@ static void 
bcm2835_property_mbox_push(BCM2835PropertyState *s, uint32_t value)
 resplen = 8;
 break;
 
+/* Frame buffer */
+
+case 0x00040001: /* Allocate buffer */
+stl_le_phys(>dma_as, value + 12, s->fbdev->base);
+stl_le_phys(>dma_as, value + 16, s->fbdev->size);
+resplen = 8;
+break;
+case 0x00048001: /* Release buffer */
+resplen = 0;
+break;
+case 0x00040002: /* Blank screen */
+resplen = 4;
+break;
+case 0x00040003: /* Get display width/height */
+case 0x00040004:
+stl_le_phys(>dma_as, value + 12, s->fbdev->xres);
+stl_le_phys(>dma_as, value + 16, s->fbdev->yres);
+resplen = 8;
+break;
+case 0x00044003: /* Test display width/height */
+case 0x00044004:
+   

[Qemu-devel] [PATCH v2 2/5] bcm2835_aux: add emulation of BCM2835 AUX (aka UART1) block

[Andrew Baumann]

At present only the core UART functions (data path for tx/rx) are
implemented, which is enough for UEFI to boot. The following
features/registers are unimplemented:
  * Line/modem control
  * Scratch register
  * Extra control
  * Baudrate
  * SPI interfaces

Signed-off-by: Andrew Baumann <andrew.baum...@microsoft.com>
---

Notes:
v2:
 * document unimplemented features, log unimplemented register accesses
 * model read path as 8-bit
 * drop incorrect event (break detection) functionality
 * implement AUX_IRQ register
 * model interrupt enables as a uint8 rather than 2 bools
 * corrected bugs in implementation of IIR bits 0-2
 * use chardev prop rather than qemu_char_get_next_serial(); the
   soc-level (bcm2835_peripherals) handling is still a little messy
   however, because uart0 is a pl011 device which still calls
   qemu_char_get_next_serial()

 hw/arm/bcm2835_peripherals.c |  29 
 hw/char/Makefile.objs|   1 +
 hw/char/bcm2835_aux.c| 316 +++
 include/hw/arm/bcm2835_peripherals.h |   2 +
 include/hw/char/bcm2835_aux.h|  33 
 5 files changed, 381 insertions(+)
 create mode 100644 hw/char/bcm2835_aux.c
 create mode 100644 include/hw/char/bcm2835_aux.h

diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 6ce9cd1..375e341 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -12,6 +12,8 @@
 #include "hw/arm/bcm2835_peripherals.h"
 #include "hw/misc/bcm2835_mbox_defs.h"
 #include "hw/arm/raspi_platform.h"
+#include "sysemu/char.h"
+#include "sysemu/sysemu.h" /* for serial_hds */
 
 /* Peripheral base address on the VC (GPU) system bus */
 #define BCM2835_VC_PERI_BASE 0x7e00
@@ -48,6 +50,11 @@ static void bcm2835_peripherals_init(Object *obj)
 object_property_add_child(obj, "uart0", OBJECT(s->uart0), NULL);
 qdev_set_parent_bus(DEVICE(s->uart0), sysbus_get_default());
 
+/* AUX / UART1 */
+object_initialize(>aux, sizeof(s->aux), TYPE_BCM2835_AUX);
+object_property_add_child(obj, "aux", OBJECT(>aux), NULL);
+qdev_set_parent_bus(DEVICE(>aux), sysbus_get_default());
+
 /* Mailboxes */
 object_initialize(>mboxes, sizeof(s->mboxes), TYPE_BCM2835_MBOX);
 object_property_add_child(obj, "mbox", OBJECT(>mboxes), NULL);
@@ -79,6 +86,7 @@ static void bcm2835_peripherals_realize(DeviceState *dev, 
Error **errp)
 MemoryRegion *ram;
 Error *err = NULL;
 uint32_t ram_size;
+CharDriverState *chr;
 int n;
 
 obj = object_property_get_link(OBJECT(dev), "ram", );
@@ -131,6 +139,27 @@ static void bcm2835_peripherals_realize(DeviceState *dev, 
Error **errp)
 qdev_get_gpio_in_named(DEVICE(>ic), BCM2835_IC_GPU_IRQ,
INTERRUPT_UART));
 
+/* AUX / UART1 */
+/* XXX: pl011 (uart0) uses qemu_char_get_next_serial(), so at this point it
+ * should have claimed the first serial device (if one exists) */
+chr = serial_hds[1];
+if (chr == NULL) {
+chr = qemu_chr_new("bcm2835.uart1", "null", NULL);
+}
+qdev_prop_set_chr(DEVICE(>aux), "chardev", chr);
+
+object_property_set_bool(OBJECT(>aux), true, "realized", );
+if (err) {
+error_propagate(errp, err);
+return;
+}
+
+memory_region_add_subregion(>peri_mr, UART1_OFFSET,
+sysbus_mmio_get_region(SYS_BUS_DEVICE(>aux), 0));
+sysbus_connect_irq(SYS_BUS_DEVICE(>aux), 0,
+qdev_get_gpio_in_named(DEVICE(>ic), BCM2835_IC_GPU_IRQ,
+   INTERRUPT_AUX));
+
 /* Mailboxes */
 object_property_set_bool(OBJECT(>mboxes), true, "realized", );
 if (err) {
diff --git a/hw/char/Makefile.objs b/hw/char/Makefile.objs
index 5931cc8..69a553c 100644
--- a/hw/char/Makefile.objs
+++ b/hw/char/Makefile.objs
@@ -16,6 +16,7 @@ obj-$(CONFIG_SH4) += sh_serial.o
 obj-$(CONFIG_PSERIES) += spapr_vty.o
 obj-$(CONFIG_DIGIC) += digic-uart.o
 obj-$(CONFIG_STM32F2XX_USART) += stm32f2xx_usart.o
+obj-$(CONFIG_RASPI) += bcm2835_aux.o
 
 common-obj-$(CONFIG_ETRAXFS) += etraxfs_ser.o
 common-obj-$(CONFIG_ISA_DEBUG) += debugcon.o
diff --git a/hw/char/bcm2835_aux.c b/hw/char/bcm2835_aux.c
new file mode 100644
index 000..0394d11
--- /dev/null
+++ b/hw/char/bcm2835_aux.c
@@ -0,0 +1,316 @@
+/*
+ * BCM2835 (Raspberry Pi / Pi 2) Aux block (mini UART and SPI).
+ * Copyright (c) 2015, Microsoft
+ * Written by Andrew Baumann
+ * Based on pl011.c, copyright terms below:
+ *
+ * Arm PrimeCell PL011 UART
+ *
+ * Copyright (c) 2006 CodeSourcery.
+ * Written by Paul Brook
+ *
+ * This code is licensed under the GPL.
+ *
+ * At present only the core UART functions (data path for tx/rx) are
+ * implemented. The following features/regis

[Qemu-devel] [PATCH v2 0/5] Raspberry Pi framebuffer, DMA and Windows support

[Andrew Baumann]

This patch series adds support for the AUX (second UART), framebuffer
and DMA controller on Raspberry Pi 2, and enables booting Windows on
this device. As with the previous series, it is heavily based on the
original (out of tree) work of Gregory Estrade, Stefan Weil and others
to support Raspberry Pi 1.

After this series, it is possible to boot Windows by following the
instructions at https://github.com/0xabu/qemu/wiki. You also boot
Raspbian to the GUI using a command such as:

  qemu-system-arm -M raspi2 -kernel raspbian-boot/kernel7.img -sd
  2015-09-24-raspbian-jessie.img -append "rw earlyprintk loglevel=8
  console=ttyAMA0 dwc_otg.lpm_enable=0 root=/dev/mmcblk0p2 rootwait"
  -dtb raspbian-boot/bcm2709-rpi-2-b.dtb -serial stdio

I plan to add USB, and remaining timers / system devices in
future patch series, along with support for pi1 (bcm2835). In the
meantime, the complete code is available at https://github.com/0xabu/qemu

v2:
 * added DMA controller
 * revised per PMM's review feedback
 * rebased on top of the patch for fixing ldl_phys/stl_phys in raspi
   devices, presently in target-arm.next

Cheers,
Andrew

Andrew Baumann (5):
  bcm2835_peripherals: enable sdhci pending-insert quirk for raspberry
pi
  bcm2835_aux: add emulation of BCM2835 AUX (aka UART1) block
  bcm2835_fb: add framebuffer device for Raspberry Pi
  bcm2835_property: implement framebuffer control/configuration
properties
  bcm2835_dma: add emulation of Raspberry Pi DMA controller

 hw/arm/bcm2835_peripherals.c | 100 -
 hw/arm/bcm2836.c |   2 +
 hw/arm/raspi.c   |  12 +-
 hw/char/Makefile.objs|   1 +
 hw/char/bcm2835_aux.c| 316 ++
 hw/display/Makefile.objs |   1 +
 hw/display/bcm2835_fb.c  | 424 +++
 hw/dma/Makefile.objs |   1 +
 hw/dma/bcm2835_dma.c | 408 +
 hw/misc/bcm2835_property.c   | 139 +++-
 include/hw/arm/bcm2835_peripherals.h |   6 +
 include/hw/char/bcm2835_aux.h|  33 +++
 include/hw/display/bcm2835_fb.h  |  47 
 include/hw/dma/bcm2835_dma.h |  47 
 include/hw/misc/bcm2835_property.h   |   5 +-
 15 files changed, 1529 insertions(+), 13 deletions(-)
 create mode 100644 hw/char/bcm2835_aux.c
 create mode 100644 hw/display/bcm2835_fb.c
 create mode 100644 hw/dma/bcm2835_dma.c
 create mode 100644 include/hw/char/bcm2835_aux.h
 create mode 100644 include/hw/display/bcm2835_fb.h
 create mode 100644 include/hw/dma/bcm2835_dma.h

-- 
2.5.3

[Qemu-devel] [PATCH v2 3/5] bcm2835_fb: add framebuffer device for Raspberry Pi

[Andrew Baumann]

The framebuffer occupies the upper portion of memory (64MiB by
default), but it can only be controlled/configured via a system
mailbox or property channel (to be added by a subsequent patch).

Signed-off-by: Andrew Baumann <andrew.baum...@microsoft.com>
---

Notes:
v2:
 * avoid ldl_phys
 * move code to increase default pi2 memory size back to the final patch

 hw/arm/bcm2835_peripherals.c |  38 +++-
 hw/arm/bcm2836.c |   2 +
 hw/arm/raspi.c   |   5 +-
 hw/display/Makefile.objs |   1 +
 hw/display/bcm2835_fb.c  | 424 +++
 include/hw/arm/bcm2835_peripherals.h |   2 +
 include/hw/display/bcm2835_fb.h  |  47 
 7 files changed, 517 insertions(+), 2 deletions(-)
 create mode 100644 hw/display/bcm2835_fb.c
 create mode 100644 include/hw/display/bcm2835_fb.h

diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 375e341..552611a 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -63,6 +63,16 @@ static void bcm2835_peripherals_init(Object *obj)
 object_property_add_const_link(OBJECT(>mboxes), "mbox-mr",
OBJECT(>mbox_mr), _abort);
 
+/* Framebuffer */
+object_initialize(>fb, sizeof(s->fb), TYPE_BCM2835_FB);
+object_property_add_child(obj, "fb", OBJECT(>fb), NULL);
+object_property_add_alias(obj, "vcram-size", OBJECT(>fb), "vcram-size",
+  _abort);
+qdev_set_parent_bus(DEVICE(>fb), sysbus_get_default());
+
+object_property_add_const_link(OBJECT(>fb), "dma-mr",
+   OBJECT(>gpu_bus_mr), _abort);
+
 /* Property channel */
 object_initialize(>property, sizeof(s->property), 
TYPE_BCM2835_PROPERTY);
 object_property_add_child(obj, "property", OBJECT(>property), NULL);
@@ -85,7 +95,7 @@ static void bcm2835_peripherals_realize(DeviceState *dev, 
Error **errp)
 Object *obj;
 MemoryRegion *ram;
 Error *err = NULL;
-uint32_t ram_size;
+uint32_t ram_size, vcram_size;
 CharDriverState *chr;
 int n;
 
@@ -173,6 +183,32 @@ static void bcm2835_peripherals_realize(DeviceState *dev, 
Error **errp)
 qdev_get_gpio_in_named(DEVICE(>ic), BCM2835_IC_ARM_IRQ,
INTERRUPT_ARM_MAILBOX));
 
+/* Framebuffer */
+vcram_size = (uint32_t)object_property_get_int(OBJECT(s), "vcram-size",
+   );
+if (err) {
+error_propagate(errp, err);
+return;
+}
+
+object_property_set_int(OBJECT(>fb), ram_size - vcram_size,
+"vcram-base", );
+if (err) {
+error_propagate(errp, err);
+return;
+}
+
+object_property_set_bool(OBJECT(>fb), true, "realized", );
+if (err) {
+error_propagate(errp, err);
+return;
+}
+
+memory_region_add_subregion(>mbox_mr, MBOX_CHAN_FB << 
MBOX_AS_CHAN_SHIFT,
+sysbus_mmio_get_region(SYS_BUS_DEVICE(>fb), 0));
+sysbus_connect_irq(SYS_BUS_DEVICE(>fb), 0,
+   qdev_get_gpio_in(DEVICE(>mboxes), MBOX_CHAN_FB));
+
 /* Property channel */
 object_property_set_int(OBJECT(>property), ram_size, "ram-size", );
 if (err) {
diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
index 0321439..89a6b35 100644
--- a/hw/arm/bcm2836.c
+++ b/hw/arm/bcm2836.c
@@ -42,6 +42,8 @@ static void bcm2836_init(Object *obj)
   _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());
 }
 
diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index 6582279..5498209 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -113,6 +113,7 @@ static void setup_boot(MachineState *machine, int version, 
size_t ram_size)
 static void raspi2_init(MachineState *machine)
 {
 RasPiState *s = g_new0(RasPiState, 1);
+uint32_t vcram_size;
 DriveInfo *di;
 BlockBackend *blk;
 BusState *bus;
@@ -149,7 +150,9 @@ static void raspi2_init(MachineState *machine)
 qdev_prop_set_drive(carddev, "drive", blk, _fatal);
 object_property_set_bool(OBJECT(carddev), true, "realized", _fatal);
 
-setup_boot(machine, 2, machine->ram_size);
+vcram_size = object_property_get_int(OBJECT(>soc), "vcram-size",
+ _abort);
+setup_boot(machine, 2, machine->ram_size - vcram_size);
 }
 
 static void raspi2_mach

[Qemu-devel] [PATCH v2 1/5] bcm2835_peripherals: enable sdhci pending-insert quirk for raspberry pi

[Andrew Baumann]

Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
Signed-off-by: Andrew Baumann <andrew.baum...@microsoft.com>
---
 hw/arm/bcm2835_peripherals.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 6d66fa0..6ce9cd1 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -171,6 +171,13 @@ static void bcm2835_peripherals_realize(DeviceState *dev, 
Error **errp)
 return;
 }
 
+object_property_set_bool(OBJECT(>sdhci), true, "pending-insert-quirk",
+ );
+if (err) {
+error_propagate(errp, err);
+return;
+}
+
 object_property_set_bool(OBJECT(>sdhci), true, "realized", );
 if (err) {
 error_propagate(errp, err);
-- 
2.5.3

Re: [Qemu-devel] [PATCH RFC] bcm2835_dma: add emulation of Raspberry Pi DMA controller

[Andrew Baumann]

> From: Peter Maydell [mailto:peter.mayd...@linaro.org]
> Sent: Thursday, 3 March 2016 8:22 AM
> 
> On 3 March 2016 at 16:16, Gerd Hoffmann  wrote:
> > usb emulation has this problem too.
> >
> > uhci queue heads can go in circles.  The emulation code keeps a linked
> > list of active queue heads, which is (among other bookkeeping things)
> > used to detect when we run in circles.  It's a legal thing to do for a
> > guest btw, so you can see that happening in practice.
> >
> > until recently ehci could be tricked into running in loops too, by
> > creating a circular chain of IDTs.  Which is not legal according to
> > specs, so this went unnoticed for a while.  But a malicious guest can do
> > it nevertheless.  That one was fixed by stopping IDT processing in case
> > no data was transfered.  This is possible because the ehci controller
> > writes back the status to the IDT, so we can figure there is nothing to
> > do (because we already processed that IDT) without additional
> > bookkeeping, by simply checking the status.
> >
> > From a brief look at the patch it seems you can not use the later for
> > the bcm2835 dma controller, I can't spot a place where the some status
> > is written back to the dma contol block ...
> 
> I guess a more general approach to the problem would be to have
> a (hopefully easy) way to say "if this has been going on for too
> long then arrange to defer continued processing of it til later,
> and for now resume the guest". That's too big a can of worms for
> this patch, though. (And for something that's only used in TCG
> emulation we care much less about malicious guests than for devices
> that can be used with KVM.)

It seems like the ultimate solution here is to model the concurrency of DMA. 
Run each DMA engine as its own thread, including modelling the ability to 
pause/abort transfers, and provide some suitable scheduling/resource 
arbitration mechanism, and then you don’t need to worry about whether a guest 
can tie up one of those threads in a loop, because it can't hurt anyone else. 
Of course, this is much more complex to implement.

But in this particular case, I _really_ hope that we don't have to worry about 
virtual raspberry pi's in multi-tenant hosting! :)

Thanks for the feedback,
Andrew

[Qemu-devel] [PATCH] bcm2835_mbox/property: replace ldl_phys/stl_phys with endian-specific accesses

[Andrew Baumann]

PMM pointed out that ldl_phys and stl_phys are dependent on the CPU's
endianness, whereas device model code should be independent of
it. This changes the relevant Raspberry Pi devices to explicitly call
the little-endian variants.

Signed-off-by: Andrew Baumann <andrew.baum...@microsoft.com>
---
bcm2835_property will probably have a trivial conflict with my current
patch series (which to add framebuffer-related properties).

 hw/misc/bcm2835_mbox.c |  6 +++---
 hw/misc/bcm2835_property.c | 38 +++---
 2 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/hw/misc/bcm2835_mbox.c b/hw/misc/bcm2835_mbox.c
index 500baba..106585a 100644
--- a/hw/misc/bcm2835_mbox.c
+++ b/hw/misc/bcm2835_mbox.c
@@ -98,7 +98,7 @@ static void bcm2835_mbox_update(BCM2835MboxState *s)
  */
 for (n = 0; n < MBOX_CHAN_COUNT; n++) {
 while (s->available[n] && !(s->mbox[0].status & ARM_MS_FULL)) {
-value = ldl_phys(>mbox_as, n << MBOX_AS_CHAN_SHIFT);
+value = ldl_le_phys(>mbox_as, n << MBOX_AS_CHAN_SHIFT);
 assert(value != MBOX_INVALID_DATA); /* Pending interrupt but no 
data */
 mbox_push(>mbox[0], value);
 }
@@ -207,12 +207,12 @@ static void bcm2835_mbox_write(void *opaque, hwaddr 
offset,
 ch = value & 0xf;
 if (ch < MBOX_CHAN_COUNT) {
 childaddr = ch << MBOX_AS_CHAN_SHIFT;
-if (ldl_phys(>mbox_as, childaddr + MBOX_AS_PENDING)) {
+if (ldl_le_phys(>mbox_as, childaddr + MBOX_AS_PENDING)) {
 /* Child busy, push delayed. Push it in the arm->vc mbox */
 mbox_push(>mbox[1], value);
 } else {
 /* Push it directly to the child device */
-stl_phys(>mbox_as, childaddr, value);
+stl_le_phys(>mbox_as, childaddr, value);
 }
 } else {
 /* Invalid channel number */
diff --git a/hw/misc/bcm2835_property.c b/hw/misc/bcm2835_property.c
index 581922a..41fbbe3 100644
--- a/hw/misc/bcm2835_property.c
+++ b/hw/misc/bcm2835_property.c
@@ -22,20 +22,20 @@ static void bcm2835_property_mbox_push(BCM2835PropertyState 
*s, uint32_t value)
 
 s->addr = value;
 
-tot_len = ldl_phys(>dma_as, value);
+tot_len = ldl_le_phys(>dma_as, value);
 
 /* @(addr + 4) : Buffer response code */
 value = s->addr + 8;
 while (value + 8 <= s->addr + tot_len) {
-tag = ldl_phys(>dma_as, value);
-bufsize = ldl_phys(>dma_as, value + 4);
+tag = ldl_le_phys(>dma_as, value);
+bufsize = ldl_le_phys(>dma_as, value + 4);
 /* @(value + 8) : Request/response indicator */
 resplen = 0;
 switch (tag) {
 case 0x: /* End tag */
 break;
 case 0x0001: /* Get firmware revision */
-stl_phys(>dma_as, value + 12, 346337);
+stl_le_phys(>dma_as, value + 12, 346337);
 resplen = 4;
 break;
 case 0x00010001: /* Get board model */
@@ -44,7 +44,7 @@ static void bcm2835_property_mbox_push(BCM2835PropertyState 
*s, uint32_t value)
 resplen = 4;
 break;
 case 0x00010002: /* Get board revision */
-stl_phys(>dma_as, value + 12, s->board_rev);
+stl_le_phys(>dma_as, value + 12, s->board_rev);
 resplen = 4;
 break;
 case 0x00010003: /* Get board MAC address */
@@ -58,24 +58,24 @@ static void bcm2835_property_mbox_push(BCM2835PropertyState 
*s, uint32_t value)
 break;
 case 0x00010005: /* Get ARM memory */
 /* base */
-stl_phys(>dma_as, value + 12, 0);
+stl_le_phys(>dma_as, value + 12, 0);
 /* size */
-stl_phys(>dma_as, value + 16, s->ram_size);
+stl_le_phys(>dma_as, value + 16, s->ram_size);
 resplen = 8;
 break;
 case 0x00028001: /* Set power state */
 /* Assume that whatever device they asked for exists,
  * and we'll just claim we set it to the desired state
  */
-tmp = ldl_phys(>dma_as, value + 16);
-stl_phys(>dma_as, value + 16, (tmp & 1));
+tmp = ldl_le_phys(>dma_as, value + 16);
+stl_le_phys(>dma_as, value + 16, (tmp & 1));
 resplen = 8;
 break;
 
 /* Clocks */
 
 case 0x00030001: /* Get clock state */
-stl_phys(>dma_as, value + 16, 0x1);
+stl_le_phys(>dma_as, value + 16, 0x1);
 resplen = 8;
 break;
 
@@ -88,15 +88,15 @@ static void bcm2835_property_mbox_push(BCM2835PropertyState 
*s, uint32_t value)
 case 0x00030002: /

Re: [Qemu-devel] [PATCH 3/4] bcm2835_fb: add framebuffer device for Raspberry Pi

[Andrew Baumann]

> From: Peter Maydell [mailto:peter.mayd...@linaro.org]
> Sent: Tuesday, 1 March 2016 11:23 AM
> 
> On 27 February 2016 at 00:16, Andrew Baumann
> <andrew.baum...@microsoft.com> wrote:
> > The framebuffer occupies the upper portion of memory (64MiB by
> > default), but it can only be controlled/configured via a system
> > mailbox or property channel (to be added by a subsequent patch).
> >
> > Signed-off-by: Andrew Baumann <andrew.baum...@microsoft.com>
> 
> Mostly looks ok, but a couple of points:
> 
> > +static void draw_line_src16(void *opaque, uint8_t *dst, const uint8_t
> *src,
> > +int width, int deststep)
> > +{
> > +BCM2835FBState *s = opaque;
> > +uint16_t rgb565;
> > +uint32_t rgb888;
> > +uint8_t r, g, b;
> > +DisplaySurface *surface = qemu_console_surface(s->con);
> > +int bpp = surface_bits_per_pixel(surface);
> > +
> > +while (width--) {
> > +switch (s->bpp) {
> > +case 8:
> > +rgb888 = ldl_phys(>dma_as, s->vcram_base + (*src << 2));
> 
> Don't use ldl_phys() in device model code, please. It means "do
> a load with the endianness of the CPU's bus", and generally
> device behaviour doesn't depend on the what the CPU happens to
> be doing. If you do a grep for 'ld[a-z]*_phys' in hw/ you'll find
> that (apart from a few spurious matches) the only things doing this
> are some bcm2835 code that you should fix and the spapr hypercall
> device (which really is legitimately cpu-behaviour-dependent).
>
> You need to determine what endianness the device uses to do
> its DMA accesses, and use either ldl_le_phys() or ldl_be_phys()
> as appropriate. (Or address_space_ldl_le/be if you care about
> memory attributes.)

Thanks for pointing this out. I'll get the other instances fixed (they are all 
le).

> More interestingly, why can't you just read from the source
> pointer you're passed in here? The framebuffer_update_display()
> code should have obtained it by looking up the location of the
> host RAM where the guest video RAM is, so if you ignore it and
> do a complete physical-address-to-memory-region lookup for every
> pixel it's going to make redraws unnecessarily slow.

That is what's happening for the 16-, 24- and 32-bit modes. For 8-bit, it 
appears to be doing a palette lookup (using the 8-bit value at the pointer as 
an index into the 256-colour palette starting at vcram_base). Your point that 
this is inefficient stands, but the translation afforded by the existing 
framebuffer code doesn't help. I'm tempted to replace it with ldl_le_phys as a 
quick fix.

Regards,
Andrew

Re: [Qemu-devel] [PATCH v1 00/17] ARM big-endian and setend support

[Andrew Baumann]

> From: Peter Crosthwaite [mailto:crosthwaitepe...@gmail.com]
> Sent: Tuesday, 1 March 2016 10:44 AM
> 
> On Mon, Feb 29, 2016 at 9:27 PM, Stefan Weil  wrote:
> > Am 18.01.2016 um 08:12 schrieb Peter Crosthwaite:
> >> Hi All,
> >>
> >> This patch series adds system-mode big-endian support for ARM. It also
> >> implements the setend instruction, and loading of BE binaries even in
> >> LE emulation mode.
> >>
> >> Based on Paolo's original work. I have moved all the BE32 related work
> >> to the back of the series. Multiple parties are interested in the BE8
> >> work just on its own, so that could potentially be merged w/o BE32.
> >> PMM requested BE32 be at least thought out architecturally, so this
> >> series sees BE32 functionality through.
> >>
> >> I have tested all of LE. BE8 and BE32 in both linux-user mode (for
> >> regressions) and system mode (BE8 and BE32 are new here).
> >> My test application is here, the README gives some example command
> >> lines you can run:
> >>
> >> https://github.com/pcrost/arm-be-test
> >>
> >> Regards,
> >> Peter
> >
> > It would be nice to get at least the emulation for 'setend' into the
> > next version, because it is needed for the Raspberry Pi emulation.
> >
> 
> BTW if you can link me binaries and a failing command line for the
> failing rPI work I can test it, I am currently doing my own standalone
> tests (of linux user as well).

Download and unzip:
https://downloads.raspberrypi.org/raspbian_lite_latest

Then, mount the boot partition, and grab a copy of kernel7.img. You should now 
be able to boot on a current qemu with:

qemu-system-arm -M raspi2 -kernel kernel7.img -sd 
2016-02-26-raspbian-jessie-lite.img -append "rw earlyprintk loglevel=8 
console=ttyAMA0 root=/dev/mmcblk0p2 rootwait" -serial stdio -d unimp

At present, this fails with:

[   10.535232] VFS: Mounted root (ext4 filesystem) on device 179:2.
[   10.729685] devtmpfs: mounted
[   10.805125] Freeing unused kernel memory: 416K (80776000 - 807de000)
[   17.317286] random: systemd urandom read with 6 bits of entropy available
arm: unimplemented setend
[   17.366656] Kernel panic - not syncing: Attempted to kill init! 
exitcode=0x0004
[   17.366656]
[   17.384625] CPU: 0 PID: 1 Comm: systemd Not tainted 4.1.7-v7+ #817
[   17.395562] Hardware name: BCM2709
[   17.404738] [<80018440>] (unwind_backtrace) from [<80013e0c>] 
(show_stack+0x20/0x24)
[   17.419042] [<80013e0c>] (show_stack) from [<80558548>] 
(dump_stack+0x98/0xe0)
[   17.432237] [<80558548>] (dump_stack) from [<8055473c>] (panic+0xa4/0x204)
[   17.444761] [<8055473c>] (panic) from [<8002937c>] (do_exit+0xa0c/0xa64)
[   17.452507] [<8002937c>] (do_exit) from [<80029470>] 
(do_group_exit+0x50/0xcc)
[   17.462827] [<80029470>] (do_group_exit) from [<80033ed4>] 
(get_signal+0x2b0/0x6e0)
[   17.474429] [<80033ed4>] (get_signal) from [<80013194>] 
(do_signal+0x98/0x3ac)
[   17.482176] [<80013194>] (do_signal) from [<80013690>] 
(do_work_pending+0xb8/0xc8)
[   17.490129] [<80013690>] (do_work_pending) from [<8000f9e4>] 
(work_pending+0xc/0x20)
[   17.501020] ---[ end Kernel panic - not syncing: Attempted to kill init! 
exitcode=0x0004
[   17.501020]

Cheers,
Andrew

Re: [Qemu-devel] [PATCH] Use special code for sigsetjmp only in cpu-exec.c

[Andrew Baumann]

> From: Stefan Weil [mailto:s...@weilnetz.de]
> Sent: Tuesday, 1 March 2016 5:16 AM
> 
> Am 01.03.2016 um 13:22 schrieb Peter Maydell:
> > On 1 March 2016 at 11:54, Stefan Weil  wrote:
> >> Am 01.03.2016 um 10:59 schrieb Peter Maydell:
> >>> I don't understand this patch. Why doesn't it work to have
> >>> sigsetjmp() be implemented the same way for every use that
> >>> QEMU makes of it?
> >> It does, as long as the "same way" is the correct one, namely
> >> the one without stack unwinding.
> >>
> >> The current code used to work, but re-arranged include files
> >> broke the working code somewhere in the past:
> >>
> >> include/sysemu/os-win32.h does the right thing at the
> >> wrong place. Its correct definition of sigsetjmp is overwritten by
> >> the definition from a Mingw-w64 system header file which
> >> triggers stack unwinding. Stack unwinding is fatal for
> >> QEMU's generated code.
> >>
> >> My patch makes sure that the critical code in cpu-exec.c
> >> gets the correct definition of sigsetjmp.
> > I think we should fix this by making sure that osdep.h
> > does the right thing -- ie that it gives us the correct
> > definition and prevents mingw's headers from overriding it
> > with the wrong thing (by ensuring that the offending system
> > header is included before we redefine things, or however
> > necessary). This is what osdep.h's purpose is -- to hide
> > annoying system-header workarounds and hacks rather than
> > putting them in the rest of QEMU code.
> >
> >> In addition, it removes code which might or might not
> >> change the default definition of sigsetjmp (depending
> >> on the order of include files). Now all other files beside
> >> cpu-exec.c will use the default behaviour with stack
> >> unwinding.
> > That seems wrong -- we should have the same behaviour for
> > sigsetjmp/siglongjmp everywhere we use it.
> >
> > thanks
> > -- PMM
> 
> Technically there is nothing wrong with using different behaviour
> for each setjmp or sigsetjmp.
> 
> The "best" solution would be to add any prologue / epilogue which
> is needed for stack unwinding to the generated code. Like that,
> no tricks with redefinitions of setjmp / sigsetjmp would be necessary.
> 
> As long as that solution is not available, I'd prefer the variant which
> is implemented by my patch and keep the workaround close to
> the single location where it is needed.
> 
> Your alternate solution would require
> inclusion of setjmp.h in include/sysemu/os-win32.h. Then every
> compilation for Windows would get that header file, resulting
> in a (small) overhead. In addition, there would be no stack
> unwinding for any setjmp/longjmp which is not the standard
> behaviour for Windows 64 bit (but which we had until it was
> broken). I simply don't know whether this has unwanted
> side effects (maybe for debugging or with crash dumps) -
> that's the reason why I'd minimize the non-standard behaviour.

FWIW, I don't see a big problem including setjmp.h from os-win32.h and then 
modifying the definition globally. The overhead you mention is just in 
compilation time, and it's pretty minor compared to all the other system header 
files already included. The lack of stack unwinding is also probably not an 
issue -- AFAIK nothing in qemu uses structured exception handling, and that is 
the only reason I'm aware of for needing to be able to unwind from a longjmp.

I have been getting along fine with the following local fix:

--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -60,6 +60,7 @@
  * If this parameter is NULL, longjump does no stack unwinding.
  * That is what we need for QEMU. Passing the value of register rsp (default)
  * lets longjmp try a stack unwinding which will crash with generated code. */
+# include 
 # undef setjmp
 # define setjmp(env) _setjmp(env, NULL)
 #endif

Cheers,
Andrew

Re: [Qemu-devel] [PATCH] Use special code for sigsetjmp only in cpu-exec.c

[Andrew Baumann]

> From: Stefan Weil [mailto:s...@weilnetz.de]
> Sent: Monday, 29 February 2016 9:08 PM
> 
> The rest of the code can use longjmp with stack unwinding.
> 
> Signed-off-by: Stefan Weil <s...@weilnetz.de>
> ---
> 
> This is a bug fix needed for 64 bit Windows.
> 
> QEMU for Windows currently gets the wrong definition for
> sigsetjmp. It uses stack unwinding for longjmp which results
> in a crash when it is called from generated code.
> 
> Thanks to Andrew Baumann for his reminder that this patch was
> still missing. Andrew, could you please test it with your
> RPi emulation?
> 
> Regards,
> Stefan
> 
>  cpu-exec.c| 9 +
>  include/sysemu/os-win32.h | 8 
>  2 files changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/cpu-exec.c b/cpu-exec.c
> index fd92452..6a725e0 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -33,6 +33,15 @@
>  #endif
>  #include "sysemu/replay.h"
> 
> +#if defined(_WIN64)
> +/* On w64, sigsetjmp is implemented by _setjmp which needs a second
> parameter.
> + * If this parameter is NULL, longjump does no stack unwinding.
> + * That is what we need for QEMU. Passing the value of register rsp
> (default)
> + * lets longjmp try a stack unwinding which will crash with generated code.
> */
> +#undef sigsetjmp
> +#define sigsetjmp(env, savesigs) _setjmp(env, NULL)
> +#endif
> +
>  /* -icount align implementation. */
> 
>  typedef struct SyncClocks {
> diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
> index fbed346..b151e74 100644
> --- a/include/sysemu/os-win32.h
> +++ b/include/sysemu/os-win32.h
> @@ -55,14 +55,6 @@
>  # define EWOULDBLOCK  WSAEWOULDBLOCK
>  #endif
> 
> -#if defined(_WIN64)
> -/* On w64, setjmp is implemented by _setjmp which needs a second
> parameter.
> - * If this parameter is NULL, longjump does no stack unwinding.
> - * That is what we need for QEMU. Passing the value of register rsp (default)
> - * lets longjmp try a stack unwinding which will crash with generated code.
> */
> -# undef setjmp
> -# define setjmp(env) _setjmp(env, NULL)
> -#endif
>  /* QEMU uses sigsetjmp()/siglongjmp() as the portable way to specify
>   * "longjmp and don't touch the signal masks". Since we know that the
>   * savemask parameter will always be zero we can safely define these
> --
> 2.1.4

Thanks Stefan. This works for me with a mingw64 native build.

Tested-by: Andrew Baumann <andrew.baum...@microsoft.com>

Andrew

[Qemu-devel] [PATCH RFC] bcm2835_dma: add emulation of Raspberry Pi DMA controller

[Andrew Baumann]

Signed-off-by: Andrew Baumann <andrew.baum...@microsoft.com>
---
This patch applies on top of the previous series for Windows and
framebuffer support:
  https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg06387.html

After preparing that, I was disappointed to discover that Raspbian
won't boot cleanly without the DMA controller. In the hope of beating
the freeze deadline (it's still February 29 here :-) I'm sending this
for review.

After applying this patch, it is possible to boot Raspbian to the GUI
using a command such as:

  qemu-system-arm -M raspi2 -kernel raspbian-boot/kernel7.img -sd
  2015-09-24-raspbian-jessie.img -append "rw earlyprintk loglevel=8
  console=ttyAMA0 dwc_otg.lpm_enable=0 root=/dev/mmcblk0p2 rootwait"
  -dtb raspbian-boot/bcm2709-rpi-2-b.dtb -serial stdio

As before, this derives from the original (out of tree) work of
Gregory Estrade, Stefan Weil and others to support Raspberry Pi
1. This patch in particulary is Gregory's code, which I have cleaned
up for submission.

Thanks,
Andrew

 hw/arm/bcm2835_peripherals.c |  26 +++
 hw/dma/Makefile.objs |   1 +
 hw/dma/bcm2835_dma.c | 397 +++
 include/hw/arm/bcm2835_peripherals.h |   2 +
 include/hw/dma/bcm2835_dma.h |  47 +
 5 files changed, 473 insertions(+)
 create mode 100644 hw/dma/bcm2835_dma.c
 create mode 100644 include/hw/dma/bcm2835_dma.h

diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index c2b812a..fdd346e 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -87,6 +87,14 @@ static void bcm2835_peripherals_init(Object *obj)
 object_initialize(>sdhci, sizeof(s->sdhci), TYPE_SYSBUS_SDHCI);
 object_property_add_child(obj, "sdhci", OBJECT(>sdhci), NULL);
 qdev_set_parent_bus(DEVICE(>sdhci), sysbus_get_default());
+
+/* DMA Channels */
+object_initialize(>dma, sizeof(s->dma), TYPE_BCM2835_DMA);
+object_property_add_child(obj, "dma", OBJECT(>dma), NULL);
+qdev_set_parent_bus(DEVICE(>dma), sysbus_get_default());
+
+object_property_add_const_link(OBJECT(>dma), "dma-mr",
+   OBJECT(>gpu_bus_mr), _abort);
 }
 
 static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
@@ -246,6 +254,24 @@ static void bcm2835_peripherals_realize(DeviceState *dev, 
Error **errp)
 return;
 }
 
+/* DMA Channels */
+object_property_set_bool(OBJECT(>dma), true, "realized", );
+if (err) {
+error_propagate(errp, err);
+return;
+}
+
+memory_region_add_subregion(>peri_mr, DMA_OFFSET,
+sysbus_mmio_get_region(SYS_BUS_DEVICE(>dma), 0));
+memory_region_add_subregion(>peri_mr, DMA15_OFFSET,
+sysbus_mmio_get_region(SYS_BUS_DEVICE(>dma), 1));
+
+for (n = 0; n <= 12; n++) {
+sysbus_connect_irq(SYS_BUS_DEVICE(>dma), n,
+   qdev_get_gpio_in_named(DEVICE(>ic),
+  BCM2835_IC_GPU_IRQ,
+  INTERRUPT_DMA0 + n));
+}
 }
 
 static void bcm2835_peripherals_class_init(ObjectClass *oc, void *data)
diff --git a/hw/dma/Makefile.objs b/hw/dma/Makefile.objs
index 0e65ed0..a1abbcf 100644
--- a/hw/dma/Makefile.objs
+++ b/hw/dma/Makefile.objs
@@ -11,3 +11,4 @@ common-obj-$(CONFIG_SUN4M) += sun4m_iommu.o
 
 obj-$(CONFIG_OMAP) += omap_dma.o soc_dma.o
 obj-$(CONFIG_PXA2XX) += pxa2xx_dma.o
+obj-$(CONFIG_RASPI) += bcm2835_dma.o
diff --git a/hw/dma/bcm2835_dma.c b/hw/dma/bcm2835_dma.c
new file mode 100644
index 000..1b3cdcf
--- /dev/null
+++ b/hw/dma/bcm2835_dma.c
@@ -0,0 +1,397 @@
+/*
+ * Raspberry Pi emulation (c) 2012 Gregory Estrade
+ * This code is licensed under the GNU GPLv2 and later.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/dma/bcm2835_dma.h"
+
+/* DMA CS Control and Status bits */
+#define BCM2708_DMA_ACTIVE  (1 << 0)
+#define BCM2708_DMA_END (1 << 1) /* GE */
+#define BCM2708_DMA_INT (1 << 2)
+#define BCM2708_DMA_ISPAUSED(1 << 4)  /* Pause requested or not active */
+#define BCM2708_DMA_ISHELD  (1 << 5)  /* Is held by DREQ flow control */
+#define BCM2708_DMA_ERR (1 << 8)
+#define BCM2708_DMA_ABORT   (1 << 30) /* stop current CB, go to next, WO */
+#define BCM2708_DMA_RESET   (1 << 31) /* WO, self clearing */
+
+/* DMA control block "info" field bits */
+#define BCM2708_DMA_INT_EN  (1 << 0)
+#define BCM2708_DMA_TDMODE  (1 << 1)
+#define BCM2708_DMA_WAIT_RESP   (1 << 3)
+#define BCM2708_DMA_D_INC   (1 << 4)
+#define BCM2708_DMA_D_WIDTH (1 << 5)
+#define BCM2708_DMA_D_DREQ  (1 << 6)
+#define BCM2708_DMA_D_IGNORE(1 << 7)
+#define BCM2708_DMA_S_INC   (1 &