[Qemu-devel] [PATCH v4] linux-user: ppc64: don't use volatile register during safe_syscall

[Shivaprasad G Bhat]

r11 is a volatile register on PPC as per calling conventions.
The safe_syscall code uses it to check if the signal_pending
is set during the safe_syscall. When a syscall is interrupted
on return from signal handling, the r11 might be corrupted
before we retry the syscall leading to a crash. The registers
r0-r13 are not to be used here as they have
volatile/designated/reserved usages.

Change the code to use r14 which is non-volatile.
Use SP+16 which is a slot for LR, for save/restore of previous value
of r14. SP+16 can be used, as LR is preserved across the syscall.

Steps to reproduce:
On PPC host, issue `qemu-x86_64 /usr/bin/cc -E -`
Attempt Ctrl-C, the issue is reproduced.

Reference:
https://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi-1.9.html#REG
https://openpowerfoundation.org/wp-content/uploads/2016/03/ABI64BitOpenPOWERv1.1_16July2015_pub4.pdf

Signed-off-by: Shivaprasad G Bhat 
Tested-by: Richard Henderson 
Tested-by: Laurent Vivier 
Reviewed-by: Richard Henderson 
Reviewed-by: Laurent Vivier 
---
v3: https://lists.gnu.org/archive/html/qemu-devel/2018-07/msg05559.html
Changes from v3:
Added cfi_offset directive as suggested and a minor comment/code line swap.

v2: https://lists.gnu.org/archive/html/qemu-devel/2018-07/msg05102.html
Changes from v2:
   Added code to store and restore r14 register. 

v1: https://lists.gnu.org/archive/html/qemu-devel/2018-07/msg05089.html
Changes from v1:
   Fixed the commit message as suggested

 linux-user/host/ppc64/safe-syscall.inc.S |8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/linux-user/host/ppc64/safe-syscall.inc.S 
b/linux-user/host/ppc64/safe-syscall.inc.S
index d30050a67c..8ed73a5b86 100644
--- a/linux-user/host/ppc64/safe-syscall.inc.S
+++ b/linux-user/host/ppc64/safe-syscall.inc.S
@@ -49,7 +49,9 @@ safe_syscall_base:
 *   and returns the result in r3
 * Shuffle everything around appropriately.
 */
-   mr  11, 3   /* signal_pending */
+   std 14, 16(1) /* Preserve r14 in SP+16 */
+   .cfi_offset 14, 16
+   mr  14, 3   /* signal_pending */
mr  0, 4/* syscall number */
mr  3, 5/* syscall arguments */
mr  4, 6
@@ -67,12 +69,13 @@ safe_syscall_base:
 */
 safe_syscall_start:
/* if signal_pending is non-zero, don't do the call */
-   lwz 12, 0(11)
+   lwz 12, 0(14)
cmpwi   0, 12, 0
bne-0f
sc
 safe_syscall_end:
/* code path when we did execute the syscall */
+   ld 14, 16(1) /* restore r14 to its original value */
bnslr+
 
/* syscall failed; return negative errno */
@@ -81,6 +84,7 @@ safe_syscall_end:
 
/* code path when we didn't execute the syscall */
 0: addi3, 0, -TARGET_ERESTARTSYS
+   ld 14, 16(1) /* restore r14 to its orginal value */
blr
.cfi_endproc
 

Re: [Qemu-devel] [PATCH v2 for-3.0] tests/libqtest: Improve kill_qemu()

[Markus Armbruster]

Eric Blake  writes:

> On 07/25/2018 11:17 AM, Markus Armbruster wrote:
>
>>>
>>> the output was produced by bash, which uses waitpid() - and therefore
>>> the fact that bash reports the core dump even when no core file is
>>> created is promising.
>>
>> Proof beats plausibility argument:
>>
>> $ cat wcordump.c
>
>> $ gcc -Wall -g -O wcordump.c
>> $ (ulimit -c unlimited; ./a.out)
>> sig 6 128
>> $ (ulimit -c 0; ./a.out)
>> sig 6 0
>
> Doesn't match my results:
>
> $ (ulimit -c 0; ./a.out)
> sig 6 128
>
> So, what's different between our two environments?
>
> kernel 4.17.7-100.fc27.x86_64

4.17.7-200.fc28.x86_64

> $ echo /proc/sys/kernel/core_*
> /proc/sys/kernel/core_pattern /proc/sys/kernel/core_pipe_limit
> /proc/sys/kernel/core_uses_pid
> $ cat /proc/sys/kernel/core_*
> |/usr/lib/systemd/systemd-coredump %P %u %g %s %t %c %e

This is "core" on my development boxes (I'm a happy caveman).

# cat /etc/sysctl.d/50-coredump.conf 
kernel.core_pattern=core

> 0
> 1
>
>>
>> Looks like WCOREDUMP() does depend on my ulimit -c.
>
> Or worse, that its behavior is kernel/environment-sensitive.

Looks like it.

Re: [Qemu-devel] [PATCH 0/5] mps2: Implement FPGAIO counters and dual-timer

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20180730162458.23186-1-peter.mayd...@linaro.org
Subject: [Qemu-devel] [PATCH 0/5] mps2: Implement FPGAIO counters and dual-timer

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
ddb70544a8 hw/arm/mps2: Wire up dual-timer in mps2-an385 and mps2-an511
bdb50460f4 hw/arm/iotkit: Wire up the dualtimer
008a768e12 hw/timer/cmsdk-apb-dualtimer: Implement CMSDK dual timer module
5a008a3545 hw/misc/mps2-fpgaio: Implement PSCNTR and COUNTER
752c65591e hw/misc/mps2-fpgaio: Implement 1Hz and 100Hz counters

=== OUTPUT BEGIN ===
Checking PATCH 1/5: hw/misc/mps2-fpgaio: Implement 1Hz and 100Hz counters...
ERROR: spaces required around that '*' (ctx:VxV)
#131: FILE: hw/misc/mps2-fpgaio.c:193:
+.subsections = (const VMStateDescription*[]) {
 ^

total: 1 errors, 0 warnings, 123 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 2/5: hw/misc/mps2-fpgaio: Implement PSCNTR and COUNTER...
Checking PATCH 3/5: hw/timer/cmsdk-apb-dualtimer: Implement CMSDK dual timer 
module...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#50: 
new file mode 100644

total: 0 errors, 1 warnings, 617 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 4/5: hw/arm/iotkit: Wire up the dualtimer...
Checking PATCH 5/5: hw/arm/mps2: Wire up dual-timer in mps2-an385 and 
mps2-an511...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupt connections

[Sebastian Bauer]

Am 2018-07-31 01:15, schrieb Peter Maydell:

It would work. But creating an OR gate is half a dozen lines or so of
code, so it's not much more work than changing the PCI controller.
So we should prefer whichever is closest to what the real hardware
does, assuming we can determine that.


The real way is not known. The only source of information is UBoot which 
sets the interrupt line to 32 for all PCI devices. At least AmigaOS 
doesn't overwrite this setting. But UBoot could be wrong of course, as 
the hardware provides only one PCI slot (I think the sm502 is also 
connected to that bus in a direct fashion, but it doesn't have an PCI 
IRQ). Technically, we could also modify UBoot to chose other lines, at 
least AmigaOS would continue to work (if this maps the QEMU wiring) but 
this UBoot would be incompatible to the original hardware.


Bye
Sebastian

Re: [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupt connections

[Sebastian Bauer]

Am 2018-07-31 02:18, schrieb David Gibson:
David, can you please drop this patch, we'll come up with a different 
fix.

Done.  Should have looked at that patch a bit closer.


I created a follow up patch, unfortunately, based on the previous patch, 
as I did not spot your mail earlier than now. Note that the previous 
patch was an improvement over the old behaviour and under normal 
conditions the difference was not visible (as the OS serves both 
interrupts in the same run and at least on AmigaOS the interrupt 
handling is deferred but IRQs are acked quickly; it is still not correct 
though). The old implementation was not correct at all, i.e., all 
interrupts were missed. So it was technical an improvement (and the 
easiest to come up without deep understanding QEMU) ;)


Anyway, I don't know if that follow up patch is the right approach. But 
building the logical or gate seems also be very much code, especially as 
in pci.c the case of multiple irqs sources are seems to be already 
handled (if I understand that code correctly). The follow up patch most 
likely does not model the hardware correctly, but the behaviour should 
be the same. The logical or gate would model the hardware more closely.


Let me know if I should rebase to the state before my initial patch (I 
just looked and the previous patch was still not dropped) if you think 
that the change is fine.


There is also the possibility to make the special (num-irqs == 1) the 
common case, as the Sam460ex platform is the only user of this bus so 
far (and probably stays the only one). I'm not sure if it is worth all 
the hassle. Also note that the entire ppc440_pcix.c source file seems to 
be created for the sam board. I have no idea why that mapping function 
based on slots was chosen in the first place so I kept it. I would also 
be fine to remove that, which would simplify things a lot.


Let me know how to proceed.

Bye
Sebastian

[Qemu-devel] [PATCH 2/2] sam460ex: Create the PCI-X bus with only one interrupt

[Sebastian Bauer]

This is done by unfolding the sysbus_create_varargs() call and by
announcing that only a single irq is needed before connecting the irqs
before the bus is initialized.

This should model the design of the SAM board better, which is that all
PCI interrupts are connected to a single interrupt pin, in particular that
the logical level of the destination pin is an combined or of all the
individual interrupt levels.

Signed-off-by: Sebastian Bauer 
---
 hw/ppc/sam460ex.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index b2b22f280d..28265bcb4c 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -515,10 +515,16 @@ static void sam460ex_init(MachineState *machine)
 
 /* PCI bus */
 ppc460ex_pcie_init(env);
-/* All PCI ints are connected to the same UIC pin (cf. UBoot source) */
-dev = sysbus_create_varargs("ppc440-pcix-host", 0xc0ec0,
-uic[1][0], uic[1][0], uic[1][0], uic[1][0],
-NULL);
+
+/* All PCI ints of the PCI-X bus are connected to the same UIC pin (cf.
+ * UBoot source) so only one connection is needed. */
+dev = qdev_create(NULL, "ppc440-pcix-host");
+qdev_prop_set_uint16(dev, "num-irqs", 1);
+sbdev = SYS_BUS_DEVICE(dev);
+qdev_init_nofail(dev);
+sysbus_mmio_map(sbdev, 0, 0xc0ec0);
+sysbus_connect_irq(sbdev, 0, uic[1][0]);
+
 pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0");
 if (!pci_bus) {
 error_report("couldn't create PCI controller!");
-- 
2.18.0

[Qemu-devel] [PATCH 1/2] ppc: Allow clients of the 440 pcix bus to specify the number of interrupts

[Sebastian Bauer]

This can be done by using the newly introduced num_irqs property. In
particular, this change introduces a special case if num_irqs is 1 in which
case any interrupt pin will be connected to the single irq. The default
case is untouched (but note that the only client is the Sam460ex board for
which the special case was actually created).

Signed-off-by: Sebastian Bauer 
---
 hw/ppc/ppc440_pcix.c | 28 +---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
index d8af04b70f..cb7d7cfd2b 100644
--- a/hw/ppc/ppc440_pcix.c
+++ b/hw/ppc/ppc440_pcix.c
@@ -57,6 +57,7 @@ typedef struct PPC440PCIXState {
 struct PLBOutMap pom[PPC440_PCIX_NR_POMS];
 struct PLBInMap pim[PPC440_PCIX_NR_PIMS];
 uint32_t sts;
+uint16_t num_irqs;
 qemu_irq irq[PCI_NUM_PINS];
 AddressSpace bm_as;
 MemoryRegion bm;
@@ -423,6 +424,12 @@ static int ppc440_pcix_map_irq(PCIDevice *pci_dev, int 
irq_num)
 return slot - 1;
 }
 
+/* All pins from each slot are tied the same and only board IRQ. */
+static int ppc440_pcix_map_irq_single(PCIDevice *pci_dev, int irq_num)
+{
+return 0;
+}
+
 static void ppc440_pcix_set_irq(void *opaque, int irq_num, int level)
 {
 qemu_irq *pci_irqs = opaque;
@@ -469,6 +476,7 @@ const MemoryRegionOps ppc440_pcix_host_data_ops = {
 
 static int ppc440_pcix_initfn(SysBusDevice *dev)
 {
+pci_map_irq_fn map_irq;
 PPC440PCIXState *s;
 PCIHostState *h;
 int i;
@@ -476,14 +484,22 @@ static int ppc440_pcix_initfn(SysBusDevice *dev)
 h = PCI_HOST_BRIDGE(dev);
 s = PPC440_PCIX_HOST_BRIDGE(dev);
 
-for (i = 0; i < ARRAY_SIZE(s->irq); i++) {
+if (s->num_irqs > 4) {
+fprintf(stderr, "%s: Number of irqs must not exceed 4\n", __func__);
+return -1;
+}
+
+for (i = 0; i < s->num_irqs; i++) {
 sysbus_init_irq(dev, >irq[i]);
 }
 
 memory_region_init(>busmem, OBJECT(dev), "pci bus memory", UINT64_MAX);
+
+map_irq = s->num_irqs == 1 ?
+ppc440_pcix_map_irq_single : ppc440_pcix_map_irq;
 h->bus = pci_register_root_bus(DEVICE(dev), NULL, ppc440_pcix_set_irq,
- ppc440_pcix_map_irq, s->irq, >busmem,
- get_system_io(), PCI_DEVFN(0, 0), 4, TYPE_PCI_BUS);
+ map_irq, s->irq, >busmem, get_system_io(),
+ PCI_DEVFN(0, 0), s->num_irqs, TYPE_PCI_BUS);
 
 s->dev = pci_create_simple(h->bus, PCI_DEVFN(0, 0), "ppc4xx-host-bridge");
 
@@ -507,6 +523,11 @@ static int ppc440_pcix_initfn(SysBusDevice *dev)
 return 0;
 }
 
+static Property ppc440_pcix_properties[] = {
+DEFINE_PROP_UINT16("num-irqs", PPC440PCIXState, num_irqs, 4),
+DEFINE_PROP_END_OF_LIST(),
+};
+
 static void ppc440_pcix_class_init(ObjectClass *klass, void *data)
 {
 SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
@@ -514,6 +535,7 @@ static void ppc440_pcix_class_init(ObjectClass *klass, void 
*data)
 
 k->init = ppc440_pcix_initfn;
 dc->reset = ppc440_pcix_reset;
+dc->props = ppc440_pcix_properties;
 }
 
 static const TypeInfo ppc440_pcix_info = {
-- 
2.18.0

[Qemu-devel] [PATCH 0/2] sam460ex: Improve logicial or'ed PCI-X interrupts

[Sebastian Bauer]

The previous change 70a8ff3fd0c27e69a598e7603112de9d0fec5380 fixed
the interrupt connection not properly as the IRQ levels would not
be logcical or'ed. While other PCI cards already have worked with
that change, it didn't model the expected hardware behaviour
close enough.

This patch series is an attempt to fix this remaining problem.
Firtsly, it introduces the num-irqs property to the pci-xbus class
and implements the one common IRQ as a special case. The PCI bus
implementation logic will handle the logical or for us then.

Secondly, it adjust the sam460ex code to take advantage of the
new property.

Tested on the SAM460ex machine (which admittely is the only user
so far).

Sebastian Bauer (2):
  ppc: Allow clients of the 440 pcix bus to specify the number of
interrupts
  sam460ex: Create the PCI-X bus with only one interrupt

 hw/ppc/ppc440_pcix.c | 28 +---
 hw/ppc/sam460ex.c| 14 ++
 2 files changed, 35 insertions(+), 7 deletions(-)

-- 
2.18.0

Re: [Qemu-devel] [PATCH for-3.1] tests/cpu-plug-test: check CPU hotplug on ppc64 with KVM

[David Gibson]

On Mon, Jul 30, 2018 at 10:41:45AM +0200, Greg Kurz wrote:
> On Mon, 30 Jul 2018 15:57:15 +1000
> David Gibson  wrote:
> 
> > On Fri, Jul 27, 2018 at 09:54:52AM +0200, Greg Kurz wrote:
> > > On Fri, 27 Jul 2018 15:27:24 +1000
> > > David Gibson  wrote:
> > >   
> > > > On Wed, Jul 25, 2018 at 04:45:26PM +0200, Greg Kurz wrote:  
> > > > > Commit b585395b655 fixed a regression introduced by some recent 
> > > > > changes
> > > > > in the XICS code, that was causing QEMU to crash instantly during CPU
> > > > > hotplug with KVM. This is typically the kind of bug we'd like our
> > > > > test suite to detect before it gets merged. Unfortunately, the current
> > > > > tests run with '-machine accel=qtest' and don't exercise KVM specific
> > > > > paths in QEMU.
> > > > > 
> > > > > This patch hence changes add_pseries_test_case() to launch QEMU with
> > > > > '-machine accel=kvm' if KVM is available.
> > > > > 
> > > > > A notable consequence is that the guest will execute SLOF, but for 
> > > > > some
> > > > > reasons SLOF sometimes hits a program exception. This causes the guest
> > > > > to loop forever and the test to be stuck.  Since we don't need the 
> > > > > guest
> > > > > to be truely running, let's pass -S to QEMU to avoid that.
> > > > > 
> > > > > Also disable machine capabilities that could be unavailable in KVM, 
> > > > > eg,
> > > > > when using PR KVM.
> > > > > 
> > > > > Signed-off-by: Greg Kurz 
> > > > 
> > > > I'm pretty sure trying to change the accelerator on a qtest test just
> > > > doesn't make sense.  We'd need a different approach for testing cpu
> > > > hotplug against kvm & tcg backends.
> > > >   
> > > 
> > > The test starts QEMU, triggers the CPU hotplug code with a QMP command
> > > and checks the command didn't fail (or QEMU didn't crash, as it would
> > > have before commit b585395b655a)... I really don't understand what
> > > is wrong with that... Please elaborate.  
> > 
> > Well, ok, let me turn that around.  A test that doesn't rely on
> > controlling the guest side behaviour at all probably shouldn't be a
> > qtest based test, since that's what qtest is all about.
> > 
> 
> The CPU hotplug test doesn't seem to do anything on the guest side: it
> just checks that 'device_add' returns a response that isn't an
> error.

Right, which makes this ill suited to being a qtest test.  The whole
point of qtest is making it easier to test qemu peripherals without
having to have specific test code within the guest, by essentially
replacing the guest's cpu with a stub controlled by the test harness.
That's what the qtest accel is all about.

I think it's confusing to have a test which tries things with both
qtest and kvm accelerators.  Looking like a qtest test, people might
reasonably think they can extend/refine the test to check behaviour
when the guest does respond to the hotplug events.  But such an
extension won't work with the kvm accel, because the qtest code used
to simulate that guest response won't have any effect with kvm.

> I'm not aware that the guest is expected to have a specific behavior
> during 'device_add', apart from not crashing or hanging. That was the
> initial idea behind passing '-S' to ensure the guest doesn't run.

Note that with qtest (or -S) we don't even test that minimal
condition.  We only test that *qemu* doesn't crash - it could fatally
compromise the guest and the test would never know.

> Your remark seems to be more general though... are you meaning that
> doing something like qtest_start("-machine accel=kvm:tcg") is just
> wrong ?

Pretty much, yes.  A non-qtest test which does that is reasonable, but
not a qtest test.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH for-3.1] tests/cpu-plug-test: check CPU hotplug on ppc64 with KVM

[David Gibson]

On Mon, Jul 30, 2018 at 11:59:00AM +0200, Greg Kurz wrote:
> On Mon, 30 Jul 2018 10:41:45 +0200
> Greg Kurz  wrote:
> 
> > On Mon, 30 Jul 2018 15:57:15 +1000
> > David Gibson  wrote:
> [...]
> > > > > I'm pretty sure trying to change the accelerator on a qtest test just
> > > > > doesn't make sense.  We'd need a different approach for testing cpu
> > > > > hotplug against kvm & tcg backends.
> > > > > 
> > > > 
> > > > The test starts QEMU, triggers the CPU hotplug code with a QMP command
> > > > and checks the command didn't fail (or QEMU didn't crash, as it would
> > > > have before commit b585395b655a)... I really don't understand what
> > > > is wrong with that... Please elaborate.
> > > 
> > > Well, ok, let me turn that around.  A test that doesn't rely on
> > > controlling the guest side behaviour at all probably shouldn't be a
> > > qtest based test, since that's what qtest is all about.
> > >   
> > 
> > The CPU hotplug test doesn't seem to do anything on the guest side: it
> > just checks that 'device_add' returns a response that isn't an error.
> > I'm not aware that the guest is expected to have a specific behavior
> > during 'device_add', apart from not crashing or hanging. That was the
> > initial idea behind passing '-S' to ensure the guest doesn't run.
> > 
> > Your remark seems to be more general though... are you meaning that
> > doing something like qtest_start("-machine accel=kvm:tcg") is just
> > wrong ?
> 
> The purpose of this test is simply to exercise a path in QEMU that
> is only used with KVM, but it can also be achieved the other way
> around:
> 
> @@ -189,7 +190,7 @@ static void xics_system_init(MachineState *machine, int 
> nr_irqs, Error **errp)
>  sPAPRMachineState *spapr = SPAPR_MACHINE(machine);
>  Error *local_err = NULL;
>  
> -if (kvm_enabled()) {
> +if (kvm_enabled() || qtest_enabled()) {
>  if (machine_kernel_irqchip_allowed(machine) &&
>  !xics_kvm_init(spapr, _err)) {
> 
> This will test the setup of the in-kernel XICS when run on a book3s host,
> and fallback to emulated XICS otherwise (eg, travis).
> 
> Would this be more acceptable ?

No, I don't think that will work.  With this we call into kvm related
code via machine_kernel_irqchip_allowed() and xics_kvm_init() even in
the qtest case.  If they work on a host which doesn't have KVM (say
x86) it will only be by sheer accident.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

[Qemu-devel] [PATCH] qemu-img-cmds.hx: Add example usage for create command

[John Arbuckle]

Add an example on how to use the create command. I believe this will make 
qemu-img easier to use.

Signed-off-by: John Arbuckle 
---
 qemu-img-cmds.hx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 69758fb6e8..92f7437944 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -50,7 +50,7 @@ STEXI
 ETEXI
 
 DEF("create", img_create,
-"create [--object objectdef] [-q] [-f fmt] [-b backing_file] [-F 
backing_fmt] [-u] [-o options] filename [size]")
+"create [--object objectdef] [-q] [-f fmt] [-b backing_file] [-F 
backing_fmt] [-u] [-o options] filename [size]\nExample: qemu-img create -f 
qcow2 WindowsXP.qcow2 10G")
 STEXI
 @item create [--object @var{objectdef}] [-q] [-f @var{fmt}] [-b 
@var{backing_file}] [-F @var{backing_fmt}] [-u] [-o @var{options}] 
@var{filename} [@var{size}]
 ETEXI
-- 
2.14.3 (Apple Git-98)

Re: [Qemu-devel] [PATCH] Add interactive mode to qemu-img command

[no-reply]

Hi,

This series failed docker-quick@centos7 build test. Please find the testing 
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

Type: series
Message-id: 20180730192955.14291-1-programmingk...@gmail.com
Subject: [Qemu-devel] [PATCH] Add interactive mode to qemu-img command

=== TEST SCRIPT BEGIN ===
#!/bin/bash
time make docker-test-quick@centos7 SHOW_ENV=1 J=8
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
cabdf929f6 Add interactive mode to qemu-img command

=== OUTPUT BEGIN ===
  BUILD   centos7
make[1]: Entering directory '/var/tmp/patchew-tester-tmp-tzwnehbx/src'
  GEN 
/var/tmp/patchew-tester-tmp-tzwnehbx/src/docker-src.2018-07-30-22.13.47.19613/qemu.tar
Cloning into 
'/var/tmp/patchew-tester-tmp-tzwnehbx/src/docker-src.2018-07-30-22.13.47.19613/qemu.tar.vroot'...
done.
Checking out files:  46% (2959/6319)   
Checking out files:  47% (2970/6319)   
Checking out files:  48% (3034/6319)   
Checking out files:  49% (3097/6319)   
Checking out files:  50% (3160/6319)   
Checking out files:  51% (3223/6319)   
Checking out files:  52% (3286/6319)   
Checking out files:  53% (3350/6319)   
Checking out files:  54% (3413/6319)   
Checking out files:  55% (3476/6319)   
Checking out files:  56% (3539/6319)   
Checking out files:  57% (3602/6319)   
Checking out files:  58% (3666/6319)   
Checking out files:  59% (3729/6319)   
Checking out files:  60% (3792/6319)   
Checking out files:  61% (3855/6319)   
Checking out files:  62% (3918/6319)   
Checking out files:  63% (3981/6319)   
Checking out files:  64% (4045/6319)   
Checking out files:  65% (4108/6319)   
Checking out files:  66% (4171/6319)   
Checking out files:  67% (4234/6319)   
Checking out files:  68% (4297/6319)   
Checking out files:  69% (4361/6319)   
Checking out files:  70% (4424/6319)   
Checking out files:  71% (4487/6319)   
Checking out files:  72% (4550/6319)   
Checking out files:  73% (4613/6319)   
Checking out files:  74% (4677/6319)   
Checking out files:  75% (4740/6319)   
Checking out files:  76% (4803/6319)   
Checking out files:  77% (4866/6319)   
Checking out files:  78% (4929/6319)   
Checking out files:  79% (4993/6319)   
Checking out files:  80% (5056/6319)   
Checking out files:  81% (5119/6319)   
Checking out files:  82% (5182/6319)   
Checking out files:  83% (5245/6319)   
Checking out files:  84% (5308/6319)   
Checking out files:  85% (5372/6319)   
Checking out files:  86% (5435/6319)   
Checking out files:  87% (5498/6319)   
Checking out files:  88% (5561/6319)   
Checking out files:  89% (5624/6319)   
Checking out files:  90% (5688/6319)   
Checking out files:  91% (5751/6319)   
Checking out files:  92% (5814/6319)   
Checking out files:  93% (5877/6319)   
Checking out files:  94% (5940/6319)   
Checking out files:  95% (6004/6319)   
Checking out files:  96% (6067/6319)   
Checking out files:  97% (6130/6319)   
Checking out files:  98% (6193/6319)   
Checking out files:  99% (6256/6319)   
Checking out files: 100% (6319/6319)   
Checking out files: 100% (6319/6319), done.
Your branch is up-to-date with 'origin/test'.
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 
'/var/tmp/patchew-tester-tmp-tzwnehbx/src/docker-src.2018-07-30-22.13.47.19613/qemu.tar.vroot/dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
Submodule 'ui/keycodemapdb' (git://git.qemu.org/keycodemapdb.git) registered 
for path 'ui/keycodemapdb'
Cloning into 
'/var/tmp/patchew-tester-tmp-tzwnehbx/src/docker-src.2018-07-30-22.13.47.19613/qemu.tar.vroot/ui/keycodemapdb'...
Submodule path 'ui/keycodemapdb': checked out 
'6b3d716e2b6472eb7189d3220552280ef3d832ce'
  COPYRUNNER
RUN test-quick in qemu:centos7 
Packages installed:
SDL-devel-1.2.15-14.el7.x86_64
bison-3.0.4-1.el7.x86_64
bzip2-devel-1.0.6-13.el7.x86_64
ccache-3.3.4-1.el7.x86_64
csnappy-devel-0-6.20150729gitd7bc683.el7.x86_64
flex-2.5.37-3.el7.x86_64
gcc-4.8.5-16.el7_4.2.x86_64
gettext-0.19.8.1-2.el7.x86_64
git-1.8.3.1-12.el7_4.x86_64
glib2-devel-2.50.3-3.el7.x86_64
libepoxy-devel-1.3.1-1.el7.x86_64
libfdt-devel-1.4.6-1.el7.x86_64
lzo-devel-2.06-8.el7.x86_64
make-3.82-23.el7.x86_64
mesa-libEGL-devel-17.0.1-6.20170307.el7.x86_64
mesa-libgbm-devel-17.0.1-6.20170307.el7.x86_64
package g++ is not installed
package librdmacm-devel is not installed
pixman-devel-0.34.0-1.el7.x86_64
spice-glib-devel-0.33-6.el7_4.1.x86_64
spice-server-devel-0.12.8-2.el7.1.x86_64
tar-1.26-32.el7.x86_64
vte-devel-0.28.2-10.el7.x86_64
xen-devel-4.6.6-10.el7.x86_64
zlib-devel-1.2.7-17.el7.x86_64

Environment variables:
PACKAGES=bison bzip2-devel ccache csnappy-devel flex g++
 gcc gettext git glib2-devel libepoxy-devel libfdt-devel
 librdmacm-devel lzo-devel make mesa-libEGL-devel 
mesa-libgbm-devel pixman-devel SDL-devel 

Re: [Qemu-devel] [PULL 1/1] slirp: fix ICMP handling on macOS hosts

[Andrew Oates via Qemu-devel]

Yeah, I suspect (but haven't tested) that this applies to all BSDs.  We
could switch CONFIG_DARWIN to CONFIG_BSD (happy to resend the patch, just
LMK).

Agreed that platform-specific ifdefs are gross, but I don't see a better
way here :/  One option would be to look at the packet length and contents
to try to determine if there's an IP header before the ICMP header, but
that seems pretty iffy.  Creating ICMP sockets often requires special
privileges or configuration (e.g. on Linux) so I don't think it could
easily be done at configure-time to test the host machine's configuration.

~Andrew


On Mon, Jul 30, 2018 at 6:38 AM Peter Maydell 
wrote:

> On 29 July 2018 at 15:35, Samuel Thibault 
> wrote:
> > From: Andrew Oates 
> >
> > On Linux, SOCK_DGRAM+IPPROTO_ICMP sockets give only the ICMP packet when
> > read from.  On macOS, however, the socket acts like a SOCK_RAW socket
> > and includes the IP header as well.
> >
> > This change strips the extra IP header from the received packet on macOS
> > before sending it to the guest.
> >
> > Signed-off-by: Andrew Oates 
> > Signed-off-by: Samuel Thibault 
> > ---
> >  slirp/ip_icmp.c | 16 +++-
> >  1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/slirp/ip_icmp.c b/slirp/ip_icmp.c
> > index 0b667a429a..6316427ed3 100644
> > --- a/slirp/ip_icmp.c
> > +++ b/slirp/ip_icmp.c
> > @@ -420,7 +420,21 @@ void icmp_receive(struct socket *so)
> >  icp = mtod(m, struct icmp *);
> >
> >  id = icp->icmp_id;
> > -len = qemu_recv(so->s, icp, m->m_len, 0);
> > +len = qemu_recv(so->s, icp, M_ROOM(m), 0);
> > +#ifdef CONFIG_DARWIN
> > +if (len >= sizeof(struct ip)) {
> > +/* Skip the IP header that OS X (unlike Linux) includes. */
> > +struct ip *inner_ip = mtod(m, struct ip *);
> > +int inner_hlen = inner_ip->ip_hl << 2;
> > +if (inner_hlen > len) {
> > +len = -1;
> > +errno = -EINVAL;
> > +} else {
> > +len -= inner_hlen;
> > +memmove(icp, (unsigned char *)icp + inner_hlen, len);
> > +}
> > +}
> > +#endif
>
> I think it's generally preferable to avoid per-OS ifdefs -- is
> this really OSX specific and not (for instance) also applicable
> to the other BSDs? Is there some other (configure or runtime) check
> we could do to identify whether this is required?
>
> For instance the FreeBSD manpage for icmp(4)
>
> https://www.freebsd.org/cgi/man.cgi?query=icmp=0=0=FreeBSD+11.2-RELEASE=default=html
>
> says "incoming packets are received with the IP header and
> options intact" and I would be unsurprised to find that
> all the BSDs behave the same way here.
>
> thanks
> -- PMM
>

[Qemu-devel] [PATCH] qemu-img.c: increase spacing between commands in documentation

[Programmingkid]

When the user uses the --help option in qemu-img, the output for the commands 
is very hard to read due to being so close to each other. With this patch the 
help for the commands is double spaced making things easier to read.

Signed-off-by: John Arbuckle 
---
qemu-img.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-img.c b/qemu-img.c
index 9b7506b8ae..6a7e63435e 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -120,7 +120,7 @@ static void QEMU_NORETURN help(void)
   "\n"
   "Command syntax:\n"
#define DEF(option, callback, arg_string)\
-   "  " arg_string "\n"
+   "  " arg_string "\n\n"
#include "qemu-img-cmds.h"
#undef DEF
   "\n"
-- 
2.14.3 (Apple Git-98)

Re: [Qemu-devel] [PATCH v3] linux-user: ppc64: don't use volatile register during safe_syscall

[David Gibson]

On Mon, 30 Jul 2018 21:16:35 +0200
Laurent Vivier  wrote:

> Le 30/07/2018 à 14:44, Richard Henderson a écrit :
>  [...]  
>  [...]  
>  [...]  
>  [...]  
>  [...]  
>  [...]  
>  [...]  
> 
> Tested-by: Laurent Vivier 
> Reviewed-by: Laurent Vivier 
> 
> I think this patch should go into the next -rc because it fixes a lot of
> failures in the LTP on ppc64 host (hundreds of failure...).
> 
> David, if you want, I can take it through the linux-user tree.

That'd be great, thanks.

> 
> Thanks,
> Laurent


-- 
David Gibson 
Principal Software Engineer, Virtualization, Red Hat


pgp1aZqLQdOs6.pgp
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupt connections

[David Gibson]

On Tue, Jul 31, 2018 at 01:31:46AM +0200, BALATON Zoltan wrote:
> On Tue, 31 Jul 2018, Peter Maydell wrote:
> > On 30 July 2018 at 23:37, BALATON Zoltan  wrote:
> > QEMU's implementation of qemu_irq signal lines is that the destination
> > end provides the qemu_irq, which under the hood is a pointer to a
> > struct containing a pointer to the function in the destination device
> > which gets called when the source end says "the line level has changed".
> > This means that there won't be a compile time or runtime error if you
> > pass that qemu_irq to multiple sources (ie device outputs) simultaneously.
> > But the behaviour will be wrong, because the destination will see all
> > the "level is 0", "level is 1" calls from all the sources intermingled, eg
> > 
> > device A output:   |^|__
> > 
> > device B output:   ___|^|___
> > 
> > destination sees:  ||___
> > 
> > (because the destination gets the "level now 0" call when B's output
> > goes to zero). To get the desired behaviour:
> > 
> > logical OR:|^|_
> > 
> > you need an OR gate. (I'm assuming wired-OR because that's the
> > usual thing when several devices share an interrupt.)
> > 
> > If your testing involves a setup which doesn't happen to assert
> > several of the interrupt lines simultaneously you won't notice this
> > problem.
> 
> I think the testing with SATA and USB mouse on a PCI card is likely to
> assert several interrupts simultanelously (eg. when moving the mouse while
> loading stuff from disk) but the OS might have some ways to still recover
> from this so maybe we won't notice it anyway. As this is now confirmed that
> using the same irq multiple times is wrong I think we need a better fix.
> 
> David, can you please drop this patch, we'll come up with a different fix.

Done.  Should have looked at that patch a bit closer.

> > > Probably we can just change the map function in ppc440_pcix.c to always
> > > return the first line then what's specified for other lines should not
> > > matter and the above problem is avoided. We could even get rid of those
> > > additional irqs by changing ppc440_pcix.c to only model a single line but
> > > I'd need someone with better understanding of this to confirm that I got
> > > this right.
> > 
> > I think that would also fix the bug. The logically preferable
> > approach would depend rather on what the actual hardware does:
> > is there a PCI controller chip with 4 interrupt outputs which
> > the board wires together to a single interrupt controller line,
> > or does the PCI controller chip itself have just one output
> > and do the merging of the different PCI interrupts itself?
> 
> Hmm, don't really know. The PCI controller is part of the SoC but we don't
> have the manual of this SoC. The physical board itself also has a single PCI
> slot so however it's wired internally it's only using one irq for that. Not
> sure what other internal PCI devices are there. A comment in U-Boot sources
> says this (it lists PCIB twice but maybe that's meant to be PCID?):
> 
> // IRQ2  = PCI_INT (PCIA, PCIB, PCIC, PCIB)
> 
> So that suggests probably there are 4 PCI irqs that are wired together but
> probably this is inside the SoC. It could also be read as there's only a
> single PCI_INT that delivers all four PCI interrupts. So if we go from that
> either adding an OR gate to sam460ex.c that ORs together the PCI lines and
> connects it to uic[1][0] would work, or changing ppc440_pcix.c to provide a
> single PCI interrupt? We don't really have definitive info other than this
> comment so whatever Sebastian prefers and you approve is fine with me.
> 
> Thank you,
> BALATON Zoltan
> 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupt connections

[BALATON Zoltan]

On Tue, 31 Jul 2018, Peter Maydell wrote:

On 30 July 2018 at 23:37, BALATON Zoltan  wrote:
QEMU's implementation of qemu_irq signal lines is that the destination
end provides the qemu_irq, which under the hood is a pointer to a
struct containing a pointer to the function in the destination device
which gets called when the source end says "the line level has changed".
This means that there won't be a compile time or runtime error if you
pass that qemu_irq to multiple sources (ie device outputs) simultaneously.
But the behaviour will be wrong, because the destination will see all
the "level is 0", "level is 1" calls from all the sources intermingled, eg

device A output:   |^|__

device B output:   ___|^|___

destination sees:  ||___

(because the destination gets the "level now 0" call when B's output
goes to zero). To get the desired behaviour:

logical OR:|^|_

you need an OR gate. (I'm assuming wired-OR because that's the
usual thing when several devices share an interrupt.)

If your testing involves a setup which doesn't happen to assert
several of the interrupt lines simultaneously you won't notice this
problem.


I think the testing with SATA and USB mouse on a PCI card is likely to 
assert several interrupts simultanelously (eg. when moving the mouse while 
loading stuff from disk) but the OS might have some ways to still recover 
from this so maybe we won't notice it anyway. As this is now confirmed 
that using the same irq multiple times is wrong I think we need a better 
fix.


David, can you please drop this patch, we'll come up with a different fix.


Probably we can just change the map function in ppc440_pcix.c to always
return the first line then what's specified for other lines should not
matter and the above problem is avoided. We could even get rid of those
additional irqs by changing ppc440_pcix.c to only model a single line but
I'd need someone with better understanding of this to confirm that I got
this right.


I think that would also fix the bug. The logically preferable
approach would depend rather on what the actual hardware does:
is there a PCI controller chip with 4 interrupt outputs which
the board wires together to a single interrupt controller line,
or does the PCI controller chip itself have just one output
and do the merging of the different PCI interrupts itself?


Hmm, don't really know. The PCI controller is part of the SoC but we don't 
have the manual of this SoC. The physical board itself also has a single 
PCI slot so however it's wired internally it's only using one irq for 
that. Not sure what other internal PCI devices are there. A comment in 
U-Boot sources says this (it lists PCIB twice but maybe that's meant to be 
PCID?):


// IRQ2  = PCI_INT (PCIA, PCIB, PCIC, PCIB)

So that suggests probably there are 4 PCI irqs that are wired together but 
probably this is inside the SoC. It could also be read as there's only a 
single PCI_INT that delivers all four PCI interrupts. So if we go from 
that either adding an OR gate to sam460ex.c that ORs together the PCI 
lines and connects it to uic[1][0] would work, or changing ppc440_pcix.c 
to provide a single PCI interrupt? We don't really have definitive info 
other than this comment so whatever Sebastian prefers and you approve is 
fine with me.


Thank you,
BALATON Zoltan

Re: [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupt connections

[Peter Maydell]

On 31 July 2018 at 00:00, BALATON Zoltan  wrote:
> On Mon, 30 Jul 2018, Peter Maydell wrote:
>>
>> On 30 July 2018 at 12:06, BALATON Zoltan  wrote:
>>> I don't understand QOM. Does this really work? It will ultimately do
>>>
>>> qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 0, uic[1][0]);
>>> qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 1, uic[1][0]);
>>> qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 2, uic[1][0]);
>>> qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 3, uic[1][0]);
>>
>>
>> You are correct; this will not do the intended thing. If you
>> want to wire up multiple outputs which are logically ORed
>> together into a single input, you need to instantiate an
>> OR gate for that (we have the TYPE_OR_IRQ for this).
>
>
> Thanks for confirming it. However after reading hw/pci/pci.c I still don't
> see where these properties are used at all. It looks like it will just call
> the map_irq and set_irq functions specified in pci_register_root_bus passing
> them the irq array given in opaque and these will change irq lines directly.

When ppc4xx_pci_set_irq() calls qemu_set_irq() it passes it a
qemu_irq which is one of the s->irq[] which was initialized
via sysbus_init_irq(). The qdev_connect_gpio_out_named() calls
above are what connect those to the interrupt controller at the
other end of the line. Without them qemu_set_irq() does nothing
(because an initialized-but-never-connected qemu_irq is actually
a NULL pointer, and qemu_set_irq() exits doing nothing for NULL).

On the output end of a device, a qemu_irq connection is a QOM property
which is a link (pointer) property (a qemu_irq is a pointer to an
opaque struct). On the input end of a device, initializing an input
GPIO creates those opaque structs and sets the callback function
which will be invoked for a level change. The various functions for
wiring up gpio connections get a pointer to the input device's struct
and pass it to the output device by setting its QOM property.

> So I think we could just change the map function to map everything to the
> first line and then the other lines don't matter at all. We could even get
> rid of them and change ppc440_pcix to have a single line as used in the
> sam460ex (which is the only board using this model currently so we can add
> more complexity when/if anything else needs it to be different). Is there
> anything why this simple solution would not work that justifies adding more
> complexity now?

It would work. But creating an OR gate is half a dozen lines or so of
code, so it's not much more work than changing the PCI controller.
So we should prefer whichever is closest to what the real hardware
does, assuming we can determine that.

thanks
-- PMM

[Qemu-devel] [PATCH v2 4/4] vfio/ccw/pci: Allow devices to opt-in for ballooning

[Alex Williamson]

If a vfio assigned device makes use of a physical IOMMU, then memory
ballooning is necessarily inhibited due to the page pinning, lack of
page level granularity at the IOMMU, and sufficient notifiers to both
remove the page on balloon inflation and add it back on deflation.
However, not all devices are backed by a physical IOMMU.  In the case
of mediated devices, if a vendor driver is well synchronized with the
guest driver, such that only pages actively used by the guest driver
are pinned by the host mdev vendor driver, then there should be no
overlap between pages available for the balloon driver and pages
actively in use by the device.  Under these conditions, ballooning
should be safe.

vfio-ccw devices are always mediated devices and always operate under
the constraints above.  Therefore we can consider all vfio-ccw devices
as balloon compatible.

The situation is far from straightforward with vfio-pci.  These
devices can be physical devices with physical IOMMU backing or
mediated devices where it is unknown whether a physical IOMMU is in
use or whether the vendor driver is well synchronized to the working
set of the guest driver.  The safest approach is therefore to assume
all vfio-pci devices are incompatible with ballooning, but allow user
opt-in should they have further insight into mediated devices.

Signed-off-by: Alex Williamson 
---
 hw/vfio/ccw.c |9 +
 hw/vfio/common.c  |   23 ++-
 hw/vfio/pci.c |   26 +-
 hw/vfio/trace-events  |1 +
 include/hw/vfio/vfio-common.h |2 ++
 5 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
index 351b305e1ae7..40e7b5623e69 100644
--- a/hw/vfio/ccw.c
+++ b/hw/vfio/ccw.c
@@ -349,6 +349,15 @@ static void vfio_ccw_get_device(VFIOGroup *group, 
VFIOCCWDevice *vcdev,
 }
 }
 
+/*
+ * All vfio-ccw devices are believed to operate compatibly with memory
+ * ballooning, ie. pages pinned in the host are in the current working
+ * set of the guest driver and therefore never overlap with pages
+ * available to the guest balloon driver.  This needs to be set before
+ * vfio_get_device() for vfio common to handle the balloon inhibitor.
+ */
+vcdev->vdev.balloon_allowed = true;
+
 if (vfio_get_device(group, vcdev->cdev.mdevid, >vdev, errp)) {
 goto out_err;
 }
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 4881b691a659..ef5f4b77548a 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -1356,7 +1356,9 @@ void vfio_put_group(VFIOGroup *group)
 return;
 }
 
-qemu_balloon_inhibit(false);
+if (!group->balloon_allowed) {
+qemu_balloon_inhibit(false);
+}
 vfio_kvm_device_del_group(group);
 vfio_disconnect_container(group);
 QLIST_REMOVE(group, next);
@@ -1392,6 +1394,25 @@ int vfio_get_device(VFIOGroup *group, const char *name,
 return ret;
 }
 
+/*
+ * Clear the balloon inhibitor for this group if the driver knows the
+ * device operates compatibly with ballooning.  Setting must be consistent
+ * per group, but since compatibility is really only possible with mdev
+ * currently, we expect singleton groups.
+ */
+if (vbasedev->balloon_allowed != group->balloon_allowed) {
+if (!QLIST_EMPTY(>device_list)) {
+error_setg(errp,
+   "Inconsistent device balloon setting within group");
+return -1;
+}
+
+if (!group->balloon_allowed) {
+group->balloon_allowed = true;
+qemu_balloon_inhibit(false);
+}
+}
+
 vbasedev->fd = fd;
 vbasedev->group = group;
 QLIST_INSERT_HEAD(>device_list, vbasedev, next);
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 6cbb8fa0549d..056f3a887a8f 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2804,12 +2804,13 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
 VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
 VFIODevice *vbasedev_iter;
 VFIOGroup *group;
-char *tmp, group_path[PATH_MAX], *group_name;
+char *tmp, *subsys, group_path[PATH_MAX], *group_name;
 Error *err = NULL;
 ssize_t len;
 struct stat st;
 int groupid;
 int i, ret;
+bool is_mdev;
 
 if (!vdev->vbasedev.sysfsdev) {
 if (!(~vdev->host.domain || ~vdev->host.bus ||
@@ -2869,6 +2870,27 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
 }
 }
 
+/*
+ * Mediated devices *might* operate compatibly with memory ballooning, but
+ * we cannot know for certain, it depends on whether the mdev vendor driver
+ * stays in sync with the active working set of the guest driver.  Prevent
+ * the x-balloon-allowed option unless this is minimally an mdev device.
+ */
+tmp = g_strdup_printf("%s/subsystem", vdev->vbasedev.sysfsdev);
+subsys = realpath(tmp, NULL);
+

[Qemu-devel] [PATCH v2 2/4] kvm: Use inhibit to prevent ballooning without synchronous mmu

[Alex Williamson]

Remove KVM specific tests in balloon_page(), instead marking
ballooning as inhibited without KVM_CAP_SYNC_MMU support.

Signed-off-by: Alex Williamson 
---
 accel/kvm/kvm-all.c|4 
 hw/virtio/virtio-balloon.c |4 +---
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index eb7db92a5e3b..38f468d8e2b1 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -39,6 +39,7 @@
 #include "trace.h"
 #include "hw/irq.h"
 #include "sysemu/sev.h"
+#include "sysemu/balloon.h"
 
 #include "hw/boards.h"
 
@@ -1698,6 +1699,9 @@ static int kvm_init(MachineState *ms)
 s->many_ioeventfds = kvm_check_many_ioeventfds();
 
 s->sync_mmu = !!kvm_vm_check_extension(kvm_state, KVM_CAP_SYNC_MMU);
+if (!s->sync_mmu) {
+qemu_balloon_inhibit(true);
+}
 
 return 0;
 
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 1f7a87f09429..b5425080c5fb 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -21,7 +21,6 @@
 #include "hw/mem/pc-dimm.h"
 #include "sysemu/balloon.h"
 #include "hw/virtio/virtio-balloon.h"
-#include "sysemu/kvm.h"
 #include "exec/address-spaces.h"
 #include "qapi/error.h"
 #include "qapi/qapi-events-misc.h"
@@ -36,8 +35,7 @@
 
 static void balloon_page(void *addr, int deflate)
 {
-if (!qemu_balloon_is_inhibited() && (!kvm_enabled() ||
- kvm_has_sync_mmu())) {
+if (!qemu_balloon_is_inhibited()) {
 qemu_madvise(addr, BALLOON_PAGE_SIZE,
 deflate ? QEMU_MADV_WILLNEED : QEMU_MADV_DONTNEED);
 }

[Qemu-devel] [PATCH v2 3/4] vfio: Inhibit ballooning based on group attachment to a container

[Alex Williamson]

We use a VFIOContainer to associate an AddressSpace to one or more
VFIOGroups.  The VFIOContainer represents the DMA context for that
AdressSpace for those VFIOGroups and is synchronized to changes in
that AddressSpace via a MemoryListener.  For IOMMU backed devices,
maintaining the DMA context for a VFIOGroup generally involves
pinning a host virtual address in order to create a stable host
physical address and then mapping a translation from the associated
guest physical address to that host physical address into the IOMMU.

While the above maintains the VFIOContainer synchronized to the QEMU
memory API of the VM, memory ballooning occurs outside of that API.
Inflating the memory balloon (ie. cooperatively capturing pages from
the guest for use by the host) simply uses MADV_DONTNEED to "zap"
pages from QEMU's host virtual address space.  The page pinning and
IOMMU mapping above remains in place, negating the host's ability to
reuse the page, but the host virtual to host physical mapping of the
page is invalidated outside of QEMU's memory API.

When the balloon is later deflated, attempting to cooperatively
return pages to the guest, the page is simply freed by the guest
balloon driver, allowing it to be used in the guest and incurring a
page fault when that occurs.  The page fault maps a new host physical
page backing the existing host virtual address, meanwhile the
VFIOContainer still maintains the translation to the original host
physical address.  At this point the guest vCPU and any assigned
devices will map different host physical addresses to the same guest
physical address.  Badness.

The IOMMU typically does not have page level granularity with which
it can track this mapping without also incurring inefficiencies in
using page size mappings throughout.  MMU notifiers in the host
kernel also provide indicators for invalidating the mapping on
balloon inflation, not for updating the mapping when the balloon is
deflated.  For these reasons we assume a default behavior that the
mapping of each VFIOGroup into the VFIOContainer is incompatible
with memory ballooning and increment the balloon inhibitor to match
the attached VFIOGroups.

Signed-off-by: Alex Williamson 
---
 hw/vfio/common.c |5 +
 1 file changed, 5 insertions(+)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index fb396cf00ac4..4881b691a659 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -32,6 +32,7 @@
 #include "hw/hw.h"
 #include "qemu/error-report.h"
 #include "qemu/range.h"
+#include "sysemu/balloon.h"
 #include "sysemu/kvm.h"
 #include "trace.h"
 #include "qapi/error.h"
@@ -1049,6 +1050,7 @@ static int vfio_connect_container(VFIOGroup *group, 
AddressSpace *as,
 group->container = container;
 QLIST_INSERT_HEAD(>group_list, group, container_next);
 vfio_kvm_device_add_group(group);
+qemu_balloon_inhibit(true);
 return 0;
 }
 }
@@ -1198,6 +1200,7 @@ static int vfio_connect_container(VFIOGroup *group, 
AddressSpace *as,
 }
 
 vfio_kvm_device_add_group(group);
+qemu_balloon_inhibit(true);
 
 QLIST_INIT(>group_list);
 QLIST_INSERT_HEAD(>containers, container, next);
@@ -1222,6 +1225,7 @@ static int vfio_connect_container(VFIOGroup *group, 
AddressSpace *as,
 listener_release_exit:
 QLIST_REMOVE(group, container_next);
 QLIST_REMOVE(container, next);
+qemu_balloon_inhibit(false);
 vfio_kvm_device_del_group(group);
 vfio_listener_release(container);
 
@@ -1352,6 +1356,7 @@ void vfio_put_group(VFIOGroup *group)
 return;
 }
 
+qemu_balloon_inhibit(false);
 vfio_kvm_device_del_group(group);
 vfio_disconnect_container(group);
 QLIST_REMOVE(group, next);

[Qemu-devel] [PATCH v2 1/4] balloon: Allow nested inhibits

[Alex Williamson]

A simple true/false internal state does not allow multiple users.  Fix
this within the existing interface by converting to a counter, so long
as the counter is elevated, ballooning is inhibited.

Signed-off-by: Alex Williamson 
---
 balloon.c |   13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/balloon.c b/balloon.c
index 6bf0a9681377..931987983858 100644
--- a/balloon.c
+++ b/balloon.c
@@ -26,6 +26,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu-common.h"
+#include "qemu/atomic.h"
 #include "exec/cpu-common.h"
 #include "sysemu/kvm.h"
 #include "sysemu/balloon.h"
@@ -37,16 +38,22 @@
 static QEMUBalloonEvent *balloon_event_fn;
 static QEMUBalloonStatus *balloon_stat_fn;
 static void *balloon_opaque;
-static bool balloon_inhibited;
+static int balloon_inhibit_count;
 
 bool qemu_balloon_is_inhibited(void)
 {
-return balloon_inhibited;
+return atomic_read(_inhibit_count) > 0;
 }
 
 void qemu_balloon_inhibit(bool state)
 {
-balloon_inhibited = state;
+if (state) {
+atomic_inc(_inhibit_count);
+} else {
+atomic_dec(_inhibit_count);
+}
+
+assert(atomic_read(_inhibit_count) >= 0);
 }
 
 static bool have_balloon(Error **errp)

[Qemu-devel] [PATCH v2 0/4] Balloon inhibit enhancements, vfio restriction

[Alex Williamson]

v2:
 - Use atomic ops for balloon inhibit counter (Peter)
 - Allow endpoint driver opt-in for ballooning, vfio-ccw opt-in by
   default, vfio-pci opt-in by device option, only allowed for mdev
   devices, no support added for platform as there are no platform
   mdev devices.

See patch 3/4 for detailed explanation why ballooning and device
assignment typically don't mix.  If this eventually changes, flags
on the iommu info struct or perhaps device info struct can inform
us for automatic opt-in.  Thanks,

Alex

---

Alex Williamson (4):
  balloon: Allow nested inhibits
  kvm: Use inhibit to prevent ballooning without synchronous mmu
  vfio: Inhibit ballooning based on group attachment to a container
  vfio/ccw/pci: Allow devices to opt-in for ballooning


 accel/kvm/kvm-all.c   |4 
 balloon.c |   13 ++---
 hw/vfio/ccw.c |9 +
 hw/vfio/common.c  |   26 ++
 hw/vfio/pci.c |   26 +-
 hw/vfio/trace-events  |1 +
 hw/virtio/virtio-balloon.c|4 +---
 include/hw/vfio/vfio-common.h |2 ++
 8 files changed, 78 insertions(+), 7 deletions(-)

Re: [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupt connections

[BALATON Zoltan]

On Mon, 30 Jul 2018, Peter Maydell wrote:

On 30 July 2018 at 12:06, BALATON Zoltan  wrote:

On Mon, 30 Jul 2018, Sebastian Bauer wrote:


The four interrupts of the PCI bus are connected to the same UIC pin on
the
real Sam460ex. Evidence for this can be found in the UBoot source for the
Sam460ex in the Sam460ex.c file where PCI_INTERRUPT_LINE in written. This
change brings the connection in line with this.

This fixes the problem that can be observed when adding further PCI cards
that get their interrupt rotated to other interrupts than PCI INT A. In
particular, the bug was observed and verified to be fixed (after this
change) with an additional OHCI PCI card.

Signed-off-by: Sebastian Bauer 
---
hw/ppc/sam460ex.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index 0999efcc1e..b2b22f280d 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -515,9 +515,9 @@ static void sam460ex_init(MachineState *machine)

/* PCI bus */
ppc460ex_pcie_init(env);
-/* FIXME: is this correct? */
+/* All PCI ints are connected to the same UIC pin (cf. UBoot source)
*/
dev = sysbus_create_varargs("ppc440-pcix-host", 0xc0ec0,
-uic[1][0], uic[1][20], uic[1][21],
uic[1][22],
+uic[1][0], uic[1][0], uic[1][0],
uic[1][0],



I don't understand QOM. Does this really work? It will ultimately do

qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 0, uic[1][0]);
qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 1, uic[1][0]);
qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 2, uic[1][0]);
qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 3, uic[1][0]);


You are correct; this will not do the intended thing. If you
want to wire up multiple outputs which are logically ORed
together into a single input, you need to instantiate an
OR gate for that (we have the TYPE_OR_IRQ for this).


Thanks for confirming it. However after reading hw/pci/pci.c I still don't 
see where these properties are used at all. It looks like it will just 
call the map_irq and set_irq functions specified in pci_register_root_bus 
passing them the irq array given in opaque and these will change irq lines 
directly. So I think we could just change the map function to map 
everything to the first line and then the other lines don't matter at all. 
We could even get rid of them and change ppc440_pcix to have a single line 
as used in the sam460ex (which is the only board using this model 
currently so we can add more complexity when/if anything else needs it to 
be different). Is there anything why this simple solution would not work 
that justifies adding more complexity now?


Regards,
BALATON Zoltan

Re: [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupt connections

[Peter Maydell]

On 30 July 2018 at 23:37, BALATON Zoltan  wrote:
> I think the number of irq lines could be set, the functions have an nirq or
> num_irq parameters so the 4 lines is only assumed because PCI defines that
> and this is what's modelled but we could use different value here. Sam460ex
> seems to connect all four PCI irq lines to a single irq but I'm not sure
> what's the best way to model this (implementing 1 line in ppc440_pcix model
> or trying to merge these at some higher level). Using wrong irq lines is
> definitely wrong and was only left there because I had no clue about this
> when I've written that so your patch is definitely closer to what the board
> does.
>
>> As I have written in the commit log, I tested this change. I used two
>> cards, the (default) SATA and the OHCI controller and everything was working
>> nicely (contrary to the previous state where only the SATA card worked
>> because this was put into slot 1). Did you have a chance to test it
>> yourself?
>
>
> I could not test it yet, I was trying to understand the code instead to make
> sure it works than just verifying it fixes one particular problem which I
> believe you've done.

QEMU's implementation of qemu_irq signal lines is that the destination
end provides the qemu_irq, which under the hood is a pointer to a
struct containing a pointer to the function in the destination device
which gets called when the source end says "the line level has changed".
This means that there won't be a compile time or runtime error if you
pass that qemu_irq to multiple sources (ie device outputs) simultaneously.
But the behaviour will be wrong, because the destination will see all
the "level is 0", "level is 1" calls from all the sources intermingled, eg

device A output:   |^|__

device B output:   ___|^|___

destination sees:  ||___

(because the destination gets the "level now 0" call when B's output
goes to zero). To get the desired behaviour:

logical OR:|^|_

you need an OR gate. (I'm assuming wired-OR because that's the
usual thing when several devices share an interrupt.)

If your testing involves a setup which doesn't happen to assert
several of the interrupt lines simultaneously you won't notice this
problem.

> Probably we can just change the map function in ppc440_pcix.c to always
> return the first line then what's specified for other lines should not
> matter and the above problem is avoided. We could even get rid of those
> additional irqs by changing ppc440_pcix.c to only model a single line but
> I'd need someone with better understanding of this to confirm that I got
> this right.

I think that would also fix the bug. The logically preferable
approach would depend rather on what the actual hardware does:
is there a PCI controller chip with 4 interrupt outputs which
the board wires together to a single interrupt controller line,
or does the PCI controller chip itself have just one output
and do the merging of the different PCI interrupts itself?

thanks
-- PMM

Re: [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupt connections

[Peter Maydell]

On 30 July 2018 at 12:06, BALATON Zoltan  wrote:
> On Mon, 30 Jul 2018, Sebastian Bauer wrote:
>>
>> The four interrupts of the PCI bus are connected to the same UIC pin on
>> the
>> real Sam460ex. Evidence for this can be found in the UBoot source for the
>> Sam460ex in the Sam460ex.c file where PCI_INTERRUPT_LINE in written. This
>> change brings the connection in line with this.
>>
>> This fixes the problem that can be observed when adding further PCI cards
>> that get their interrupt rotated to other interrupts than PCI INT A. In
>> particular, the bug was observed and verified to be fixed (after this
>> change) with an additional OHCI PCI card.
>>
>> Signed-off-by: Sebastian Bauer 
>> ---
>> hw/ppc/sam460ex.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
>> index 0999efcc1e..b2b22f280d 100644
>> --- a/hw/ppc/sam460ex.c
>> +++ b/hw/ppc/sam460ex.c
>> @@ -515,9 +515,9 @@ static void sam460ex_init(MachineState *machine)
>>
>> /* PCI bus */
>> ppc460ex_pcie_init(env);
>> -/* FIXME: is this correct? */
>> +/* All PCI ints are connected to the same UIC pin (cf. UBoot source)
>> */
>> dev = sysbus_create_varargs("ppc440-pcix-host", 0xc0ec0,
>> -uic[1][0], uic[1][20], uic[1][21],
>> uic[1][22],
>> +uic[1][0], uic[1][0], uic[1][0],
>> uic[1][0],
>
>
> I don't understand QOM. Does this really work? It will ultimately do
>
> qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 0, uic[1][0]);
> qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 1, uic[1][0]);
> qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 2, uic[1][0]);
> qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 3, uic[1][0]);

You are correct; this will not do the intended thing. If you
want to wire up multiple outputs which are logically ORed
together into a single input, you need to instantiate an
OR gate for that (we have the TYPE_OR_IRQ for this).

thanks
-- PMM

Re: [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupt connections

[BALATON Zoltan]

On Mon, 30 Jul 2018, Sebastian Bauer wrote:

Am 2018-07-30 13:06, schrieb BALATON Zoltan:

On Mon, 30 Jul 2018, Sebastian Bauer wrote:
The four interrupts of the PCI bus are connected to the same UIC pin on 
the

real Sam460ex. Evidence for this can be found in the UBoot source for the
Sam460ex in the Sam460ex.c file where PCI_INTERRUPT_LINE in written. This
change brings the connection in line with this.

This fixes the problem that can be observed when adding further PCI cards
that get their interrupt rotated to other interrupts than PCI INT A. In
particular, the bug was observed and verified to be fixed (after this
change) with an additional OHCI PCI card.

Signed-off-by: Sebastian Bauer 
---
hw/ppc/sam460ex.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index 0999efcc1e..b2b22f280d 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -515,9 +515,9 @@ static void sam460ex_init(MachineState *machine)

/* PCI bus */
ppc460ex_pcie_init(env);
-/* FIXME: is this correct? */
+/* All PCI ints are connected to the same UIC pin (cf. UBoot source) 
*/

dev = sysbus_create_varargs("ppc440-pcix-host", 0xc0ec0,
-uic[1][0], uic[1][20], uic[1][21], 
uic[1][22],
+uic[1][0], uic[1][0], uic[1][0], 
uic[1][0],


I don't understand QOM. Does this really work? It will ultimately do

qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 0, uic[1][0]);
qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 1, uic[1][0]);
qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 2, uic[1][0]);
qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 3, uic[1][0]);

which will call for each of these

object_property_set_link(dev, uic[1][0], "some name", );


I don't know QOM myself, but my research was that "some name" is different on 
each invocation. So you associate the same value to the same object (dev) 
under a property different name. What was not obvious to me was who is the 
owner of the value. But according to the doc QOM has the notation of strong 
references, but it is not created like that in qdev_connect_gpio_out_named(). 
So I assume that it is weak reference and the parent is not the owner.


Yes, you are right, different name is generated for each irq line. But all 
this does not seem to matter because at the end irqs are raised/lowered 
directly in the irq array through functions passed when calling 
pci_register_root_bus so I'm not sure what these properties are used for 
if they are used at all so their value probably don't matter.



Would this correctly add all device interrupts to the uic[1][0] irq or
will this replace it so only the last line will be connected instead
of the first after this patch? Could someone with more understanding
about QOM confirm this?


I don't know how this affects QOM, but as I see it the PCI bus has four 
interrupts and so you need to specify all of them (and specifying wrong ones 
seems to be not ideal either). The code assumes this throughout, e.g., see 
ppc440_pcix_initfn().


I think the number of irq lines could be set, the functions have an nirq 
or num_irq parameters so the 4 lines is only assumed because PCI defines 
that and this is what's modelled but we could use different value here. 
Sam460ex seems to connect all four PCI irq lines to a single irq but I'm 
not sure what's the best way to model this (implementing 1 line in 
ppc440_pcix model or trying to merge these at some higher level). Using 
wrong irq lines is definitely wrong and was only left there because I had 
no clue about this when I've written that so your patch is definitely 
closer to what the board does.


As I have written in the commit log, I tested this change. I used two cards, 
the (default) SATA and the OHCI controller and everything was working nicely 
(contrary to the previous state where only the SATA card worked because this 
was put into slot 1). Did you have a chance to test it yourself?


I could not test it yet, I was trying to understand the code instead to 
make sure it works than just verifying it fixes one particular problem 
which I believe you've done.



If this does not work this way would mapping all PCI interrupts to
first line in ppc440_pcix_map_irq at hw/ppc/ppc440_pcix.c:417 and
assign only that to uic[1][0] be a better fix?


I thought about this, but then it needs to passed that we are dealing with a 
SAM board, which seems to be more complicated than this solution. You also 
would need to adjust that ppc440_pcix_initfn() and I'm not sure if it is 
possible to register a root PCI bus with only one interrupt and how the 
remaining code deals with that. Overall, this solution seem to be much 
simpler. Of course, if it doesn't work the way I proposed it must be changed 
again.


The ppc440_pcix model is only used on the sam460ex so I think we can 
change it to model that and care about other cases when/if another 

Re: [Qemu-devel] [PATCH 2/5] hw/misc/mps2-fpgaio: Implement PSCNTR and COUNTER

[Alistair Francis]

On Mon, Jul 30, 2018 at 9:24 AM, Peter Maydell  wrote:
> In the MPS2 FPGAIO, PSCNTR is a free-running downcounter with
> a reload value configured via the PRESCALE register, and
> COUNTER counts up by 1 every time PSCNTR reaches zero.
> Implement these counters.
>
> We can just increment the counters migration subsection's
> version ID because we only added it in the previous commit,
> so no released QEMU versions will be using it.
>
> Signed-off-by: Peter Maydell 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  include/hw/misc/mps2-fpgaio.h |  6 +++
>  hw/misc/mps2-fpgaio.c | 97 +--
>  2 files changed, 99 insertions(+), 4 deletions(-)
>
> diff --git a/include/hw/misc/mps2-fpgaio.h b/include/hw/misc/mps2-fpgaio.h
> index ec057d38c76..69e265cd4b2 100644
> --- a/include/hw/misc/mps2-fpgaio.h
> +++ b/include/hw/misc/mps2-fpgaio.h
> @@ -37,6 +37,12 @@ typedef struct {
>  uint32_t prescale;
>  uint32_t misc;
>
> +/* QEMU_CLOCK_VIRTUAL time at which counter and pscntr were last synced 
> */
> +int64_t pscntr_sync_ticks;
> +/* Values of COUNTER and PSCNTR at time pscntr_sync_ticks */
> +uint32_t counter;
> +uint32_t pscntr;
> +
>  uint32_t prescale_clk;
>
>  /* These hold the CLOCK_VIRTUAL ns tick when the CLK1HZ/CLK100HZ was 
> zero */
> diff --git a/hw/misc/mps2-fpgaio.c b/hw/misc/mps2-fpgaio.c
> index bbc28f641f0..5cf10ebd66a 100644
> --- a/hw/misc/mps2-fpgaio.c
> +++ b/hw/misc/mps2-fpgaio.c
> @@ -43,6 +43,77 @@ static int64_t tickoff_from_counter(int64_t now, uint32_t 
> count, int frq)
>  return now - muldiv64(count, NANOSECONDS_PER_SECOND, frq);
>  }
>
> +static void resync_counter(MPS2FPGAIO *s)
> +{
> +/*
> + * Update s->counter and s->pscntr to their true current values
> + * by calculating how many times PSCNTR has ticked since the
> + * last time we did a resync.
> + */
> +int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> +int64_t elapsed = now - s->pscntr_sync_ticks;
> +
> +/*
> + * Round elapsed down to a whole number of PSCNTR ticks, so we don't
> + * lose time if we do multiple resyncs in a single tick.
> + */
> +uint64_t ticks = muldiv64(elapsed, s->prescale_clk, 
> NANOSECONDS_PER_SECOND);
> +
> +/*
> + * Work out what PSCNTR and COUNTER have moved to. We assume that
> + * PSCNTR reloads from PRESCALE one tick-period after it hits zero,
> + * and that COUNTER increments at the same moment.
> + */
> +if (ticks == 0) {
> +/* We haven't ticked since the last time we were asked */
> +return;
> +} else if (ticks < s->pscntr) {
> +/* We haven't yet reached zero, just reduce the PSCNTR */
> +s->pscntr -= ticks;
> +} else {
> +if (s->prescale == 0) {
> +/*
> + * If the reload value is zero then the PSCNTR will stick
> + * at zero once it reaches it, and so we will increment
> + * COUNTER every tick after that.
> + */
> +s->counter += ticks - s->pscntr;
> +s->pscntr = 0;
> +} else {
> +/*
> + * This is the complicated bit. This ASCII art diagram gives an
> + * example with PRESCALE==5 PSCNTR==7:
> + *
> + * ticks  0  1  2  3  4  5  6  7  8  9 10 11 12 13 14
> + * PSCNTR 7  6  5  4  3  2  1  0  5  4  3  2  1  0  5
> + * cinc   1 2
> + * y0  1  2  3  4  5  6  7  8  9 10 11 12
> + * x0  1  2  3  4  5  0  1  2  3  4  5  0
> + *
> + * where x = y % (s->prescale + 1)
> + * and so PSCNTR = s->prescale - x
> + * and COUNTER is incremented by y / (s->prescale + 1)
> + *
> + * The case where PSCNTR < PRESCALE works out the same,
> + * though we must be careful to calculate y as 64-bit unsigned
> + * for all parts of the expression.
> + * y < 0 is not possible because that implies ticks < s->pscntr.
> + */
> +uint64_t y = ticks - s->pscntr + s->prescale;
> +s->pscntr = s->prescale - (y % (s->prescale + 1));
> +s->counter += y / (s->prescale + 1);
> +}
> +}
> +
> +/*
> + * Only advance the sync time to the timestamp of the last PSCNTR tick,
> + * not all the way to 'now', so we don't lose time if we do multiple
> + * resyncs in a single tick.
> + */
> +s->pscntr_sync_ticks += muldiv64(ticks, NANOSECONDS_PER_SECOND,
> + s->prescale_clk);
> +}
> +
>  static uint64_t mps2_fpgaio_read(void *opaque, hwaddr offset, unsigned size)
>  {
>  MPS2FPGAIO *s = MPS2_FPGAIO(opaque);
> @@ -74,9 +145,12 @@ static uint64_t mps2_fpgaio_read(void *opaque, hwaddr 
> offset, unsigned size)
>  r = counter_from_tickoff(now, 

[Qemu-devel] [PATCH v3 for-3.0] tests/libqtest: Improve kill_qemu()

[Eric Blake]

In kill_qemu() we have an assert that checks that the QEMU process
didn't dump core:
assert(!WCOREDUMP(wstatus));

Unfortunately the WCOREDUMP macro here means the resulting message
is not very easy to comprehend on at least some systems:

ahci-test: tests/libqtest.c:113: kill_qemu: Assertion `!(((__extension__ 
(((union { __typeof(wstatus) __in; int __i; }) { .__in = (wstatus) }).__i))) & 
0x80)' failed.

and it doesn't identify what signal the process took.

Furthermore, we are NOT detecting EINTR (while EINTR shouldn't be
happening if we didn't install signal handlers, it's still better
to always be robust), and also want to log unexpected non-zero status
that was not accompanied by a core dump.

Instead of using a raw assert, print the information in an
easier to understand way:

/i386/ahci/sanity: tests/libqtest.c:119: kill_qemu() detected QEMU death with 
core dump from signal 11 (Segmentation fault)
Aborted (core dumped)

(Of course, the really useful information would be why the QEMU
process dumped core in the first place, but we don't have that
by the time the test program has picked up the exit status.)

Suggested-by: Peter Maydell 
Signed-off-by: Eric Blake 

---
v3: use TFR() instead of open-coding the retry loop [Thomas]
---
 tests/libqtest.c | 37 ++---
 1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index 098af6aec44..3aa3e4c2a46 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -105,12 +105,43 @@ static void kill_qemu(QTestState *s)
 if (s->qemu_pid != -1) {
 int wstatus = 0;
 pid_t pid;
+bool die = false;

 kill(s->qemu_pid, SIGTERM);
-pid = waitpid(s->qemu_pid, , 0);
+TFR(pid = waitpid(s->qemu_pid, , 0));

-if (pid == s->qemu_pid && WIFSIGNALED(wstatus)) {
-assert(!WCOREDUMP(wstatus));
+assert(pid == s->qemu_pid);
+/*
+ * We expect qemu to exit with status 0; anything else is
+ * fishy and should be logged.  Abort except when death by
+ * signal is not accompanied by a coredump (as that's the only
+ * time it was likely that the user is trying to kill the
+ * testsuite early).
+ */
+if (wstatus) {
+die = true;
+if (WIFEXITED(wstatus)) {
+fprintf(stderr, "%s:%d: kill_qemu() tried to terminate QEMU "
+"process but encountered exit status %d\n",
+__FILE__, __LINE__, WEXITSTATUS(wstatus));
+} else if (WIFSIGNALED(wstatus)) {
+int sig = WTERMSIG(wstatus);
+const char *signame = strsignal(sig) ?: "unknown ???";
+
+if (!WCOREDUMP(wstatus)) {
+die = false;
+fprintf(stderr, "%s:%d: kill_qemu() ignoring QEMU death "
+"by signal %d (%s)\n",
+__FILE__, __LINE__, sig, signame);
+} else {
+fprintf(stderr, "%s:%d: kill_qemu() detected QEMU death "
+"with core dump from signal %d (%s)\n",
+__FILE__, __LINE__, sig, signame);
+}
+}
+}
+if (die) {
+abort();
 }
 }
 }
-- 
2.14.4

Re: [Qemu-devel] [PATCH v2 for 3.0 1/2] linux-user/mmap.c: handle invalid len maps correctly

[Richard Henderson]

On 07/30/2018 09:43 AM, Alex Bennée wrote:
> I've slightly re-organised the check to more closely match the
> sequence that the kernel uses in do_mmap(). We check for both the zero
> case (EINVAL) and the overflow length case (ENOMEM).
> 
> Signed-off-by: Alex Bennée 
> Cc: umarcor <1783...@bugs.launchpad.net>
> 
> ---
> v2
>   - add comment on overflow
> ---
>  linux-user/mmap.c | 15 ---
>  1 file changed, 12 insertions(+), 3 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v2 for-3.0] tests/libqtest: Improve kill_qemu()

[Eric Blake]

On 07/24/2018 01:44 AM, Thomas Huth wrote:



Furthermore, we are NOT detecting EINTR (while EINTR shouldn't be
happening if we didn't install signal handlers, it's still better
to always be robust), and also want to log unexpected non-zero status
that was not accompanied by a core dump.




  kill(s->qemu_pid, SIGTERM);
+retry:
  pid = waitpid(s->qemu_pid, , 0);
+if (pid == -1 && errno == EINTR) {
+goto retry;
+}


 do {
 pid = waitpid(s->qemu_pid, , 0);
 } while (pid == -1 && errno == EINTR);

?

Or use the TFR macro from include/qemu-common.h ?


Cool, I didn't know that macro existed! Will send v3.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v3] block/gluster: Handle changed glfs_ftruncate signature

[Jeff Cody]

On Sat, Jul 28, 2018 at 09:50:05AM +0200, Niels de Vos wrote:
> On Sat, Jul 28, 2018 at 12:18:39AM -0400, Jeff Cody wrote:
> > On Fri, Jul 27, 2018 at 08:24:05AM -0500, Eric Blake wrote:
> > > On 07/27/2018 03:19 AM, Niels de Vos wrote:
> > > >From: Prasanna Kumar Kalever 
> > > >
> > > >New versions of Glusters libgfapi.so have an updated glfs_ftruncate()
> > > >function that returns additional 'struct stat' structures to enable
> > > >advanced caching of attributes. This is useful for file servers, not so
> > > >much for QEMU. Nevertheless, the API has changed and needs to be
> > > >adopted.
> > > >
> > > 
> > > Oh, one other comment.
> > > 
> > > >+++ b/block/gluster.c
> > > >@@ -20,6 +20,10 @@
> > > >  #include "qemu/option.h"
> > > >  #include "qemu/cutils.h"
> > > >+#ifdef CONFIG_GLUSTERFS_LEGACY_FTRUNCATE
> > > >+# define glfs_ftruncate(fd, offset, _u1, _u2) glfs_ftruncate(fd, offset)
> > > >+#endif
> > > 
> > > Someday, when we can assume new enough gluster everywhere, we can drop 
> > > this
> > > hunk...
> > > 
> > 
> > Part of me wishes that libgfapi had just created a new function
> > 'glfs_ftruncate2', so that existing users don't need to handle the api
> > change.  But I guess in the grand scheme, not a huge deal either way.
> 
> Gluster uses versioned symbols, so older binaries will keep working with
> new libraries. It is (hopefully) rare that existing symbols get updated.
> We try to send patches for these kind of changes to the projects we know
> well in advance, reducing the number of surprises.
> 
> > > >+++ b/configure
> > > 
> > > >+/* new glfs_ftruncate() passes two additional args */
> > > >+return glfs_ftruncate(NULL, 0 /*, NULL, NULL */);
> > > >+}
> > > >+EOF
> > > >+if compile_prog "$glusterfs_cflags" "$glusterfs_libs" ; then
> > > >+  glusterfs_legacy_ftruncate="yes"
> > > >+fi
> > > 
> > > ...but it will be easier to remember to do so if this comment in configure
> > > calls out the upstream gluster version that no longer requires the legacy
> > > workaround, as our hint for when...
> > > 
> > 
> > I can go ahead and add that to the comment in my branch after applying, if
> > Niels can let me know what that version is/will be (if known).
> 
> The new glfs_ftruncate() will be part of glusterfs-5 (planned for
> October). We're changing the numbering scheme, it was expected to come
> in glusterfs-4.2, but that is a version that never will be released.
> 
> Thanks for correcting the last bits of the patch!
> Niels

So that there is no confusion or miscommunication: I'm not going to pull
this patch in through my tree for 3.0 (or 3.1 yet), since the new API isn't
part of a released version of gluster yet.  (And hopefully we won't ever
need it, if the gluster changes can happen without breaking the existing
API)

-Jeff

Re: [Qemu-devel] [PATCH v2 for-3.0] tests/libqtest: Improve kill_qemu()

[Eric Blake]

On 07/25/2018 11:17 AM, Markus Armbruster wrote:



the output was produced by bash, which uses waitpid() - and therefore
the fact that bash reports the core dump even when no core file is
created is promising.


Proof beats plausibility argument:

$ cat wcordump.c



$ gcc -Wall -g -O wcordump.c
$ (ulimit -c unlimited; ./a.out)
sig 6 128
$ (ulimit -c 0; ./a.out)
sig 6 0


Doesn't match my results:

$ (ulimit -c 0; ./a.out)
sig 6 128

So, what's different between our two environments?

kernel 4.17.7-100.fc27.x86_64

$ echo /proc/sys/kernel/core_*
/proc/sys/kernel/core_pattern /proc/sys/kernel/core_pipe_limit 
/proc/sys/kernel/core_uses_pid

$ cat /proc/sys/kernel/core_*
|/usr/lib/systemd/systemd-coredump %P %u %g %s %t %c %e
0
1



Looks like WCOREDUMP() does depend on my ulimit -c.


Or worse, that its behavior is kernel/environment-sensitive.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 0/5] mps2: Implement FPGAIO counters and dual-timer

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20180730162458.23186-1-peter.mayd...@linaro.org
Subject: [Qemu-devel] [PATCH 0/5] mps2: Implement FPGAIO counters and dual-timer

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
72316845d2 hw/arm/mps2: Wire up dual-timer in mps2-an385 and mps2-an511
1e40b97496 hw/arm/iotkit: Wire up the dualtimer
b4970bc615 hw/timer/cmsdk-apb-dualtimer: Implement CMSDK dual timer module
222de88154 hw/misc/mps2-fpgaio: Implement PSCNTR and COUNTER
201fa0eef0 hw/misc/mps2-fpgaio: Implement 1Hz and 100Hz counters

=== OUTPUT BEGIN ===
Checking PATCH 1/5: hw/misc/mps2-fpgaio: Implement 1Hz and 100Hz counters...
ERROR: spaces required around that '*' (ctx:VxV)
#131: FILE: hw/misc/mps2-fpgaio.c:193:
+.subsections = (const VMStateDescription*[]) {
 ^

total: 1 errors, 0 warnings, 123 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 2/5: hw/misc/mps2-fpgaio: Implement PSCNTR and COUNTER...
Checking PATCH 3/5: hw/timer/cmsdk-apb-dualtimer: Implement CMSDK dual timer 
module...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#50: 
new file mode 100644

total: 0 errors, 1 warnings, 617 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 4/5: hw/arm/iotkit: Wire up the dualtimer...
Checking PATCH 5/5: hw/arm/mps2: Wire up dual-timer in mps2-an385 and 
mps2-an511...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

[Qemu-devel] [PATCH v3 3/3] i.MX6UL: Add Freescale i.MX6 UltraLite 14x14 EVK Board

[Jean-Christophe Dubois]

Tested by booting linux 4.18 (built using imx_v6_v7_defconfig) on the
emulated board.

Signed-off-by: Jean-Christophe Dubois 
---

Changes in V3:
 * None

Changes in V2:
 * use object_initialize_child instead of several funcions

 hw/arm/Makefile.objs  |  2 +-
 hw/arm/mcimx6ul-evk.c | 85 +++
 2 files changed, 86 insertions(+), 1 deletion(-)
 create mode 100644 hw/arm/mcimx6ul-evk.c

diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index e419ad040b..2902f47b4c 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -36,4 +36,4 @@ obj-$(CONFIG_MSF2) += msf2-soc.o msf2-som.o
 obj-$(CONFIG_IOTKIT) += iotkit.o
 obj-$(CONFIG_FSL_IMX7) += fsl-imx7.o mcimx7d-sabre.o
 obj-$(CONFIG_ARM_SMMUV3) += smmu-common.o smmuv3.o
-obj-$(CONFIG_FSL_IMX6UL) += fsl-imx6ul.o
+obj-$(CONFIG_FSL_IMX6UL) += fsl-imx6ul.o mcimx6ul-evk.o
diff --git a/hw/arm/mcimx6ul-evk.c b/hw/arm/mcimx6ul-evk.c
new file mode 100644
index 00..fb2b015bf6
--- /dev/null
+++ b/hw/arm/mcimx6ul-evk.c
@@ -0,0 +1,85 @@
+/*
+ * Copyright (c) 2018 Jean-Christophe Dubois 
+ *
+ * MCIMX6UL_EVK Board System emulation.
+ *
+ * This code is licensed under the GPL, version 2 or later.
+ * See the file `COPYING' in the top level directory.
+ *
+ * It (partially) emulates a mcimx6ul_evk board, with a Freescale
+ * i.MX6ul SoC
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu-common.h"
+#include "hw/arm/fsl-imx6ul.h"
+#include "hw/boards.h"
+#include "sysemu/sysemu.h"
+#include "qemu/error-report.h"
+#include "sysemu/qtest.h"
+
+typedef struct {
+FslIMX6ULState soc;
+MemoryRegion ram;
+} MCIMX6ULEVK;
+
+static void mcimx6ul_evk_init(MachineState *machine)
+{
+static struct arm_boot_info boot_info;
+MCIMX6ULEVK *s = g_new0(MCIMX6ULEVK, 1);
+int i;
+
+if (machine->ram_size > FSL_IMX6UL_MMDC_SIZE) {
+error_report("RAM size " RAM_ADDR_FMT " above max supported (%08x)",
+ machine->ram_size, FSL_IMX6UL_MMDC_SIZE);
+exit(1);
+}
+
+boot_info = (struct arm_boot_info) {
+.loader_start = FSL_IMX6UL_MMDC_ADDR,
+.board_id = -1,
+.ram_size = machine->ram_size,
+.kernel_filename = machine->kernel_filename,
+.kernel_cmdline = machine->kernel_cmdline,
+.initrd_filename = machine->initrd_filename,
+.nb_cpus = smp_cpus,
+};
+
+object_initialize_child(OBJECT(machine), "soc", >soc,  sizeof(s->soc),
+TYPE_FSL_IMX6UL, _fatal, NULL);
+
+object_property_set_bool(OBJECT(>soc), true, "realized", _fatal);
+
+memory_region_allocate_system_memory(>ram, NULL, "mcimx6ul-evk.ram",
+ machine->ram_size);
+memory_region_add_subregion(get_system_memory(),
+FSL_IMX6UL_MMDC_ADDR, >ram);
+
+for (i = 0; i < FSL_IMX6UL_NUM_USDHCS; i++) {
+BusState *bus;
+DeviceState *carddev;
+DriveInfo *di;
+BlockBackend *blk;
+
+di = drive_get_next(IF_SD);
+blk = di ? blk_by_legacy_dinfo(di) : NULL;
+bus = qdev_get_child_bus(DEVICE(>soc.usdhc[i]), "sd-bus");
+carddev = qdev_create(bus, TYPE_SD_CARD);
+qdev_prop_set_drive(carddev, "drive", blk, _fatal);
+object_property_set_bool(OBJECT(carddev), true,
+ "realized", _fatal);
+}
+
+if (!qtest_enabled()) {
+arm_load_kernel(>soc.cpu[0], _info);
+}
+}
+
+static void mcimx6ul_evk_machine_init(MachineClass *mc)
+{
+mc->desc = "Freescale i.MX6UL Evaluation Kit (Cortex A7)";
+mc->init = mcimx6ul_evk_init;
+mc->max_cpus = FSL_IMX6UL_NUM_CPUS;
+}
+DEFINE_MACHINE("mcimx6ul-evk", mcimx6ul_evk_machine_init)
-- 
2.17.1

[Qemu-devel] [PATCH v3 2/3] i.MX6UL: Add i.MX6UL SOC

[Jean-Christophe Dubois]

Signed-off-by: Jean-Christophe Dubois 
---

Changes in V3:
 * Fix coding style issue with indent.

Changes in V2:
 * use object_initialize_child instead of several funcions
 * use sysbus_init_child_obj instead for several functions

 default-configs/arm-softmmu.mak |   1 +
 hw/arm/Makefile.objs|   1 +
 hw/arm/fsl-imx6ul.c | 617 
 include/hw/arm/fsl-imx6ul.h | 339 ++
 4 files changed, 958 insertions(+)
 create mode 100644 hw/arm/fsl-imx6ul.c
 create mode 100644 include/hw/arm/fsl-imx6ul.h

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 834d45cfaf..311584fd74 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -133,6 +133,7 @@ CONFIG_FSL_IMX6=y
 CONFIG_FSL_IMX31=y
 CONFIG_FSL_IMX25=y
 CONFIG_FSL_IMX7=y
+CONFIG_FSL_IMX6UL=y
 
 CONFIG_IMX_I2C=y
 
diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index d51fcecaf2..e419ad040b 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -36,3 +36,4 @@ obj-$(CONFIG_MSF2) += msf2-soc.o msf2-som.o
 obj-$(CONFIG_IOTKIT) += iotkit.o
 obj-$(CONFIG_FSL_IMX7) += fsl-imx7.o mcimx7d-sabre.o
 obj-$(CONFIG_ARM_SMMUV3) += smmu-common.o smmuv3.o
+obj-$(CONFIG_FSL_IMX6UL) += fsl-imx6ul.o
diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c
new file mode 100644
index 00..258f470623
--- /dev/null
+++ b/hw/arm/fsl-imx6ul.c
@@ -0,0 +1,617 @@
+/*
+ * Copyright (c) 2018 Jean-Christophe Dubois 
+ *
+ * i.MX6UL SOC emulation.
+ *
+ * Based on hw/arm/fsl-imx7.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu-common.h"
+#include "hw/arm/fsl-imx6ul.h"
+#include "hw/misc/unimp.h"
+#include "sysemu/sysemu.h"
+#include "qemu/error-report.h"
+
+#define NAME_SIZE 20
+
+static void fsl_imx6ul_init(Object *obj)
+{
+FslIMX6ULState *s = FSL_IMX6UL(obj);
+char name[NAME_SIZE];
+int i;
+
+for (i = 0; i < MIN(smp_cpus, FSL_IMX6UL_NUM_CPUS); i++) {
+snprintf(name, NAME_SIZE, "cpu%d", i);
+object_initialize_child(obj, name, >cpu[i], sizeof(s->cpu[i]),
+"cortex-a7-" TYPE_ARM_CPU, _abort, NULL);
+}
+
+/*
+ * A7MPCORE
+ */
+sysbus_init_child_obj(obj, "a7mpcore", >a7mpcore, sizeof(s->a7mpcore),
+  TYPE_A15MPCORE_PRIV);
+
+/*
+ * CCM
+ */
+sysbus_init_child_obj(obj, "ccm", >ccm, sizeof(s->ccm), 
TYPE_IMX6UL_CCM);
+
+/*
+ * SRC
+ */
+sysbus_init_child_obj(obj, "src", >src, sizeof(s->src), TYPE_IMX6_SRC);
+
+/*
+ * GPCv2
+ */
+sysbus_init_child_obj(obj, "gpcv2", >gpcv2, sizeof(s->gpcv2),
+  TYPE_IMX_GPCV2);
+
+/*
+ * SNVS
+ */
+sysbus_init_child_obj(obj, "snvs", >snvs, sizeof(s->snvs),
+  TYPE_IMX7_SNVS);
+
+/*
+ * GPR
+ */
+sysbus_init_child_obj(obj, "gpr", >gpr, sizeof(s->gpr),
+  TYPE_IMX7_GPR);
+
+/*
+ * GPIOs 1 to 5
+ */
+for (i = 0; i < FSL_IMX6UL_NUM_GPIOS; i++) {
+snprintf(name, NAME_SIZE, "gpio%d", i);
+sysbus_init_child_obj(obj, name, >gpio[i], sizeof(s->gpio[i]),
+  TYPE_IMX_GPIO);
+}
+
+/*
+ * GPT 1, 2
+ */
+for (i = 0; i < FSL_IMX6UL_NUM_GPTS; i++) {
+snprintf(name, NAME_SIZE, "gpt%d", i);
+sysbus_init_child_obj(obj, name, >gpt[i], sizeof(s->gpt[i]),
+  TYPE_IMX7_GPT);
+}
+
+/*
+ * EPIT 1, 2
+ */
+for (i = 0; i < FSL_IMX6UL_NUM_EPITS; i++) {
+snprintf(name, NAME_SIZE, "epit%d", i + 1);
+sysbus_init_child_obj(obj, name, >epit[i], sizeof(s->epit[i]),
+  TYPE_IMX_EPIT);
+}
+
+/*
+ * eCSPI
+ */
+for (i = 0; i < FSL_IMX6UL_NUM_ECSPIS; i++) {
+snprintf(name, NAME_SIZE, "spi%d", i + 1);
+sysbus_init_child_obj(obj, name, >spi[i], sizeof(s->spi[i]),
+  TYPE_IMX_SPI);
+}
+
+/*
+ * I2C
+ */
+for (i = 0; i < FSL_IMX6UL_NUM_I2CS; i++) {
+snprintf(name, NAME_SIZE, "i2c%d", i + 1);
+sysbus_init_child_obj(obj, name, >i2c[i], sizeof(s->i2c[i]),
+  TYPE_IMX_I2C);
+}
+
+/*
+ * UART
+ */
+for (i = 0; i < FSL_IMX6UL_NUM_UARTS; i++) {
+snprintf(name, NAME_SIZE, "uart%d", i);
+sysbus_init_child_obj(obj, name, >uart[i], 

[Qemu-devel] [PATCH v3 0/3] i.MX: Add the i.MX6UL SOC and a reference board.

[Jean-Christophe Dubois]

This series adds the i.MX6UL SOC from NXP/Freescale and the reference
evaluation board.

This series was tested by booting linux 4.18 (built using imx_v6_v7_defconfig)
on the emulated board (with the appropriate device tree).

Jean-Christophe Dubois (3):
  i.MX6UL: Add i.MX6UL specific CCM device
  i.MX6UL: Add i.MX6UL SOC
  i.MX6UL: Add Freescale i.MX6 UltraLite 14x14 EVK Board

 default-configs/arm-softmmu.mak |   1 +
 hw/arm/Makefile.objs|   1 +
 hw/arm/fsl-imx6ul.c | 617 ++
 hw/arm/mcimx6ul-evk.c   |  85 +++
 hw/misc/Makefile.objs   |   1 +
 hw/misc/imx6ul_ccm.c| 890 
 hw/misc/trace-events|   7 +
 include/hw/arm/fsl-imx6ul.h | 339 
 include/hw/misc/imx6ul_ccm.h| 226 
 9 files changed, 2167 insertions(+)
 create mode 100644 hw/arm/fsl-imx6ul.c
 create mode 100644 hw/arm/mcimx6ul-evk.c
 create mode 100644 hw/misc/imx6ul_ccm.c
 create mode 100644 include/hw/arm/fsl-imx6ul.h
 create mode 100644 include/hw/misc/imx6ul_ccm.h

-- 
2.17.1

[Qemu-devel] [PATCH v3 1/3] i.MX6UL: Add i.MX6UL specific CCM device

[Jean-Christophe Dubois]

Signed-off-by: Jean-Christophe Dubois 
---

Changes in V3:
 * None

Changes in V2:
 * move all CCM "debug" to the "trace" framework for i.MX6UL
 * remove unecessary breaks
 * prevent g_assert_not_reached triggered by guest.
 * Add assert to help static analyzer.
 * use FIELD_EX32 from hw/registerfields.h instead of defining my own.

 hw/misc/Makefile.objs|   1 +
 hw/misc/imx6ul_ccm.c | 890 +++
 hw/misc/trace-events |   7 +
 include/hw/misc/imx6ul_ccm.h | 226 +
 4 files changed, 1124 insertions(+)
 create mode 100644 hw/misc/imx6ul_ccm.c
 create mode 100644 include/hw/misc/imx6ul_ccm.h

diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 9350900845..51d27b3af1 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -36,6 +36,7 @@ obj-$(CONFIG_IMX) += imx_ccm.o
 obj-$(CONFIG_IMX) += imx31_ccm.o
 obj-$(CONFIG_IMX) += imx25_ccm.o
 obj-$(CONFIG_IMX) += imx6_ccm.o
+obj-$(CONFIG_IMX) += imx6ul_ccm.o
 obj-$(CONFIG_IMX) += imx6_src.o
 obj-$(CONFIG_IMX) += imx7_ccm.o
 obj-$(CONFIG_IMX) += imx2_wdt.o
diff --git a/hw/misc/imx6ul_ccm.c b/hw/misc/imx6ul_ccm.c
new file mode 100644
index 00..32197b2435
--- /dev/null
+++ b/hw/misc/imx6ul_ccm.c
@@ -0,0 +1,890 @@
+/*
+ * IMX6UL Clock Control Module
+ *
+ * Copyright (c) 2018 Jean-Christophe Dubois 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ * To get the timer frequencies right, we need to emulate at least part of
+ * the CCM.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/registerfields.h"
+#include "hw/misc/imx6ul_ccm.h"
+#include "qemu/log.h"
+
+#include "trace.h"
+
+static const char *imx6ul_ccm_reg_name(uint32_t reg)
+{
+static char unknown[20];
+
+switch (reg) {
+case CCM_CCR:
+return "CCR";
+case CCM_CCDR:
+return "CCDR";
+case CCM_CSR:
+return "CSR";
+case CCM_CCSR:
+return "CCSR";
+case CCM_CACRR:
+return "CACRR";
+case CCM_CBCDR:
+return "CBCDR";
+case CCM_CBCMR:
+return "CBCMR";
+case CCM_CSCMR1:
+return "CSCMR1";
+case CCM_CSCMR2:
+return "CSCMR2";
+case CCM_CSCDR1:
+return "CSCDR1";
+case CCM_CS1CDR:
+return "CS1CDR";
+case CCM_CS2CDR:
+return "CS2CDR";
+case CCM_CDCDR:
+return "CDCDR";
+case CCM_CHSCCDR:
+return "CHSCCDR";
+case CCM_CSCDR2:
+return "CSCDR2";
+case CCM_CSCDR3:
+return "CSCDR3";
+case CCM_CDHIPR:
+return "CDHIPR";
+case CCM_CTOR:
+return "CTOR";
+case CCM_CLPCR:
+return "CLPCR";
+case CCM_CISR:
+return "CISR";
+case CCM_CIMR:
+return "CIMR";
+case CCM_CCOSR:
+return "CCOSR";
+case CCM_CGPR:
+return "CGPR";
+case CCM_CCGR0:
+return "CCGR0";
+case CCM_CCGR1:
+return "CCGR1";
+case CCM_CCGR2:
+return "CCGR2";
+case CCM_CCGR3:
+return "CCGR3";
+case CCM_CCGR4:
+return "CCGR4";
+case CCM_CCGR5:
+return "CCGR5";
+case CCM_CCGR6:
+return "CCGR6";
+case CCM_CMEOR:
+return "CMEOR";
+default:
+sprintf(unknown, "%d ?", reg);
+return unknown;
+}
+}
+
+static const char *imx6ul_analog_reg_name(uint32_t reg)
+{
+static char unknown[20];
+
+switch (reg) {
+case CCM_ANALOG_PLL_ARM:
+return "PLL_ARM";
+case CCM_ANALOG_PLL_ARM_SET:
+return "PLL_ARM_SET";
+case CCM_ANALOG_PLL_ARM_CLR:
+return "PLL_ARM_CLR";
+case CCM_ANALOG_PLL_ARM_TOG:
+return "PLL_ARM_TOG";
+case CCM_ANALOG_PLL_USB1:
+return "PLL_USB1";
+case CCM_ANALOG_PLL_USB1_SET:
+return "PLL_USB1_SET";
+case CCM_ANALOG_PLL_USB1_CLR:
+return "PLL_USB1_CLR";
+case CCM_ANALOG_PLL_USB1_TOG:
+return "PLL_USB1_TOG";
+case CCM_ANALOG_PLL_USB2:
+return "PLL_USB2";
+case CCM_ANALOG_PLL_USB2_SET:
+return "PLL_USB2_SET";
+case CCM_ANALOG_PLL_USB2_CLR:
+return "PLL_USB2_CLR";
+case CCM_ANALOG_PLL_USB2_TOG:
+return "PLL_USB2_TOG";
+case CCM_ANALOG_PLL_SYS:
+return "PLL_SYS";
+case CCM_ANALOG_PLL_SYS_SET:
+return "PLL_SYS_SET";
+case CCM_ANALOG_PLL_SYS_CLR:
+return "PLL_SYS_CLR";
+case CCM_ANALOG_PLL_SYS_TOG:
+return "PLL_SYS_TOG";
+case CCM_ANALOG_PLL_SYS_SS:
+return "PLL_SYS_SS";
+case CCM_ANALOG_PLL_SYS_NUM:
+return "PLL_SYS_NUM";
+case CCM_ANALOG_PLL_SYS_DENOM:
+return "PLL_SYS_DENOM";
+case CCM_ANALOG_PLL_AUDIO:
+return "PLL_AUDIO";
+case CCM_ANALOG_PLL_AUDIO_SET:
+return "PLL_AUDIO_SET";
+case CCM_ANALOG_PLL_AUDIO_CLR:
+return "PLL_AUDIO_CLR";
+case CCM_ANALOG_PLL_AUDIO_TOG:
+return "PLL_AUDIO_TOG";
+case CCM_ANALOG_PLL_AUDIO_NUM:
+return 

Re: [Qemu-devel] [PATCH v5 23/76] target/mips: Add emulation of nanoMIPS 16-bit load and store instructions

[Richard Henderson]

On 07/30/2018 12:11 PM, Aleksandar Markovic wrote:
>  case NM_P16_LB:
> +rd = extract32(ctx->opcode, 0, 2);
> +switch (extract32(ctx->opcode, 2, 2)) {
...
>  case NM_P16_LH:
> +rd = extract32(ctx->opcode, 1, 2) << 1;
> +switch ((extract32(ctx->opcode, 3, 1) << 1) | (ctx->opcode & 1)) {
...
>  case NM_LW16:
> +rd = extract32(ctx->opcode, 0, 4) << 2;
> +gen_ld(ctx, OPC_LW, rt, rs, rd);

Again, do not use RD to hold anything other than a register.
In these cases and more through this patch, RD being set to
the immediate offset to the memory operation.


r~

Re: [Qemu-devel] [PATCH v5 22/76] target/mips: Add emulation of nanoMIPS 16-bit misc instructions

[Richard Henderson]

On 07/30/2018 12:11 PM, Aleksandar Markovic wrote:
> From: Yongbok Kim 
> 
> Add emulation of misc nanoMIPS 16-bit instructions.
> 
> Signed-off-by: Yongbok Kim 
> Signed-off-by: Aleksandar Markovic 
> Signed-off-by: Stefan Markovic 
> ---
>  target/mips/translate.c | 41 +
>  1 file changed, 41 insertions(+)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v5 21/76] target/mips: Add emulation of nanoMIPS 16-bit shift instructions

[Richard Henderson]

On 07/30/2018 12:11 PM, Aleksandar Markovic wrote:
> From: Yongbok Kim 
> 
> Add emulation of nanoMIPS 16-bit shift instructions.
> 
> Signed-off-by: Yongbok Kim 
> Signed-off-by: Aleksandar Markovic 
> Signed-off-by: Stefan Markovic 
> ---
>  target/mips/translate.c | 15 +++
>  1 file changed, 15 insertions(+)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v5 20/76] target/mips: Add emulation of nanoMIPS 16-bit branch instructions

[Richard Henderson]

On 07/30/2018 12:11 PM, Aleksandar Markovic wrote:
> From: Yongbok Kim 
> 
> Add emulation of nanoMIPS 16-bit branch instructions.
> 
> Signed-off-by: Yongbok Kim 
> Signed-off-by: Aleksandar Markovic 
> Signed-off-by: Stefan Markovic 
> ---
>  target/mips/translate.c | 36 
>  1 file changed, 36 insertions(+)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [Qemu-block] [PATCH] Add interactive mode to qemu-img command

[Programmingkid]

> On Jul 30, 2018, at 3:48 PM, Eric Blake  wrote:
> 
> On 07/30/2018 02:14 PM, John Arbuckle wrote:
>> Changes qemu-img so if the user runs it without any
>> arguments, it will walk the user thru making an image
>> file.
> 
> Please remember to cc qemu-devel on ALL patches, as suggested by 
> ./scripts/getmaintainer.pl.

Sorry about that. I did remember a few minutes
after sending the patch and sent it.

> 
>> Signed-off-by: John Arbuckle 
>> ---
>>  qemu-img.c | 31 +--
>>  1 file changed, 29 insertions(+), 2 deletions(-)
> 
> Missing documentation patches (at a minimum, 'man qemu-img' should be updated 
> to mention this new mode of operation).  I'm also going to be a stickler and 
> require an addition to tests/qemu-iotests to ensure this new feature gets 
> coverage and doesn't regress (at least, if others are even in favor of the 
> idea. Personally, I'm 60-40 against the bloat, as telling the user to read 
> --help is sufficient in my mind).

After talking to Max Reitz about this issue I'm
not even sure I should continue with this plan.

Another idea I have is to improve the
documentation to qemu-img. Right now it looks
very hard to look at. I'm thinking a few examples
might make things easier for the user.

> 
>> diff --git a/qemu-img.c b/qemu-img.c
>> index 9b7506b8ae..aa3df3b431 100644
>> --- a/qemu-img.c
>> +++ b/qemu-img.c
>> @@ -4873,6 +4873,32 @@ out:
>>  return ret;
>>  }
>>  +/* Guides the user on making an image file */
>> +static int interactive_mode()
>> +{
>> +char format[100];
>> +char size[100];
>> +char name[1000];
> 
> Eww. Fixed size buffers for user-provided input are evil.  getline() is your 
> friend.
> 
>> +
>> +printf("\nInteractive mode (Enter Control-C to cancel)\n");
>> +printf("Please select a format (qcow, qcow2, raw, vdi, vhdx, vmdk, 
>> vpc): ");
> 
> Incomplete.  Better to generate the list dynamically by iterating over the 
> QAPI enum than it is to hard-code an incomplete list - particularly since 
> distros can white- or black-list particular drivers.
> 
>> +scanf("%100s", format);
> 
> Eww. Repeating the limit of your fixed size buffer, without checking that you 
> actually read a complete line, or that the scanf actually succeeded in 
> reading.  That's dangerous if a later programmer changes one but not both of 
> the two lines that are supposed to be using the same fixed size.  Even if we 
> accept the direction that this patch is heading in, it would need to be done 
> using more robust code.
> 
>> +printf("Please enter a size (e.g. 100M, 10G): ");
>> +scanf("%100s", size);
>> +printf("Please enter a name: ");
>> +scanf("%1000s", name);
> 
> A limit of 1000 has no bearing on actual file name lengths; a more typical 
> limit of NAME_MAX and/or PATH_MAX may come into play first (as low as 256 on 
> many systems, but on some systems, that value is intentionally undefined 
> because there is no inherent limit).  Again, you should be using getline() 
> and malloc'ing your result rather than using arbitrary (and probably 
> wrong-size) fixed-size buffers.
> 
>> +
>> +const char *arguments[] = {"create", "-f", format, name, size};
> 
> Although this is valid C99, qemu tends to prefer that you declare all 
> variables at the start of the scope rather than after statements.
> 
> You did not do ANY error checking that you actually parsed arguments from the 
> user; that in turn could result in passing bogus arguments on to img_create() 
> that could not normally happen via command line usage.

I was going to do error handling until I realized
img_create() is very good at telling the user
what is wrong. Anything I would add would be
redundant to what img_create() does.

> 
> Why are you hard-coding a create, rather than going FULLY interactive and 
> asking the user which sub-command (create, compare, ...) that they want to 
> use?

I was only thinking about what I use qemu-img for.
You do have a good point. A fully interactive
qemu-img command would be better.

>  Note that if we do want the level of interactivity to encompass the choice 
> of subcommand, that you still need to make things programmatic, not 
> hard-coded.  Also, John Snow had started some work on making qemu-img.c and 
> associated documentation saner, including making subcommands more uniform; 
> you may want to rebase your work on top of his cleanups, if there is even 
> demand for this.
> 
>> +int arg_count = 5;
>> +int return_value;
>> +return_value = img_create(arg_count, (char **)arguments);
> 
> Can we come up with a way to not have to cast away the const, if we are 
> locally supplying the arguments?
> 
>> +if (return_value == 0) {
>> +printf("Done creating image file\n");
>> +}
>> +
>> +return return_value;
>> +}
>> +
>>  static const img_cmd_t img_cmds[] = {
>>  #define DEF(option, callback, arg_string)\
>>  { option, callback },
>> @@ -4912,8 +4938,9 @@ int main(int argc, 

Re: [Qemu-devel] [PATCH v5 19/76] target/mips: Add emulation of nanoMIPS 16-bit arithmetic instructions

[Richard Henderson]

On 07/30/2018 12:11 PM, Aleksandar Markovic wrote:
> +case NM_P16_A2:
> +switch (extract32(ctx->opcode, 3, 1)) {
> +case NM_ADDIUR2:
> +rd = extract32(ctx->opcode, 0, 3) << 2;
> +gen_arith_imm(ctx, OPC_ADDIU, rt, rs, rd);
> +break;
> +case NM_P_ADDIURS5:
> +rt = extract32(ctx->opcode, 5, 5);
> +if (rt != 0) {
> +rs = (sextract32(ctx->opcode, 4, 1) << 3) |
> +  extract32(ctx->opcode, 0, 3);
> +/* s = sign_extend( s[3] . s[2:0] , from_nbits = 4)*/
> +gen_arith_imm(ctx, OPC_ADDIU, rt, rt, rs);
> +}
> +break;
> +}

Now, these re-uses of RD and RS variables are misleading.
These are immediates that you are extracting, not register numbers.

I suggest a "target_long imm;" at the top of the function to handle all such
that will be required when this function is filled out.


r~

Re: [Qemu-devel] [PATCH v5 18/76] target/mips: Add nanoMIPS decoding and extraction utilities

[Richard Henderson]

On 07/30/2018 12:11 PM, Aleksandar Markovic wrote:
> From: Aleksandar Markovic 
> 
> Add some basic utility functions and macros for nanoMIPS decoding
> engine.
> 
> Signed-off-by: Yongbok Kim 
> Signed-off-by: Aleksandar Markovic 
> Signed-off-by: Stefan Markovic 
> ---
>  target/mips/translate.c | 45 +
>  1 file changed, 45 insertions(+)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v5 17/76] target/mips: Add placeholder and invocation of decode_nanomips_opc()

[Richard Henderson]

On 07/30/2018 12:11 PM, Aleksandar Markovic wrote:
> From: Aleksandar Markovic 
> 
> Add empty body and invocation of decode_nanomips_opc() if the bit
> ISA_NANOMIPS32 is set in ctx->insn_flags.
> 
> Signed-off-by: Yongbok Kim 
> Signed-off-by: Aleksandar Markovic 
> Signed-off-by: Stefan Markovic 
> ---
>  target/mips/translate.c | 16 
>  1 file changed, 16 insertions(+)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v5 16/76] target/mips: Mark switch fallthroughs with interpretable comments

[Richard Henderson]

On 07/30/2018 12:11 PM, Aleksandar Markovic wrote:
> From: Aleksandar Markovic 
> 
> Mark switch fallthroughs with comments, in cases fallthroughs
> are intentional.
> 
> The comments "/* fall through */" are interpreted by compilers and
> other tools, and they will not issue warnings in such cases. For gcc,
> the warning is turnend on by -Wimplicit-fallthrough. With this patch,
> there will be no such warnings in target/mips directory. If such
> warning appears in future, it should be checked if it is intentional,
> and, if yes, marked with a comment similar to those from this patch.
> 
> The comment must be just before next "case", otherwise gcc won't
> understand it.
> 
> Signed-off-by: Aleksandar Markovic 
> Signed-off-by: Stefan Markovic 
> ---
>  target/mips/translate.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v5 15/76] target/mips: Fix two instances of shadow variables

[Richard Henderson]

On 07/30/2018 12:11 PM, Aleksandar Markovic wrote:
> From: Aleksandar Markovic 
> 
> Fix two instances of shadow variables. This cleans up entire file
> translate.c from shadow variables.
> 
> Signed-off-by: Aleksandar Markovic 
> Signed-off-by: Stefan Markovic 
> ---
>  target/mips/translate.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v5 14/76] target/mips: Add gen_op_addr_addi()

[Richard Henderson]

On 07/30/2018 12:11 PM, Aleksandar Markovic wrote:
> From: Stefan Markovic 
> 
> Add gen_op_addr_addi(). This function will be used in emulation of
> some nanoMIPS instructions.
> 
> Signed-off-by: Aleksandar Markovic 
> Signed-off-by: Stefan Markovic 
> ---
>  target/mips/translate.c | 12 
>  1 file changed, 12 insertions(+)

Reviewed-by: Richard Henderson 

For your general mips to-do list, there are lots of places in
the existing code that can be cleaned up to make use of this
new function.


r~

[Qemu-devel] ARM: SVE issue in saturated subtraction

[Laurent Desnogues]

Hello Richard,

the signed overflow computation for 64-bit saturated subtraction in
do_sat_addsub_64 looks wrong:  I think the second xor should use t1
instead of t0.

Thanks,

Laurent

Re: [Qemu-devel] [PATCH for-3.0 v2] pc: acpi: fix memory hotplug regression by reducing stub SRAT entry size

[Eduardo Habkost]

On Mon, Jul 30, 2018 at 11:41:41AM +0200, Igor Mammedov wrote:
> Commit 848a1cc1e (hw/acpi-build: build SRAT memory affinity structures for 
> DIMM devices)
> broke the first dimm hotplug in following cases:
> 
>  1: there is no coldplugged dimm in the last numa node
> but there is a coldplugged dimm in another node
> 
>   -m 4096,slots=4,maxmem=32G   \
>   -object memory-backend-ram,id=m0,size=2G \
>   -device pc-dimm,memdev=m0,node=0 \
>   -numa node,nodeid=0  \
>   -numa node,nodeid=1
> 
>  2: if order of dimms on CLI is:
>1st plugged dimm in node1
>2nd plugged dimm in node0
> 
>   -m 4096,slots=4,maxmem=32G   \
>   -object memory-backend-ram,size=2G,id=m0 \
>   -device pc-dimm,memdev=m0,node=1 \
>   -object memory-backend-ram,id=m1,size=2G \
>   -device pc-dimm,memdev=m1,node=0 \
>   -numa node,nodeid=0  \
>   -numa node,nodeid=1
> 
> (qemu) object_add memory-backend-ram,id=m2,size=1G
> (qemu) device_add pc-dimm,memdev=m2,node=0
> 
> the first DIMM hotplug to any node except the last one
> fails (Windows is unable to online it).
> 
> Length reduction of stub hotplug memory SRAT entry,
> fixes issue for some reason.
> 

I'm really bothered by the lack of automated testing for all
these NUMA/ACPI corner cases.

This looks like a good candidate for an avocado_qemu test case.
Can you show pseudo-code of how exactly the bug fix could be
verified, so we can try to write a test case?


> RHBZ: 1609234

I suggest including full URL of bug report[1].  I doubt anybody
outside Red Hat knows what "RHBZ" means.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1609234

> 
> Signed-off-by: Igor Mammedov 
> ---
> NOTE TO MAINTAINER: UPDATE REFERENCE APCI TABLE BLOBS
> 
> v2:
>  fixup examples in commit message
>  (they were in wrong order and with duplicate memdev=m1)
> ---
>  hw/i386/acpi-build.c | 19 ++-
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 9e8350c..b52fdb2 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2269,7 +2269,16 @@ static void build_srat_hotpluggable_memory(GArray 
> *table_data, uint64_t base,
>  numamem = acpi_data_push(table_data, sizeof *numamem);
>  
>  if (!info) {
> -build_srat_memory(numamem, cur, end - cur, default_node,
> +/*
> + * Entry is required for Windows to enable memory hotplug in OS
> + * and for Linux to enable SWIOTLB when booted with less than
> + * 4G of RAM. Windows works better if the entry sets proximity
> + * to the highest NUMA node in the machine at the end of the
> + * reserved space.
> + * Memory devices may override proximity set by this entry,
> + * providing _PXM method if necessary.
> + */
> +build_srat_memory(numamem, end - 1, 1, default_node,
>MEM_AFFINITY_HOTPLUGGABLE | 
> MEM_AFFINITY_ENABLED);
>  break;
>  }
> @@ -2402,14 +2411,6 @@ build_srat(GArray *table_data, BIOSLinker *linker, 
> MachineState *machine)
>  build_srat_memory(numamem, 0, 0, 0, MEM_AFFINITY_NOFLAGS);
>  }
>  
> -/*
> - * Entry is required for Windows to enable memory hotplug in OS
> - * and for Linux to enable SWIOTLB when booted with less than
> - * 4G of RAM. Windows works better if the entry sets proximity
> - * to the highest NUMA node in the machine.
> - * Memory devices may override proximity set by this entry,
> - * providing _PXM method if necessary.
> - */
>  if (hotplugabble_address_space_size) {
>  build_srat_hotpluggable_memory(table_data, 
> machine->device_memory->base,
> hotplugabble_address_space_size,
> -- 
> 2.7.4
> 

-- 
Eduardo

[Qemu-devel] [PATCH 1/4] linux-user: Disallow setting newsp for fork

[Richard Henderson]

Or really, just clone devolving into fork.  This should not ever happen
in practice.  We do want to reserve calling cpu_clone_regs for the case
in which we are actually performing a clone.

Signed-off-by: Richard Henderson 
---
 linux-user/syscall.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index dfc851cc35..5bf8d13de7 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -6502,10 +6502,14 @@ static int do_fork(CPUArchState *env, unsigned int 
flags, abi_ulong newsp,
 pthread_mutex_destroy();
 pthread_mutex_unlock(_lock);
 } else {
-/* if no CLONE_VM, we consider it is a fork */
+/* If no CLONE_VM, we consider it is a fork.  */
 if (flags & CLONE_INVALID_FORK_FLAGS) {
 return -TARGET_EINVAL;
 }
+/* As a fork, setting a new sp does not make sense.  */
+if (newsp) {
+return -TARGET_EINVAL;
+}
 
 /* We can't support custom termination signals */
 if ((flags & CSIGNAL) != TARGET_SIGCHLD) {
@@ -6520,7 +6524,6 @@ static int do_fork(CPUArchState *env, unsigned int flags, 
abi_ulong newsp,
 ret = fork();
 if (ret == 0) {
 /* Child Process.  */
-cpu_clone_regs(env, newsp);
 fork_end(1);
 /* There is a race condition here.  The parent process could
theoretically read the TID in the child process before the child
-- 
2.17.1

[Qemu-devel] [PATCH 2/4] linux-user: Pass the parent env to cpu_clone_regs

[Richard Henderson]

Implementing clone for sparc requires that we make modifications
to both the parent and child cpu state.  In all other cases, the
new argument can be ignored.

Signed-off-by: Richard Henderson 
---
 linux-user/aarch64/target_cpu.h| 3 ++-
 linux-user/alpha/target_cpu.h  | 3 ++-
 linux-user/arm/target_cpu.h| 3 ++-
 linux-user/cris/target_cpu.h   | 3 ++-
 linux-user/hppa/target_cpu.h   | 3 ++-
 linux-user/i386/target_cpu.h   | 3 ++-
 linux-user/m68k/target_cpu.h   | 3 ++-
 linux-user/microblaze/target_cpu.h | 3 ++-
 linux-user/mips/target_cpu.h   | 3 ++-
 linux-user/nios2/target_cpu.h  | 3 ++-
 linux-user/openrisc/target_cpu.h   | 4 +++-
 linux-user/ppc/target_cpu.h| 3 ++-
 linux-user/riscv/target_cpu.h  | 3 ++-
 linux-user/s390x/target_cpu.h  | 3 ++-
 linux-user/sh4/target_cpu.h| 3 ++-
 linux-user/sparc/target_cpu.h  | 3 ++-
 linux-user/tilegx/target_cpu.h | 3 ++-
 linux-user/xtensa/target_cpu.h | 3 ++-
 linux-user/syscall.c   | 2 +-
 19 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/linux-user/aarch64/target_cpu.h b/linux-user/aarch64/target_cpu.h
index a021c95fa4..130177115e 100644
--- a/linux-user/aarch64/target_cpu.h
+++ b/linux-user/aarch64/target_cpu.h
@@ -19,7 +19,8 @@
 #ifndef AARCH64_TARGET_CPU_H
 #define AARCH64_TARGET_CPU_H
 
-static inline void cpu_clone_regs(CPUARMState *env, target_ulong newsp)
+static inline void cpu_clone_regs(CPUARMState *env, CPUARMState *old_env,
+  target_ulong newsp)
 {
 if (newsp) {
 env->xregs[31] = newsp;
diff --git a/linux-user/alpha/target_cpu.h b/linux-user/alpha/target_cpu.h
index ac4d255ae7..750ffb50d7 100644
--- a/linux-user/alpha/target_cpu.h
+++ b/linux-user/alpha/target_cpu.h
@@ -19,7 +19,8 @@
 #ifndef ALPHA_TARGET_CPU_H
 #define ALPHA_TARGET_CPU_H
 
-static inline void cpu_clone_regs(CPUAlphaState *env, target_ulong newsp)
+static inline void cpu_clone_regs(CPUAlphaState *env, CPUAlphaState *old_env,
+  target_ulong newsp)
 {
 if (newsp) {
 env->ir[IR_SP] = newsp;
diff --git a/linux-user/arm/target_cpu.h b/linux-user/arm/target_cpu.h
index 8a3764919a..5538b6cb29 100644
--- a/linux-user/arm/target_cpu.h
+++ b/linux-user/arm/target_cpu.h
@@ -23,7 +23,8 @@
See validate_guest_space in linux-user/elfload.c.  */
 #define MAX_RESERVED_VA  0xul
 
-static inline void cpu_clone_regs(CPUARMState *env, target_ulong newsp)
+static inline void cpu_clone_regs(CPUARMState *env, CPUARMState *old_env,
+  target_ulong newsp)
 {
 if (newsp) {
 env->regs[13] = newsp;
diff --git a/linux-user/cris/target_cpu.h b/linux-user/cris/target_cpu.h
index 2309343979..baf842b400 100644
--- a/linux-user/cris/target_cpu.h
+++ b/linux-user/cris/target_cpu.h
@@ -20,7 +20,8 @@
 #ifndef CRIS_TARGET_CPU_H
 #define CRIS_TARGET_CPU_H
 
-static inline void cpu_clone_regs(CPUCRISState *env, target_ulong newsp)
+static inline void cpu_clone_regs(CPUCRISState *env, CPUCRISState *old_env,
+  target_ulong newsp)
 {
 if (newsp) {
 env->regs[14] = newsp;
diff --git a/linux-user/hppa/target_cpu.h b/linux-user/hppa/target_cpu.h
index 1c539bdbd6..7cd8d168a7 100644
--- a/linux-user/hppa/target_cpu.h
+++ b/linux-user/hppa/target_cpu.h
@@ -19,7 +19,8 @@
 #ifndef HPPA_TARGET_CPU_H
 #define HPPA_TARGET_CPU_H
 
-static inline void cpu_clone_regs(CPUHPPAState *env, target_ulong newsp)
+static inline void cpu_clone_regs(CPUHPPAState *env, CPUHPPAState *old_env,
+  target_ulong newsp)
 {
 if (newsp) {
 env->gr[30] = newsp;
diff --git a/linux-user/i386/target_cpu.h b/linux-user/i386/target_cpu.h
index ece04d0966..8fbe36670f 100644
--- a/linux-user/i386/target_cpu.h
+++ b/linux-user/i386/target_cpu.h
@@ -20,7 +20,8 @@
 #ifndef I386_TARGET_CPU_H
 #define I386_TARGET_CPU_H
 
-static inline void cpu_clone_regs(CPUX86State *env, target_ulong newsp)
+static inline void cpu_clone_regs(CPUX86State *env, CPUX86State *old_env,
+  target_ulong newsp)
 {
 if (newsp) {
 env->regs[R_ESP] = newsp;
diff --git a/linux-user/m68k/target_cpu.h b/linux-user/m68k/target_cpu.h
index 611df065ca..1f0939aea7 100644
--- a/linux-user/m68k/target_cpu.h
+++ b/linux-user/m68k/target_cpu.h
@@ -21,7 +21,8 @@
 #ifndef M68K_TARGET_CPU_H
 #define M68K_TARGET_CPU_H
 
-static inline void cpu_clone_regs(CPUM68KState *env, target_ulong newsp)
+static inline void cpu_clone_regs(CPUM68KState *env, CPUM68KState *old_env,
+  target_ulong newsp)
 {
 if (newsp) {
 env->aregs[7] = newsp;
diff --git a/linux-user/microblaze/target_cpu.h 
b/linux-user/microblaze/target_cpu.h
index 73e139938c..3394e98918 100644
--- a/linux-user/microblaze/target_cpu.h
+++ b/linux-user/microblaze/target_cpu.h
@@ -19,7 +19,8 @@
 #ifndef MICROBLAZE_TARGET_CPU_H
 #define 

[Qemu-devel] [PATCH 4/4] linux-user/sparc: Flush register windows before clone

[Richard Henderson]

As seen as the very first instruction of sys_clone in the kernel.

Ideally this would be done in or before cpu_copy, and not with a
separate explicit test vs the syscall number, but this is a more
minimal solution.

Signed-off-by: Richard Henderson 
---
 linux-user/sparc/cpu_loop.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/linux-user/sparc/cpu_loop.c b/linux-user/sparc/cpu_loop.c
index 91f714afc6..fe83f25686 100644
--- a/linux-user/sparc/cpu_loop.c
+++ b/linux-user/sparc/cpu_loop.c
@@ -169,6 +169,9 @@ void cpu_loop (CPUSPARCState *env)
 case 0x110:
 case 0x16d:
 #endif
+if (env->gregs[1] == TARGET_NR_clone) {
+flush_windows(env);
+}
 ret = do_syscall (env, env->gregs[1],
   env->regwptr[0], env->regwptr[1],
   env->regwptr[2], env->regwptr[3],
-- 
2.17.1

[Qemu-devel] [PATCH 3/4] linux-user/sparc: Fix cpu_clone_regs

[Richard Henderson]

We failed to set the secondary return value in %o1
we failed to advance the PC past the syscall, and
we failed to adjust regwptr into the new structure.

Signed-off-by: Richard Henderson 
---
 linux-user/sparc/target_cpu.h | 20 ++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/linux-user/sparc/target_cpu.h b/linux-user/sparc/target_cpu.h
index a92748cae3..c223f865e9 100644
--- a/linux-user/sparc/target_cpu.h
+++ b/linux-user/sparc/target_cpu.h
@@ -23,11 +23,21 @@
 static inline void cpu_clone_regs(CPUSPARCState *env, CPUSPARCState *old_env,
   target_ulong newsp)
 {
+/*
+ * After cpu_copy, env->regwptr is pointing into old_env.
+ * Update the new cpu to use its own register window.
+ */
+env->regwptr = env->regbase + (env->cwp * 16);
+
+/* Set a new stack, if requested.  */
 if (newsp) {
 env->regwptr[22] = newsp;
 }
-/* syscall return for clone child: 0, and clear CF since
- * this counts as a success return value.
+
+/*
+ * Syscall return for clone child: %o0 = 0 and clear CF since
+ * this counts as a success return value.  %o1 = 1 to indicate
+ * this is the child.  Advance the PC past the syscall.
  */
 env->regwptr[0] = 0;
 #if defined(TARGET_SPARC64) && !defined(TARGET_ABI32)
@@ -35,6 +45,12 @@ static inline void cpu_clone_regs(CPUSPARCState *env, 
CPUSPARCState *old_env,
 #else
 env->psr &= ~PSR_CARRY;
 #endif
+env->regwptr[1] = 1;
+env->pc = env->npc;
+env->npc = env->npc + 4;
+
+/* Set the second return value for the parent: %o1 = 0.  */
+old_env->regwptr[1] = 0;
 }
 
 static inline void cpu_set_tls(CPUSPARCState *env, target_ulong newtls)
-- 
2.17.1

[Qemu-devel] [PATCH 0/3] linux-user/sparc: Fixes for clone

[Richard Henderson]

There are at least 4 separate bugs preventing clone from working.

(1) cpu_copy left both cpus sharing the same register window (!)

(2) cpu_clone_regs did not initialize %o1, so the new thread path
in the guest __clone was always taken, even for the parent
(old %o1 value was newsp, and so non-zero).

(3) cpu_clone_regs did not advance the pc past the syscall in the
child, which meant that the child re-executed the syscall
(and because of (1), with essentially random inputs).

(4) clone did not flush register windows, which would cause the
parent stack to be clobbered by the child writing out old
windows in order to allocate a new one.

This is enough for Alex's atomic-test to make progress, but not
quite enough for it to actually work.  What I'm seeing now is a
legitimate SEGV for a write to a r-xp memory segment.  I'll need
to examine the testcase further to see why that is happening.


r~


Richard Henderson (4):
  linux-user: Disallow setting newsp for fork
  linux-user: Pass the parent env to cpu_clone_regs
  linux-user/sparc: Fix cpu_clone_regs
  linux-user/sparc: Flush register windows before clone

 linux-user/aarch64/target_cpu.h|  3 ++-
 linux-user/alpha/target_cpu.h  |  3 ++-
 linux-user/arm/target_cpu.h|  3 ++-
 linux-user/cris/target_cpu.h   |  3 ++-
 linux-user/hppa/target_cpu.h   |  3 ++-
 linux-user/i386/target_cpu.h   |  3 ++-
 linux-user/m68k/target_cpu.h   |  3 ++-
 linux-user/microblaze/target_cpu.h |  3 ++-
 linux-user/mips/target_cpu.h   |  3 ++-
 linux-user/nios2/target_cpu.h  |  3 ++-
 linux-user/openrisc/target_cpu.h   |  4 +++-
 linux-user/ppc/target_cpu.h|  3 ++-
 linux-user/riscv/target_cpu.h  |  3 ++-
 linux-user/s390x/target_cpu.h  |  3 ++-
 linux-user/sh4/target_cpu.h|  3 ++-
 linux-user/sparc/target_cpu.h  | 23 ---
 linux-user/tilegx/target_cpu.h |  3 ++-
 linux-user/xtensa/target_cpu.h |  3 ++-
 linux-user/sparc/cpu_loop.c|  3 +++
 linux-user/syscall.c   |  9 ++---
 20 files changed, 64 insertions(+), 23 deletions(-)

-- 
2.17.1

Re: [Qemu-devel] [qemu-s390x] [PATCH v1] s390x/cpu_models: Add "-cpu max" support

[Eduardo Habkost]

On Mon, Jul 30, 2018 at 11:16:38AM +0200, David Hildenbrand wrote:
> On 27.07.2018 14:55, Cornelia Huck wrote:
> > On Wed, 25 Jul 2018 11:12:33 +0200
> > David Hildenbrand  wrote:
> > 
> >> The "max" CPU model behaves like "-cpu host" when KVM is enabled, and like
> >> a CPU with the maximum possible feature set when TCG is enabled.
> >>
> >> While the "host" model can not be used under TCG ("kvm_required"), the
> >> "max" model can and "Enables all features supported by the accelerator in
> >> the current host".
> >>
> >> So we can treat "host" just as a special case of "max" (like x86 does).
> >> It differs to the "qemu" CPU model under TCG such that compatibility
> >> handling will not be performed and that some experimental CPU features
> >> not yet part of the "qemu" model might be indicated.
> >>
> >> These are right now under TCG (see "qemu_MAX"):
> >> - stfle53
> >> - msa5-base
> >> - zpci
> >>
> >> This will result right now in the following warning when starting QEMU TCG
> >> with the "max" model:
> >> "qemu-system-s390x: warning: 'msa5-base' requires 'kimd-sha-512'."
> >>
> >> The "qemu" model (used as default in QEMU under TCG) will continue to
> >> work without such warnings. The "max" mdel in the current form
> >> might be interesting for kvm-unit-tests (where we would e.g. now also
> >> test "msa5-base").
> >>
> >> The "max" model is neither static nor migration safe (like the "host"
> >> model). It is independent of the machine but dependends on the accelerator.
> >> It can be used to detect the maximum CPU model also under TCG from upper
> >> layers without having to care about CPU model names for CPU model
> >> expansion.
> >>
> >> Signed-off-by: David Hildenbrand 
> >> ---
> >>  target/s390x/cpu_models.c | 81 +++
> >>  1 file changed, 56 insertions(+), 25 deletions(-)
> > 
> > So, what's the outcome? Can I merge this with the discussed minor
> > edits, or should I wait for a v2?
> > 
> 
> Eduardo identified possible optimizations independent of this patch, so
> we should be good to go. @Eduardo, please correct me if I'm wrong!

This version still looks good to me, my Reviewed-by line still
applies.  Thanks!

-- 
Eduardo

Re: [Qemu-devel] [Qemu-block] [PATCH] Add interactive mode to qemu-img command

[Eric Blake]

On 07/30/2018 02:14 PM, John Arbuckle wrote:

Changes qemu-img so if the user runs it without any
arguments, it will walk the user thru making an image
file.


Please remember to cc qemu-devel on ALL patches, as suggested by 
./scripts/getmaintainer.pl.




Signed-off-by: John Arbuckle 
---
  qemu-img.c | 31 +--
  1 file changed, 29 insertions(+), 2 deletions(-)



Missing documentation patches (at a minimum, 'man qemu-img' should be 
updated to mention this new mode of operation).  I'm also going to be a 
stickler and require an addition to tests/qemu-iotests to ensure this 
new feature gets coverage and doesn't regress (at least, if others are 
even in favor of the idea. Personally, I'm 60-40 against the bloat, as 
telling the user to read --help is sufficient in my mind).



diff --git a/qemu-img.c b/qemu-img.c
index 9b7506b8ae..aa3df3b431 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -4873,6 +4873,32 @@ out:
  return ret;
  }
  
+/* Guides the user on making an image file */

+static int interactive_mode()
+{
+char format[100];
+char size[100];
+char name[1000];


Eww. Fixed size buffers for user-provided input are evil.  getline() is 
your friend.



+
+printf("\nInteractive mode (Enter Control-C to cancel)\n");
+printf("Please select a format (qcow, qcow2, raw, vdi, vhdx, vmdk, vpc): 
");


Incomplete.  Better to generate the list dynamically by iterating over 
the QAPI enum than it is to hard-code an incomplete list - particularly 
since distros can white- or black-list particular drivers.



+scanf("%100s", format);


Eww. Repeating the limit of your fixed size buffer, without checking 
that you actually read a complete line, or that the scanf actually 
succeeded in reading.  That's dangerous if a later programmer changes 
one but not both of the two lines that are supposed to be using the same 
fixed size.  Even if we accept the direction that this patch is heading 
in, it would need to be done using more robust code.



+printf("Please enter a size (e.g. 100M, 10G): ");
+scanf("%100s", size);
+printf("Please enter a name: ");
+scanf("%1000s", name);


A limit of 1000 has no bearing on actual file name lengths; a more 
typical limit of NAME_MAX and/or PATH_MAX may come into play first (as 
low as 256 on many systems, but on some systems, that value is 
intentionally undefined because there is no inherent limit).  Again, you 
should be using getline() and malloc'ing your result rather than using 
arbitrary (and probably wrong-size) fixed-size buffers.



+
+const char *arguments[] = {"create", "-f", format, name, size};


Although this is valid C99, qemu tends to prefer that you declare all 
variables at the start of the scope rather than after statements.


You did not do ANY error checking that you actually parsed arguments 
from the user; that in turn could result in passing bogus arguments on 
to img_create() that could not normally happen via command line usage.


Why are you hard-coding a create, rather than going FULLY interactive 
and asking the user which sub-command (create, compare, ...) that they 
want to use?  Note that if we do want the level of interactivity to 
encompass the choice of subcommand, that you still need to make things 
programmatic, not hard-coded.  Also, John Snow had started some work on 
making qemu-img.c and associated documentation saner, including making 
subcommands more uniform; you may want to rebase your work on top of his 
cleanups, if there is even demand for this.



+int arg_count = 5;
+int return_value;
+return_value = img_create(arg_count, (char **)arguments);


Can we come up with a way to not have to cast away the const, if we are 
locally supplying the arguments?



+if (return_value == 0) {
+printf("Done creating image file\n");
+}
+
+return return_value;
+}
+
  static const img_cmd_t img_cmds[] = {
  #define DEF(option, callback, arg_string)\
  { option, callback },
@@ -4912,8 +4938,9 @@ int main(int argc, char **argv)
  
  module_call_init(MODULE_INIT_QOM);

  bdrv_init();
-if (argc < 2) {
-error_exit("Not enough arguments");
+
+if (argc == 1) { /* If no arguments passed to qemu-img */
+return interactive_mode();


This is placed early-enough in main() that you never use interactive 
mode if the user passed options but no subcommand (such as 'qemu-img -T 
foo'), is that intentional?  Conversely, your patch does not help if a 
user calls 'qemu-img create' without options, while it can be argued 
that if we want (partial) interactivity, why not be nice to the user and 
provide it at any step of the way rather than just when no subcommand 
name was given.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

[Qemu-devel] [PATCH] Add interactive mode to qemu-img command

[John Arbuckle]

Changes qemu-img so if the user runs it without any
arguments, it will walk the user thru making an image
file.

Signed-off-by: John Arbuckle 
---
 qemu-img.c | 31 +--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 9b7506b8ae..aa3df3b431 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -4873,6 +4873,32 @@ out:
 return ret;
 }
 
+/* Guides the user on making an image file */
+static int interactive_mode()
+{
+char format[100];
+char size[100];
+char name[1000];
+
+printf("\nInteractive mode (Enter Control-C to cancel)\n");
+printf("Please select a format (qcow, qcow2, raw, vdi, vhdx, vmdk, vpc): 
");
+scanf("%100s", format);
+printf("Please enter a size (e.g. 100M, 10G): ");
+scanf("%100s", size);
+printf("Please enter a name: ");
+scanf("%1000s", name);
+
+const char *arguments[] = {"create", "-f", format, name, size};
+int arg_count = 5;
+int return_value;
+return_value = img_create(arg_count, (char **)arguments);
+if (return_value == 0) {
+printf("Done creating image file\n");
+}
+
+return return_value;
+}
+
 static const img_cmd_t img_cmds[] = {
 #define DEF(option, callback, arg_string)\
 { option, callback },
@@ -4912,8 +4938,9 @@ int main(int argc, char **argv)
 
 module_call_init(MODULE_INIT_QOM);
 bdrv_init();
-if (argc < 2) {
-error_exit("Not enough arguments");
+
+if (argc == 1) { /* If no arguments passed to qemu-img */
+return interactive_mode();
 }
 
 qemu_add_opts(_object_opts);
-- 
2.14.3 (Apple Git-98)

Re: [Qemu-devel] [PATCH v3] block/gluster: Handle changed glfs_ftruncate signature

[Jeff Cody]

On Mon, Jul 30, 2018 at 10:07:27AM -0500, Eric Blake wrote:
> On 07/28/2018 02:50 AM, Niels de Vos wrote:
> >>
> >>Part of me wishes that libgfapi had just created a new function
> >>'glfs_ftruncate2', so that existing users don't need to handle the api
> >>change.  But I guess in the grand scheme, not a huge deal either way.
> >
> >Gluster uses versioned symbols, so older binaries will keep working with
> >new libraries. It is (hopefully) rare that existing symbols get updated.
> >We try to send patches for these kind of changes to the projects we know
> >well in advance, reducing the number of surprises.
> 
> >>I can go ahead and add that to the comment in my branch after applying, if
> >>Niels can let me know what that version is/will be (if known).
> >
> >The new glfs_ftruncate() will be part of glusterfs-5 (planned for
> >October). We're changing the numbering scheme, it was expected to come
> >in glusterfs-4.2, but that is a version that never will be released.
> >
> 
> Wait - so you're saying gluster has not yet released the incompatible
> change? Now would be the right time to get rid of the API breakage, before
> you bake it in, rather than relying solely on the versioned symbols to avoid
> an ABI breakage but forcing all clients to compensate to the API breakage.
> 

If this is not yet in a released version of Gluster, I'm not real eager to
pollute the QEMU driver codebase with #ifdef's, especially if there is a
possibility the API change may not actually materialize.

Is there any reason that this change is being approached in a way that
breaks API usage, and is it too late in the Gluster development pipeline to
change that?

Re: [Qemu-devel] [PATCH v3] linux-user: ppc64: don't use volatile register during safe_syscall

[Laurent Vivier]

Le 30/07/2018 à 14:44, Richard Henderson a écrit :
> On 07/30/2018 06:09 AM, Shivaprasad G Bhat wrote:
>> r11 is a volatile register on PPC as per calling conventions.
>> The safe_syscall code uses it to check if the signal_pending
>> is set during the safe_syscall. When a syscall is interrupted
>> on return from signal handling, the r11 might be corrupted
>> before we retry the syscall leading to a crash. The registers
>> r0-r13 are not to be used here as they have
>> volatile/designated/reserved usages.
>>
>> Change the code to use r14 which is non-volatile.
>> Use SP+16 which is a slot for LR, for save/restore of previous value
>> of r14. SP+16 can be used, as LR is preserved across the syscall.
>>
>> Steps to reproduce:
>> On PPC host, issue `qemu-x86_64 /usr/bin/cc -E -`
>> Attempt Ctrl-C, the issue is reproduced.
>>
>> Reference:
>> https://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi-1.9.html#REG
>> https://openpowerfoundation.org/wp-content/uploads/2016/03/ABI64BitOpenPOWERv1.1_16July2015_pub4.pdf
>>
>> Signed-off-by: Shivaprasad G Bhat 
> 
> This does fix the bug, so
> Tested-by: Richard Henderson 
> 
> But we can do slightly better:
> 
>> @@ -49,7 +49,8 @@ safe_syscall_base:
>>   *   and returns the result in r3
>>   * Shuffle everything around appropriately.
>>   */
>> -mr  11, 3   /* signal_pending */
>> +std 14, 16(1) /* Preserve r14 in SP+16 */
> 
> Above this context, we have a .cfi_startproc directive, which indicates we are
> providing unwind information for this function (trivial up to this point).
> 
> To preserve that, you need to add
> 
>   .cfi_offset 14, 16
> 
> right here, indicating r14 is saved 16 bytes "up" the stack frame.
> 
> Since this portion of the stack frame is allocated by the caller, this saved
> value remains valid through the end of the function, so we do not need to do
> anything at the points where the value is restored.
> 
>>  safe_syscall_end:
>> +ld 14, 16(1) /* restore r14 to its original value */
>>  /* code path when we did execute the syscall */
> 
> Swap these two lines.
> 
> With those two changes,
> Reviewed-by: Richard Henderson 

Tested-by: Laurent Vivier 
Reviewed-by: Laurent Vivier 

I think this patch should go into the next -rc because it fixes a lot of
failures in the LTP on ppc64 host (hundreds of failure...).

David, if you want, I can take it through the linux-user tree.

Thanks,
Laurent

[Qemu-devel] [Bug 1720969] Re: qemu/memory.c:206: pointless copies of large structs ?

[Tristan Burgess]

Fix committed and sent upstream:
https://github.com/qemu/qemu/commit/73bb753d24a702b37913ce4b5ddb6dca40dab067

** Changed in: qemu
   Status: New => Fix Committed

** Changed in: qemu
 Assignee: (unassigned) => Tristan Burgess (tburgessdev)

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

Title:
  qemu/memory.c:206:  pointless copies of large structs ?

Status in QEMU:
  Fix Committed

Bug description:
  [qemu/memory.c:206]: (performance) Function parameter 'a' should be passed by 
reference.
  [qemu/memory.c:207]: (performance) Function parameter 'b' should be passed by 
reference.

  Source code is

  static bool memory_region_ioeventfd_equal(MemoryRegionIoeventfd a,
MemoryRegionIoeventfd b)

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

Re: [Qemu-devel] [PATCH v2 0/3] i.MX: Add the i.MX6UL SOC and a reference board.

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: cover.1532963204.git@tribudubois.net
Subject: [Qemu-devel] [PATCH v2 0/3] i.MX: Add the i.MX6UL SOC and a reference 
board.

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
   6d9dd5fb9d..7aefc14565  master -> master
 t [tag update]
patchew/20180730141748.430-1-peter.mayd...@linaro.org -> 
patchew/20180730141748.430-1-peter.mayd...@linaro.org
 * [new tag]   patchew/20180730173712.gg4...@os.inf.tu-dresden.de 
-> patchew/20180730173712.gg4...@os.inf.tu-dresden.de
Switched to a new branch 'test'
d305c7d6d0 i.MX6UL: Add Freescale i.MX6 UltraLite 14x14 EVK Board
d45a2a8602 i.MX6UL: Add i.MX6UL SOC
07fff3d865 i.MX6UL: Add i.MX6UL specific CCM device

=== OUTPUT BEGIN ===
Checking PATCH 1/3: i.MX6UL: Add i.MX6UL specific CCM device...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#22: 
new file mode 100644

total: 0 errors, 1 warnings, 1133 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 2/3: i.MX6UL: Add i.MX6UL SOC...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#31: 
new file mode 100644

ERROR: suspect code indent for conditional statements (4, 7)
#113: FILE: hw/arm/fsl-imx6ul.c:78:
+for (i = 0; i < FSL_IMX6UL_NUM_GPIOS; i++) {
+   snprintf(name, NAME_SIZE, "gpio%d", i);

ERROR: suspect code indent for conditional statements (4, 7)
#122: FILE: hw/arm/fsl-imx6ul.c:87:
+for (i = 0; i < FSL_IMX6UL_NUM_GPTS; i++) {
+   snprintf(name, NAME_SIZE, "gpt%d", i);

ERROR: suspect code indent for conditional statements (4, 7)
#131: FILE: hw/arm/fsl-imx6ul.c:96:
+for (i = 0; i < FSL_IMX6UL_NUM_EPITS; i++) {
+   snprintf(name, NAME_SIZE, "epit%d", i + 1);

ERROR: suspect code indent for conditional statements (4, 7)
#140: FILE: hw/arm/fsl-imx6ul.c:105:
+for (i = 0; i < FSL_IMX6UL_NUM_ECSPIS; i++) {
+   snprintf(name, NAME_SIZE, "spi%d", i + 1);

ERROR: suspect code indent for conditional statements (4, 7)
#149: FILE: hw/arm/fsl-imx6ul.c:114:
+for (i = 0; i < FSL_IMX6UL_NUM_I2CS; i++) {
+   snprintf(name, NAME_SIZE, "i2c%d", i + 1);

ERROR: suspect code indent for conditional statements (4, 7)
#158: FILE: hw/arm/fsl-imx6ul.c:123:
+for (i = 0; i < FSL_IMX6UL_NUM_UARTS; i++) {
+   snprintf(name, NAME_SIZE, "uart%d", i);

ERROR: suspect code indent for conditional statements (4, 7)
#167: FILE: hw/arm/fsl-imx6ul.c:132:
+for (i = 0; i < FSL_IMX6UL_NUM_ETHS; i++) {
+   snprintf(name, NAME_SIZE, "eth%d", i);

ERROR: suspect code indent for conditional statements (4, 7)
#176: FILE: hw/arm/fsl-imx6ul.c:141:
+for (i = 0; i < FSL_IMX6UL_NUM_USDHCS; i++) {
+   snprintf(name, NAME_SIZE, "usdhc%d", i);

ERROR: suspect code indent for conditional statements (4, 7)
#185: FILE: hw/arm/fsl-imx6ul.c:150:
+for (i = 0; i < FSL_IMX6UL_NUM_WDTS; i++) {
+   snprintf(name, NAME_SIZE, "wdt%d", i);

total: 9 errors, 1 warnings, 968 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 3/3: i.MX6UL: Add Freescale i.MX6 UltraLite 14x14 EVK Board...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#23: 
new file mode 100644

total: 0 errors, 1 warnings, 90 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [Qemu-devel] [PATCH v8 0/3] wakeup-from-suspend and system_wakeup changes

[Daniel Henrique Barboza]

Ping

On 07/05/2018 05:08 PM, Daniel Henrique Barboza wrote:

changes in v8:
- created 'query-current-machine' API to hold the wakeup-suspend-support
   flag
- wake-up flag now considers the --no-acpi config option for PC archs
- fixes in patch 3 proposed by Markus.
Previous series link:
https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg04234.html

Daniel Henrique Barboza (3):
   qmp: query-current-machine with wakeup-suspend-support
   qga: update guest-suspend-ram and guest-suspend-hybrid descriptions
   qmp hmp: Make system_wakeup check wake-up support and run state

  hmp.c   |  5 -
  include/sysemu/sysemu.h |  1 +
  qapi/misc.json  | 29 -
  qga/qapi-schema.json| 12 ++--
  qmp.c   | 11 ++-
  vl.c| 30 ++
  6 files changed, 79 insertions(+), 9 deletions(-)

[Qemu-devel] [Bug 1777777] Re: arm9 clock pending (SP804)

[Peter Maydell]

Sorry, I should have been clearer. Please can you provide a test case
binary and QEMU command line that reproduces the problem (including what
the expected output is and what the actual output is)?

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

Title:
  arm9 clock pending (SP804)

Status in QEMU:
  New

Bug description:
  Hello all,

  I'm using the versatilepb board and the timer Interrupt Mask Status
  register (offset 0x14 of the SP804) does not seem to be working
  properly on the latest qemu-2.12. I tried on the 2.5 (i believe this
  is the mainstream version that comes with Linux) and it works
  perfectly.

  What happens is that the pending bit does not seem to be set in some
  scenarios. In my case, I see the timer value decreasing to 0 and then
  being reset to the reload value and neither does the interrupt is
  triggered nor the pending bit is set.

  I believe this is a matter of timing since in the "long" run the
  system eventually catches up (after a few microseconds).

  Thank you

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

Re: [Qemu-devel] [PATCH v1 1/1] configure: Add RISC-V host support

[Peter Maydell]

On 30 July 2018 at 19:20, Philippe Mathieu-Daudé  wrote:
> Before this patch:
>
> $ ./configure
>
> ERROR: Unsupported CPU = riscv64, try --enable-tcg-interpreter
>
> $ ./configure --enable-tcg-interpreter
> Unsupported CPU = riscv64, will use TCG with TCI (experimental)
>
> [...]
>
> WARNING: SUPPORT FOR THIS HOST CPU WILL GO AWAY IN FUTURE RELEASES!


> It is unlikely the RISC-V port goes away in the next future releases :)

...but it is not supported as a *host* CPU.

We should fix this somewhat nonsensical error message by
completing the deprecate-and-drop cycle for this bit of
configure, ie by outright rejecting attempts to build on
host CPU types we don't recognize and support, the same
way we do with unrecognized host OS types.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 1/1] configure: Add RISC-V host support

[Peter Maydell]

On 30 July 2018 at 18:24, Alistair Francis  wrote:
> On Sun, Jul 29, 2018 at 4:28 AM, Peter Maydell  
> wrote:
>>> Another piece that even Michael Clark does not have is
>>> linux-user/host/*/safe-syscall.S.
>>
>> It might be nice to complete the safe-syscall stuff for all hosts,
>> and then remove the fallback that lets you build an unreliable
>> linux-user binary without it...
>
> At the moment though QEMU won't build with Linux user. So some work is
> required just to get the build working.

Right -- it's not really sufficient to just change configure.
I'd rather see:
 (a) a proper backend for any host CPU type we actually care about,
 which is what Michael's work is doing
 (b) support for the various per-host-arch things that are needed
 like the safe-syscall shim
 (c) QEMU's build infrastructure guiding the process of implementing
 both of those, by making it a build failure to not provide
 all the set of things, rather than producing a configuration
 that will build but not work reliably

thanks
-- PMM

[Qemu-devel] [Bug 1777777] Re: arm9 clock pending (SP804)

[RTOS Pharos]

Hello,

Let me explain a bit how to reproduce the problem. I'm developing a RTOS
with qemu (and boards,etc) -
https://sourceforge.net/projects/rtospharos/. The first version used
ARM926 with the versatilepb board and I used the qemu 2.5 version.

In my test (to check that the RTOS is working correctly) I produced a
loop that is always reading the current time ("pharosClockGet") and
checking that it is always increasing. This works perfectly on qemu 2.5.
When qemu 2.12 was released I updated to the new version to add support
for the raspberry pi 3.

Here is a piece of the test:


void aperiodicThread0_0()
{
uint64_t microseconds;
uint64_t previousMicroseconds = 0;
uint32_t i;

for(i = 0; i < NUMBER_REPETITIONS; i++)
{
microseconds = pharosClockGetSinceBoot();
if(previousMicroseconds > microseconds) 
{
print("ERROR! Clock got lower\r\n");
break;
}
}
}


I'll show you the "internals" of Pharos next (this is file clock.c):


uint64_t pharosIClockGetSinceBoot(void)
{
/* microseconds since last clock tick*/
uint32_t microseconds;

/* number of clock ticks since boot */
ClockTick clockTicks;

/* >> interrupts are disabled when we get here << */

/* read the number of microseconds on the counter */
clockTicks = pharosVClockTicks;

/* read the timer value */
microseconds = pharosCpuClockReadCounter();

/* if the clock interrupt is pending we might have read the microseconds 
before or after the clock tick occurred (but not processed since interrupts are 
disabled) */
if(pharosCpuClockIsrIsPending() == TRUE)
{
/* interrupt is pending, so read again the microseconds and subtract 2 
clock ticks */
microseconds = (2U * pharosIMicrosecondsPerClockTick()) - 
pharosCpuClockReadCounter();
}
else
{
/* the timer counts backwards so we must subtract to get the number of 
microseconds since last tick */
microseconds = pharosIMicrosecondsPerClockTick() - microseconds;
}

/* calculate the result using the last us read */
return (uint64_t) ((pharosIMicrosecondsPerClockTick() * clockTicks) + 
microseconds);
}


And the part that is hw dependent (for ARM926) - this is in file ClockSp804.h: 

INLINE bool armSp804TimerInterruptIsPending(const ptrArmSp804Timer c)
{
/* get the first bit of the interrupt status (ignore other bits) to 
check if the interrupt is pending */
return (c->timerInterruptMaskStatus & 0x1U) == 1U ? TRUE : FALSE;
}


I hope the code is easy enough to read.
Thank you

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

Title:
  arm9 clock pending (SP804)

Status in QEMU:
  New

Bug description:
  Hello all,

  I'm using the versatilepb board and the timer Interrupt Mask Status
  register (offset 0x14 of the SP804) does not seem to be working
  properly on the latest qemu-2.12. I tried on the 2.5 (i believe this
  is the mainstream version that comes with Linux) and it works
  perfectly.

  What happens is that the pending bit does not seem to be set in some
  scenarios. In my case, I see the timer value decreasing to 0 and then
  being reset to the reload value and neither does the interrupt is
  triggered nor the pending bit is set.

  I believe this is a matter of timing since in the "long" run the
  system eventually catches up (after a few microseconds).

  Thank you

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

Re: [Qemu-devel] [PATCH v1 1/1] configure: Add RISC-V host support

[Philippe Mathieu-Daudé]

On 07/27/2018 08:49 PM, Alistair Francis wrote:
> Allow QEMU to be built to run on a RISC-V host.
> 
> QEMU does not yet have a RISC-V TCG or user mode target port, but
> running other architectures on RISC-V using TCI does work.
> 
> Signed-off-by: Alistair Francis 
> ---
>  configure | 18 +-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index 2a7796ea80..c3ff3ae146 100755
> --- a/configure
> +++ b/configure
> @@ -606,6 +606,16 @@ EOF
>compile_object
>  }
>  
> +check_define_value() {
> +cat > $TMPC < +#if (($1) != ($2))
> +#error $1 != ($2)
> +#endif
> +int main(void) { return 0; }
> +EOF
> +  compile_object
> +}
> +
>  check_include() {
>  cat > $TMPC <  #include <$1>
> @@ -704,6 +714,12 @@ elif check_define __arm__ ; then
>cpu="arm"
>  elif check_define __aarch64__ ; then
>cpu="aarch64"
> +elif check_define __riscv ; then
> +  if check_define_value __riscv_xlen 64 ; then
> +cpu="riscv64"
> +  else
> +cpu="riscv32"
> +  fi
>  else
>cpu=$(uname -m)
>  fi
> @@ -712,7 +728,7 @@ ARCH=
>  # Normalise host CPU name and set ARCH.
>  # Note that this case should only have supported host CPUs, not guests.
>  case "$cpu" in
> -  ppc|ppc64|s390|s390x|sparc64|x32)
> +  ppc|ppc64|s390|s390x|sparc64|x32|riscv32|riscv64)
>  cpu="$cpu"
>  supported_cpu="yes"
>  eval "cross_cc_${cpu}=\$host_cc"
> 

Before this patch:

$ ./configure

ERROR: Unsupported CPU = riscv64, try --enable-tcg-interpreter

$ ./configure --enable-tcg-interpreter
Unsupported CPU = riscv64, will use TCG with TCI (experimental)

[...]

WARNING: SUPPORT FOR THIS HOST CPU WILL GO AWAY IN FUTURE RELEASES!

CPU host architecture riscv64 support is not currently maintained.
The QEMU project intends to remove support for this host CPU in
a future release if nobody volunteers to maintain it and to
provide a build host for our continuous integration setup.
configure has succeeded and you can continue to build, but
if you care about QEMU on this platform you should contact
us upstream at qemu-devel@nongnu.org.

It is unlikely the RISC-V port goes away in the next future releases :)

With this patch we can now build/use QEMU tools such qemu-img qemu-io
qemu-nbd ivshmem-client ivshmem-server scsi/qemu-pr-helper
qemu-bridge-helper qemu-keymap fsdev/virtfs-proxy-helper qemu-ga
vhost-user-scsi vhost-user-blk.

This is still not enough for system/user emulation, but this is
certainly an improvement, a starting point for the built system tests
(we have other crippled targets such TriCore).

Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 

Re: [Qemu-devel] [PULL 0/6] target-arm queue

[Peter Maydell]

On 30 July 2018 at 15:17, Peter Maydell  wrote:
> A set of small bugfixes for arm for 3.0; the "migration was
> broken" fixes for SMMUv3 and v7M NVIC with security extensions
> are the most significant.
>
> thanks
> -- PMM
>
> The following changes since commit 6d9dd5fb9d0e9f4a174f53a0e20a39fbe809c71e:
>
>   Merge remote-tracking branch 
> 'remotes/armbru/tags/pull-qobject-2018-07-27-v2' into staging (2018-07-30 
> 09:55:47 +0100)
>
> are available in the Git repository at:
>
>   git://git.linaro.org/people/pmaydell/qemu-arm.git 
> tags/pull-target-arm-20180730
>
> for you to fetch changes up to 0261fb805c00a6f97d143235e7b06b0906bdf898:
>
>   target/arm: Remove duplicate 'host' entry in '-cpu ?' output (2018-07-30 
> 15:07:08 +0100)
>
> 
> target-arm queue:
>  * arm/smmuv3: Fix broken VM state migration
>  * armv7m_nvic: Fix broken VM state migration
>  * hw/arm/sysbus-fdt: Fix assertion in copy_properties_from_host()
>  * hw/arm/iotkit: Fix IRQ number for timer1
>  * hw/misc/tz-mpc: Zero the LUT on initialization, not just reset
>  * target/arm: Remove duplicate 'host' entry in '-cpu ?' output
>
> 

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH v2 1/2] hw/arm: check fw_cfg return value before using it

[Peter Maydell]

On 25 July 2018 at 06:30, Hongbo Zhang  wrote:
> The fw_cfg value returned from fw_cfg_find() may be NULL, so check it
> before using.
>
> Signed-off-by: Hongbo Zhang 
> ---
>  hw/arm/boot.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index e09201c..43b217f 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -930,6 +930,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info 
> *info)
>  hwaddr entry;
>  static const ARMInsnFixup *primary_loader;
>  AddressSpace *as = arm_boot_address_space(cpu, info);
> +FWCfgState *fw_cfg;
>
>  /* CPU objects (unlike devices) are not automatically reset on system
>   * reset, so we must always register a handler to do so. If we're
> @@ -960,11 +961,10 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info 
> *info)
>  info->dtb_start = info->loader_start;
>  }
>
> -if (info->kernel_filename) {
> -FWCfgState *fw_cfg;
> +fw_cfg = fw_cfg_find();
> +if (info->kernel_filename && fw_cfg) {
>  bool try_decompressing_kernel;

This can only happen if:
 * the user provided a firmware blob
 * the user provided a -kernel option
 * the board does not have a fw_cfg so we can't pass the kernel to
   the firmware blob
right?

I think in this situation we should exit with a helpful error
message to the user (telling them that this board model does
not support using the -kernel option when a guest firmware image
is being booted), rather than silently ignoring the option.

thanks
-- PMM

[Qemu-devel] [PATCH] arm: Fix return code of arm_load_elf

[Adam Lackorzynski]

Use an int64_t as a return type to restore
the negative check for arm_load_as.

Signed-off-by: Adam Lackorzynski 
---
 hw/arm/boot.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index e09201cc97..ca9467e583 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -818,9 +818,9 @@ static int do_arm_linux_init(Object *obj, void *opaque)
 return 0;
 }
 
-static uint64_t arm_load_elf(struct arm_boot_info *info, uint64_t *pentry,
- uint64_t *lowaddr, uint64_t *highaddr,
- int elf_machine, AddressSpace *as)
+static int64_t arm_load_elf(struct arm_boot_info *info, uint64_t *pentry,
+uint64_t *lowaddr, uint64_t *highaddr,
+int elf_machine, AddressSpace *as)
 {
 bool elf_is64;
 union {
@@ -829,7 +829,7 @@ static uint64_t arm_load_elf(struct arm_boot_info *info, 
uint64_t *pentry,
 } elf_header;
 int data_swab = 0;
 bool big_endian;
-uint64_t ret = -1;
+int64_t ret = -1;
 Error *err = NULL;
 
 
-- 
2.18.0

Re: [Qemu-devel] [PATCH v3 6/7] loader: Implement .hex file loader

[Peter Maydell]

On 25 July 2018 at 09:59, Stefan Hajnoczi  wrote:
> From: Su Hang 
>
> This patch adds Intel Hexadecimal Object File format support to the
> loader.  The file format specification is available here:
> http://www.piclist.com/techref/fileext/hex/intel.htm
>
> This file format is often used with microcontrollers such as the
> micro:bit, Arduino, STM32, etc.  Users expect to be able to run them
> directly with qemu -kernel program.hex instead of converting to ELF or
> binary.

I'm still not convinced we want to add another random
special case only-works-on-one-architecture-and-some-boards
feature to the -kernel command line option.

Adding it to the "generic loader" device might be more plausible?
Or just get the user to create ELF files or plain binary files,
both of which we already handle.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v5 00/76] Add nanoMIPS support to QEMU

[Aleksandar Markovic]

> From: Aleksandar Markovic 
> 
> v4->v5:
> 
>   - merged series "Mips maintenance and misc fixes and improvements"
> and this one for easier handling (there are build dependencies)
>   - eliminated shadow variables from translate.c
>   - replaced shift/mask combination with extract32()
>   - added new function gen_op_addr_addi()
>   - added "fall through" comments at appropriate places
>   - eliminated micromips flag from I7200 definition
>   - numerous other enhancements originating from reviewer's
> comments
>   - some of the patches split into two or more for easier
> handling and review
>   - rebased to the latest code

I forgot to include an important item here:

  - reimplemented emulation of LLWP/SCWP pair

Aleksandar M.

[Qemu-devel] [PATCH v5 73/76] linux-user: Add support for nanoMIPS signal trampoline

[Aleksandar Markovic]

From: Aleksandar Rikalo 

Add signal trampoline support for nanoMIPS.

Signed-off-by: Aleksandar Rikalo 
Signed-off-by: Aleksandar Markovic 
Signed-off-by: Stefan Markovic 
---
 linux-user/mips/signal.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/linux-user/mips/signal.c b/linux-user/mips/signal.c
index ab66429..c6f5504 100644
--- a/linux-user/mips/signal.c
+++ b/linux-user/mips/signal.c
@@ -101,6 +101,17 @@ static inline int install_sigtramp(unsigned int *tramp,   
unsigned int syscall)
 {
 int err = 0;
 
+#if defined(TARGET_ABI_MIPSP32)
+uint16_t *tramp16 = (uint16_t *)tramp;
+/*
+ * li  $2, __NR__foo_sigreturn
+ * syscall 0
+ */
+ __put_user(0x6040 , tramp16 + 0);
+ __put_user(syscall, tramp16 + 1);
+ __put_user(0  , tramp16 + 2);
+ __put_user(0x1008 , tramp16 + 3);
+#else
 /*
  * Set up the return code ...
  *
@@ -110,7 +121,7 @@ static inline int install_sigtramp(unsigned int *tramp,   
unsigned int syscall)
 
 __put_user(0x2402 + syscall, tramp + 0);
 __put_user(0x000c  , tramp + 1);
-
+#endif
 return err;
 }
 
-- 
2.7.4

Re: [Qemu-devel] [PATCH v1 1/1] configure: Add RISC-V host support

[Alistair Francis]

On Sun, Jul 29, 2018 at 4:28 AM, Peter Maydell  wrote:
> On 28 July 2018 at 17:36, Richard Henderson
>  wrote:
>> On 07/27/2018 04:49 PM, Alistair Francis wrote:
>>> Allow QEMU to be built to run on a RISC-V host.
>>>
>>> QEMU does not yet have a RISC-V TCG or user mode target port, but
>>> running other architectures on RISC-V using TCI does work.
>>>
>>> Signed-off-by: Alistair Francis 
>>> ---
>>>  configure | 18 +-
>>>  1 file changed, 17 insertions(+), 1 deletion(-)
>>
>> This is ok as far as it goes.
>>
>> Even for TCI, you need some more.
>>
>> At minimum, see Michael Clark's branch changes to accel/tcg/user-exec.c for
>> host signal handling.  While you can run *-softmmu without this, none of
>> *-linux-user will work reliably.
>>
>> Another piece that even Michael Clark does not have is
>> linux-user/host/*/safe-syscall.S.
>
> It might be nice to complete the safe-syscall stuff for all hosts,
> and then remove the fallback that lets you build an unreliable
> linux-user binary without it...

At the moment though QEMU won't build with Linux user. So some work is
required just to get the build working.

Alistair

>
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH for-3.0 0/7] fix persistent bitmaps migration logic

[Michael Roth]

Quoting John Snow (2018-07-23 17:22:03)
> This is an updated version of Vladimir's proposal for fixing the
> handling around migration and persistent dirty bitmaps.

Are these still being considered for 3.0 rc3/rc4? 2.12.1 releases this week
and I'm not sure how badly these are needed.

> 
> Patches 1, 4, 6, and 7 update the testing for this feature.
> Patch 2 touches up an error message.
> Patch 3 removes dead code.
> Patch 5 contains the real fix.
> 
> v2:
>  - Add a new patch 4 as a prerequisite for what's now patch 5
>  - Rework the fix to be (hopefully) cleaner, see patch 5 notes
>  - Adjust error message in patch 2 (Eric)
>  - Adjust test logic slightly (patches 6, 7) to deal with changes
>in patch 5.
> 
> John Snow (2):
>   iotests: 169: actually test block migration
>   dirty-bitmaps: clean-up bitmaps loading and migration logic
> 
> Vladimir Sementsov-Ogievskiy (5):
>   iotests: 169: drop deprecated 'autoload' parameter
>   block/qcow2: improve error message in qcow2_inactivate
>   block/qcow2: drop dirty_bitmaps_loaded state variable
>   iotests: improve 169
>   iotests: 169: add cases for source vm resuming
> 
>  block.c|  4 ---
>  block/dirty-bitmap.c   | 20 
>  block/qcow2-bitmap.c   | 16 +
>  block/qcow2.c  | 26 ---
>  block/qcow2.h  |  1 -
>  include/block/dirty-bitmap.h   |  2 +-
>  migration/block-dirty-bitmap.c | 11 ---
>  tests/qemu-iotests/169 | 74 
> --
>  tests/qemu-iotests/169.out |  4 +--
>  9 files changed, 103 insertions(+), 55 deletions(-)
> 
> -- 
> 2.14.4
> 
> 

[Qemu-devel] [PATCH v3 1/1] ppc/pnv: Add model for Power8 PHB3 PCIe Host bridge

[Cédric Le Goater]

From: Benjamin Herrenschmidt 

This is a model of the PCIe Host Bridge (PHB3) found on a Power8
processor. It includes the PowerBus logic interface (PBCQ), IOMMU
support, a single PCIe Gen.3 Root Complex, and support for MSI and LSI
interrupt sources as found on a Power8 system using the XICS interrupt
controller.

The Power8 processor comes in different flavors: Venice, Murano,
Naple, each having a different number of PHBs. To make things simpler,
the PHB3 model provides 3 per chip. Some platforms, like the
Firestone, can also couple PHBs on the first chip to provide more
bandwidth but this is too specific to model in QEMU.

No default device layout is provided and PCI devices can be added on
any of the available PCIe Root Port (pcie.0 .. 2 of a Power8 chip)
with address 0x0 as the firwware (skiboot) only accepts a single
device per root port. To run a simple system with a network and a
storage adapters, use a command line options such as :

  -device e1000e,netdev=net0,mac=C0:FF:EE:00:00:02,bus=pcie.0,addr=0x0
  -netdev 
bridge,id=net0,helper=/usr/libexec/qemu-bridge-helper,br=virbr0,id=hostnet0

  -device megasas,id=scsi0,bus=pcie.1,addr=0x0
  -drive file=$disk,if=none,id=drive-scsi0-0-0-0,format=qcow2,cache=none
  -device 
scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=2

If more are needed, include a bridge.

Multi chip is supported, each chip adding its set of PHB3 controllers
and its PCI busses. The model doesn't emulate the EEH error handling
(and may never do).

Signed-off-by: Benjamin Herrenschmidt 
[clg: rewrote the QOM models
  misc fixes
  lots of love and care]
Signed-off-by: Cédric Le Goater 
---
 default-configs/ppc64-softmmu.mak   |1 +
 include/hw/pci-host/pnv_phb3.h  |  171 
 include/hw/pci-host/pnv_phb3_regs.h |  432 ++
 include/hw/ppc/pnv.h|   22 +
 include/hw/ppc/pnv_xscom.h  |9 +
 include/hw/ppc/xics.h   |1 +
 hw/intc/xics.c  |2 +-
 hw/pci-host/pnv_phb3.c  | 1146 +++
 hw/pci-host/pnv_phb3_msi.c  |  318 
 hw/pci-host/pnv_phb3_pbcq.c |  347 
 hw/ppc/pnv.c|   75 +-
 hw/ppc/pnv_xscom.c  |6 +-
 hw/pci-host/Makefile.objs   |1 +
 13 files changed, 2526 insertions(+), 5 deletions(-)
 create mode 100644 include/hw/pci-host/pnv_phb3.h
 create mode 100644 include/hw/pci-host/pnv_phb3_regs.h
 create mode 100644 hw/pci-host/pnv_phb3.c
 create mode 100644 hw/pci-host/pnv_phb3_msi.c
 create mode 100644 hw/pci-host/pnv_phb3_pbcq.c

diff --git a/default-configs/ppc64-softmmu.mak 
b/default-configs/ppc64-softmmu.mak
index b94af6c7c62a..deebba2b044a 100644
--- a/default-configs/ppc64-softmmu.mak
+++ b/default-configs/ppc64-softmmu.mak
@@ -9,6 +9,7 @@ CONFIG_IPMI=y
 CONFIG_IPMI_LOCAL=y
 CONFIG_IPMI_EXTERN=y
 CONFIG_ISA_IPMI_BT=y
+CONFIG_PCIE_PORT=y
 
 # For pSeries
 CONFIG_PSERIES=y
diff --git a/include/hw/pci-host/pnv_phb3.h b/include/hw/pci-host/pnv_phb3.h
new file mode 100644
index ..eb9efaf4e13e
--- /dev/null
+++ b/include/hw/pci-host/pnv_phb3.h
@@ -0,0 +1,171 @@
+/*
+ * QEMU PowerPC PowerNV PHB3 model
+ *
+ * Copyright (c) 2014-2018, IBM Corporation.
+ *
+ * This code is licensed under the GPL version 2 or later. See the
+ * COPYING file in the top-level directory.
+ */
+
+#ifndef PCI_HOST_PNV_PHB3_H
+#define PCI_HOST_PNV_PHB3_H
+
+#include "hw/pci/pcie_host.h"
+#include "hw/pci/pcie_port.h"
+#include "hw/ppc/xics.h"
+
+typedef struct PnvPHB3 PnvPHB3;
+typedef struct PnvChip PnvChip;
+
+/*
+ * PHB3 XICS Source for MSIs
+ */
+#define TYPE_PHB3_MSI "phb3-msi"
+#define PHB3_MSI(obj) OBJECT_CHECK(Phb3MsiState, (obj), TYPE_PHB3_MSI)
+
+#define PHB3_MAX_MSI 2048
+
+typedef struct Phb3MsiState {
+ICSState ics;
+
+PnvPHB3 *phb;
+uint64_t rba[PHB3_MAX_MSI / 64];
+uint32_t rba_sum;
+} Phb3MsiState;
+
+void pnv_phb3_msi_update_config(Phb3MsiState *msis, uint32_t base,
+uint32_t count);
+void pnv_phb3_msi_send(Phb3MsiState *msis, uint64_t addr, uint16_t data,
+   int32_t dev_pe);
+void pnv_phb3_msi_ffi(Phb3MsiState *msis, uint64_t val);
+
+
+/* We have one such address space wrapper per possible device
+ * under the PHB since they need to be assigned statically at
+ * qemu device creation time. The relationship to a PE is done
+ * later dynamically. This means we can potentially create a lot
+ * of these guys. Q35 stores them as some kind of radix tree but
+ * we never really need to do fast lookups so instead we simply
+ * keep a QLIST of them for now, we can add the radix if needed
+ * later on.
+ *
+ * We do cache the PE number to speed things up a bit though.
+ */
+typedef struct PnvPhb3DMASpace {
+PCIBus *bus;
+uint8_t devfn;
+int pe_num; /* Cached PE number */
+#define PHB_INVALID_PE (-1)
+PnvPHB3 *phb;
+AddressSpace dma_as;
+

[Qemu-devel] [PATCH v5 71/76] linux-user: Add target_elf.h header for nanoMIPS

[Aleksandar Markovic]

From: Dimitrije Nikolic 

This header includes common elf header, and adds cpu_get_model()
function.

Signed-off-by: Aleksandar Rikalo 
Signed-off-by: Aleksandar Markovic 
Signed-off-by: Stefan Markovic 
---
 linux-user/nanomips/target_elf.h | 14 ++
 1 file changed, 14 insertions(+)
 create mode 100644 linux-user/nanomips/target_elf.h

diff --git a/linux-user/nanomips/target_elf.h b/linux-user/nanomips/target_elf.h
new file mode 100644
index 000..ca68dab
--- /dev/null
+++ b/linux-user/nanomips/target_elf.h
@@ -0,0 +1,14 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation, or (at your option) any
+ * later version. See the COPYING file in the top-level directory.
+ */
+
+#ifndef NANOMIPS_TARGET_ELF_H
+#define NANOMIPS_TARGET_ELF_H
+static inline const char *cpu_get_model(uint32_t eflags)
+{
+return "I7200";
+}
+#endif
-- 
2.7.4

[Qemu-devel] [PATCH v5 70/76] linux-user: Add target_structs.h header for nanoMIPS

[Aleksandar Markovic]

From: Dimitrije Nikolic 

Add target_structs.h header for nanoMIPS, that redirects to the
corresponding MIPS header.

Signed-off-by: Aleksandar Rikalo 
Signed-off-by: Aleksandar Markovic 
Signed-off-by: Stefan Markovic 
---
 linux-user/nanomips/target_structs.h | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 linux-user/nanomips/target_structs.h

diff --git a/linux-user/nanomips/target_structs.h 
b/linux-user/nanomips/target_structs.h
new file mode 100644
index 000..cc6c6ea
--- /dev/null
+++ b/linux-user/nanomips/target_structs.h
@@ -0,0 +1 @@
+#include "../mips/target_structs.h"
-- 
2.7.4

Re: [Qemu-devel] [PATCH v5 00/20] arm_gic: add virtualization extensions support

[Peter Maydell]

On 27 July 2018 at 10:54, Luc Michel  wrote:
> v5:
>   - Patch 7: dropped unreachable 'return NULL' [Peter]
>   - Patch 12: reworked the way invalid vIRQ are handled. The
> specification being uncleared, I used real hardware to check our
> previous hypothesises. See commit message for the adopted behaviour.
> The main difference is that when EOIMode is true (EOI split is
> enabled), a write to V_EOIR never increments H_HCR.EOICount. [Peter]
>   - Patch 14-15: fixed commit message regarding mirrored regions. [Peter

Thanks (and especially for taking the time to check the hardware
behaviour where the spec was unclear).

I've applied this series to my target-arm.for-3.1 branch ready
for when we reopen master after the 3.0 release.

thanks
-- PMM

[Qemu-devel] [PATCH v5 75/76] linux-user: Amend sigaction syscall support for nanoMIPS

[Aleksandar Markovic]

From: Aleksandar Rikalo 

Amend sigaction syscall support for nanoMIPS. This must be done
since nanoMIPS' signal handling is different than MIPS' signal
handling.

Signed-off-by: Aleksandar Rikalo 
Signed-off-by: Aleksandar Markovic 
Signed-off-by: Stefan Markovic 
---
 linux-user/syscall.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 3d57966..bced9b8 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8825,7 +8825,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 old_act->sa_flags = oact.sa_flags;
 unlock_user_struct(old_act, arg3, 1);
 }
-#elif defined(TARGET_MIPS)
+#elif defined(TARGET_MIPS) && !defined(TARGET_NANOMIPS)
struct target_sigaction act, oact, *pact, *old_act;
 
if (arg2) {
-- 
2.7.4

Re: [Qemu-devel] [PATCH v3 4/7] target/arm: add "cortex-m0" CPU model

[Peter Maydell]

On 25 July 2018 at 09:59, Stefan Hajnoczi  wrote:
> Define a "cortex-m0" ARMv6-M CPU model.
>
> Most of the register reset values set by other CPU models are not
> relevant for the cut-down ARMv6-M architecture.
>
> Signed-off-by: Stefan Hajnoczi 
> ---
>  target/arm/cpu.c | 11 +++
>  1 file changed, 11 insertions(+)
>
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 3848ef46aa..7e477c0d23 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1255,6 +1255,15 @@ static void arm11mpcore_initfn(Object *obj)
>  cpu->reset_auxcr = 1;
>  }
>
> +static void cortex_m0_initfn(Object *obj)
> +{
> +ARMCPU *cpu = ARM_CPU(obj);
> +set_feature(>env, ARM_FEATURE_V6);
> +set_feature(>env, ARM_FEATURE_M);
> +
> +cpu->midr = 0x410cc200;
> +}

We have all the patches for turning off not-v6M bits of
behaviour either in master or in target-arm.for-3.1 already,
right?

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-3.0 0/7] fix persistent bitmaps migration logic

[John Snow]

On 07/30/2018 01:06 PM, Michael Roth wrote:
> Quoting John Snow (2018-07-23 17:22:03)
>> This is an updated version of Vladimir's proposal for fixing the
>> handling around migration and persistent dirty bitmaps.
> 
> Are these still being considered for 3.0 rc3/rc4? 2.12.1 releases this week
> and I'm not sure how badly these are needed.
> 

Good question. I'd still LIKE them in 3.0, but further fixes are
actually still needed, and I'm working on this right now.

I think you'll find that you might have trouble applying them to 2.12.1
without a fairly long trail of prerequisites, so it might not be easily
plausible.

>>
>> Patches 1, 4, 6, and 7 update the testing for this feature.
>> Patch 2 touches up an error message.
>> Patch 3 removes dead code.
>> Patch 5 contains the real fix.
>>
>> v2:
>>  - Add a new patch 4 as a prerequisite for what's now patch 5
>>  - Rework the fix to be (hopefully) cleaner, see patch 5 notes
>>  - Adjust error message in patch 2 (Eric)
>>  - Adjust test logic slightly (patches 6, 7) to deal with changes
>>in patch 5.
>>
>> John Snow (2):
>>   iotests: 169: actually test block migration
>>   dirty-bitmaps: clean-up bitmaps loading and migration logic
>>
>> Vladimir Sementsov-Ogievskiy (5):
>>   iotests: 169: drop deprecated 'autoload' parameter
>>   block/qcow2: improve error message in qcow2_inactivate
>>   block/qcow2: drop dirty_bitmaps_loaded state variable
>>   iotests: improve 169
>>   iotests: 169: add cases for source vm resuming
>>
>>  block.c|  4 ---
>>  block/dirty-bitmap.c   | 20 
>>  block/qcow2-bitmap.c   | 16 +
>>  block/qcow2.c  | 26 ---
>>  block/qcow2.h  |  1 -
>>  include/block/dirty-bitmap.h   |  2 +-
>>  migration/block-dirty-bitmap.c | 11 ---
>>  tests/qemu-iotests/169 | 74 
>> --
>>  tests/qemu-iotests/169.out |  4 +--
>>  9 files changed, 103 insertions(+), 55 deletions(-)
>>
>> -- 
>> 2.14.4
>>
>>

Re: [Qemu-devel] [RFC PATCH 0/3] Balloon inhibit enhancements

[Michael S. Tsirkin]

On Mon, Jul 30, 2018 at 10:42:05AM -0600, Alex Williamson wrote:
> On Mon, 30 Jul 2018 18:49:58 +0300
> "Michael S. Tsirkin"  wrote:
> 
> > On Mon, Jul 30, 2018 at 09:01:37AM -0600, Alex Williamson wrote:
> > > > > but I don't think it can be done
> > > > > atomically with respect to inflight DMA of a physical device where we
> > > > > cannot halt the device without interfering with its state.
> > > > 
> > > > Guests never add pages to the balloon if they are under DMA,
> > > > so that's fine - there's never an in-flight DMA, if
> > > > there is guest is buggy and it's ok to crash it.  
> > > 
> > > It's not the ballooned page that I'm trying to note, it's the entire
> > > remainder of the SubRegion which needs to be unmapped to remove that
> > > one page.  It's more compatible from an IOMMU perspective in that we're
> > > only unmapping with the same granularity with which we mapped, but it's
> > > incompatible with inflight DMA as we have no idea what DMA targets may
> > > reside within the remainder of that mapping while it's temporarily
> > > unmapped.  
> > 
> > I see. Yes you need to be careful to replace the host IOMMU PTE
> > atomically. Same applies to vIOMMU though - if guest changes
> > a PTE atomically host should do the same.
> 
> I'm not sure the hardware supports atomic updates in these cases and
> therefore I don't think the vIOMMU does either.  Thanks,
> 
> Alex

Interesting. What makes you think simply writing into PTE
then flushing the cache isn't atomic?

-- 
MST

[Qemu-devel] [PATCH v5 64/76] linux-user: Add termbits.h header for nanoMIPS

[Aleksandar Markovic]

From: Aleksandar Rikalo 

Add termbits.h header for nanoMIPS. Reuse MIPS' termbits.h as
the functionalities are almost identical.

Signed-off-by: Aleksandar Rikalo 
Signed-off-by: Aleksandar Markovic 
Signed-off-by: Stefan Markovic 
---
 linux-user/mips/termbits.h | 4 
 linux-user/nanomips/termbits.h | 1 +
 2 files changed, 5 insertions(+)
 create mode 100644 linux-user/nanomips/termbits.h

diff --git a/linux-user/mips/termbits.h b/linux-user/mips/termbits.h
index 49a72c5..c7254f4 100644
--- a/linux-user/mips/termbits.h
+++ b/linux-user/mips/termbits.h
@@ -1,6 +1,10 @@
 /* from asm/termbits.h */
 
+#ifdef TARGET_NANOMIPS
+#define TARGET_NCCS 32
+#else
 #define TARGET_NCCS 23
+#endif
 
 struct target_termios {
 unsigned int c_iflag;   /* input mode flags */
diff --git a/linux-user/nanomips/termbits.h b/linux-user/nanomips/termbits.h
new file mode 100644
index 000..ea4e962
--- /dev/null
+++ b/linux-user/nanomips/termbits.h
@@ -0,0 +1 @@
+#include "../mips/termbits.h"
-- 
2.7.4

Re: [Qemu-devel] [PATCH v5 08/76] elf: Add ELF flags for MIPS machine variants

[Laurent Vivier]

Le 30/07/2018 à 18:11, Aleksandar Markovic a écrit :
> From: Aleksandar Markovic 
> 
> Add MIPS machine variants ELF flags so that the emulation behavior
> can be adjusted if needed.
> 
> Signed-off-by: Aleksandar Markovic 
> Signed-off-by: Stefan Markovic 
> Reviewed-by: Richard Henderson 
> ---
>  include/elf.h | 23 +++
>  1 file changed, 23 insertions(+)

Reviewed-by: Laurent Vivier 

Re: [Qemu-devel] [PATCH 1/1] s390x/sclp: fix maxram calculation

[Michael Roth]

Quoting Christian Borntraeger (2018-07-30 10:31:12)
> Are we still able to get things into 2.12.1 or are we too late?

Freeze is EOD today, but I can grab them if they hit master/rc3 tomorrow.

> 
> 
> On 07/30/2018 04:09 PM, Christian Borntraeger wrote:
> > We clamp down ram_size to match the sclp increment size. We do
> > not do the same for maxram_size, which means for large guests
> > with some sizes (e.g. -m 5) maxram_size differs from ram_size.
> > This can break other code (e.g. CMMA migration) which uses maxram_size
> > to calculate the number of pages and then throws some errors.
> > 
> > Fixes: 82fab5c5b90e468f3e9d54c ("s390x/sclp: remove memory hotplug support")
> > Signed-off-by: Christian Borntraeger 
> > CC: qemu-sta...@nongnu.org
> > CC: David Hildenbrand 
> > ---
> >  hw/s390x/sclp.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> > index bd2a024..4510a80 100644
> > --- a/hw/s390x/sclp.c
> > +++ b/hw/s390x/sclp.c
> > @@ -320,6 +320,7 @@ static void sclp_memory_init(SCLPDevice *sclp)
> >  initial_mem = initial_mem >> increment_size << increment_size;
> >  
> >  machine->ram_size = initial_mem;
> > +machine->maxram_size = initial_mem;
> >  /* let's propagate the changed ram size into the global variable. */
> >  ram_size = initial_mem;
> >  }
> > 
> 

Re: [Qemu-devel] [PATCH v3 1/7] hw/arm: rename armv7m_load_kernel()

[Peter Maydell]

On 25 July 2018 at 09:59, Stefan Hajnoczi  wrote:
> ARMv6-M and ARMv8-M need the same kernel loading functionality as
> ARMv7-M.  Rename armv7m_load_kernel() to arm_m_profile_load_kernel() so
> it's clear that this function isn't specific to ARMv7-M.
>
> Signed-off-by: Stefan Hajnoczi 
> ---
>  hw/arm/Makefile.objs|  1 +
>  include/hw/arm/arm.h| 11 +++--
>  hw/arm/arm-m-profile.c  | 81 +
>  hw/arm/armv7m.c | 60 
>  hw/arm/mps2-tz.c|  3 +-
>  hw/arm/mps2.c   |  4 +-
>  hw/arm/msf2-som.c   |  4 +-
>  hw/arm/netduino2.c  |  4 +-
>  hw/arm/stellaris.c  |  3 +-
>  default-configs/arm-softmmu.mak |  1 +
>  10 files changed, 99 insertions(+), 73 deletions(-)
>  create mode 100644 hw/arm/arm-m-profile.c

Another rename that shows up in the patch as "delete the old
one and have a copy somewhere else". Maybe this can be fixed
with suitable git rename-detection options?

thanks
-- PMM

Re: [Qemu-devel] [Qemu-arm] [PATCH v3 4/7] target/arm: add "cortex-m0" CPU model

[Peter Maydell]

On 27 July 2018 at 06:26, Philippe Mathieu-Daudé  wrote:
> Hi Stefan,
>
> On 07/25/2018 05:59 AM, Stefan Hajnoczi wrote:
>> Define a "cortex-m0" ARMv6-M CPU model.
>>
>> Most of the register reset values set by other CPU models are not
>> relevant for the cut-down ARMv6-M architecture.
>>
>> Signed-off-by: Stefan Hajnoczi 
>> ---
>>  target/arm/cpu.c | 11 +++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
>> index 3848ef46aa..7e477c0d23 100644
>> --- a/target/arm/cpu.c
>> +++ b/target/arm/cpu.c
>> @@ -1255,6 +1255,15 @@ static void arm11mpcore_initfn(Object *obj)
>>  cpu->reset_auxcr = 1;
>>  }
>>
>> +static void cortex_m0_initfn(Object *obj)
>> +{
>> +ARMCPU *cpu = ARM_CPU(obj);
>> +set_feature(>env, ARM_FEATURE_V6);
>> +set_feature(>env, ARM_FEATURE_M);
>
> What about ARM_FEATURE_THUMB2 (T32)?

No, the M0 doesn't have Thumb2. It has Thumb1 plus a tiny set
of 32-bit instructions (which we deal with specially in translate.c).

(As it happens we generally can't get to the checks on the THUMB2
feature for a v6M core, so I think but have not checked thoroughly
that it would make no difference to QEMU's behaviour whether the
feature bit was set or not.)

> Peter: Since the M0 (optionally?) supports 32x32 multiply, should this
> cpu use the ARM_FEATURE_THUMB_DSP feature? Else this might trigger an
> 'Undefined Instruction' in disas_thumb2_insn().

ARM_FEATURE_THUMB_DSP only enables checks in disas_thumb2_insn()
(and 32x32->32 multiply is not one of the insns it gates).
The only insns in that function that a v6M core can execute are
msr, mrs, dsb, dmb, isb and bl, none of which are affected by that
feature switch.

> And what about optional ARM_FEATURE_PMSA?
> Oh this would be cortex_m0plus_initfn() for "cortex-m0-plus", ok.

Yep, plain M0 has no MPU (and so we're postponing the work
of implementing the v6M PMSA, which IIRC isn't quite the
same as the v7M one).

thanks
-- PMM

[Qemu-devel] [PATCH v5 65/76] linux-user: Update syscall_defs.h header for nanoMIPS

[Aleksandar Markovic]

From: Aleksandar Markovic 

Update constants and structures related to syscall support in
nanoMIPS.

Signed-off-by: Aleksandar Rikalo 
Signed-off-by: Aleksandar Markovic 
Signed-off-by: Stefan Markovic 
---
 linux-user/syscall_defs.h | 57 ++-
 1 file changed, 52 insertions(+), 5 deletions(-)

diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 40bb60e..abf94b8 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -374,7 +374,7 @@ struct target_dirent64 {
 #define TARGET_SIG_IGN ((abi_long)1)   /* ignore signal */
 #define TARGET_SIG_ERR ((abi_long)-1)  /* error return from signal */
 
-#ifdef TARGET_MIPS
+#if defined(TARGET_MIPS) && !defined(TARGET_NANOMIPS)
 #define TARGET_NSIG   128
 #else
 #define TARGET_NSIG   64
@@ -445,7 +445,7 @@ struct target_sigaction {
 target_sigset_t sa_mask;
 abi_ulong sa_restorer;
 };
-#elif defined(TARGET_MIPS)
+#elif defined(TARGET_MIPS) && !defined(TARGET_NANOMIPS)
 struct target_sigaction {
uint32_tsa_flags;
 #if defined(TARGET_ABI_MIPSN32)
@@ -459,6 +459,14 @@ struct target_sigaction {
 abi_ulong sa_restorer;
 #endif
 };
+#elif defined(TARGET_NANOMIPS)
+struct target_sigaction {
+abi_ulong _sa_handler;
+abi_uint sa_flags;
+target_sigset_t sa_mask;
+abi_ulong sa_restorer;
+};
+
 #else
 struct target_old_sigaction {
 abi_ulong _sa_handler;
@@ -537,7 +545,7 @@ typedef struct {
 #define QEMU_SI_RT 5
 
 typedef struct target_siginfo {
-#ifdef TARGET_MIPS
+#if defined(TARGET_MIPS) && !defined(TARGET_NANOMIPS)
int si_signo;
int si_code;
int si_errno;
@@ -665,13 +673,16 @@ struct target_rlimit {
 
 #if defined(TARGET_ALPHA)
 #define TARGET_RLIM_INFINITY   0x7fffull
-#elif defined(TARGET_MIPS) || (defined(TARGET_SPARC) && TARGET_ABI_BITS == 32)
+#elif (defined(TARGET_MIPS) && !defined(TARGET_NANOMIPS)) \
+  || (defined(TARGET_SPARC) && TARGET_ABI_BITS == 32)
 #define TARGET_RLIM_INFINITY   0x7fffUL
+#elif defined(TARGET_NANOMIPS)
+#define TARGET_RLIM_INFINITY0x76ffeec4UL
 #else
 #define TARGET_RLIM_INFINITY   ((abi_ulong)-1)
 #endif
 
-#if defined(TARGET_MIPS)
+#if defined(TARGET_MIPS) && !defined(TARGET_NANOMIPS)
 #define TARGET_RLIMIT_CPU  0
 #define TARGET_RLIMIT_FSIZE1
 #define TARGET_RLIMIT_DATA 2
@@ -687,6 +698,22 @@ struct target_rlimit {
 #define TARGET_RLIMIT_MSGQUEUE 12
 #define TARGET_RLIMIT_NICE 13
 #define TARGET_RLIMIT_RTPRIO   14
+#elif defined(TARGET_NANOMIPS)
+#define TARGET_RLIMIT_CPU   0
+#define TARGET_RLIMIT_FSIZE 1
+#define TARGET_RLIMIT_DATA  2
+#define TARGET_RLIMIT_STACK 3
+#define TARGET_RLIMIT_CORE  4
+#define TARGET_RLIMIT_RSS   5
+#define TARGET_RLIMIT_NPROC 6
+#define TARGET_RLIMIT_NOFILE7
+#define TARGET_RLIMIT_MEMLOCK   8
+#define TARGET_RLIMIT_AS9
+#define TARGET_RLIMIT_LOCKS 10
+#define TARGET_RLIMIT_SIGPENDING11
+#define TARGET_RLIMIT_MSGQUEUE  12
+#define TARGET_RLIMIT_NICE  13
+#define TARGET_RLIMIT_RTPRIO14
 #else
 #define TARGET_RLIMIT_CPU  0
 #define TARGET_RLIMIT_FSIZE1
@@ -1657,6 +1684,10 @@ struct target_stat64 {
int64_t st_blocks;
 };
 
+#elif defined(TARGET_ABI_MIPSP32)
+
+/* No struct stat and struct stat64 structures */
+
 #elif defined(TARGET_ALPHA)
 
 struct target_stat {
@@ -2009,6 +2040,22 @@ struct target_statfs {
int32_t f_flags;
int32_t f_spare[5];
 };
+#elif defined(TARGET_ABI_MIPSP32)
+struct target_statfs {
+abi_longf_type;
+abi_longf_bsize;
+abi_longf_blocks;
+abi_longf_bfree;
+abi_longf_bavail;
+abi_longf_files;
+abi_longf_ffree;
+
+/* Linux specials */
+target_fsid_t f_fsid;
+abi_longf_namelen;
+abi_llong   f_frsize;   /* Fragment size - unsupported */
+abi_longf_spare[6];
+};
 #else
 struct target_statfs {
abi_longf_type;
-- 
2.7.4

[Qemu-devel] [PATCH v5 53/76] elf: Add nanoMIPS specific variations in ELF header fields

[Aleksandar Markovic]

From: Aleksandar Rikalo 

Add nanoMIPS-related values in ELF header fields as specified in
nanoMIPS' "ELF ABI Supplement".

Acked-by: Richard Henderson 
Signed-off-by: Aleksandar Markovic 
Signed-off-by: Stefan Markovic 
---
 include/elf.h | 20 
 1 file changed, 20 insertions(+)

diff --git a/include/elf.h b/include/elf.h
index 2c4fe7a..fff5967 100644
--- a/include/elf.h
+++ b/include/elf.h
@@ -62,6 +62,24 @@ typedef int64_t  Elf64_Sxword;
 #define EF_MIPS_NAN2008   0x0400
 #define EF_MIPS_ARCH  0xf000
 
+/* nanoMIPS architecture bits, EF_NANOMIPS_ARCH */
+#define EF_NANOMIPS_ARCH_32R6 0x  /* 32-bit nanoMIPS Release 6 ISA   */
+#define EF_NANOMIPS_ARCH_64R6 0x1000  /* 62-bit nanoMIPS Release 6 ISA   */
+
+/* nanoMIPS ABI bits, EF_NANOMIPS_ABI */
+#define EF_NANOMIPS_ABI_P32   0x1000  /* 32-bit nanoMIPS ABI */
+#define EF_NANOMIPS_ABI_P64   0x2000  /* 64-bit nanoMIPS ABI */
+
+/* nanoMIPS processor specific flags, e_flags */
+#define EF_NANOMIPS_LINKRELAX 0x0001  /* Link-time relaxation*/
+#define EF_NANOMIPS_PIC   0x0002  /* Position independant code   */
+#define EF_NANOMIPS_32BITMODE 0x0004  /* 32-bit object for 64-bit arch.  */
+#define EF_NANOMIPS_PID   0x0008  /* Position independant data   */
+#define EF_NANOMIPS_PCREL 0x0010  /* PC-relative mode*/
+#define EF_NANOMIPS_ABI   0xf000  /* nanoMIPS ABI*/
+#define EF_NANOMIPS_MACH  0x00ff  /* Machine variant */
+#define EF_NANOMIPS_ARCH  0xf000  /* nanoMIPS architecture   */
+
 /* MIPS machine variant */
 #define EF_MIPS_MACH_NONE 0x  /* A standard MIPS implementation  */
 #define EF_MIPS_MACH_3900 0x0081  /* Toshiba R3900   */
@@ -143,6 +161,8 @@ typedef int64_t  Elf64_Sxword;
 
 #define EM_RISCV243 /* RISC-V */
 
+#define EM_NANOMIPS 249 /* Wave Computing nanoMIPS */
+
 /*
  * This is an interim value that we will use until the committee comes
  * up with a final number.
-- 
2.7.4

Re: [Qemu-devel] [PATCH v3 2/7] hw/arm: rename TYPE_ARMV7M to TYPE_ARM_M_PROFILE

[Peter Maydell]

On 25 July 2018 at 09:59, Stefan Hajnoczi  wrote:
> The TYPE_ARMV7M class is really a container for an ARM M Profile CPU,
> NVIC, and related pieces.  It can also be used for ARMv6-M and ARMv8-M.
> Rename the class since it is not exclusive to ARMv7-M.
>
> Signed-off-by: Stefan Hajnoczi 
> ---
>  hw/arm/Makefile.objs |   1 -
>  include/hw/arm/{armv7m.h => arm-m-profile.h} |  37 ++-
>  include/hw/arm/iotkit.h  |   4 +-
>  include/hw/arm/msf2-soc.h|   4 +-
>  include/hw/arm/stm32f205_soc.h   |   4 +-
>  hw/arm/arm-m-profile.c   | 271 +
>  hw/arm/armv7m.c  | 290 ---
>  hw/arm/iotkit.c  |   2 +-
>  hw/arm/mps2-tz.c |   1 -
>  hw/arm/mps2.c|   6 +-
>  hw/arm/msf2-soc.c|   2 +-
>  hw/arm/stellaris.c   |   4 +-
>  hw/arm/stm32f205_soc.c   |   2 +-
>  13 files changed, 313 insertions(+), 315 deletions(-)
>  rename include/hw/arm/{armv7m.h => arm-m-profile.h} (65%)
>  delete mode 100644 hw/arm/armv7m.c

It would be nice if this rename showed up in the patch as
a rename, rather than a "delete everything and have a new one"...
Reviewing this is going to be painful enough to make me wonder
whether it really matters that we call this "m-profile" rather
than "v7m" (we have a lot of places that use 'v7m' as a somewhat
inaccurate name for "M profile" anyway, including a bunch
of new code I wrote that's really v8m related.)

thanks
-- PMM

[Qemu-devel] [PATCH v5 45/76] target/mips: Implement emulation of nanoMIPS LLWP/SCWP pair

[Aleksandar Markovic]

From: Aleksandar Rikalo 

Implement nanoMIPS LLWP and SCWP instruction pair.

Signed-off-by: Dimitrije Nikolic 
Signed-off-by: Aleksandar Markovic 
Signed-off-by: Stefan Markovic 
---
 linux-user/mips/cpu_loop.c | 25 +++---
 target/mips/cpu.h  |  2 ++
 target/mips/helper.h   |  2 ++
 target/mips/op_helper.c| 35 
 target/mips/translate.c| 81 ++
 5 files changed, 140 insertions(+), 5 deletions(-)

diff --git a/linux-user/mips/cpu_loop.c b/linux-user/mips/cpu_loop.c
index 084ad6a..1d3dc9e 100644
--- a/linux-user/mips/cpu_loop.c
+++ b/linux-user/mips/cpu_loop.c
@@ -397,10 +397,13 @@ static int do_store_exclusive(CPUMIPSState *env)
 target_ulong addr;
 target_ulong page_addr;
 target_ulong val;
+uint32_t val_wp = 0;
+uint32_t llnewval_wp = 0;
 int flags;
 int segv = 0;
 int reg;
 int d;
+int wp;
 
 addr = env->lladdr;
 page_addr = addr & TARGET_PAGE_MASK;
@@ -412,19 +415,31 @@ static int do_store_exclusive(CPUMIPSState *env)
 } else {
 reg = env->llreg & 0x1f;
 d = (env->llreg & 0x20) != 0;
-if (d) {
-segv = get_user_s64(val, addr);
+wp = (env->llreg & 0x40) != 0;
+if (!wp) {
+if (d) {
+segv = get_user_s64(val, addr);
+} else {
+segv = get_user_s32(val, addr);
+}
 } else {
 segv = get_user_s32(val, addr);
+segv |= get_user_s32(val_wp, addr);
+llnewval_wp = env->llnewval_wp;
 }
 if (!segv) {
-if (val != env->llval) {
+if (val != env->llval && val_wp == llnewval_wp) {
 env->active_tc.gpr[reg] = 0;
 } else {
-if (d) {
-segv = put_user_u64(env->llnewval, addr);
+if (!wp) {
+if (d) {
+segv = put_user_u64(env->llnewval, addr);
+} else {
+segv = put_user_u32(env->llnewval, addr);
+}
 } else {
 segv = put_user_u32(env->llnewval, addr);
+segv |= put_user_u32(env->llnewval_wp, addr + 4);
 }
 if (!segv) {
 env->active_tc.gpr[reg] = 1;
diff --git a/target/mips/cpu.h b/target/mips/cpu.h
index 009202c..28af4d1 100644
--- a/target/mips/cpu.h
+++ b/target/mips/cpu.h
@@ -506,6 +506,8 @@ struct CPUMIPSState {
 uint64_t lladdr;
 target_ulong llval;
 target_ulong llnewval;
+uint64_t llval_wp;
+uint32_t llnewval_wp;
 target_ulong llreg;
 uint64_t CP0_LLAddr_rw_bitmask;
 int CP0_LLAddr_shift;
diff --git a/target/mips/helper.h b/target/mips/helper.h
index b2a780a..deca307 100644
--- a/target/mips/helper.h
+++ b/target/mips/helper.h
@@ -14,6 +14,8 @@ DEF_HELPER_4(swr, void, env, tl, tl, int)
 #ifndef CONFIG_USER_ONLY
 DEF_HELPER_3(ll, tl, env, tl, int)
 DEF_HELPER_4(sc, tl, env, tl, tl, int)
+DEF_HELPER_5(llwp, void, env, tl, i32, i32, i32)
+DEF_HELPER_4(scwp, tl, env, tl, i64, int)
 #ifdef TARGET_MIPS64
 DEF_HELPER_3(lld, tl, env, tl, int)
 DEF_HELPER_4(scd, tl, env, tl, tl, int)
diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
index b3eef9f..cb83b6d 100644
--- a/target/mips/op_helper.c
+++ b/target/mips/op_helper.c
@@ -380,6 +380,19 @@ HELPER_LD_ATOMIC(lld, ld, 0x7)
 #endif
 #undef HELPER_LD_ATOMIC
 
+void helper_llwp(CPUMIPSState *env, target_ulong addr, uint32_t reg1,
+ uint32_t reg2, uint32_t mem_idx)
+{
+if (addr & 0x7) {
+env->CP0_BadVAddr = addr;
+do_raise_exception(env, EXCP_AdEL, GETPC());
+}
+env->lladdr = do_translate_address(env, addr, 0, GETPC());
+env->active_tc.gpr[reg1] = env->llval = do_lw(env, addr, mem_idx, GETPC());
+env->active_tc.gpr[reg2] = env->llval_wp = do_lw(env, addr + 4, mem_idx,
+ GETPC());
+}
+
 #define HELPER_ST_ATOMIC(name, ld_insn, st_insn, almask)  \
 target_ulong helper_##name(CPUMIPSState *env, target_ulong arg1,  \
target_ulong arg2, int mem_idx)\
@@ -406,6 +419,28 @@ HELPER_ST_ATOMIC(sc, lw, sw, 0x3)
 HELPER_ST_ATOMIC(scd, ld, sd, 0x7)
 #endif
 #undef HELPER_ST_ATOMIC
+
+target_ulong helper_scwp(CPUMIPSState *env, target_ulong addr,
+ uint64_t data, int mem_idx)
+{
+uint32_t tmp;
+uint32_t tmp2;
+
+if (addr & 0x7) {
+env->CP0_BadVAddr = addr;
+do_raise_exception(env, EXCP_AdES, GETPC());
+}
+if (do_translate_address(env, addr, 1, GETPC()) == env->lladdr) {
+tmp = do_lw(env, addr, mem_idx, GETPC());
+tmp2 = do_lw(env, addr + 4, mem_idx, GETPC());
+if (tmp == env->llval && tmp2 == env->llval_wp) {
+do_sw(env, addr, (uint32_t) data, mem_idx, 

Re: [Qemu-devel] [PATCH v3 3/7] hw/arm: make bitbanded IO optional on ARM M Profile

[Peter Maydell]

On 25 July 2018 at 09:59, Stefan Hajnoczi  wrote:
> Some ARM CPUs have bitbanded IO, a memory region that allows convenient
> bit access via 32-bit memory loads/stores.  This eliminates the need for
> read-modify-update instruction sequences.
>
> This patch makes this optional feature a ARMMProfile qdev property,
> allowing boards to choose whether they want bitbanding or not.
>
> Status of boards:
>  * iotkit (Cortex M33), no bitband
>  * mps2 (Cortex M3), bitband
>  * msf2 (Cortex M3), bitband
>  * stellaris (Cortex M3), bitband
>  * stm32f205 (Cortex M3), bitband
>
> Signed-off-by: Stefan Hajnoczi 

Reviewed-by: Peter Maydell 

thanks
-- PMM

[Qemu-devel] [PATCH v5 72/76] linux-user: Add signal.c for nanoMIPS

[Aleksandar Markovic]

From: Dimitrije Nikolic 

Add signal.c as a dredirection of regular mips' signal.c, but also
amend regular mips' signal.c. this is done to avoid the duplication
of large pieces of code.

Signed-off-by: Aleksandar Rikalo 
Signed-off-by: Aleksandar Markovic 
Signed-off-by: Stefan Markovic 
---
 linux-user/mips/signal.c | 25 -
 linux-user/nanomips/signal.c |  1 +
 2 files changed, 21 insertions(+), 5 deletions(-)
 create mode 100644 linux-user/nanomips/signal.c

diff --git a/linux-user/mips/signal.c b/linux-user/mips/signal.c
index 6aa303e..ab66429 100644
--- a/linux-user/mips/signal.c
+++ b/linux-user/mips/signal.c
@@ -21,7 +21,15 @@
 #include "signal-common.h"
 #include "linux-user/trace.h"
 
-# if defined(TARGET_ABI_MIPSO32)
+#if defined(TARGET_ABI_MIPSP32)
+struct target_sigcontext {
+uint64_t sc_regs[32];
+uint64_t sc_pc;
+uint32_t sc_used_math;
+uint32_t sc_reserved;
+};
+#define TARGET_ALMASK  (~15)
+#elif defined(TARGET_ABI_MIPSO32)
 struct target_sigcontext {
 uint32_t   sc_regmask; /* Unused */
 uint32_t   sc_status;
@@ -43,6 +51,7 @@ struct target_sigcontext {
 target_ulong   sc_hi3;
 target_ulong   sc_lo3;
 };
+#define TARGET_ALMASK  (~7)
 # else /* N32 || N64 */
 struct target_sigcontext {
 uint64_t sc_regs[32];
@@ -61,6 +70,7 @@ struct target_sigcontext {
 uint32_t sc_dsp;
 uint32_t sc_reserved;
 };
+#define TARGET_ALMASK  (~15)
 # endif /* O32 */
 
 struct sigframe {
@@ -100,6 +110,7 @@ static inline int install_sigtramp(unsigned int *tramp,   
unsigned int syscall)
 
 __put_user(0x2402 + syscall, tramp + 0);
 __put_user(0x000c  , tramp + 1);
+
 return err;
 }
 
@@ -116,6 +127,7 @@ static inline void setup_sigcontext(CPUMIPSState *regs,
 __put_user(regs->active_tc.gpr[i], >sc_regs[i]);
 }
 
+#if !defined(TARGET_ABI_MIPSP32)
 __put_user(regs->active_tc.HI[0], >sc_mdhi);
 __put_user(regs->active_tc.LO[0], >sc_mdlo);
 
@@ -137,6 +149,7 @@ static inline void setup_sigcontext(CPUMIPSState *regs,
 for (i = 0; i < 32; ++i) {
 __put_user(regs->active_fpu.fpr[i].d, >sc_fpregs[i]);
 }
+#endif
 }
 
 static inline void
@@ -146,13 +159,14 @@ restore_sigcontext(CPUMIPSState *regs, struct 
target_sigcontext *sc)
 
 __get_user(regs->CP0_EPC, >sc_pc);
 
-__get_user(regs->active_tc.HI[0], >sc_mdhi);
-__get_user(regs->active_tc.LO[0], >sc_mdlo);
-
 for (i = 1; i < 32; ++i) {
 __get_user(regs->active_tc.gpr[i], >sc_regs[i]);
 }
 
+#if !defined(TARGET_ABI_MIPSP32)
+__get_user(regs->active_tc.HI[0], >sc_mdhi);
+__get_user(regs->active_tc.LO[0], >sc_mdlo);
+
 __get_user(regs->active_tc.HI[1], >sc_hi1);
 __get_user(regs->active_tc.HI[2], >sc_hi2);
 __get_user(regs->active_tc.HI[3], >sc_hi3);
@@ -168,6 +182,7 @@ restore_sigcontext(CPUMIPSState *regs, struct 
target_sigcontext *sc)
 for (i = 0; i < 32; ++i) {
 __get_user(regs->active_fpu.fpr[i].d, >sc_fpregs[i]);
 }
+#endif
 }
 
 /*
@@ -185,7 +200,7 @@ get_sigframe(struct target_sigaction *ka, CPUMIPSState 
*regs, size_t frame_size)
  */
 sp = target_sigsp(get_sp_from_cpustate(regs) - 32, ka);
 
-return (sp - frame_size) & ~7;
+return (sp - frame_size) & TARGET_ALMASK;
 }
 
 static void mips_set_hflags_isa_mode_from_pc(CPUMIPSState *env)
diff --git a/linux-user/nanomips/signal.c b/linux-user/nanomips/signal.c
new file mode 100644
index 000..86efc21
--- /dev/null
+++ b/linux-user/nanomips/signal.c
@@ -0,0 +1 @@
+#include "../mips/signal.c"
-- 
2.7.4

Re: [Qemu-devel] [PATCH v3 5/7] loader: add rom transaction API

[Peter Maydell]

On 25 July 2018 at 09:59, Stefan Hajnoczi  wrote:
> Image file loaders may add a series of roms.  If an error occurs partway
> through loading there is no easy way to drop previously added roms.
>
> This patch adds a transaction mechanism that works like this:
>
>   rom_transaction_begin();
>   ...call rom_add_*()...
>   rom_transaction_end(ok);
>
> If ok is false then roms added in this transaction are dropped.
>
> Signed-off-by: Stefan Hajnoczi 
> ---
> +void rom_transaction_end(bool commit)
> +{
> +Rom *rom;
> +Rom *tmp;
> +
> +QTAILQ_FOREACH_SAFE(rom, , next, tmp) {
> +if (rom->committed) {
> +continue;
> +}
> +if (commit) {
> +rom->committed = true;
> +} else {
> +QTAILQ_REMOVE(, rom, next);
> +g_free(rom->data);
> +g_free(rom->path);
> +g_free(rom->name);
> +g_free(rom->fw_dir);
> +g_free(rom->fw_file);
> +g_free(rom);

Is it worth having a rom_free() function so we can
share the "free all the pointers" code between this
and the error-exit codepath at the end of rom_add_file() ?

Either way
Reviewed-by: Peter Maydell 

thanks
-- PMM

[Qemu-devel] [PATCH v5 67/76] linux-user: Add sockbits.h header for nanoMIPS

[Aleksandar Markovic]

From: Aleksandar Rikalo 

Add sockbits.h header for nanoMIPS.

Signed-off-by: Aleksandar Rikalo 
Signed-off-by: Aleksandar Markovic 
Signed-off-by: Stefan Markovic 
---
 linux-user/nanomips/sockbits.h | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 linux-user/nanomips/sockbits.h

diff --git a/linux-user/nanomips/sockbits.h b/linux-user/nanomips/sockbits.h
new file mode 100644
index 000..e6b6d31
--- /dev/null
+++ b/linux-user/nanomips/sockbits.h
@@ -0,0 +1 @@
+#include "../mips/sockbits.h"
-- 
2.7.4

[Qemu-devel] [PATCH v5 63/76] linux-user: Add target_signal.h header for nanoMIPS

[Aleksandar Markovic]

From: Aleksandar Rikalo 

nanoMIPS signal handling is much closer to the signal handling in
other mainstream platforms that to the signal handling in preceding
MIPS platforms.

Signed-off-by: Aleksandar Rikalo 
Signed-off-by: Aleksandar Markovic 
Signed-off-by: Stefan Markovic 
---
 linux-user/nanomips/target_signal.h | 22 ++
 1 file changed, 22 insertions(+)
 create mode 100644 linux-user/nanomips/target_signal.h

diff --git a/linux-user/nanomips/target_signal.h 
b/linux-user/nanomips/target_signal.h
new file mode 100644
index 000..604e853
--- /dev/null
+++ b/linux-user/nanomips/target_signal.h
@@ -0,0 +1,22 @@
+#ifndef NANOMIPS_TARGET_SIGNAL_H
+#define NANOMIPS_TARGET_SIGNAL_H
+
+#include "../generic/signal.h"
+#undef TARGET_SIGRTMIN
+#define TARGET_SIGRTMIN   35
+
+/* this struct defines a stack used during syscall handling */
+typedef struct target_sigaltstack {
+abi_long ss_sp;
+abi_ulong ss_size;
+abi_long ss_flags;
+} target_stack_t;
+
+/* sigaltstack controls */
+#define TARGET_SS_ONSTACK 1
+#define TARGET_SS_DISABLE 2
+
+#define TARGET_MINSIGSTKSZ6144
+#define TARGET_SIGSTKSZ   12288
+
+#endif
-- 
2.7.4

[Qemu-devel] [PATCH v3 0/1] ppc/pnv: Add model for Power8 PHB3 PCIe Host bridge

[Cédric Le Goater]

This is a model of the PCIe Host Bridge (PHB3) controller found on a
Power8 processor. The Power8 processor comes in different flavors:
Venice, Murano, Naple, each having a different number of PHBs. Multi
chip is supported, each chip adding its set of PHB3 controllers.

There is no default device layout and PCI devices should be added to
the machine using command line options such as :

  -device e1000e,netdev=net0,mac=C0:FF:EE:00:00:02,bus=pcie.0,addr=0x0
  -netdev 
bridge,id=net0,helper=/usr/libexec/qemu-bridge-helper,br=virbr0,id=hostnet0

  -device megasas,id=scsi0,bus=pcie.1,addr=0x0
  -drive file=$disk,if=none,id=drive-scsi0-0-0-0,format=qcow2,cache=none
  -device 
scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=2

Git tree available here for testing, based on David's branch:

https://github.com/legoater/qemu/tree/phb3-3.0

Thanks,

C.

Changes since v2 :

 - kept user creatable PHB3 for later.
 - machine: the default number of PHBs is set to 3 per chip.
 - refreshed the PnvPHB3 object hierarchy with PCIe objects
 - introduced a static PCIe Root Port object under the PHB3 host
   bridge object
 - cleanup the register definitions to fit skiboot current ones
 - introduced a phb3_error() helper routine
 - fixed mask in pnv_phb3_config_write()
 - reworked init and realize routine of PnvPHB3
 - removed the creation of a default PCI bridge under the Root Port
 - simplified the PnvPHB3 properties using the DEFINE_PROP_UINT32 macros
 - MSI: fixed a resend error when P|Q was set
 
What did not change since v2 :

 - the MMIO ops are still the same. The controller has many registers,
   more or less 150, and the current model works well enough not to
   pollute the read/write ops.

Changes since v1 :

 - removed duplication of macros for the register definitions
 - fixed multi chip support
 - introduced a chip class attribute to create all possible PHB3
   devices
 - introduced property handlers to check the validity of the phb index
   and the chip id
 - explored user creatable PHB3 devices


Benjamin Herrenschmidt (1):
  ppc/pnv: Add model for Power8 PHB3 PCIe Host bridge

 default-configs/ppc64-softmmu.mak   |1 +
 include/hw/pci-host/pnv_phb3.h  |  171 
 include/hw/pci-host/pnv_phb3_regs.h |  432 ++
 include/hw/ppc/pnv.h|   22 +
 include/hw/ppc/pnv_xscom.h  |9 +
 include/hw/ppc/xics.h   |1 +
 hw/intc/xics.c  |2 +-
 hw/pci-host/pnv_phb3.c  | 1146 +++
 hw/pci-host/pnv_phb3_msi.c  |  318 
 hw/pci-host/pnv_phb3_pbcq.c |  347 
 hw/ppc/pnv.c|   75 +-
 hw/ppc/pnv_xscom.c  |6 +-
 hw/pci-host/Makefile.objs   |1 +
 13 files changed, 2526 insertions(+), 5 deletions(-)
 create mode 100644 include/hw/pci-host/pnv_phb3.h
 create mode 100644 include/hw/pci-host/pnv_phb3_regs.h
 create mode 100644 hw/pci-host/pnv_phb3.c
 create mode 100644 hw/pci-host/pnv_phb3_msi.c
 create mode 100644 hw/pci-host/pnv_phb3_pbcq.c

-- 
2.17.1