[PATCH v2 1/2] net/eth: Simplify _eth_get_rss_ex_dst_addr()

2021-01-15 Thread Philippe Mathieu-Daudé
The length field is already contained in the ip6_ext_hdr structure.
Check it direcly in eth_parse_ipv6_hdr() before calling
_eth_get_rss_ex_dst_addr(), which gets a bit simplified.

Reviewed-by: Miroslav Rezanina 
Signed-off-by: Philippe Mathieu-Daudé 
---
 net/eth.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/net/eth.c b/net/eth.c
index 1e0821c5f81..7d4dd48c1ff 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -407,9 +407,7 @@ _eth_get_rss_ex_dst_addr(const struct iovec *pkt, int 
pkt_frags,
 {
 struct ip6_ext_hdr_routing *rthdr = (struct ip6_ext_hdr_routing *) ext_hdr;
 
-if ((rthdr->rtype == 2) &&
-(rthdr->len == sizeof(struct in6_address) / 8) &&
-(rthdr->segleft == 1)) {
+if ((rthdr->rtype == 2) && (rthdr->segleft == 1)) {
 
 size_t input_size = iov_size(pkt, pkt_frags);
 size_t bytes_read;
@@ -528,10 +526,12 @@ bool eth_parse_ipv6_hdr(const struct iovec *pkt, int 
pkt_frags,
 }
 
 if (curr_ext_hdr_type == IP6_ROUTING) {
-info->rss_ex_dst_valid =
-_eth_get_rss_ex_dst_addr(pkt, pkt_frags,
- ip6hdr_off + info->full_hdr_len,
- _hdr, >rss_ex_dst);
+if (ext_hdr.ip6r_len == sizeof(struct in6_address) / 8) {
+info->rss_ex_dst_valid =
+_eth_get_rss_ex_dst_addr(pkt, pkt_frags,
+ ip6hdr_off + info->full_hdr_len,
+ _hdr, >rss_ex_dst);
+}
 } else if (curr_ext_hdr_type == IP6_DESTINATON) {
 info->rss_ex_src_valid =
 _eth_get_rss_ex_src_addr(pkt, pkt_frags,
-- 
2.26.2




[PATCH v2 2/2] net/eth: Fix stack-buffer-overflow in _eth_get_rss_ex_dst_addr()

2021-01-15 Thread Philippe Mathieu-Daudé
QEMU fuzzer reported a buffer overflow in _eth_get_rss_ex_dst_addr()
reproducible as:

  $ cat << EOF | ./qemu-system-i386 -M pc-q35-5.0 \
-accel qtest -monitor none \
-serial none -nographic -qtest stdio
  outl 0xcf8 0x80001010
  outl 0xcfc 0xe102
  outl 0xcf8 0x80001004
  outw 0xcfc 0x7
  write 0x25 0x1 0x86
  write 0x26 0x1 0xdd
  write 0x4f 0x1 0x2b
  write 0xe1020030 0x4 0x190002e1
  write 0xe102003a 0x2 0x0807
  write 0xe1020048 0x4 0x12077cdd
  write 0xe1020400 0x4 0xba077cdd
  write 0xe1020420 0x4 0x190002e1
  write 0xe1020428 0x4 0x3509d807
  write 0xe1020438 0x1 0xe2
  EOF
  =
  ==2859770==ERROR: AddressSanitizer: stack-buffer-overflow on address 
0x7ffdef904902 at pc 0x561ceefa78de bp 0x7ffdef904820 sp 0x7ffdef904818
  READ of size 1 at 0x7ffdef904902 thread T0
  #0 0x561ceefa78dd in _eth_get_rss_ex_dst_addr net/eth.c:410:17
  #1 0x561ceefa41fb in eth_parse_ipv6_hdr net/eth.c:532:17
  #2 0x561cef7de639 in net_tx_pkt_parse_headers hw/net/net_tx_pkt.c:228:14
  #3 0x561cef7dbef4 in net_tx_pkt_parse hw/net/net_tx_pkt.c:273:9
  #4 0x561ceec29f22 in e1000e_process_tx_desc hw/net/e1000e_core.c:730:29
  #5 0x561ceec28eac in e1000e_start_xmit hw/net/e1000e_core.c:927:9
  #6 0x561ceec1baab in e1000e_set_tdt hw/net/e1000e_core.c:2444:9
  #7 0x561ceebf300e in e1000e_core_write hw/net/e1000e_core.c:3256:9
  #8 0x561cef3cd4cd in e1000e_mmio_write hw/net/e1000e.c:110:5

  Address 0x7ffdef904902 is located in stack of thread T0 at offset 34 in frame
  #0 0x561ceefa320f in eth_parse_ipv6_hdr net/eth.c:486

This frame has 1 object(s):
  [32, 34) 'ext_hdr' (line 487) <== Memory access at offset 34 overflows 
this variable
  HINT: this may be a false positive if your program uses some custom stack 
unwind mechanism, swapcontext or vfork
(longjmp and C++ exceptions *are* supported)
  SUMMARY: AddressSanitizer: stack-buffer-overflow net/eth.c:410:17 in 
_eth_get_rss_ex_dst_addr
  Shadow bytes around the buggy address:
0x10003df188d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10003df188e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10003df188f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10003df18900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10003df18910: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
  =>0x10003df18920:[02]f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
0x10003df18930: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10003df18940: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10003df18950: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10003df18960: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10003df18970: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable:   00
Partially addressable: 01 02 03 04 05 06 07
Stack left redzone:  f1
Stack right redzone: f3
  ==2859770==ABORTING

Similarly GCC 11 reports:

  net/eth.c: In function 'eth_parse_ipv6_hdr':
  net/eth.c:410:15: error: array subscript 'struct ip6_ext_hdr_routing[0]' is 
partly outside array bounds of 'struct ip6_ext_hdr[1]' [-Werror=array-bounds]
410 | if ((rthdr->rtype == 2) && (rthdr->segleft == 1)) {
|  ~^~~
  net/eth.c:485:24: note: while referencing 'ext_hdr'
485 | struct ip6_ext_hdr ext_hdr;
|^~~
  net/eth.c:410:38: error: array subscript 'struct ip6_ext_hdr_routing[0]' is 
partly outside array bounds of 'struct ip6_ext_hdr[1]' [-Werror=array-bounds]
410 | if ((rthdr->rtype == 2) && (rthdr->segleft == 1)) {
| ~^
  net/eth.c:485:24: note: while referencing 'ext_hdr'
485 | struct ip6_ext_hdr ext_hdr;
|^~~

In eth_parse_ipv6_hdr() we called iov_to_buf() to fill the 2 bytes of
the 'ext_hdr' buffer, then _eth_get_rss_ex_dst_addr() tries to access
beside the 2 filled bytes.

Fix by reworking the function, filling the full rt_hdr buffer on the
stack calling iov_to_buf() again.

Add the corresponding qtest case with the fuzzer reproducer.

Cc: qemu-sta...@nongnu.org
Buglink: https://bugs.launchpad.net/qemu/+bug/1879531
Reported-by: Alexander Bulekov 
Reported-by: Miroslav Rezanina 
Fixes: eb700029c78 ("net_pkt: Extend packet abstraction as required by e1000e 
functionality")
Reviewed-by: Miroslav Rezanina 
Signed-off-by: Philippe Mathieu-Daudé 
---
v2: Do no run test if device not available
---
 net/eth.c  | 25 +++-
 tests/qtest/fuzz-e1000e-test.c | 53 ++
 MAINTAINERS|  1 +
 tests/qtest/meson.build|  1 +
 4 files changed, 66 insertions(+), 14 deletions(-)
 create mode 100644 tests/qtest/fuzz-e1000e-test.c

diff --git a/net/eth.c b/net/eth.c
index 

[PATCH v2 0/2] net/eth: Fix stack-buffer-overflow in _eth_get_rss_ex_dst_addr()

2021-01-15 Thread Philippe Mathieu-Daudé
I had a look at the patch from Miroslav trying to silence a
compiler warning which in fact is a nasty bug. Here is a fix.
https://www.mail-archive.com/qemu-devel@nongnu.org/msg772735.html

v2: Restrict tests so they don't fail when device aren't available

Based-on: <20210115150936.282-1-phi...@redhat.com>
  "tests/qtest: Fixes fuzz-tests"

Philippe Mathieu-Daudé (2):
  net/eth: Simplify _eth_get_rss_ex_dst_addr()
  net/eth: Fix stack-buffer-overflow in _eth_get_rss_ex_dst_addr()

 net/eth.c  | 37 +++-
 tests/qtest/fuzz-e1000e-test.c | 53 ++
 MAINTAINERS|  1 +
 tests/qtest/meson.build|  1 +
 4 files changed, 72 insertions(+), 20 deletions(-)
 create mode 100644 tests/qtest/fuzz-e1000e-test.c

-- 
2.26.2





Re: Windows installer builds apparently broken since October?

2021-01-15 Thread Peter Maydell
On Fri, 15 Jan 2021 at 14:01, Peter Maydell  wrote:
>
> I was just trying to see what updates the qemu.nsi file needed for
> the merge-all-the-manuals-into-one-place change, and I discovered
> that it's been broken since October when we removed the Changelog file:
>
> File: "/tmp/qemu-test/src\Changelog" -> no files found.
> Usage: File [/nonfatal] [/a] ([/r] [/x filespec [...]] filespec [...] |
>/oname=outfile one_file_only)
> Error in script "../src/qemu.nsi" on line 119 -- aborting creation process

...and if you fix that it then fails because it's looking for
a subdirectory share/ in the install dir and it doesn't exist...

thanks
-- PMM



Re: [PATCH] fuzz: Add virtio-9p configurations for fuzzing

2021-01-15 Thread Alexander Bulekov
On 210115 1033, Darren Kenny wrote:
> Hi Alex,
> 
> On Thursday, 2021-01-14 at 17:17:48 -05, Alexander Bulekov wrote:
> > Signed-off-by: Alexander Bulekov 
> 
> In general this look good, so:
> 
> Reviewed-by: Darren Kenny 
> 
> but I do have a question below...
> 
> > ---
> >  tests/qtest/fuzz/generic_fuzz_configs.h | 12 
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/tests/qtest/fuzz/generic_fuzz_configs.h 
> > b/tests/qtest/fuzz/generic_fuzz_configs.h
> > index 7fed035345..ffdb590c58 100644
> > --- a/tests/qtest/fuzz/generic_fuzz_configs.h
> > +++ b/tests/qtest/fuzz/generic_fuzz_configs.h
> > @@ -59,6 +59,18 @@ const generic_fuzz_config predefined_configs[] = {
> >  .name = "virtio-mouse",
> >  .args = "-machine q35 -nodefaults -device virtio-mouse",
> >  .objects = "virtio*",
> > +},{
> > +.name = "virtio-9p",
> > +.args = "-machine q35 -nodefaults "
> > +"-device virtio-9p,fsdev=hshare,mount_tag=hshare "
> > +"-fsdev local,id=hshare,path=/tmp/,security_model=none",
> > +.objects = "virtio*",
> 
> I wonder about the use of "/tmp" rather than maybe some generated name
> using mkdtemp() - I also realise that the ability to generate this and
> plug it in here probably doesn't exist either, hence not holding you to
> it for this patch. Also the fact that in OSS-Fuzz this is run in limited
> containers.
> 
> Have you observed any changes to "/tmp" while this is running? My
> concerns may be unfounded since I don't really know what state things
> are in while this is executed with no running OS.
> 

So far, I haven't seen any files created, however this might be due to
the fact that I used security_model=none. In any case, I agree that this
is not a nice solution. I'll think of a way to use mkdtemp() for v2.
-Alex

> Thanks,
> 
> Darren.
> 



Re: [RFC PATCH v6 04/11] hw/ssi: imx_spi: Reduce 'change_mask' variable scope

2021-01-15 Thread Philippe Mathieu-Daudé
On 1/13/21 2:47 PM, Juan Quintela wrote:
> Juan Quintela  wrote:
>> Philippe Mathieu-Daudé  wrote:
>>> Signed-off-by: Philippe Mathieu-Daudé 
>>
>> I think this one is wrong.
> 
> Wrong is a strong word.  I mean that it changes behaviour and the commit
> message don't talk about changing behaviour.

Indeed. Well I'll simply drop this patch as it is not essential.

Thanks for reviewing the series!

Phil.



Re: [PULL v2 00/69] MIPS patches for 2021-01-14

2021-01-15 Thread Peter Maydell
On Thu, 14 Jan 2021 at 16:56, Philippe Mathieu-Daudé  wrote:
>
> Resending the MIPS pull request from MIPS patches from last week
> (2021-01-07) now than the "decodetree: Open files with encoding='utf-8'"
> patch got merged (commit 4cacecaaa2b).
>
> 
> MIPS patches queue
>
> - Simplify CPU/ISA definitions
> - Various maintenance code movements in translate.c
> - Convert part of the MSA ASE instructions to decodetree
> - Convert some instructions removed from Release 6 to decodetree
> - Remove deprecated 'fulong2e' machine alias
>
> 
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/6.0
for any user-visible changes.

-- PMM



[PATCH v7 0/3] MTE support for KVM guest

2021-01-15 Thread Steven Price
After chasing down a bug[1] with MTE assisted KASAN and KVM, I've now
been able to rebase on v5.11-rc1 and test the combination of
KVM-with-MTE and KASAN.

For anyone new to this series, or simply pretending 2020 didn't happen,
this series adds support for Arm's Memory Tagging Extension (MTE) to KVM,
allowing KVM guests to make use of it. The first patch adds definitions
for the new registers and saves/restores them as necessary. The second
adds a new VM feature which allows the guest access to MTE.

The third patch is new and RFC for now. It adds a new ioctl allowing a
VMM to easily read/write the tags in the guest's memory even if the
memory isn't mapped with PROT_MTE in userspace. I'd particularly welcome
feedback on this new ABI.

Changes since v6[2]:
 * Moved the save/restore of RGSR_EL1, GCR_EL1 and TFSRE0_EL into asm
 * Correctly set TCO when injecting an exception to an MTE-enabled guest
 * Rebased on v5.11-rc1
 * RFC patch for new MTE tag copy ioctl

[1] https://lore.kernel.org/r/20210108161254.53674-1-steven.pr...@arm.com
[2] https://lore.kernel.org/r/20201127152113.13099-1-steven.pr...@arm.com

Steven Price (3):
  arm64: kvm: Save/restore MTE registers
  arm64: kvm: Introduce MTE VCPU feature
  KVM: arm64: ioctl to fetch/store tags in a guest

 arch/arm64/include/asm/kvm_emulate.h   |  3 +
 arch/arm64/include/asm/kvm_host.h  |  7 ++
 arch/arm64/include/asm/kvm_mte.h   | 74 ++
 arch/arm64/include/asm/pgtable.h   |  2 +-
 arch/arm64/include/asm/sysreg.h|  3 +-
 arch/arm64/include/uapi/asm/kvm.h  | 13 
 arch/arm64/kernel/asm-offsets.c|  3 +
 arch/arm64/kernel/mte.c| 36 +++
 arch/arm64/kvm/arm.c   | 68 
 arch/arm64/kvm/hyp/entry.S |  7 ++
 arch/arm64/kvm/hyp/exception.c |  3 +-
 arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h |  4 ++
 arch/arm64/kvm/mmu.c   | 16 +
 arch/arm64/kvm/sys_regs.c  | 20 --
 include/uapi/linux/kvm.h   |  2 +
 15 files changed, 239 insertions(+), 22 deletions(-)
 create mode 100644 arch/arm64/include/asm/kvm_mte.h

-- 
2.20.1




[PATCH v7 2/3] arm64: kvm: Introduce MTE VCPU feature

2021-01-15 Thread Steven Price
Add a new VM feature 'KVM_ARM_CAP_MTE' which enables memory tagging
for a VM. This exposes the feature to the guest and automatically tags
memory pages touched by the VM as PG_mte_tagged (and clears the tags
storage) to ensure that the guest cannot see stale tags, and so that the
tags are correctly saved/restored across swap.

Signed-off-by: Steven Price 
---
 arch/arm64/include/asm/kvm_emulate.h |  3 +++
 arch/arm64/include/asm/kvm_host.h|  3 +++
 arch/arm64/include/asm/pgtable.h |  2 +-
 arch/arm64/kernel/mte.c  | 36 +---
 arch/arm64/kvm/arm.c |  9 +++
 arch/arm64/kvm/hyp/exception.c   |  3 ++-
 arch/arm64/kvm/mmu.c | 16 +
 arch/arm64/kvm/sys_regs.c|  6 -
 include/uapi/linux/kvm.h |  1 +
 9 files changed, 62 insertions(+), 17 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_emulate.h 
b/arch/arm64/include/asm/kvm_emulate.h
index f612c090f2e4..6bf776c2399c 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -84,6 +84,9 @@ static inline void vcpu_reset_hcr(struct kvm_vcpu *vcpu)
if (cpus_have_const_cap(ARM64_MISMATCHED_CACHE_TYPE) ||
vcpu_el1_is_32bit(vcpu))
vcpu->arch.hcr_el2 |= HCR_TID2;
+
+   if (kvm_has_mte(vcpu->kvm))
+   vcpu->arch.hcr_el2 |= HCR_ATA;
 }
 
 static inline unsigned long *vcpu_hcr(struct kvm_vcpu *vcpu)
diff --git a/arch/arm64/include/asm/kvm_host.h 
b/arch/arm64/include/asm/kvm_host.h
index 51590a397e4b..1ca5785fb0e9 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -132,6 +132,8 @@ struct kvm_arch {
 
u8 pfr0_csv2;
u8 pfr0_csv3;
+   /* Memory Tagging Extension enabled for the guest */
+   bool mte_enabled;
 };
 
 struct kvm_vcpu_fault_info {
@@ -749,6 +751,7 @@ bool kvm_arm_vcpu_is_finalized(struct kvm_vcpu *vcpu);
 #define kvm_arm_vcpu_sve_finalized(vcpu) \
((vcpu)->arch.flags & KVM_ARM64_VCPU_SVE_FINALIZED)
 
+#define kvm_has_mte(kvm) (system_supports_mte() && (kvm)->arch.mte_enabled)
 #define kvm_vcpu_has_pmu(vcpu) \
(test_bit(KVM_ARM_VCPU_PMU_V3, (vcpu)->arch.features))
 
diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index 501562793ce2..27416d52f6a9 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -312,7 +312,7 @@ static inline void set_pte_at(struct mm_struct *mm, 
unsigned long addr,
__sync_icache_dcache(pte);
 
if (system_supports_mte() &&
-   pte_present(pte) && pte_tagged(pte) && !pte_special(pte))
+   pte_present(pte) && pte_valid_user(pte) && !pte_special(pte))
mte_sync_tags(ptep, pte);
 
__check_racy_pte_update(mm, ptep, pte);
diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
index dc9ada64feed..f9e089be1603 100644
--- a/arch/arm64/kernel/mte.c
+++ b/arch/arm64/kernel/mte.c
@@ -25,27 +25,33 @@
 
 u64 gcr_kernel_excl __ro_after_init;
 
-static void mte_sync_page_tags(struct page *page, pte_t *ptep, bool check_swap)
+static void mte_sync_page_tags(struct page *page, pte_t *ptep, bool check_swap,
+  bool pte_is_tagged)
 {
pte_t old_pte = READ_ONCE(*ptep);
 
if (check_swap && is_swap_pte(old_pte)) {
swp_entry_t entry = pte_to_swp_entry(old_pte);
 
-   if (!non_swap_entry(entry) && mte_restore_tags(entry, page))
+   if (!non_swap_entry(entry) && mte_restore_tags(entry, page)) {
+   set_bit(PG_mte_tagged, >flags);
return;
+   }
}
 
-   page_kasan_tag_reset(page);
-   /*
-* We need smp_wmb() in between setting the flags and clearing the
-* tags because if another thread reads page->flags and builds a
-* tagged address out of it, there is an actual dependency to the
-* memory access, but on the current thread we do not guarantee that
-* the new page->flags are visible before the tags were updated.
-*/
-   smp_wmb();
-   mte_clear_page_tags(page_address(page));
+   if (pte_is_tagged) {
+   set_bit(PG_mte_tagged, >flags);
+   page_kasan_tag_reset(page);
+   /*
+* We need smp_wmb() in between setting the flags and clearing 
the
+* tags because if another thread reads page->flags and builds a
+* tagged address out of it, there is an actual dependency to 
the
+* memory access, but on the current thread we do not guarantee 
that
+* the new page->flags are visible before the tags were updated.
+*/
+   smp_wmb();
+   mte_clear_page_tags(page_address(page));
+   }
 }
 
 void mte_sync_tags(pte_t *ptep, pte_t pte)
@@ -53,11 +59,13 @@ void 

Re: [PATCH 7/7] block/rbd: change request alignment to 1 byte

2021-01-15 Thread Jason Dillaman
On Thu, Jan 14, 2021 at 2:59 PM Peter Lieven  wrote:
>
> Am 14.01.21 um 20:19 schrieb Jason Dillaman:
> > On Sun, Dec 27, 2020 at 11:42 AM Peter Lieven  wrote:
> >> since we implement byte interfaces and librbd supports aio on byte 
> >> granularity we can lift
> >> the 512 byte alignment.
> >>
> >> Signed-off-by: Peter Lieven 
> >> ---
> >>  block/rbd.c | 2 --
> >>  1 file changed, 2 deletions(-)
> >>
> >> diff --git a/block/rbd.c b/block/rbd.c
> >> index 27b4404adf..8673e8f553 100644
> >> --- a/block/rbd.c
> >> +++ b/block/rbd.c
> >> @@ -223,8 +223,6 @@ done:
> >>  static void qemu_rbd_refresh_limits(BlockDriverState *bs, Error **errp)
> >>  {
> >>  BDRVRBDState *s = bs->opaque;
> >> -/* XXX Does RBD support AIO on less than 512-byte alignment? */
> >> -bs->bl.request_alignment = 512;
> > Just a suggestion, but perhaps improve discard alignment, max discard,
> > optimal alignment (if that's something QEMU handles internally) if not
> > overridden by the user.
>
>
> Qemu supports max_discard and discard_alignment. Is there a call to get these 
> limits
>
> from librbd?
>
>
> What do you mean by optimal_alignment? The object size?

krbd does a good job of initializing defaults [1] where optimal and
discard alignment is 64KiB (can actually be 4KiB now), max IO size for
writes, discards, and write-zeroes is the object size * the stripe
count.

> Peter
>
>
>

[1] https://github.com/torvalds/linux/blob/master/drivers/block/rbd.c#L4981

-- 
Jason




[PATCH v7 1/3] arm64: kvm: Save/restore MTE registers

2021-01-15 Thread Steven Price
Define the new system registers that MTE introduces and context switch
them. The MTE feature is still hidden from the ID register as it isn't
supported in a VM yet.

Signed-off-by: Steven Price 
---
 arch/arm64/include/asm/kvm_host.h  |  4 ++
 arch/arm64/include/asm/kvm_mte.h   | 74 ++
 arch/arm64/include/asm/sysreg.h|  3 +-
 arch/arm64/kernel/asm-offsets.c|  3 +
 arch/arm64/kvm/hyp/entry.S |  7 ++
 arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h |  4 ++
 arch/arm64/kvm/sys_regs.c  | 14 ++--
 7 files changed, 104 insertions(+), 5 deletions(-)
 create mode 100644 arch/arm64/include/asm/kvm_mte.h

diff --git a/arch/arm64/include/asm/kvm_host.h 
b/arch/arm64/include/asm/kvm_host.h
index 11beda85ee7e..51590a397e4b 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -148,6 +148,8 @@ enum vcpu_sysreg {
SCTLR_EL1,  /* System Control Register */
ACTLR_EL1,  /* Auxiliary Control Register */
CPACR_EL1,  /* Coprocessor Access Control */
+   RGSR_EL1,   /* Random Allocation Tag Seed Register */
+   GCR_EL1,/* Tag Control Register */
ZCR_EL1,/* SVE Control */
TTBR0_EL1,  /* Translation Table Base Register 0 */
TTBR1_EL1,  /* Translation Table Base Register 1 */
@@ -164,6 +166,8 @@ enum vcpu_sysreg {
TPIDR_EL1,  /* Thread ID, Privileged */
AMAIR_EL1,  /* Aux Memory Attribute Indirection Register */
CNTKCTL_EL1,/* Timer Control Register (EL1) */
+   TFSRE0_EL1, /* Tag Fault Status Register (EL0) */
+   TFSR_EL1,   /* Tag Fault Stauts Register (EL1) */
PAR_EL1,/* Physical Address Register */
MDSCR_EL1,  /* Monitor Debug System Control Register */
MDCCINT_EL1,/* Monitor Debug Comms Channel Interrupt Enable Reg */
diff --git a/arch/arm64/include/asm/kvm_mte.h b/arch/arm64/include/asm/kvm_mte.h
new file mode 100644
index ..62bbfae77f33
--- /dev/null
+++ b/arch/arm64/include/asm/kvm_mte.h
@@ -0,0 +1,74 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2020 ARM Ltd.
+ */
+#ifndef __ASM_KVM_MTE_H
+#define __ASM_KVM_MTE_H
+
+#ifdef __ASSEMBLY__
+
+#include 
+
+#ifdef CONFIG_ARM64_MTE
+
+.macro mte_switch_to_guest g_ctxt, h_ctxt, reg1
+alternative_if_not ARM64_MTE
+   b   .L__skip_switch\@
+alternative_else_nop_endif
+   mrs \reg1, hcr_el2
+   and \reg1, \reg1, #(HCR_ATA)
+   cbz \reg1, .L__skip_switch\@
+
+   mrs_s   \reg1, SYS_RGSR_EL1
+   str \reg1, [\h_ctxt, #CPU_RGSR_EL1]
+   mrs_s   \reg1, SYS_GCR_EL1
+   str \reg1, [\h_ctxt, #CPU_GCR_EL1]
+   mrs_s   \reg1, SYS_TFSRE0_EL1
+   str \reg1, [\h_ctxt, #CPU_TFSRE0_EL1]
+
+   ldr \reg1, [\g_ctxt, #CPU_RGSR_EL1]
+   msr_s   SYS_RGSR_EL1, \reg1
+   ldr \reg1, [\g_ctxt, #CPU_GCR_EL1]
+   msr_s   SYS_GCR_EL1, \reg1
+   ldr \reg1, [\g_ctxt, #CPU_TFSRE0_EL1]
+   msr_s   SYS_TFSRE0_EL1, \reg1
+
+.L__skip_switch\@:
+.endm
+
+.macro mte_switch_to_hyp g_ctxt, h_ctxt, reg1
+alternative_if_not ARM64_MTE
+   b   .L__skip_switch\@
+alternative_else_nop_endif
+   mrs \reg1, hcr_el2
+   and \reg1, \reg1, #(HCR_ATA)
+   cbz \reg1, .L__skip_switch\@
+
+   mrs_s   \reg1, SYS_RGSR_EL1
+   str \reg1, [\g_ctxt, #CPU_RGSR_EL1]
+   mrs_s   \reg1, SYS_GCR_EL1
+   str \reg1, [\g_ctxt, #CPU_GCR_EL1]
+   mrs_s   \reg1, SYS_TFSRE0_EL1
+   str \reg1, [\g_ctxt, #CPU_TFSRE0_EL1]
+
+   ldr \reg1, [\h_ctxt, #CPU_RGSR_EL1]
+   msr_s   SYS_RGSR_EL1, \reg1
+   ldr \reg1, [\h_ctxt, #CPU_GCR_EL1]
+   msr_s   SYS_GCR_EL1, \reg1
+   ldr \reg1, [\h_ctxt, #CPU_TFSRE0_EL1]
+   msr_s   SYS_TFSRE0_EL1, \reg1
+
+.L__skip_switch\@:
+.endm
+
+#else /* CONFIG_ARM64_MTE */
+
+.macro mte_switch_to_guest g_ctxt, h_ctxt, reg1
+.endm
+
+.macro mte_switch_to_hyp g_ctxt, h_ctxt, reg1
+.endm
+
+#endif /* CONFIG_ARM64_MTE */
+#endif /* __ASSEMBLY__ */
+#endif /* __ASM_KVM_MTE_H */
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 8b5e7e5c3cc8..0a01975d331d 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -574,7 +574,8 @@
 #define SCTLR_ELx_M(BIT(0))
 
 #define SCTLR_ELx_FLAGS(SCTLR_ELx_M  | SCTLR_ELx_A | SCTLR_ELx_C | \
-SCTLR_ELx_SA | SCTLR_ELx_I | SCTLR_ELx_IESB)
+SCTLR_ELx_SA | SCTLR_ELx_I | SCTLR_ELx_IESB | \
+SCTLR_ELx_ITFSB)
 
 /* SCTLR_EL2 specific flags. */
 #define SCTLR_EL2_RES1 ((BIT(4))  | (BIT(5))  | (BIT(11)) | (BIT(16)) | \
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index f42fd9e33981..801531e1fa5c 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -105,6 +105,9 

[PATCH v7 0/9] hw/ssi: imx_spi: Fix various bugs in the imx_spi model

2021-01-15 Thread Philippe Mathieu-Daudé
Hi,

This is how I understand the ecSPI reset works, after
looking at the IMX6DQRM.pdf datasheet.

This is a respin of Ben's v5 series [*].

Since v6:
- Dropped "Reduce 'change_mask' variable scope" patch
- Fixed inverted reset logic
- Added Juan R-b tags
- Removed 'RFC' tag as tests pass

Based-on: <1608688825-81519-1-git-send-email-bmeng...@gmail.com>
(queued on riscv-next).

Copy of Ben's v5 cover:

  This series fixes a bunch of bugs in current implementation of the imx
  spi controller, including the following issues:

  - chip select signal was not lower down when spi controller is disabled
  - remove imx_spi_update_irq() in imx_spi_reset()
  - round up the tx burst length to be multiple of 8
  - transfer incorrect data when the burst length is larger than 32 bit
  - spi controller tx and rx fifo endianness is incorrect

[*] https://lists.gnu.org/archive/html/qemu-devel/2021-01/msg02333.html

Diff with v6:

Key:
[] : patches are identical
[] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respective=
ly

001/9:[] [--] 'hw/ssi: imx_spi: Use a macro for number of chip selects su=
pported'
002/9:[] [--] 'hw/ssi: imx_spi: Remove pointless variable initialization'
003/9:[] [-C] 'hw/ssi: imx_spi: Rework imx_spi_reset() to keep CONREG reg=
ister value'
004/9:[] [-C] 'hw/ssi: imx_spi: Rework imx_spi_read() to handle block dis=
abled'
005/9:[0003] [FC] 'hw/ssi: imx_spi: Rework imx_spi_write() to handle block di=
sabled'
006/9:[] [--] 'hw/ssi: imx_spi: Disable chip selects when controller is d=
isabled'
007/9:[] [--] 'hw/ssi: imx_spi: Round up the burst length to be multiple =
of 8'
008/9:[] [--] 'hw/ssi: imx_spi: Correct the burst length > 32 bit transfe=
r logic'
009/9:[] [--] 'hw/ssi: imx_spi: Correct tx and rx fifo endianness'

Bin Meng (4):
  hw/ssi: imx_spi: Use a macro for number of chip selects supported
  hw/ssi: imx_spi: Round up the burst length to be multiple of 8
  hw/ssi: imx_spi: Correct the burst length > 32 bit transfer logic
  hw/ssi: imx_spi: Correct tx and rx fifo endianness

Philippe Mathieu-Daud=C3=A9 (4):
  hw/ssi: imx_spi: Remove pointless variable initialization
  hw/ssi: imx_spi: Rework imx_spi_reset() to keep CONREG register value
  hw/ssi: imx_spi: Rework imx_spi_read() to handle block disabled
  hw/ssi: imx_spi: Rework imx_spi_write() to handle block disabled

Xuzhou Cheng (1):
  hw/ssi: imx_spi: Disable chip selects when controller is disabled

 include/hw/ssi/imx_spi.h |   5 +-
 hw/ssi/imx_spi.c | 137 +++
 2 files changed, 86 insertions(+), 56 deletions(-)

--=20
2.26.2




[PATCH v7 2/9] hw/ssi: imx_spi: Remove pointless variable initialization

2021-01-15 Thread Philippe Mathieu-Daudé
'burst_length' is cleared in imx_spi_reset(), which is called
after imx_spi_realize(). Remove the initialization to simplify.

Reviewed-by: Juan Quintela 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/ssi/imx_spi.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c
index e605049a213..40f72c36b61 100644
--- a/hw/ssi/imx_spi.c
+++ b/hw/ssi/imx_spi.c
@@ -428,8 +428,6 @@ static void imx_spi_realize(DeviceState *dev, Error **errp)
 sysbus_init_irq(SYS_BUS_DEVICE(dev), >cs_lines[i]);
 }
 
-s->burst_length = 0;
-
 fifo32_create(>tx_fifo, ECSPI_FIFO_SIZE);
 fifo32_create(>rx_fifo, ECSPI_FIFO_SIZE);
 }
-- 
2.26.2




[RFC PATCH v7 3/3] KVM: arm64: ioctl to fetch/store tags in a guest

2021-01-15 Thread Steven Price
The VMM may not wish to have it's own mapping of guest memory mapped
with PROT_MTE because this causes problems if the VMM has tag checking
enabled (the guest controls the tags in physical RAM and it's unlikely
the tags are correct for the VMM).

Instead add a new ioctl which allows the VMM to easily read/write the
tags from guest memory, allowing the VMM's mapping to be non-PROT_MTE
while the VMM can still read/write the tags for the purpose of
migration.

Signed-off-by: Steven Price 
---
 arch/arm64/include/uapi/asm/kvm.h | 13 +++
 arch/arm64/kvm/arm.c  | 59 +++
 include/uapi/linux/kvm.h  |  1 +
 3 files changed, 73 insertions(+)

diff --git a/arch/arm64/include/uapi/asm/kvm.h 
b/arch/arm64/include/uapi/asm/kvm.h
index 24223adae150..5fc2534ac5df 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -184,6 +184,19 @@ struct kvm_vcpu_events {
__u32 reserved[12];
 };
 
+struct kvm_arm_copy_mte_tags {
+   __u64 guest_ipa;
+   __u64 length;
+   union {
+   void __user *addr;
+   __u64 padding;
+   };
+   __u64 flags;
+};
+
+#define KVM_ARM_TAGS_TO_GUEST  0
+#define KVM_ARM_TAGS_FROM_GUEST1
+
 /* If you need to interpret the index values, here is the key: */
 #define KVM_REG_ARM_COPROC_MASK0x0FFF
 #define KVM_REG_ARM_COPROC_SHIFT   16
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index f4c2fd2e7c49..d6dd6b79bb77 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1303,6 +1303,55 @@ static int kvm_vm_ioctl_set_device_addr(struct kvm *kvm,
}
 }
 
+static int kvm_vm_ioctl_mte_copy_tags(struct kvm *kvm,
+ struct kvm_arm_copy_mte_tags *copy_tags)
+{
+   gpa_t guest_ipa = copy_tags->guest_ipa;
+   size_t length = copy_tags->length;
+   void __user *tags = copy_tags->addr;
+   gpa_t gfn;
+   size_t pages;
+   bool write = !(copy_tags->flags & KVM_ARM_TAGS_FROM_GUEST);
+
+   if (copy_tags->flags & ~KVM_ARM_TAGS_FROM_GUEST)
+   return -EINVAL;
+
+   if (length & ~PAGE_MASK || guest_ipa & ~PAGE_MASK)
+   return -EINVAL;
+
+   gfn = gpa_to_gfn(guest_ipa);
+   pages = length >> PAGE_SHIFT;
+
+   while (length > 0) {
+   kvm_pfn_t pfn = gfn_to_pfn_prot(kvm, gfn, write, NULL);
+   void *maddr;
+   unsigned long num_tags = PAGE_SIZE / MTE_GRANULE_SIZE;
+
+   if (is_error_noslot_pfn(pfn))
+   return -ENOENT;
+
+   maddr = page_address(pfn_to_page(pfn));
+
+   if (!write) {
+   num_tags = mte_copy_tags_to_user(tags, maddr, num_tags);
+   kvm_release_pfn_clean(pfn);
+   } else {
+   num_tags = mte_copy_tags_from_user(maddr, tags,
+  num_tags);
+   kvm_release_pfn_dirty(pfn);
+   }
+
+   if (num_tags != PAGE_SIZE / MTE_GRANULE_SIZE)
+   return -EFAULT;
+
+   gfn++;
+   tags += num_tags;
+   length -= PAGE_SIZE;
+   }
+
+   return 0;
+}
+
 long kvm_arch_vm_ioctl(struct file *filp,
   unsigned int ioctl, unsigned long arg)
 {
@@ -1339,6 +1388,16 @@ long kvm_arch_vm_ioctl(struct file *filp,
 
return 0;
}
+   case KVM_ARM_MTE_COPY_TAGS: {
+   struct kvm_arm_copy_mte_tags copy_tags;
+
+   if (!kvm_has_mte(kvm))
+   return -EINVAL;
+
+   if (copy_from_user(_tags, argp, sizeof(copy_tags)))
+   return -EFAULT;
+   return kvm_vm_ioctl_mte_copy_tags(kvm, _tags);
+   }
default:
return -EINVAL;
}
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index de737d5102ca..76fccb33d025 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1397,6 +1397,7 @@ struct kvm_s390_ucas_mapping {
 /* Available with KVM_CAP_PMU_EVENT_FILTER */
 #define KVM_SET_PMU_EVENT_FILTER  _IOW(KVMIO,  0xb2, struct 
kvm_pmu_event_filter)
 #define KVM_PPC_SVM_OFF  _IO(KVMIO,  0xb3)
+#define KVM_ARM_MTE_COPY_TAGS_IOR(KVMIO,  0xb4, struct 
kvm_arm_copy_mte_tags)
 
 /* ioctl for vm fd */
 #define KVM_CREATE_DEVICE_IOWR(KVMIO,  0xe0, struct kvm_create_device)
-- 
2.20.1




[PATCH v7 6/9] hw/ssi: imx_spi: Disable chip selects when controller is disabled

2021-01-15 Thread Philippe Mathieu-Daudé
From: Xuzhou Cheng 

When a write to ECSPI_CONREG register to disable the SPI controller,
imx_spi_reset() is called to reset the controller, but chip select
lines should have been disabled, otherwise the state machine of any
devices (e.g.: SPI flashes) connected to the SPI master is stuck to
its last state and responds incorrectly to any follow-up commands.

Fixes: c906a3a01582 ("i.MX: Add the Freescale SPI Controller")
Signed-off-by: Xuzhou Cheng 
Signed-off-by: Bin Meng 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20210112145526.31095-4-bmeng...@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/ssi/imx_spi.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c
index f06bbf317e2..c132f99ba5b 100644
--- a/hw/ssi/imx_spi.c
+++ b/hw/ssi/imx_spi.c
@@ -254,6 +254,10 @@ static void imx_spi_reset(DeviceState *dev)
 
 imx_spi_update_irq(s);
 
+for (i = 0; i < ECSPI_NUM_CS; i++) {
+qemu_set_irq(s->cs_lines[i], 1);
+}
+
 s->burst_length = 0;
 }
 
-- 
2.26.2




[PATCH v7 1/9] hw/ssi: imx_spi: Use a macro for number of chip selects supported

2021-01-15 Thread Philippe Mathieu-Daudé
From: Bin Meng 

Avoid using a magic number (4) everywhere for the number of chip
selects supported.

Signed-off-by: Bin Meng 
Reviewed-by: Alistair Francis 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20210112145526.31095-2-bmeng...@gmail.com>
Reviewed-by: Juan Quintela 
Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/ssi/imx_spi.h | 5 -
 hw/ssi/imx_spi.c | 4 ++--
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/include/hw/ssi/imx_spi.h b/include/hw/ssi/imx_spi.h
index b82b17f3643..eeaf49bbac3 100644
--- a/include/hw/ssi/imx_spi.h
+++ b/include/hw/ssi/imx_spi.h
@@ -77,6 +77,9 @@
 
 #define EXTRACT(value, name) extract32(value, name##_SHIFT, name##_LENGTH)
 
+/* number of chip selects supported */
+#define ECSPI_NUM_CS 4
+
 #define TYPE_IMX_SPI "imx.spi"
 OBJECT_DECLARE_SIMPLE_TYPE(IMXSPIState, IMX_SPI)
 
@@ -89,7 +92,7 @@ struct IMXSPIState {
 
 qemu_irq irq;
 
-qemu_irq cs_lines[4];
+qemu_irq cs_lines[ECSPI_NUM_CS];
 
 SSIBus *bus;
 
diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c
index d8885ae454e..e605049a213 100644
--- a/hw/ssi/imx_spi.c
+++ b/hw/ssi/imx_spi.c
@@ -361,7 +361,7 @@ static void imx_spi_write(void *opaque, hwaddr offset, 
uint64_t value,
 
 /* We are in master mode */
 
-for (i = 0; i < 4; i++) {
+for (i = 0; i < ECSPI_NUM_CS; i++) {
 qemu_set_irq(s->cs_lines[i],
  i == imx_spi_selected_channel(s) ? 0 : 1);
 }
@@ -424,7 +424,7 @@ static void imx_spi_realize(DeviceState *dev, Error **errp)
 sysbus_init_mmio(SYS_BUS_DEVICE(dev), >iomem);
 sysbus_init_irq(SYS_BUS_DEVICE(dev), >irq);
 
-for (i = 0; i < 4; ++i) {
+for (i = 0; i < ECSPI_NUM_CS; ++i) {
 sysbus_init_irq(SYS_BUS_DEVICE(dev), >cs_lines[i]);
 }
 
-- 
2.26.2




[PATCH v7 3/9] hw/ssi: imx_spi: Rework imx_spi_reset() to keep CONREG register value

2021-01-15 Thread Philippe Mathieu-Daudé
When the block is disabled, all registers are reset with the
exception of the ECSPI_CONREG. It is initialized to zero
when the instance is created.

Ref: i.MX 6DQ Applications Processor Reference Manual (IMX6DQRM),
 chapter 21.7.3: Control Register (ECSPIx_CONREG)

Reviewed-by: Juan Quintela 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/ssi/imx_spi.c | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c
index 40f72c36b61..78b19c2eb91 100644
--- a/hw/ssi/imx_spi.c
+++ b/hw/ssi/imx_spi.c
@@ -231,12 +231,23 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)
 static void imx_spi_reset(DeviceState *dev)
 {
 IMXSPIState *s = IMX_SPI(dev);
+unsigned i;
 
 DPRINTF("\n");
 
-memset(s->regs, 0, sizeof(s->regs));
-
-s->regs[ECSPI_STATREG] = 0x0003;
+for (i = 0; i < ARRAY_SIZE(s->regs); i++) {
+switch (i) {
+case ECSPI_CONREG:
+/* CONREG is not updated on reset */
+break;
+case ECSPI_STATREG:
+s->regs[i] = 0x0003;
+break;
+default:
+s->regs[i] = 0;
+break;
+}
+}
 
 imx_spi_rxfifo_reset(s);
 imx_spi_txfifo_reset(s);
-- 
2.26.2




[PATCH v7 4/9] hw/ssi: imx_spi: Rework imx_spi_read() to handle block disabled

2021-01-15 Thread Philippe Mathieu-Daudé
When the block is disabled, it stay it is 'internal reset logic'
(internal clocks are gated off). Reading any register returns
its reset value. Only update this value if the device is enabled.

Ref: i.MX 6DQ Applications Processor Reference Manual (IMX6DQRM),
 chapter 21.7.3: Control Register (ECSPIx_CONREG)

Reviewed-by: Juan Quintela 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/ssi/imx_spi.c | 60 +++-
 1 file changed, 29 insertions(+), 31 deletions(-)

diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c
index 78b19c2eb91..ba7d3438d87 100644
--- a/hw/ssi/imx_spi.c
+++ b/hw/ssi/imx_spi.c
@@ -269,42 +269,40 @@ static uint64_t imx_spi_read(void *opaque, hwaddr offset, 
unsigned size)
 return 0;
 }
 
-switch (index) {
-case ECSPI_RXDATA:
-if (!imx_spi_is_enabled(s)) {
-value = 0;
-} else if (fifo32_is_empty(>rx_fifo)) {
-/* value is undefined */
-value = 0xdeadbeef;
-} else {
-/* read from the RX FIFO */
-value = fifo32_pop(>rx_fifo);
+value = s->regs[index];
+
+if (imx_spi_is_enabled(s)) {
+switch (index) {
+case ECSPI_RXDATA:
+if (fifo32_is_empty(>rx_fifo)) {
+/* value is undefined */
+value = 0xdeadbeef;
+} else {
+/* read from the RX FIFO */
+value = fifo32_pop(>rx_fifo);
+}
+break;
+case ECSPI_TXDATA:
+qemu_log_mask(LOG_GUEST_ERROR,
+  "[%s]%s: Trying to read from TX FIFO\n",
+  TYPE_IMX_SPI, __func__);
+
+/* Reading from TXDATA gives 0 */
+break;
+case ECSPI_MSGDATA:
+qemu_log_mask(LOG_GUEST_ERROR,
+  "[%s]%s: Trying to read from MSG FIFO\n",
+  TYPE_IMX_SPI, __func__);
+/* Reading from MSGDATA gives 0 */
+break;
+default:
+break;
 }
 
-break;
-case ECSPI_TXDATA:
-qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Trying to read from TX FIFO\n",
-  TYPE_IMX_SPI, __func__);
-
-/* Reading from TXDATA gives 0 */
-
-break;
-case ECSPI_MSGDATA:
-qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Trying to read from MSG 
FIFO\n",
-  TYPE_IMX_SPI, __func__);
-
-/* Reading from MSGDATA gives 0 */
-
-break;
-default:
-value = s->regs[index];
-break;
+imx_spi_update_irq(s);
 }
-
 DPRINTF("reg[%s] => 0x%" PRIx32 "\n", imx_spi_reg_name(index), value);
 
-imx_spi_update_irq(s);
-
 return (uint64_t)value;
 }
 
-- 
2.26.2




[PATCH v7 5/9] hw/ssi: imx_spi: Rework imx_spi_write() to handle block disabled

2021-01-15 Thread Philippe Mathieu-Daudé
When the block is disabled, only the ECSPI_CONREG register can
be modified. Setting the EN bit enabled the device, clearing it
"disables the block and resets the internal logic with the
exception of the ECSPI_CONREG" register.

Move the imx_spi_is_enabled() check earlier.

Ref: i.MX 6DQ Applications Processor Reference Manual (IMX6DQRM),
 chapter 21.7.3: Control Register (ECSPIx_CONREG)

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/ssi/imx_spi.c | 26 --
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c
index ba7d3438d87..f06bbf317e2 100644
--- a/hw/ssi/imx_spi.c
+++ b/hw/ssi/imx_spi.c
@@ -322,6 +322,21 @@ static void imx_spi_write(void *opaque, hwaddr offset, 
uint64_t value,
 DPRINTF("reg[%s] <= 0x%" PRIx32 "\n", imx_spi_reg_name(index),
 (uint32_t)value);
 
+if (!imx_spi_is_enabled(s)) {
+/* Block is disabled */
+if (index != ECSPI_CONREG) {
+/* Ignore access */
+return;
+}
+s->regs[ECSPI_CONREG] = value;
+if (!(value & ECSPI_CONREG_EN)) {
+/* Keep disabled */
+return;
+}
+/* Enable the block */
+imx_spi_reset(DEVICE(s));
+}
+
 change_mask = s->regs[index] ^ value;
 
 switch (index) {
@@ -330,10 +345,7 @@ static void imx_spi_write(void *opaque, hwaddr offset, 
uint64_t value,
   TYPE_IMX_SPI, __func__);
 break;
 case ECSPI_TXDATA:
-if (!imx_spi_is_enabled(s)) {
-/* Ignore writes if device is disabled */
-break;
-} else if (fifo32_is_full(>tx_fifo)) {
+if (fifo32_is_full(>tx_fifo)) {
 /* Ignore writes if queue is full */
 break;
 }
@@ -359,12 +371,6 @@ static void imx_spi_write(void *opaque, hwaddr offset, 
uint64_t value,
 case ECSPI_CONREG:
 s->regs[ECSPI_CONREG] = value;
 
-if (!imx_spi_is_enabled(s)) {
-/* device is disabled, so this is a reset */
-imx_spi_reset(DEVICE(s));
-return;
-}
-
 if (imx_spi_channel_is_master(s)) {
 int i;
 
-- 
2.26.2




[PATCH v7 8/9] hw/ssi: imx_spi: Correct the burst length > 32 bit transfer logic

2021-01-15 Thread Philippe Mathieu-Daudé
From: Bin Meng 

For the ECSPIx_CONREG register BURST_LENGTH field, the manual says:

0x020 A SPI burst contains the 1 LSB in first word and all 32 bits in second 
word.
0x021 A SPI burst contains the 2 LSB in first word and all 32 bits in second 
word.

Current logic uses either s->burst_length or 32, whichever smaller,
to determine how many bits it should read from the tx fifo each time.
For example, for a 48 bit burst length, current logic transfers the
first 32 bit from the first word in the tx fifo, followed by a 16
bit from the second word in the tx fifo, which is wrong. The correct
logic should be: transfer the first 16 bit from the first word in
the tx fifo, followed by a 32 bit from the second word in the tx fifo.

With this change, SPI flash can be successfully probed by U-Boot on
imx6 sabrelite board.

  => sf probe
  SF: Detected sst25vf016b with page size 256 Bytes, erase size 4 KiB, total 2 
MiB

Fixes: c906a3a01582 ("i.MX: Add the Freescale SPI Controller")
Signed-off-by: Bin Meng 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20210112145526.31095-6-bmeng...@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/ssi/imx_spi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c
index b79304d93d9..707defb8b3f 100644
--- a/hw/ssi/imx_spi.c
+++ b/hw/ssi/imx_spi.c
@@ -191,7 +191,7 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)
 
 DPRINTF("data tx:0x%08x\n", tx);
 
-tx_burst = MIN(s->burst_length, 32);
+tx_burst = (s->burst_length % 32) ? : 32;
 
 rx = 0;
 
-- 
2.26.2




Re: [PATCH v7 09/13] confidential guest support: Update documentation

2021-01-15 Thread Cornelia Huck
On Thu, 14 Jan 2021 10:58:07 +1100
David Gibson  wrote:

> Now that we've implemented a generic machine option for configuring various
> confidential guest support mechanisms:
>   1. Update docs/amd-memory-encryption.txt to reference this rather than
>  the earlier SEV specific option
>   2. Add a docs/confidential-guest-support.txt to cover the generalities of
>  the confidential guest support scheme
> 
> Signed-off-by: David Gibson 
> ---
>  docs/amd-memory-encryption.txt  |  2 +-
>  docs/confidential-guest-support.txt | 43 +
>  2 files changed, 44 insertions(+), 1 deletion(-)
>  create mode 100644 docs/confidential-guest-support.txt
> 
> diff --git a/docs/amd-memory-encryption.txt b/docs/amd-memory-encryption.txt
> index 80b8eb00e9..145896aec7 100644
> --- a/docs/amd-memory-encryption.txt
> +++ b/docs/amd-memory-encryption.txt
> @@ -73,7 +73,7 @@ complete flow chart.
>  To launch a SEV guest
>  
>  # ${QEMU} \
> --machine ...,memory-encryption=sev0 \
> +-machine ...,confidential-guest-support=sev0 \
>  -object sev-guest,id=sev0,cbitpos=47,reduced-phys-bits=1
>  
>  Debugging
> diff --git a/docs/confidential-guest-support.txt 
> b/docs/confidential-guest-support.txt
> new file mode 100644
> index 00..2790425b38
> --- /dev/null
> +++ b/docs/confidential-guest-support.txt

Maybe make this a proper .rst from the start and hook this up into the
system guide? It is already almost there.

> @@ -0,0 +1,43 @@
> +Confidential Guest Support
> +==
> +
> +Traditionally, hypervisors such as qemu have complete access to a

s/qemu/QEMU/ ?

> +guest's memory and other state, meaning that a compromised hypervisor
> +can compromise any of its guests.  A number of platforms have added
> +mechanisms in hardware and/or firmware which give guests at least some
> +protection from a compromised hypervisor.  This is obviously
> +especially desirable for public cloud environments.
> +
> +These mechanisms have different names and different modes of
> +operation, but are often referred to as Secure Guests or Confidential
> +Guests.  We use the term "Confidential Guest Support" to distinguish
> +this from other aspects of guest security (such as security against
> +attacks from other guests, or from network sources).
> +
> +Running a Confidential Guest
> +
> +
> +To run a confidential guest you need to add two command line parameters:
> +
> +1. Use "-object" to create a "confidential guest support" object.  The
> +   type and parameters will vary with the specific mechanism to be
> +   used
> +2. Set the "confidential-guest-support" machine parameter to the ID of
> +   the object from (1).
> +
> +Example (for AMD SEV)::
> +
> +qemu-system-x86_64 \
> + \
> +-machine ...,confidential-guest-support=sev0 \
> +-object sev-guest,id=sev0,cbitpos=47,reduced-phys-bits=1
> +
> +Supported mechanisms
> +
> +
> +Currently supported confidential guest mechanisms are:
> +
> +AMD Secure Encrypted Virtualization (SEV)
> +docs/amd-memory-encryption.txt
> +
> +Other mechanisms may be supported in future.

LGTM.




[PATCH v7 7/9] hw/ssi: imx_spi: Round up the burst length to be multiple of 8

2021-01-15 Thread Philippe Mathieu-Daudé
From: Bin Meng 

Current implementation of the imx spi controller expects the burst
length to be multiple of 8, which is the most common use case.

In case the burst length is not what we expect, log it to give user
a chance to notice it, and round it up to be multiple of 8.

Signed-off-by: Bin Meng 
Message-Id: <20210112145526.31095-5-bmeng...@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/ssi/imx_spi.c | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c
index c132f99ba5b..b79304d93d9 100644
--- a/hw/ssi/imx_spi.c
+++ b/hw/ssi/imx_spi.c
@@ -128,7 +128,20 @@ static uint8_t imx_spi_selected_channel(IMXSPIState *s)
 
 static uint32_t imx_spi_burst_length(IMXSPIState *s)
 {
-return EXTRACT(s->regs[ECSPI_CONREG], ECSPI_CONREG_BURST_LENGTH) + 1;
+uint32_t burst;
+
+burst = EXTRACT(s->regs[ECSPI_CONREG], ECSPI_CONREG_BURST_LENGTH) + 1;
+if (burst % 8) {
+qemu_log_mask(LOG_UNIMP,
+  "[%s]%s: burst length (%d) not multiple of 8!\n",
+  TYPE_IMX_SPI, __func__, burst);
+burst = ROUND_UP(burst, 8);
+qemu_log_mask(LOG_UNIMP,
+  "[%s]%s: burst length rounded up to %d; this may not 
work.\n",
+  TYPE_IMX_SPI, __func__, burst);
+}
+
+return burst;
 }
 
 static bool imx_spi_is_enabled(IMXSPIState *s)
-- 
2.26.2




[PATCH v7 9/9] hw/ssi: imx_spi: Correct tx and rx fifo endianness

2021-01-15 Thread Philippe Mathieu-Daudé
From: Bin Meng 

The endianness of data exchange between tx and rx fifo is incorrect.
Earlier bytes are supposed to show up on MSB and later bytes on LSB,
ie: in big endian. The manual does not explicitly say this, but the
U-Boot and Linux driver codes have a swap on the data transferred
to tx fifo and from rx fifo.

With this change, U-Boot read from / write to SPI flash tests pass.

  => sf test 1ff000 1000
  SPI flash test:
  0 erase: 0 ticks, 4096000 KiB/s 32768.000 Mbps
  1 check: 3 ticks, 1333 KiB/s 10.664 Mbps
  2 write: 235 ticks, 17 KiB/s 0.136 Mbps
  3 read: 2 ticks, 2000 KiB/s 16.000 Mbps
  Test passed
  0 erase: 0 ticks, 4096000 KiB/s 32768.000 Mbps
  1 check: 3 ticks, 1333 KiB/s 10.664 Mbps
  2 write: 235 ticks, 17 KiB/s 0.136 Mbps
  3 read: 2 ticks, 2000 KiB/s 16.000 Mbps

Fixes: c906a3a01582 ("i.MX: Add the Freescale SPI Controller")
Signed-off-by: Bin Meng 
Message-Id: <20210112145526.31095-7-bmeng...@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/ssi/imx_spi.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c
index 707defb8b3f..081b7e464ff 100644
--- a/hw/ssi/imx_spi.c
+++ b/hw/ssi/imx_spi.c
@@ -175,7 +175,6 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)
 
 while (!fifo32_is_empty(>tx_fifo)) {
 int tx_burst = 0;
-int index = 0;
 
 if (s->burst_length <= 0) {
 s->burst_length = imx_spi_burst_length(s);
@@ -196,7 +195,7 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)
 rx = 0;
 
 while (tx_burst > 0) {
-uint8_t byte = tx & 0xff;
+uint8_t byte = tx >> (tx_burst - 8);
 
 DPRINTF("writing 0x%02x\n", (uint32_t)byte);
 
@@ -205,13 +204,11 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)
 
 DPRINTF("0x%02x read\n", (uint32_t)byte);
 
-tx = tx >> 8;
-rx |= (byte << (index * 8));
+rx = (rx << 8) | byte;
 
 /* Remove 8 bits from the actual burst */
 tx_burst -= 8;
 s->burst_length -= 8;
-index++;
 }
 
 DPRINTF("data rx:0x%08x\n", rx);
-- 
2.26.2




Re: [PULL 00/30] testing, gdbstub and semihosting

2021-01-15 Thread Peter Maydell
On Fri, 15 Jan 2021 at 13:08, Alex Bennée  wrote:
>
> The following changes since commit 7c79721606be11b5bc556449e5bcbc331ef6867d:
>
>   Merge remote-tracking branch 'remotes/rth-gitlab/tags/pull-tcg-20210113' 
> into staging (2021-01-14 09:54:29 +)
>
> are available in the Git repository at:
>
>   https://github.com/stsquad/qemu.git tags/pull-testing-and-misc-150121-1
>
> for you to fetch changes up to be846761ca8b5a7e2e7b7108c8c156126b799824:
>
>   semihosting: Implement SYS_ISERROR (2021-01-15 11:12:34 +)
>
> 
> Testing, gdbstub and semihosting patches:
>
>   - clean-ups to docker images
>   - drop duplicate jobs from shippable
>   - prettier tag generation (+gtags)
>   - generate browsable source tree
>   - more Travis->GitLab migrations
>   - fix checkpatch to deal with commits
>   - gate gdbstub tests on 8.3.1, expand tests
>   - support Xfer:auxv:read gdb packet
>   - better gdbstub cleanup
>   - use GDB's SVE register layout
>   - make arm-compat-semihosting common
>   - add riscv semihosting support
>   - add HEAPINFO, ELAPSED, TICKFREQ, TMPNAM and ISERROR to semihosting

Fails to build, netbsd:

../src/gdbstub.c: In function 'handle_query_xfer_auxv':
../src/gdbstub.c:2258:26: error: 'struct image_info' has no member
named 'saved_auxv'
 saved_auxv = ts->info->saved_auxv;
  ^~
../src/gdbstub.c:2259:24: error: 'struct image_info' has no member
named 'auxv_len'
 auxv_len = ts->info->auxv_len;
^~

thanks
-- PMM



Re: [PATCH] fuzz: Add virtio-9p configurations for fuzzing

2021-01-15 Thread Alexander Bulekov
On 210115 1323, Greg Kurz wrote:
> On Thu, 14 Jan 2021 17:17:48 -0500
> Alexander Bulekov  wrote:
> 
> > Signed-off-by: Alexander Bulekov 
> > ---
> 
> No changelog at all ? 
> 
> >  tests/qtest/fuzz/generic_fuzz_configs.h | 12 
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/tests/qtest/fuzz/generic_fuzz_configs.h 
> > b/tests/qtest/fuzz/generic_fuzz_configs.h
> > index 7fed035345..ffdb590c58 100644
> > --- a/tests/qtest/fuzz/generic_fuzz_configs.h
> > +++ b/tests/qtest/fuzz/generic_fuzz_configs.h
> > @@ -59,6 +59,18 @@ const generic_fuzz_config predefined_configs[] = {
> >  .name = "virtio-mouse",
> >  .args = "-machine q35 -nodefaults -device virtio-mouse",
> >  .objects = "virtio*",
> > +},{
> > +.name = "virtio-9p",
> > +.args = "-machine q35 -nodefaults "
> > +"-device virtio-9p,fsdev=hshare,mount_tag=hshare "
> > +"-fsdev local,id=hshare,path=/tmp/,security_model=none",
> 
> Sharing a general purpose directory like "/tmp" is definitely not a
> recommended practice. This is typically the kind of thing that I'd
> like to see documented in the changelog to help me understand ;-)

Hi Greg,
Yes it is not a great solution. The fuzzers in this file are mainly
configured to run on OSS-Fuzz (https://github.com/google/oss-fuzz),
where fuzzers are executed in individual containers, and there shouldn't
be anything sensitive in /tmp/. In v2, I'll use a safer solution.

> 
> What operations does the fuzz test perform on the device ?

The generic-fuzzer will interact with the Port IO/MMIO and PCI Config
Space regions associated with the virtio-9p device. When the
device tries to access some guest memory using DMA, the fuzzer will
place some fuzzed data at the corresponding location. For many devices,
this is sufficient to achieve high coverage. If this doesn't work well
for the virtio-9p, we can add a tailored fuzzer based on the libqos
interface, in the future.

> 
> > +.objects = "virtio*",
> > +},{
> > +.name = "virtio-9p-synth",
> > +.args = "-machine q35 -nodefaults "
> > +"-device virtio-9p,fsdev=hshare,mount_tag=hshare "
> > +"-fsdev synth,id=hshare",
> > +.objects = "virtio*",
> 
> Not sure this is super useful since the only known use case for
> the synth fsdev driver is running the virtio-9p qtest, but
> it looks fine anyway.

My hope here was is that this configureation will cut down on syscalls
(improve fuzzing speed) and avoid leaky state (files left in the /tmp/
directory between individual fuzzer inputs).
-Alex

> 
> >  },{
> >  .name = "e1000",
> >  .args = "-M q35 -nodefaults "
> 



Re: [PATCH v7 10/13] spapr: Add PEF based confidential guest support

2021-01-15 Thread Cornelia Huck
On Thu, 14 Jan 2021 10:58:08 +1100
David Gibson  wrote:

> Some upcoming POWER machines have a system called PEF (Protected
> Execution Facility) which uses a small ultravisor to allow guests to
> run in a way that they can't be eavesdropped by the hypervisor.  The
> effect is roughly similar to AMD SEV, although the mechanisms are
> quite different.
> 
> Most of the work of this is done between the guest, KVM and the
> ultravisor, with little need for involvement by qemu.  However qemu
> does need to tell KVM to allow secure VMs.
> 
> Because the availability of secure mode is a guest visible difference
> which depends on having the right hardware and firmware, we don't
> enable this by default.  In order to run a secure guest you need to
> create a "pef-guest" object and set the confidential-guest-support
> property to point to it.
> 
> Note that this just *allows* secure guests, the architecture of PEF is
> such that the guest still needs to talk to the ultravisor to enter
> secure mode.  Qemu has no directl way of knowing if the guest is in
> secure mode, and certainly can't know until well after machine
> creation time.
> 
> To start a PEF-capable guest, use the command line options:
> -object pef-guest,id=pef0 -machine confidential-guest-support=pef0
> 
> Signed-off-by: David Gibson 
> ---
>  docs/confidential-guest-support.txt |   3 +
>  docs/papr-pef.txt   |  30 +++
>  hw/ppc/meson.build  |   1 +
>  hw/ppc/pef.c| 119 
>  hw/ppc/spapr.c  |   6 ++
>  include/hw/ppc/pef.h|  25 ++
>  target/ppc/kvm.c|  18 -
>  target/ppc/kvm_ppc.h|   6 --
>  8 files changed, 184 insertions(+), 24 deletions(-)
>  create mode 100644 docs/papr-pef.txt
>  create mode 100644 hw/ppc/pef.c
>  create mode 100644 include/hw/ppc/pef.h
> 
> diff --git a/docs/confidential-guest-support.txt 
> b/docs/confidential-guest-support.txt
> index 2790425b38..f0801814ff 100644
> --- a/docs/confidential-guest-support.txt
> +++ b/docs/confidential-guest-support.txt
> @@ -40,4 +40,7 @@ Currently supported confidential guest mechanisms are:
>  AMD Secure Encrypted Virtualization (SEV)
>  docs/amd-memory-encryption.txt
>  
> +POWER Protected Execution Facility (PEF)
> +docs/papr-pef.txt
> +
>  Other mechanisms may be supported in future.
> diff --git a/docs/papr-pef.txt b/docs/papr-pef.txt
> new file mode 100644
> index 00..6419e995cf
> --- /dev/null
> +++ b/docs/papr-pef.txt

Same here, make this .rst and add it to the system guide?

> @@ -0,0 +1,30 @@
> +POWER (PAPR) Protected Execution Facility (PEF)
> +===
> +
> +Protected Execution Facility (PEF), also known as Secure Guest support
> +is a feature found on IBM POWER9 and POWER10 processors.
> +
> +If a suitable firmware including an Ultravisor is installed, it adds
> +an extra memory protection mode to the CPU.  The ultravisor manages a
> +pool of secure memory which cannot be accessed by the hypervisor.
> +
> +When this feature is enabled in qemu, a guest can use ultracalls to

s/qemu/QEMU/

> +enter "secure mode".  This transfers most of its memory to secure
> +memory, where it cannot be eavesdropped by a compromised hypervisor.
> +
> +Launching
> +-
> +
> +To launch a guest which will be permitted to enter PEF secure mode:
> +
> +# ${QEMU} \
> +-object pef-guest,id=pef0 \
> +-machine confidential-guest-support=pef0 \
> +...
> +
> +Live Migration
> +
> +
> +Live migration is not yet implemented for PEF guests.  For
> +consistency, we currently prevent migration if the PEF feature is
> +enabled, whether or not the guest has actually entered secure mode.
> diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
> index ffa2ec37fa..218631c883 100644
> --- a/hw/ppc/meson.build
> +++ b/hw/ppc/meson.build
> @@ -27,6 +27,7 @@ ppc_ss.add(when: 'CONFIG_PSERIES', if_true: files(
>'spapr_nvdimm.c',
>'spapr_rtas_ddw.c',
>'spapr_numa.c',
> +  'pef.c',
>  ))
>  ppc_ss.add(when: 'CONFIG_SPAPR_RNG', if_true: files('spapr_rng.c'))
>  ppc_ss.add(when: ['CONFIG_PSERIES', 'CONFIG_LINUX'], if_true: files(
> diff --git a/hw/ppc/pef.c b/hw/ppc/pef.c
> new file mode 100644
> index 00..02b9b3b460
> --- /dev/null
> +++ b/hw/ppc/pef.c
> @@ -0,0 +1,119 @@
> +/*
> + * PEF (Protected Execution Facility) for POWER support
> + *
> + * Copyright David Gibson, Redhat Inc. 2020

2021?

> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +




Re: Windows installer builds apparently broken since October?

2021-01-15 Thread Stefan Weil

Am 15.01.21 um 15:01 schrieb Peter Maydell:


I was just trying to see what updates the qemu.nsi file needed for
the merge-all-the-manuals-into-one-place change, and I discovered
that it's been broken since October when we removed the Changelog file:

File: "/tmp/qemu-test/src\Changelog" -> no files found.
Usage: File [/nonfatal] [/a] ([/r] [/x filespec [...]] filespec [...] |
/oname=outfile one_file_only)
Error in script "../src/qemu.nsi" on line 119 -- aborting creation process

You can reproduce that with:

make -C my-build-dir docker-image-fedora V=1 NETWORK=1
make -C my-build-dir docker-test-mingw@fedora J=8 NETWORK=1

This used to be in CI (patchew ran this config) but it clearly can't
be being CI'd any more, or we'd have noticed.

Stefan, I see you have more recent installer binaries on your
site than that -- do you have some local patches for this?



Hello Peter,

although I have some local fixes (available for example in 
https://github.com/stweil/qemu/) I am still struggling with 5.2.0.


One problem which was recently discussed on the list is the directory 
structure of the installation (especially the location for BIOS and 
similar files) which still needs changes (which als require updates for 
qemu.nsi). I'd prefer a similar hierarchical structure for both Linux 
and Windows (instead of a flat one which does not work with the current 
code).


Other problems are caused by the new QEMU build system in my special 
build context (Debian cross build with Cygwin packages).


A third challenge comes from users who would like to see new features 
like zstd or braille which up to now were missing in my binaries.


As I am quite busy with other things, too, I am afraid that it will take 
some more weeks until I can send a set of patches to fix the most urgent 
issues.


Removing Changelog from qemu.nsi is easy, but not nearly sufficient: 
https://github.com/stweil/qemu/commit/923c93a663e4e51231f6ea389c19c0a960fa9f99.


Kind regards,

Stefan





Realize methods realizing "sideways" in the composition tree

2021-01-15 Thread Markus Armbruster
Perhaps I'm slow on the uptake today...

We have

typedef struct XHCIPciState {
/*< private >*/
PCIDevice parent_obj;
/*< public >*/
(1) XHCIState xhci;
OnOffAuto msi;
OnOffAuto msix;
} XHCIPciState;

This is a PCI device that contains a (bus-less) TYPE_XHCI device, at
(1).

static void xhci_instance_init(Object *obj)
{
XHCIPciState *s = XHCI_PCI(obj);
/*
 * QEMU_PCI_CAP_EXPRESS initialization does not depend on QEMU command
 * line, therefore, no need to wait to realize like other devices
 */
PCI_DEVICE(obj)->cap_present |= QEMU_PCI_CAP_EXPRESS;
(2) object_initialize_child(obj, "xhci-core", >xhci, TYPE_XHCI);
qdev_alias_all_properties(DEVICE(>xhci), obj);
}

The .instance_init() method initializes the child as it should, at (2).

static void usb_xhci_pci_realize(struct PCIDevice *dev, Error **errp)
{
int ret;
Error *err = NULL;
XHCIPciState *s = XHCI_PCI(dev);

[a few dev->config[] modifications...]

(1) object_property_set_link(OBJECT(>xhci), "host", OBJECT(s), NULL);
s->xhci.intr_update = xhci_pci_intr_update;
s->xhci.intr_raise = xhci_pci_intr_raise;
(2) object_property_set_bool(OBJECT(>xhci), "realized", true, );
if (err) {
error_propagate(errp, err);
return;
}

The .realize() method realizes the child at (1).  It should use
qdev_realize() like we do everywhere else, since commit ce189ab230
"qdev: Convert bus-less devices to qdev_realize() with Coccinelle".

It sets a link property from the child back to the parent at (2).  Why
do we need a link?  Each QOM Object contains a pointer to its parent,
doesn't it?

Same for xhci_sysbus_realize().




[PATCH v3] docs: Build and install all the docs in a single manual

2021-01-15 Thread Peter Maydell
When we first converted our documentation to Sphinx, we split it into
multiple manuals (system, interop, tools, etc), which are all built
separately.  The primary driver for this was wanting to be able to
avoid shipping the 'devel' manual to end-users.  However, this is
working against the grain of the way Sphinx wants to be used and
causes some annoyances:
 * Cross-references between documents become much harder or
   possibly impossible
 * There is no single index to the whole documentation
 * Within one manual there's no links or table-of-contents info
   that lets you easily navigate to the others
 * The devel manual doesn't get published on the QEMU website
   (it would be nice to able to refer to it there)

Merely hiding our developer documentation from end users seems like
it's not enough benefit for these costs.  Combine all the
documentation into a single manual (the same way that the readthedocs
site builds it) and install the whole thing.  The previous manual
divisions remain as the new top level sections in the manual.

 * The per-manual conf.py files are no longer needed
 * The man_pages[] specifications previously in each per-manual
   conf.py move to the top level conf.py
 * docs/meson.build logic is simplified as we now only need to run
   Sphinx once for the HTML and then once for the manpages5B
 * The old index.html.in that produced the top-level page with
   links to each manual is no longer needed

Unfortunately this means that we now have to build the HTML
documentation into docs/manual in the build tree rather than directly
into docs/; otherwise it is too awkward to ensure we install only the
built manual and not also the dependency info, stamp file, etc.  The
manual still ends up in the same place in the final installed
directory, but anybody who was consulting documentation from within
the build tree will have to adjust where they're looking.

Signed-off-by: Peter Maydell 
Reviewed-by: Paolo Bonzini 
---
Based-on: <20210114165730.31607-1-alex.ben...@linaro.org>
"[PATCH v2 00/12] testing/next (tags!, shippable/travis deprecation, regression 
fixes, checkpatch)"
to avoid a textual conflict in .gitlab-ci.yml

Changes v2->v3:
 * just the addition of the .gitlab-ci.yml change to fix the Pages CI job
Since this is a 2-line change I've taken the liberty of keeping Paolo's
R-by tag on the basis that it's not a material change to the bulk of the patch.


 docs/conf.py | 46 ++-
 docs/devel/conf.py   | 15 ---
 docs/index.html.in   | 17 
 docs/interop/conf.py | 28 ---
 docs/meson.build | 64 +---
 docs/specs/conf.py   | 16 ---
 docs/system/conf.py  | 28 ---
 docs/tools/conf.py   | 37 -
 docs/user/conf.py| 15 ---
 .gitlab-ci.yml   |  4 +--
 10 files changed, 72 insertions(+), 198 deletions(-)
 delete mode 100644 docs/devel/conf.py
 delete mode 100644 docs/index.html.in
 delete mode 100644 docs/interop/conf.py
 delete mode 100644 docs/specs/conf.py
 delete mode 100644 docs/system/conf.py
 delete mode 100644 docs/tools/conf.py
 delete mode 100644 docs/user/conf.py

diff --git a/docs/conf.py b/docs/conf.py
index d40d8ff37ba..2ee61118725 100644
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -224,7 +224,51 @@ latex_documents = [
 
 # -- Options for manual page output ---
 # Individual manual/conf.py can override this to create man pages
-man_pages = []
+man_pages = [
+('interop/qemu-ga', 'qemu-ga',
+ 'QEMU Guest Agent',
+ ['Michael Roth '], 8),
+('interop/qemu-ga-ref', 'qemu-ga-ref',
+ 'QEMU Guest Agent Protocol Reference',
+ [], 7),
+('interop/qemu-qmp-ref', 'qemu-qmp-ref',
+ 'QEMU QMP Reference Manual',
+ [], 7),
+('interop/qemu-storage-daemon-qmp-ref', 'qemu-storage-daemon-qmp-ref',
+ 'QEMU Storage Daemon QMP Reference Manual',
+ [], 7),
+('system/qemu-manpage', 'qemu',
+ 'QEMU User Documentation',
+ ['Fabrice Bellard'], 1),
+('system/qemu-block-drivers', 'qemu-block-drivers',
+ 'QEMU block drivers reference',
+ ['Fabrice Bellard and the QEMU Project developers'], 7),
+('system/qemu-cpu-models', 'qemu-cpu-models',
+ 'QEMU CPU Models',
+ ['The QEMU Project developers'], 7),
+('tools/qemu-img', 'qemu-img',
+ 'QEMU disk image utility',
+ ['Fabrice Bellard'], 1),
+('tools/qemu-nbd', 'qemu-nbd',
+ 'QEMU Disk Network Block Device Server',
+ ['Anthony Liguori '], 8),
+('tools/qemu-pr-helper', 'qemu-pr-helper',
+ 'QEMU persistent reservation helper',
+ [], 8),
+('tools/qemu-storage-daemon', 'qemu-storage-daemon',
+ 'QEMU storage daemon',
+ [], 1),
+('tools/qemu-trace-stap', 'qemu-trace-stap',
+ 'QEMU SystemTap trace tool',
+ [], 1),
+('tools/virtfs-proxy-helper', 'virtfs-proxy-helper',
+ 'QEMU 9p virtfs proxy filesystem helper',
+ ['M. Mohan 

Re: [PATCH] fuzz: Add virtio-9p configurations for fuzzing

2021-01-15 Thread Alexander Bulekov
On 210115 1351, Christian Schoenebeck wrote:
> On Freitag, 15. Januar 2021 13:23:08 CET Greg Kurz wrote:
> > On Thu, 14 Jan 2021 17:17:48 -0500
> > 
> > Alexander Bulekov  wrote:
> > > Signed-off-by: Alexander Bulekov 
> > > ---
> > 
> > No changelog at all ?
> 
> Yeah, that's indeed quite short. :)
> 
> > >  tests/qtest/fuzz/generic_fuzz_configs.h | 12 
> > >  1 file changed, 12 insertions(+)
> > > 
> > > diff --git a/tests/qtest/fuzz/generic_fuzz_configs.h
> > > b/tests/qtest/fuzz/generic_fuzz_configs.h index 7fed035345..ffdb590c58
> > > 100644
> > > --- a/tests/qtest/fuzz/generic_fuzz_configs.h
> > > +++ b/tests/qtest/fuzz/generic_fuzz_configs.h
> > > @@ -59,6 +59,18 @@ const generic_fuzz_config predefined_configs[] = {
> > > 
> > >  .name = "virtio-mouse",
> > >  .args = "-machine q35 -nodefaults -device virtio-mouse",
> > >  .objects = "virtio*",
> > > 
> > > +},{
> > > +.name = "virtio-9p",
> > > +.args = "-machine q35 -nodefaults "
> > > +"-device virtio-9p,fsdev=hshare,mount_tag=hshare "
> > > +"-fsdev local,id=hshare,path=/tmp/,security_model=none",
> > 
> > Sharing a general purpose directory like "/tmp" is definitely not a
> > recommended practice. This is typically the kind of thing that I'd
> > like to see documented in the changelog to help me understand ;-)
> 
> For the (non fuzzing) 9p 'local' tests Peter wanted either an auto generated 
> (e.g. mkdtemp()) or at least a hard coded test path that allows a person to 
> easily identify where the data came from. So I guess that applies to fuzzing 
> as well, i.e. something like "/tmp/qemu-fuzz/9pfs/" at least.
> 
> Also note that the existing (non fuzzing) 9p 'local' tests auto generate  
> individual test pathes with mkdtemp() already. This was necessary there, 
> because tests are often run by "make -jN ..." in which case tests were 
> accessing concurrently the same single test directory before. Probably less 
> of 
> a problem here, but you might consider using the same approach that
> virtio-9p-test.c already does.
> 
> Also note that 'security_model=none' is maybe Ok as a starting point for 
> fuzzing, but a realistic 9p config is rather 'security_model=xattr', because 
> 'security_model=none' requires the qemu process to be run as root to avoid 
> permission denied errors with any minor operation. I would expect these 
> fuzzing tests to mostly error out with permission denied errors early instead 
> of entering relevant execution pathes.
> 

Ah. That's good to know. I just copied the security_model from the bug
report for the recent CVE (https://bugs.launchpad.net/qemu/+bug/1911666)
I'll switch to mapped-xattr, in v2
-Alex

> > What operations does the fuzz test perform on the device ?
> > 
> > > +.objects = "virtio*",
> > > +},{
> > > +.name = "virtio-9p-synth",
> > > +.args = "-machine q35 -nodefaults "
> > > +"-device virtio-9p,fsdev=hshare,mount_tag=hshare "
> > > +"-fsdev synth,id=hshare",
> > > +.objects = "virtio*",
> > 
> > Not sure this is super useful since the only known use case for
> > the synth fsdev driver is running the virtio-9p qtest, but
> > it looks fine anyway.
> 
> Yeah, that's ok. Maybe it raises the chance to enter some execution pathes 
> here and there. So I would keep the 'synth' driver config.
> 
> > 
> > >  },{
> > >  
> > >  .name = "e1000",
> > >  .args = "-M q35 -nodefaults "
> 
> Nice to see fuzzing coming for 9p BTW!
> 
> Best regards,
> Christian Schoenebeck
> 
> 



Re: [PATCH 7/7] block/rbd: change request alignment to 1 byte

2021-01-15 Thread Peter Lieven
Am 15.01.21 um 16:27 schrieb Jason Dillaman:
> On Thu, Jan 14, 2021 at 2:59 PM Peter Lieven  wrote:
>> Am 14.01.21 um 20:19 schrieb Jason Dillaman:
>>> On Sun, Dec 27, 2020 at 11:42 AM Peter Lieven  wrote:
 since we implement byte interfaces and librbd supports aio on byte 
 granularity we can lift
 the 512 byte alignment.

 Signed-off-by: Peter Lieven 
 ---
  block/rbd.c | 2 --
  1 file changed, 2 deletions(-)

 diff --git a/block/rbd.c b/block/rbd.c
 index 27b4404adf..8673e8f553 100644
 --- a/block/rbd.c
 +++ b/block/rbd.c
 @@ -223,8 +223,6 @@ done:
  static void qemu_rbd_refresh_limits(BlockDriverState *bs, Error **errp)
  {
  BDRVRBDState *s = bs->opaque;
 -/* XXX Does RBD support AIO on less than 512-byte alignment? */
 -bs->bl.request_alignment = 512;
>>> Just a suggestion, but perhaps improve discard alignment, max discard,
>>> optimal alignment (if that's something QEMU handles internally) if not
>>> overridden by the user.
>>
>> Qemu supports max_discard and discard_alignment. Is there a call to get 
>> these limits
>>
>> from librbd?
>>
>>
>> What do you mean by optimal_alignment? The object size?
> krbd does a good job of initializing defaults [1] where optimal and
> discard alignment is 64KiB (can actually be 4KiB now), max IO size for
> writes, discards, and write-zeroes is the object size * the stripe
> count.


Okay, I will have a look at it. If qemu issues a write, discard, write_zero 
greater than

obj_size  * stripe count will librbd split it internally or will the request 
fail?


Regarding the alignment it seems that rbd_dev->opts->alloc_size is something 
that comes from the device

configuration and not from rbd? I don't have that information inside the Qemu 
RBD driver.


Peter





Re: Windows installer builds apparently broken since October?

2021-01-15 Thread Peter Maydell
On Fri, 15 Jan 2021 at 15:34, Stefan Weil  wrote:
> although I have some local fixes (available for example in
> https://github.com/stweil/qemu/) I am still struggling with 5.2.0.
>
> One problem which was recently discussed on the list is the directory
> structure of the installation (especially the location for BIOS and
> similar files) which still needs changes (which als require updates for
> qemu.nsi). I'd prefer a similar hierarchical structure for both Linux
> and Windows (instead of a flat one which does not work with the current
> code).

Yeah, that was about where I got stuck. Somehow the way the
temporary install directory is created for the Windows build
shoves everything in a single directory (you don't get this if
you do a normal Linux install -- that has separate bin, docs,
icons, etc subdirectories). This gets even worse with the single-manual
documentation because all the files for the manual are then in
that same directory too :-(

I don't think I can really get this to work with the single-manual
documentation setup as qemu.nsi stands in master. So my plan is
to just commit the single-manual changes (probably next week some
time) and then we I'm happy to help with the Windows installer
changes necessary for the manual part once it's basically working
again.

thanks
-- PMM



Re: Realize methods realizing "sideways" in the composition tree

2021-01-15 Thread Peter Maydell
On Fri, 15 Jan 2021 at 15:45, Markus Armbruster  wrote:
>
> The .realize() method realizes the child at (1).  It should use
> qdev_realize() like we do everywhere else, since commit ce189ab230
> "qdev: Convert bus-less devices to qdev_realize() with Coccinelle".
>
> It sets a link property from the child back to the parent at (2).  Why
> do we need a link?  Each QOM Object contains a pointer to its parent,
> doesn't it?

It does, but what should parent object pointers be used for?
My assumption is that you'd only use those where you really
wanted to traverse the QOM tree. Generally I would use a link
property when I wanted one object to have a pointer to the
other regardless of what the QOM-tree relationship happens to
be. Today all the users of XHCIState happen to create it in a
way that means they're parents of it, but that doesn't seem
like it should be an inherent requirement that we bake into
its API.

thanks
-- PMM



Re: [PATCH 1/4] tests/qtest: Remove TPM tests

2021-01-15 Thread Philippe Mathieu-Daudé
Subject is incorrect, this is not a removal of the tests, but
removal of their execution. The tests are still in the repository.
This is more of a disablement.

On 1/15/21 4:09 PM, Philippe Mathieu-Daudé wrote:
> The TPM tests are failing, and no further tests are run,
> making the rest of the testsuite pointless:
> 
>   $ make check-qtest
>   =
>   ==3330026==ERROR: LeakSanitizer: detected memory leaks
...
>   SUMMARY: AddressSanitizer: 449172 byte(s) leaked in 324 allocation(s).
>   make: *** [Makefile.mtest:1025: run-test-126] Error 1
> 
> Remove these tests to be able to run the rest.
> 
> Cc: Stefan Berger 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  tests/qtest/meson.build | 4 
>  1 file changed, 4 deletions(-)
> 
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index 16d04625b8b..bcbb04d2bb4 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -41,10 +41,6 @@
>(config_all_devices.has_key('CONFIG_USB_UHCI') and 
>\
> config_all_devices.has_key('CONFIG_USB_EHCI') ? ['usb-hcd-ehci-test'] : 
> []) +\
>(config_all_devices.has_key('CONFIG_USB_XHCI_NEC') ? ['usb-hcd-xhci-test'] 
> : []) +\
> -  (config_all_devices.has_key('CONFIG_TPM_CRB') ? ['tpm-crb-test'] : []) +   
>\
> -  (config_all_devices.has_key('CONFIG_TPM_CRB') ? ['tpm-crb-swtpm-test'] : 
> []) +\
> -  (config_all_devices.has_key('CONFIG_TPM_TIS_ISA') ? ['tpm-tis-test'] : []) 
> +  \
> -  (config_all_devices.has_key('CONFIG_TPM_TIS_ISA') ? ['tpm-tis-swtpm-test'] 
> : []) +\
>(config_all_devices.has_key('CONFIG_RTL8139_PCI') ? ['rtl8139-test'] : []) 
> +  \
>qtests_pci +   
>\
>['fdc-test',
> 




Re: Windows installer builds apparently broken since October?

2021-01-15 Thread Stefan Weil

Am 15.01.21 um 16:19 schrieb Peter Maydell:


On Fri, 15 Jan 2021 at 14:01, Peter Maydell  wrote:

I was just trying to see what updates the qemu.nsi file needed for
the merge-all-the-manuals-into-one-place change, and I discovered
that it's been broken since October when we removed the Changelog file:

File: "/tmp/qemu-test/src\Changelog" -> no files found.
Usage: File [/nonfatal] [/a] ([/r] [/x filespec [...]] filespec [...] |
/oname=outfile one_file_only)
Error in script "../src/qemu.nsi" on line 119 -- aborting creation process

...and if you fix that it then fails because it's looking for
a subdirectory share/ in the install dir and it doesn't exist...



That's one of the problems which I mentioned in my other mail.

There should be a subdirectory share/ for Windows, too. The current 
special case with a flat file structure for Windows does not work and is 
one of the reasons why I still did not finish a working installer for 5.2.0.


I forgot to mention that some of the problems with the Meson build also 
occur on macOS with Homebrew: they always happen when a software package 
requires special compiler flags to find its include files or libraries, 
but the Meson build does not use the result from pkg-config for them.


Stefan





Re: [PATCH 1/4] tests/qtest: Remove TPM tests

2021-01-15 Thread Stefan Berger

On 1/15/21 10:52 AM, Philippe Mathieu-Daudé wrote:

Subject is incorrect, this is not a removal of the tests, but
removal of their execution. The tests are still in the repository.
This is more of a disablement.


How do you compile / run them to have the LeakSanitizer checks?





Re: Windows installer builds apparently broken since October?

2021-01-15 Thread Peter Maydell
On Fri, 15 Jan 2021 at 15:52, Stefan Weil  wrote:
> I forgot to mention that some of the problems with the Meson build also
> occur on macOS with Homebrew: they always happen when a software package
> requires special compiler flags to find its include files or libraries,
> but the Meson build does not use the result from pkg-config for them.

Yeah -- we fixed that in commit 3eacf70bb5a83e4 for gnutls, which
is the main one homebrew was running into. Is Windows having
problems with other libs too?

Paolo: did we come up with a generic solution for this or
do we really have to add entries to dependency lists all
over the build system for every library we use?

thanks
-- PMM



Re: [PATCH 1/4] tests/qtest: Remove TPM tests

2021-01-15 Thread Philippe Mathieu-Daudé
On 1/15/21 4:53 PM, Stefan Berger wrote:
> On 1/15/21 10:52 AM, Philippe Mathieu-Daudé wrote:
>> Subject is incorrect, this is not a removal of the tests, but
>> removal of their execution. The tests are still in the repository.
>> This is more of a disablement.
> 
> How do you compile / run them to have the LeakSanitizer checks?

I used:

../configure --cc=clang --enable-sanitizers && make check-qtest

$ clang -v
clang version 10.0.1 (Fedora 10.0.1-3.fc32)

This was previously covered by patchew CI. I just figured
patchew is running without the LeakSanitizer since commit
6f89ec7442e ("docker: test-debug: disable LeakSanitizer"):

 docker: test-debug: disable LeakSanitizer

 There are just too many leaks in device-introspect-test (especially for
 the plethora of arm and aarch64 boards) to make LeakSanitizer useful;
 disable it for now.




Re: [PATCH 3/3] net/colo-compare: Add handler for passthrough connection

2021-01-15 Thread Lukas Straub
On Fri, 15 Jan 2021 09:07:47 +
"Zhang, Chen"  wrote:

> > -Original Message-
> > From: Lukas Straub 
> > Sent: Thursday, January 14, 2021 9:45 PM
> > To: Zhang, Chen 
> > Cc: Jason Wang ; qemu-dev  > de...@nongnu.org>; Eric Blake ; Dr. David Alan  
> > Gilbert ; Markus Armbruster ;
> > Zhang Chen 
> > Subject: Re: [PATCH 3/3] net/colo-compare: Add handler for passthrough
> > connection
> > 
> > On Thu, 24 Dec 2020 09:09:18 +0800
> > Zhang Chen  wrote:
> >   
> > > From: Zhang Chen 
> > >
> > > Currently, we just use guest's TCP/UDP source port as the key to
> > > bypass certain network traffic.
> > >
> > > Signed-off-by: Zhang Chen 
> > > ---
> > >  net/colo-compare.c | 49
> > > ++
> > >  net/colo-compare.h |  2 ++
> > >  net/net.c  | 27 +
> > >  3 files changed, 78 insertions(+)
> > >
> > > diff --git a/net/colo-compare.c b/net/colo-compare.c index
> > > 337025b44f..11a32caa9b 100644
> > > --- a/net/colo-compare.c
> > > +++ b/net/colo-compare.c
> > > @@ -46,6 +46,9 @@ static QTAILQ_HEAD(, CompareState) net_compares =
> > > static NotifierList colo_compare_notifiers =
> > >  NOTIFIER_LIST_INITIALIZER(colo_compare_notifiers);
> > >
> > > +static QLIST_HEAD(, PassthroughEntry) passthroughlist =
> > > +QLIST_HEAD_INITIALIZER(passthroughlist);
> > > +  
> > 
> > Hi,
> > I think this should be per colo-compare instance e.g. inside 'struct
> > CompareState'.  
> 
> It looks QMP and HMP also need to add colo-compare object ID to control it.
> Do we need make this command more general?

Yes, it gives more flexibility. For example if the VM a "public" and a separate 
"management" network interface, passthrough can then be enabled just on the 
"management" interface.

Regards,
Lukas Straub

> Thanks
> Chen
> 
> >   
> > >  [...]


pgp8Kmy1e0Qfg.pgp
Description: OpenPGP digital signature


Re: [PATCH 1/4] tests/qtest: Remove TPM tests

2021-01-15 Thread Philippe Mathieu-Daudé
On 1/15/21 5:06 PM, Philippe Mathieu-Daudé wrote:
> On 1/15/21 4:53 PM, Stefan Berger wrote:
>> On 1/15/21 10:52 AM, Philippe Mathieu-Daudé wrote:
>>> Subject is incorrect, this is not a removal of the tests, but
>>> removal of their execution. The tests are still in the repository.
>>> This is more of a disablement.
>>
>> How do you compile / run them to have the LeakSanitizer checks?
> 
> I used:
> 
> ../configure --cc=clang --enable-sanitizers && make check-qtest
> 
> $ clang -v
> clang version 10.0.1 (Fedora 10.0.1-3.fc32)
> 
> This was previously covered by patchew CI. I just figured
> patchew is running without the LeakSanitizer since commit
> 6f89ec7442e ("docker: test-debug: disable LeakSanitizer"):
> 
>  docker: test-debug: disable LeakSanitizer
> 
>  There are just too many leaks in device-introspect-test (especially for
>  the plethora of arm and aarch64 boards) to make LeakSanitizer useful;
>  disable it for now.

So if this expected, maybe the correct fix is to have meson use
ASAN_OPTIONS=detect_leaks=0 automatically when running the qtests?




[Bug 1911839] Re: [OSS-Fuzz] Issue 29586 e1000e: Memcpy-param-overlap in flatview_write_continue

2021-01-15 Thread Peter Maydell
** Tags added: fuzzer

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

Title:
  [OSS-Fuzz] Issue 29586 e1000e: Memcpy-param-overlap in
  flatview_write_continue

Status in QEMU:
  New

Bug description:
  === Reproducer ===
  cat << EOF | ./qemu-system-i386 -M q35 -accel qtest \
  -qtest stdio -nographic -nodefaults -device \
  e1000e,netdev=net0 -netdev user,id=net0 
  outl 0xcf8 0x8811
  outl 0xcfc 0x5ac600
  outl 0xcf8 0x8801
  outl 0xcfc 0x2600
  write 0x5ac60100 0x4 0x56000302
  write 0x5ac6011a 0x2 0x1006
  write 0x5ac60120 0x1 0x25
  write 0x5ac6042a 0x2 0x4048
  write 0x5ac60431 0x1 0x04
  write 0x4240 0x1 0xff
  write 0x4241 0x1 0x01
  write 0x4249 0x1 0xf5
  write 0x1ff 0x1 0x11
  write 0x5ac60401 0x1 0x12
  write 0x5ac6043a 0x2 0x3000
  write 0x5ac60112 0x2 0xf090
  write 0x5ac60430 0x1 0x0
  write 0x239 0x1 0xff
  write 0x2bb 0x1 0x41
  write 0x9531 0x1 0xff
  write 0x9532 0x1 0xff
  write 0x9533 0x1 0xff
  write 0x9534 0x1 0xff
  write 0x9535 0x1 0xff
  write 0x9536 0x1 0xff
  write 0x9537 0x1 0xff
  write 0x5ac60403 0x1 0x12
  EOF

  === Stack Trace ===
  ==1364==ERROR: AddressSanitizer: memcpy-param-overlap: memory ranges 
[0x7f90b7e00025,0x7f90b7e00604) and [0x7f90b7e00225, 0x7f90b7e00804) overlap
  #0 __asan_memcpy 
/src/llvm-project/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp:22:3
  #1 flatview_write_continue /src/qemu/softmmu/physmem.c:2764:13
  #2 flatview_write /src/qemu/softmmu/physmem.c:2799:14
  #3 address_space_write /src/qemu/softmmu/physmem.c:2891:18
  #4 address_space_rw /src/qemu/softmmu/physmem.c:2901:16
  #5 dma_memory_rw_relaxed /src/qemu/include/sysemu/dma.h:88:12
  #6 dma_memory_rw /src/qemu/include/sysemu/dma.h:127:12
  #7 pci_dma_rw /src/qemu/include/hw/pci/pci.h:801:12
  #8 pci_dma_write /src/qemu/include/hw/pci/pci.h:837:12
  #9 e1000e_write_to_rx_buffers /src/qemu/hw/net/e1000e_core.c:1405:9
  #10 e1000e_write_packet_to_guest /src/qemu/hw/net/e1000e_core.c:1575:21
  #11 e1000e_receive_iov /src/qemu/hw/net/e1000e_core.c:1702:9
  #12 e1000e_nc_receive_iov /src/qemu/hw/net/e1000e.c:214:12
  #13 net_tx_pkt_sendv /src/qemu/hw/net/net_tx_pkt.c:556:9
  #14 net_tx_pkt_send /src/qemu/hw/net/net_tx_pkt.c:633:9
  #15 net_tx_pkt_send_loopback /src/qemu/hw/net/net_tx_pkt.c:646:11
  #16 e1000e_tx_pkt_send /src/qemu/hw/net/e1000e_core.c:657:16
  #17 e1000e_process_tx_desc /src/qemu/hw/net/e1000e_core.c:736:17
  #18 e1000e_start_xmit /src/qemu/hw/net/e1000e_core.c:927:9
  #19 e1000e_set_tctl /src/qemu/hw/net/e1000e_core.c:2424:9
  #20 e1000e_core_write /src/qemu/hw/net/e1000e_core.c:3256:9
  #21 e1000e_mmio_write /src/qemu/hw/net/e1000e.c:110:5
  #22 memory_region_write_accessor /src/qemu/softmmu/memory.c:491:5
  #23 access_with_adjusted_size /src/qemu/softmmu/memory.c:552:18
  #24 memory_region_dispatch_write /src/qemu/softmmu/memory.c:0:13
  #25 flatview_write_continue /src/qemu/softmmu/physmem.c:2759:23
  #26 flatview_write /src/qemu/softmmu/physmem.c:2799:14
  #27 address_space_write /src/qemu/softmmu/physmem.c:2891:18
  #28 __wrap_qtest_writeq /src/qemu/tests/qtest/fuzz/qtest_wrappers.c:187:9
  #29 op_write /src/qemu/tests/qtest/fuzz/generic_fuzz.c:479:13
  #30 generic_fuzz /src/qemu/tests/qtest/fuzz/generic_fuzz.c:681:17

  OSS-Fuzz Report: https://bugs.chromium.org/p/oss-
  fuzz/issues/detail?id=29586

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



[Bug 1911075] Re: [OSS-Fuzz] ahci: stack overflow in ahci_cond_start_engines

2021-01-15 Thread Peter Maydell
** Tags added: fuzzer

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

Title:
  [OSS-Fuzz] ahci: stack overflow in ahci_cond_start_engines

Status in QEMU:
  Confirmed

Bug description:
  === Reproducer ===
  while true; do cat << EOF; done | ./qemu-system-i386 -machine q35 -nodefaults 
-nographic -qtest stdio -accel qtest
  outl 0xcf8 0x8000fa27
  outl 0xcfc 0x37414537
  outl 0xcf8 0x8000fa01
  outl 0xcfc 0x4606ce74
  writew 0x37000f01 0x215a
  writeq 0x37000100 0xfffaf
  writeq 0x37000115 0x373d27004037
  outl 0xcf8 0x8000fa01
  outl 0xcfc 0x4606ce74
  writeq 0x37ff 0x3700011500
  writeq 0x37000115 0xc41ff035a5a
  outl 0xcf8 0x8000ea04
  outb 0xcfc 0x15
  outl 0xcf8 0x8000ea00
  outw 0xcfc 0x5a1f
  writeq 0x37000115 0x17765746972
  writeq 0x37000115 0xbf00
  outl 0xcf8 0x8000ea04
  outb 0xcfc 0x15
  outl 0xcf8 0x8000fa46
  outb 0xcfc 0xff
  clock_step
  writeq 0x37000115 0xaf
  writeq 0x37000115 0x6301275541af7415
  writeq 0x37000115 0xafaf5a5a743715
  outb 0x64 0xfe
  EOF

  === Stack Trace ===
  ==887446==ERROR: UndefinedBehaviorSanitizer: stack-overflow on address 
0x7ffe567cae0c (pc 0x7fdd9100819e bp 0x7ffe567cb2b0 sp 0x7ffe567cad40 T887446)

  #0 vfprintf
  #1 fprintf
  #2 ahci_mem_write /src/qemu/hw/ide/ahci.c:468:9
  #3 memory_region_write_accessor /src/qemu/softmmu/memory.c:491:5
  #4 access_with_adjusted_size /src/qemu/softmmu/memory.c:552:18
  #5 memory_region_dispatch_write /src/qemu/softmmu/memory.c:0:13
  #6 flatview_write_continue /src/qemu/softmmu/physmem.c:2759:23
  #7 flatview_write /src/qemu/softmmu/physmem.c:2799:14
  #8 address_space_write /src/qemu/softmmu/physmem.c:2891:18
  #9 address_space_unmap /src/qemu/softmmu/physmem.c:3217:9
  #10 dma_memory_unmap /src/qemu/include/sysemu/dma.h:226:5
  #11 map_page /src/qemu/hw/ide/ahci.c:249:9
  #12 ahci_map_clb_address /src/qemu/hw/ide/ahci.c:748:5
  #13 ahci_cond_start_engines /src/qemu/hw/ide/ahci.c:276:14
  #14 ahci_port_write /src/qemu/hw/ide/ahci.c:339:9
  #15 ahci_mem_write /src/qemu/hw/ide/ahci.c:513:9
  #16 memory_region_write_accessor /src/qemu/softmmu/memory.c:491:5
  #17 access_with_adjusted_size /src/qemu/softmmu/memory.c:552:18
  #18 memory_region_dispatch_write /src/qemu/softmmu/memory.c:0:13
  #19 flatview_write_continue /src/qemu/softmmu/physmem.c:2759:23
  #20 flatview_write /src/qemu/softmmu/physmem.c:2799:14
  #21 address_space_write /src/qemu/softmmu/physmem.c:2891:18
  #22 address_space_unmap /src/qemu/softmmu/physmem.c:3217:9
  #23 dma_memory_unmap /src/qemu/include/sysemu/dma.h:226:5
  #24 map_page /src/qemu/hw/ide/ahci.c:249:9
  #25 ahci_map_clb_address /src/qemu/hw/ide/ahci.c:748:5
  #26 ahci_cond_start_engines /src/qemu/hw/ide/ahci.c:276:14
  #27 ahci_port_write /src/qemu/hw/ide/ahci.c:339:9
  #28 ahci_mem_write /src/qemu/hw/ide/ahci.c:513:9
  ... Repeat until we run out of stack

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



[Bug 1910941] Re: Assertion `addr < cache->len && 2 <= cache->len - addr' in virtio-blk

2021-01-15 Thread Peter Maydell
** Tags added: fuzzer

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

Title:
  Assertion `addr < cache->len && 2 <= cache->len - addr' in virtio-blk

Status in QEMU:
  New

Bug description:
  Hello,

  Using hypervisor fuzzer, hyfuzz, I found an assertion failure through
  virtio-blk emulator.

  A malicious guest user/process could use this flaw to abort the QEMU
  process on the host, resulting in a denial of service.

  This was found in version 5.2.0 (master)

  ```

  qemu-system-i386: 
/home/cwmyung/prj/hyfuzz/src/qemu-master/include/exec/memory_ldst_cached.h.inc:88:
 void address_space_stw_le_cached(MemoryRegionCache *, hwaddr, uint32_t, 
MemTxAttrs, MemTxResult *): Assertion `addr < cache->len && 2 <= cache->len - 
addr' failed.
  [1]1877 abort (core dumped)  
/home/cwmyung/prj/hyfuzz/src/qemu-master/build/i386-softmmu/qemu-system-i386

  Program terminated with signal SIGABRT, Aborted.
  #0  0x7f71cc171f47 in __GI_raise (sig=sig@entry=0x6) at 
../sysdeps/unix/sysv/linux/raise.c:51
  #1  0x7f71cc1738b1 in __GI_abort () at abort.c:79
  #2  0x7f71cc16342a in __assert_fail_base (fmt=0x7f71cc2eaa38 "%s%s%s:%u: 
%s%sAssertion `%s' failed.\n%n", assertion=assertion@entry=0x56537b324230 "addr 
< cache->len && 2 <= cache->len - addr", file=file@entry=0x56537b32425c 
"/home/cwmyung/prj/hyfuzz/src/qemu-master/include/exec/memory_ldst_cached.h.inc",
 line=line@entry=0x58, function=function@entry=0x56537b3242ab "void 
address_space_stw_le_cached(MemoryRegionCache *, hwaddr, uint32_t, MemTxAttrs, 
MemTxResult *)") at assert.c:92
  #3  0x7f71cc1634a2 in __GI___assert_fail (assertion=0x56537b324230 "addr 
< cache->len && 2 <= cache->len - addr", file=0x56537b32425c 
"/home/cwmyung/prj/hyfuzz/src/qemu-master/include/exec/memory_ldst_cached.h.inc",
 line=0x58, function=0x56537b3242ab "void 
address_space_stw_le_cached(MemoryRegionCache *, hwaddr, uint32_t, MemTxAttrs, 
MemTxResult *)") at assert.c:101
  #4  0x56537af3c917 in address_space_stw_le_cached (attrs=..., 
result=, cache=, addr=, 
val=) at 
/home/cwmyung/prj/hyfuzz/src/qemu-master/include/exec/memory_ldst_cached.h.inc:88
  #5  0x56537af3c917 in stw_le_phys_cached (cache=, 
addr=, val=) at 
/home/cwmyung/prj/hyfuzz/src/qemu-master/include/exec/memory_ldst_phys.h.inc:121
  #6  0x56537af3c917 in virtio_stw_phys_cached (vdev=, 
cache=, pa=, value=) at 
/home/cwmyung/prj/hyfuzz/src/qemu-master/include/hw/virtio/virtio-access.h:196
  #7  0x56537af2b809 in vring_set_avail_event (vq=, val=0x0) 
at ../hw/virtio/virtio.c:429
  #8  0x56537af2b809 in virtio_queue_split_set_notification (vq=, enable=) at ../hw/virtio/virtio.c:438
  #9  0x56537af2b809 in virtio_queue_set_notification (vq=, 
enable=0x1) at ../hw/virtio/virtio.c:499
  #10 0x56537b07ce1c in virtio_blk_handle_vq (s=0x56537d6bb3a0, 
vq=0x56537d6c0680) at ../hw/block/virtio-blk.c:795
  #11 0x56537af3eb4d in virtio_queue_notify_aio_vq (vq=0x56537d6c0680) at 
../hw/virtio/virtio.c:2326
  #12 0x56537af3ba04 in virtio_queue_host_notifier_aio_read (n=) at ../hw/virtio/virtio.c:3533
  #13 0x56537b20901c in aio_dispatch_handler (ctx=0x56537c4179f0, 
node=0x7f71a810b370) at ../util/aio-posix.c:329
  #14 0x56537b20838c in aio_dispatch_handlers (ctx=) at 
../util/aio-posix.c:372
  #15 0x56537b20838c in aio_dispatch (ctx=0x56537c4179f0) at 
../util/aio-posix.c:382
  #16 0x56537b1f99cb in aio_ctx_dispatch (source=0x2, 
callback=0x7ffc8add9f90, user_data=0x0) at ../util/async.c:306
  #17 0x7f71d1c10417 in g_main_context_dispatch () at 
/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
  #18 0x56537b1f1bab in glib_pollfds_poll () at ../util/main-loop.c:232
  #19 0x56537b1f1bab in os_host_main_loop_wait (timeout=) at 
../util/main-loop.c:255
  #20 0x56537b1f1bab in main_loop_wait (nonblocking=) at 
../util/main-loop.c:531
  #21 0x56537af879d7 in qemu_main_loop () at ../softmmu/runstate.c:720
  #22 0x56537a928a3b in main (argc=, argc@entry=0x15, 
argv=, argv@entry=0x7ffc8adda718, envp=) at 
../softmmu/main.c:50
  #23 0x7f71cc154b97 in __libc_start_main (main=0x56537a928a30 , 
argc=0x15, argv=0x7ffc8adda718, init=, fini=, 
rtld_fini=, stack_end=0x7ffc8adda708) at ../csu/libc-start.c:310
  #24 0x56537a92894a in _start ()

  ```

  To reproduce this issue, please run the QEMU with the following
  command line.

  ```

  # To reproduce this issue, please run the QEMU process with the
  following command line.

  $ qemu-system-i386 -m 512  -drive
  file=hyfuzz.img,index=0,media=disk,format=raw -device virtio-blk-
  pci,drive=drive0,id=virtblk0,num-queues=4 -drive
  file=disk.img,if=none,id=drive0

  ```

  Please let me know if I can provide any further info.

  Thank you.

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



[Bug 1909261] Re: [OSS-Fuzz] Issue 28929 xhci: ASSERT: xfer->packet.status != USB_RET_NAK

2021-01-15 Thread Peter Maydell
** Tags added: fuzzer

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

Title:
  [OSS-Fuzz]  Issue 28929 xhci: ASSERT: xfer->packet.status !=
  USB_RET_NAK

Status in QEMU:
  New

Bug description:
  === Reproducer ===

  ./qemu-system-i386 -m 512M -machine q35,accel=qtest \
   -drive file=null-co://,if=none,format=raw,id=disk0 \
  -device qemu-xhci,id=xhci -device usb-tablet,bus=xhci.0 \
  -device usb-bot -device usb-storage,drive=disk0 \
  -chardev null,id=cd0 -chardev null,id=cd1 \
  -device usb-braille,chardev=cd0 -device usb-ccid \
  -device usb-ccid -device usb-kbd -device usb-mouse \
  -device usb-serial,chardev=cd1 -device usb-tablet \
  -device usb-wacom-tablet -device usb-audio \
  -qtest stdio -nographic -nodefaults < attachment

  === Stack Trace ===
  #0 raise
  #1 abort
  #2 libc.so.6
  #3 __assert_fail
  #4 xhci_kick_epctx /src/qemu/hw/usb/hcd-xhci.c:1865:13
  #5 xhci_ep_kick_timer /src/qemu/hw/usb/hcd-xhci.c:1034:5
  #6 timerlist_run_timers /src/qemu/util/qemu-timer.c:574:9
  #7 qemu_clock_run_timers /src/qemu/util/qemu-timer.c:588:12
  #8 qtest_clock_warp /src/qemu/softmmu/qtest.c:356:9
  #9 qtest_process_command /src/qemu/softmmu/qtest.c:752:9
  #10 qtest_process_inbuf /src/qemu/softmmu/qtest.c:797:9
  #11 qtest_server_inproc_recv /src/qemu/softmmu/qtest.c:904:9
  #12 send_wrapper /src/qemu/tests/qtest/libqtest.c:1390:5
  #13 qtest_sendf /src/qemu/tests/qtest/libqtest.c:438:5
  #14 qtest_clock_step_next /src/qemu/tests/qtest/libqtest.c:912:5
  #15 op_clock_step /src/qemu/tests/qtest/fuzz/generic_fuzz.c:574:5

  OSS-Fuzz Report:
  https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28929

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



[Bug 1910826] Re: [OSS-Fuzz] Issue 29224 rtl8139: Stack-overflow in rtlNUMBER_transmit_one

2021-01-15 Thread Peter Maydell
** Tags added: fuzzer

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

Title:
  [OSS-Fuzz] Issue 29224 rtl8139: Stack-overflow in
  rtlNUMBER_transmit_one

Status in QEMU:
  New

Bug description:
  === Reproducer ===
  cat << EOF | ../build/qemu-system-i386 -machine q35 \
  -nodefaults  -device rtl8139,netdev=net0 \
  -netdev user,id=net0 -display none -qtest stdio
  outl 0xcf8 0x8804
  outb 0xcfc 0x26
  outl 0xcf8 0x8817
  outb 0xcfc 0xff
  write 0x1 0x1 0x42
  write 0x5 0x1 0x42
  write 0x9 0x1 0x42
  write 0xd 0x1 0x42
  write 0xff44 0x4 0x11
  write 0xff37 0x1 0x1c
  writel 0xff30 0xff00
  write 0xff40 0x4 0x16
  write 0xff10 0x4 0x01020
  EOF

  === Stack Trace ===
  ==2819215==ERROR: AddressSanitizer: stack-overflow on address 0x7ffd2c714040 
(pc 0x5639b3a933d9 bp 0x7ffd2c716210 sp 0x7ffd2c714040 T0)
  #0 rtl8139_transmit_one /src/qemu/hw/net/rtl8139.c:1815
  #1 rtl8139_transmit /src/qemu/hw/net/rtl8139.c:2388:9
  #2 rtl8139_TxStatus_write /src/qemu/hw/net/rtl8139.c:2442:5
  #3 rtl8139_io_writel /src/qemu/hw/net/rtl8139.c:2865:13
  #4 rtl8139_ioport_write /src/qemu/hw/net/rtl8139.c:3290:9
  #5 memory_region_write_accessor /src/qemu/softmmu/memory.c:491:5
  #6 access_with_adjusted_size /src/qemu/softmmu/memory.c:552:18
  #7 memory_region_dispatch_write /src/qemu/softmmu/memory.c:0:13
  #8 flatview_write_continue /src/qemu/softmmu/physmem.c:2759:23
  #9 flatview_write /src/qemu/softmmu/physmem.c:2799:14
  #10 address_space_write /src/qemu/softmmu/physmem.c:2891:18
  #11 address_space_rw /src/qemu/softmmu/physmem.c:2901:16
  #12 dma_memory_rw_relaxed /src/qemu/include/sysemu/dma.h:88:12
  #13 dma_memory_rw /src/qemu/include/sysemu/dma.h:127:12
  #14 pci_dma_rw /src/qemu/include/hw/pci/pci.h:801:12
  #15 pci_dma_write /src/qemu/include/hw/pci/pci.h:837:12
  #16 rtl8139_write_buffer /src/qemu/hw/net/rtl8139.c:778:5
  #17 rtl8139_do_receive /src/qemu/hw/net/rtl8139.c:1172:9
  #18 rtl8139_transfer_frame /src/qemu/hw/net/rtl8139.c:1798:9
  #19 rtl8139_transmit_one /src/qemu/hw/net/rtl8139.c:1845:5
  #20 rtl8139_transmit /src/qemu/hw/net/rtl8139.c:2388:9
  #21 rtl8139_TxStatus_write /src/qemu/hw/net/rtl8139.c:2442:5
  #22 rtl8139_io_writel /src/qemu/hw/net/rtl8139.c:2865:13
  #23 rtl8139_ioport_write /src/qemu/hw/net/rtl8139.c:3290:9
  #24 memory_region_write_accessor /src/qemu/softmmu/memory.c:491:5
  #25 access_with_adjusted_size /src/qemu/softmmu/memory.c:552:18
  #26 memory_region_dispatch_write /src/qemu/softmmu/memory.c:0:13
  #27 flatview_write_continue /src/qemu/softmmu/physmem.c:2759:23
  #28 flatview_write /src/qemu/softmmu/physmem.c:2799:14
  #29 address_space_write /src/qemu/softmmu/physmem.c:2891:18
  #30 address_space_rw /src/qemu/softmmu/physmem.c:2901:16
  #31 dma_memory_rw_relaxed /src/qemu/include/sysemu/dma.h:88:12
  #32 dma_memory_rw /src/qemu/include/sysemu/dma.h:127:12
  #33 pci_dma_rw /src/qemu/include/hw/pci/pci.h:801:12
  #34 pci_dma_write /src/qemu/include/hw/pci/pci.h:837:12
  #35 rtl8139_write_buffer /src/qemu/hw/net/rtl8139.c:778:5
  #36 rtl8139_do_receive /src/qemu/hw/net/rtl8139.c:1172:9
  #37 rtl8139_transfer_frame /src/qemu/hw/net/rtl8139.c:1798:9
  Repeat until we run out of stack

  https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=29224

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



[Bug 1910603] Re: [OSS-Fuzz] Issue 29174 sb16: Abrt in audio_bug

2021-01-15 Thread Peter Maydell
** Tags added: fuzzer

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

Title:
  [OSS-Fuzz] Issue 29174 sb16: Abrt in audio_bug

Status in QEMU:
  New

Bug description:
  === Reproducer ===
  cat << EOF | ../build-system/qemu-system-i386 \
  -machine q35 -device sb16,audiodev=snd0 \
  -audiodev none,id=snd0 -nographic -nodefaults \
  -qtest stdio
  outw 0x22c 0x41
  outb 0x22c 0x0
  outw 0x22c 0x1004
  outw 0x22c 0x1c
  EOF

  === Stack Trace ===
  A bug was just triggered in audio_calloc
  Save all your work and restart without audio
  I am sorry
  Context:
  Aborted

  #0 raise
  #1 abort
  #2 audio_bug /src/qemu/audio/audio.c:119:9
  #3 audio_calloc /src/qemu/audio/audio.c:154:9
  #4 audio_pcm_sw_alloc_resources_out /src/qemu/audio/audio_template.h:116:15
  #5 audio_pcm_sw_init_out /src/qemu/audio/audio_template.h:175:11
  #6 audio_pcm_create_voice_pair_out /src/qemu/audio/audio_template.h:410:9
  #7 AUD_open_out /src/qemu/audio/audio_template.h:503:14
  #8 continue_dma8 /src/qemu/hw/audio/sb16.c:216:20
  #9 dma_cmd8 /src/qemu/hw/audio/sb16.c:276:5
  #10 command /src/qemu/hw/audio/sb16.c:0
  #11 dsp_write /src/qemu/hw/audio/sb16.c:949:13
  #12 portio_write /src/qemu/softmmu/ioport.c:205:13
  #13 memory_region_write_accessor /src/qemu/softmmu/memory.c:491:5
  #14 access_with_adjusted_size /src/qemu/softmmu/memory.c:552:18
  #15 memory_region_dispatch_write /src/qemu/softmmu/memory.c:0:13
  #16 flatview_write_continue /src/qemu/softmmu/physmem.c:2759:23
  #17 flatview_write /src/qemu/softmmu/physmem.c:2799:14
  #18 address_space_write /src/qemu/softmmu/physmem.c:2891:18
  #19 cpu_outw /src/qemu/softmmu/ioport.c:70:5

  
  OSS-Fuzz Report:
  https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=29174

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



[Bug 1908513] Re: assertion failure in mptsas1068 emulator

2021-01-15 Thread Peter Maydell
** Tags added: fuzzer

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

Title:
  assertion failure in mptsas1068 emulator

Status in QEMU:
  New

Bug description:
  Using hypervisor fuzzer, hyfuzz, I found an assertion failure through
  mptsas1068 emulator.

  A malicious guest user/process could use this flaw to abort the QEMU
  process on the host, resulting in a denial of service.

  This was found in version 5.2.0 (master)

  
  qemu-system-i386: ../hw/scsi/mptsas.c:968: void 
mptsas_interrupt_status_write(MPTSASState *): Assertion
  `s->intr_status & MPI_HIS_DOORBELL_INTERRUPT' failed.
  [1]16951 abort (core dumped)  
/home/cwmyung/prj/hyfuzz/src/qemu-5.2/build/qemu-system-i386 -m 512 -drive

  Program terminated with signal SIGABRT, Aborted.
  #0  __GI_raise (sig=sig@entry=0x6) at ../sysdeps/unix/sysv/linux/raise.c:51
  51  ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
  [Current thread is 1 (Thread 0x7fc7d6023700 (LWP 23475))]
  gdb-peda$ bt
  #0  0x7fc7efa13f47 in __GI_raise (sig=sig@entry=0x6) at 
../sysdeps/unix/sysv/linux/raise.c:51
  #1  0x7fc7efa158b1 in __GI_abort () at abort.c:79
  #2  0x7fc7efa0542a in __assert_fail_base (fmt=0x7fc7efb8ca38 "%s%s%s:%u: 
%s%sAssertion `%s' failed.\\n%n", assertion=assertion@entry=0x56439214d593 
"s->intr_status & MPI_HIS_DOORBELL_INTERRUPT", file=file@entry=0x56439214d4a7 
"../hw/scsi/mptsas.c", line=line@entry=0x3c8, 
function=function@entry=0x56439214d81c "void 
mptsas_interrupt_status_write(MPTSASState *)") at assert.c:92
  #3  0x7fc7efa054a2 in __GI___assert_fail (assertion=0x56439214d593 
"s->intr_status & MPI_HIS_DOORBELL_INTERRUPT", file=0x56439214d4a7 
"../hw/scsi/mptsas.c", line=0x3c8, function=0x56439214d81c "void 
mptsas_interrupt_status_write(MPTSASState *)") at assert.c:101
  #4  0x564391a43963 in mptsas_interrupt_status_write (s=) 
at ../hw/scsi/mptsas.c:968
  #5  0x564391a43963 in mptsas_mmio_write (opaque=0x5643943dd5b0, 
addr=0x30, val=0x1800, size=) at ../hw/scsi/mptsas.c:1052
  #6  0x564391e08798 in memory_region_write_accessor (mr=, 
addr=, value=, size=, 
shift=, mask=, attrs=...)
  at ../softmmu/memory.c:491
  #7  0x564391e0858e in access_with_adjusted_size (addr=, 
value=, size=, access_size_min=, 
access_size_max=, access_fn=, mr=, 
attrs=...) at ../softmmu/memory.c:552
  #8  0x564391e0858e in memory_region_dispatch_write (mr=0x5643943ddea0, 
addr=, data=, op=, attrs=...) at 
../softmmu/memory.c:1501
  #9  0x564391eff228 in io_writex (iotlbentry=, 
mmu_idx=, val=, addr=, 
retaddr=, op=, env=)
  at ../accel/tcg/cputlb.c:1378
  #10 0x564391eff228 in store_helper (env=, addr=, val=, oi=, retaddr=, 
op=MO_32) at ../accel/tcg/cputlb.c:2397
  #11 0x564391eff228 in helper_le_stl_mmu (env=, 
addr=, val=0x2, oi=, retaddr=0x7fc78841b401) at 
../accel/tcg/cputlb.c:2463
  #12 0x7fc78841b401 in code_gen_buffer ()
  #13 0x564391dd0da0 in cpu_tb_exec (cpu=0x56439363e650, itb=) at ../accel/tcg/cpu-exec.c:178
  #14 0x564391dd19eb in cpu_loop_exec_tb (tb=, 
cpu=, last_tb=, tb_exit=) at 
../accel/tcg/cpu-exec.c:658
  #15 0x564391dd19eb in cpu_exec (cpu=0x56439363e650) at 
../accel/tcg/cpu-exec.c:771
  #16 0x564391e00b9f in tcg_cpu_exec (cpu=) at 
../accel/tcg/tcg-cpus.c:243
  #17 0x564391e00b9f in tcg_cpu_thread_fn (arg=0x56439363e650) at 
../accel/tcg/tcg-cpus.c:427
  #18 0x5643920d8775 in qemu_thread_start (args=) at 
../util/qemu-thread-posix.c:521
  #19 0x7fc7efdcd6db in start_thread (arg=0x7fc7d6023700) at 
pthread_create.c:463

  To reproduce this issue, please run the QEMU with the following
  command line.

  
  # To enable ASan option, please set configuration with the following command
  $ ./configure --target-list=i386-softmmu --disable-werror --enable-sanitizers
  $ make

  # To reproduce this issue, please run the QEMU process with the following 
command line.
  $ ./qemu-system-i386 -m 512 -drive 
file=./hyfuzz.img,index=0,media=disk,format=raw -device mptsas1068,id=scsi 
-device scsi-hd,drive=SysDisk -drive id=SysDisk,if=none,file=./disk.img

  Please let me know if I can provide any further info.
  Thank you.

  - Cheolwoo, Myung (Seoul National University)

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



[Bug 1908515] Re: assertion failure in lsi53c810 emulator

2021-01-15 Thread Peter Maydell
** Tags added: fuzzer

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

Title:
  assertion failure in lsi53c810 emulator

Status in QEMU:
  New

Bug description:
  Hello,

  Using hypervisor fuzzer, hyfuzz, I found an assertion failure through
  lsi53c810 emulator.

  A malicious guest user/process could use this flaw to abort the QEMU
  process on the host, resulting in a denial of service.

  This was found in version 5.2.0 (master)

  
  qemu-system-i386: ../hw/scsi/lsi53c895a.c:624: void lsi_do_dma(LSIState *, 
int): Assertion `s->current'
  failed.
  [1]1406 abort (core dumped)  
/home/cwmyung/prj/hyfuzz/src/qemu-5.2/build/i386-softmmu/qemu-system-i386 -m

  Program terminated with signal SIGABRT, Aborted.
  #0  __GI_raise (sig=sig@entry=0x6) at ../sysdeps/unix/sysv/linux/raise.c:51
  51  ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
  [Current thread is 1 (Thread 0x7fa9310a8700 (LWP 2076))]
  gdb-peda$ bt
  #0  0x7fa94aa98f47 in __GI_raise (sig=sig@entry=0x6) at 
../sysdeps/unix/sysv/linux/raise.c:51
  #1  0x7fa94aa9a8b1 in __GI_abort () at abort.c:79
  #2  0x7fa94aa8a42a in __assert_fail_base (fmt=0x7fa94ac11a38 "%s%s%s:%u: 
%s%sAssertion `%s' failed.\\n%n", assertion=assertion@entry=0x562851c9eab9 
"s->current", file=file@entry=0x562851c9d4f9 "../hw/scsi/lsi53c895a.c", 
line=line@entry=0x270, function=function@entry=0x562851c9de43 "void 
lsi_do_dma(LSIState *, int)") at assert.c:92
  #3  0x7fa94aa8a4a2 in __GI___assert_fail (assertion=0x562851c9eab9 
"s->current", file=0x562851c9d4f9 "../hw/scsi/lsi53c895a.c", line=0x270, 
function=0x562851c9de43 "void lsi_do_dma(LSIState *, int)")
  at assert.c:101
  #4  0x5628515d9605 in lsi_do_dma (s=0x56289060, out=0x1) at 
../hw/scsi/lsi53c895a.c:624
  #5  0x5628515d5317 in lsi_execute_script (s=) at 
../hw/scsi/lsi53c895a.c:1250
  #6  0x5628515cec49 in lsi_reg_writeb (s=0x56289060, offset=0x2f, 
val=0x1e)
  at ../hw/scsi/lsi53c895a.c:2005
  #7  0x562851952798 in memory_region_write_accessor (mr=, 
addr=, value=, size=, 
shift=, mask=, attrs=...)
  at ../softmmu/memory.c:491
  #8  0x56285195258e in access_with_adjusted_size (addr=, 
value=, size=, access_size_min=, 
access_size_max=, access_fn=, mr=, 
attrs=...) at ../softmmu/memory.c:552
  #9  0x56285195258e in memory_region_dispatch_write (mr=0x56289960, 
addr=, data=, op=, attrs=...) at 
../softmmu/memory.c:1501
  #10 0x5628518e5305 in flatview_write_continue (fv=0x7fa92871f040, 
addr=0xfebf302c, attrs=..., ptr=0x7fa9310a49b8, len=0x4, addr1=0x7fa9310a3410, 
l=, mr=0x56289960)
  at ../softmmu/physmem.c:2759
  #11 0x5628518e6ef6 in flatview_write (fv=0x7fa92871f040, addr=0xfebf302c, 
attrs=..., len=0x4, buf=) at ../softmmu/physmem.c:2799
  #12 0x5628518e6ef6 in subpage_write (opaque=, 
addr=, value=, len=, attrs=...) at 
../softmmu/physmem.c:2465
  #13 0x5628519529a2 in memory_region_write_with_attrs_accessor 
(mr=, addr=, value=, 
size=, shift=, mask=, attrs=...) 
at ../softmmu/memory.c:511
  #14 0x5628519525e1 in access_with_adjusted_size (addr=, 
size=, access_size_min=, 
access_size_max=, mr=, attrs=..., 
value=, access_fn=) at ../softmmu/memory.c:552
  #15 0x5628519525e1 in memory_region_dispatch_write (mr=, 
addr=, data=, op=, attrs=...) at 
../softmmu/memory.c:1508
  #16 0x562851a49228 in io_writex (iotlbentry=, 
mmu_idx=, val=, addr=, 
retaddr=, op=, env=)
  at ../accel/tcg/cputlb.c:1378
  #17 0x562851a49228 in store_helper (env=, addr=, val=, oi=, retaddr=, 
op=MO_32) at ../accel/tcg/cputlb.c:2397
  #18 0x562851a49228 in helper_le_stl_mmu (env=, 
addr=, val=0x2, oi=, retaddr=0x7fa8e44032ee) at 
../accel/tcg/cputlb.c:2463
  #19 0x7fa8e44032ee in code_gen_buffer ()
  #20 0x56285191ada0 in cpu_tb_exec (cpu=0x5628547b81a0, itb=)
  at ../accel/tcg/cpu-exec.c:178
  #21 0x56285191b9eb in cpu_loop_exec_tb (tb=, 
cpu=, last_tb=, tb_exit=) at 
../accel/tcg/cpu-exec.c:658
  #22 0x56285191b9eb in cpu_exec (cpu=0x5628547b81a0) at 
../accel/tcg/cpu-exec.c:771
  #23 0x56285194ab9f in tcg_cpu_exec (cpu=) at 
../accel/tcg/tcg-cpus.c:243
  #24 0x56285194ab9f in tcg_cpu_thread_fn (arg=0x5628547b81a0) at 
../accel/tcg/tcg-cpus.c:427
  #25 0x562851c22775 in qemu_thread_start (args=) at 
../util/qemu-thread-posix.c:521
  #26 0x7fa94ae526db in start_thread (arg=0x7fa9310a8700) at 
pthread_create.c:463
  #27 0x7fa94ab7ba3f in clone () at 
../sysdeps/unix/sysv/linux/x86_64/clone.S:95

  To reproduce this issue, please run the QEMU with the following
  command line.

  
  # To enable ASan option, please set configuration with the following command
  $ ./configure --target-list=i386-softmmu --disable-werror --enable-sanitizers
  $ make

  # To reproduce this issue, please run the QEMU process with the following 
command line.
  $ ./qemu-system-i386 

[Bug 1908062] Re: qemu-system-i386 virtio-vga: Assertion in address_space_stw_le_cached failed again

2021-01-15 Thread Peter Maydell
** Tags added: fuzzer

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

Title:
  qemu-system-i386 virtio-vga: Assertion in address_space_stw_le_cached
  failed again

Status in QEMU:
  New

Bug description:
  When I was fuzzing virtio-vga device of the latest QEMU (1758428, Dec
  12, built with --enable-sanitizers --enable-fuzzing), an assertion
  failed in include/exec/memory_ldst_cached.h.inc.

  --[ Reproducer

  cat << EOF | ./build/i386-softmmu/qemu-system-i386 -machine accel=qtest \
  -machine q35 -display none -nodefaults -device virtio-vga -qtest stdio
  outl 0xcf8 0x881c
  outb 0xcfc 0xc3
  outl 0xcf8 0x8804
  outb 0xcfc 0x06
  write 0xc31024 0x2 0x0040
  write 0xc31028 0x1 0x5a
  write 0xc3101c 0x1 0x01
  writel 0xc3100c 0x2000
  write 0xc31016 0x3 0x80a080
  write 0xc33002 0x1 0x80
  write 0x5c 0x1 0x10
  EOF

  --[ Output

  ==35337==WARNING: ASan doesn't fully support makecontext/swapcontext 
functions and may produce false positives in some cases!
  [I 1607946348.442865] OPENED
  [R +0.059305] outl 0xcf8 0x881c
  OK
  [S +0.059326] OK
  [R +0.059338] outb 0xcfc 0xc3
  OK
  [S +0.059355] OK
  [R +0.059363] outl 0xcf8 0x8804
  OK
  [S +0.059369] OK
  [R +0.059381] outb 0xcfc 0x06
  OK
  [S +0.061094] OK
  [R +0.061107] write 0xc31024 0x2 0x0040
  OK
  [S +0.061120] OK
  [R +0.061127] write 0xc31028 0x1 0x5a
  OK
  [S +0.061135] OK
  [R +0.061142] write 0xc3101c 0x1 0x01
  OK
  [S +0.061158] OK
  [R +0.061167] writel 0xc3100c 0x2000
  OK
  [S +0.061212] OK
  [R +0.061222] write 0xc31016 0x3 0x80a080
  OK
  [S +0.061231] OK
  [R +0.061238] write 0xc33002 0x1 0x80
  OK
  [S +0.061247] OK
  [R +0.061253] write 0x5c 0x1 0x10
  OK
  [S +0.061403] OK
  qemu-system-i386: 
/home/qiuhao/hack/qemu/include/exec/memory_ldst_cached.h.inc:88: void 
address_space_stw_le_cached(MemoryRegionCache *, hwaddr, uint32_t, MemTxAttrs, 
MemTxResult *): Assertion `addr < cache->len && 2 <= cache->len - addr' failed.

  --[ Environment

  Ubuntu 20.04.1 5.4.0-58-generic x86_64
  clang: 10.0.0-4ubuntu1
  glibc: 2.31-0ubuntu9.1
  libglib2.0-dev: 2.64.3-1~ubuntu20.04.1

  --[ Note

  Alexander Bulekov found the same assertion failure on 2020-08-04,
  https://bugs.launchpad.net/qemu/+bug/1890333, and it had been fixed in
  commit 2d69eba5fe52045b2c8b0d04fd3806414352afc1.

  Fam Zheng found the same assertion failure on 2018-09-29,
  https://bugs.launchpad.net/qemu/+bug/1795148, and it had been fixed in
  commit db812c4073c77c8a64db8d6663b3416a587c7b4a.

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



[Bug 1907938] Re: [OSS-Fuzz] Issue 28524 virtio-blk: ASSERT: !s->dataplane_started

2021-01-15 Thread Peter Maydell
** Tags added: fuzzer

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

Title:
  [OSS-Fuzz] Issue 28524 virtio-blk: ASSERT: !s->dataplane_started

Status in QEMU:
  New

Bug description:
   affects qemu

  === Reproducer ===

  cat << EOF |./qemu-system-i386 -display none -m 512M -machine q35 \
  -device virtio-blk,drive=disk0 \
  -drive file=null-co://,id=disk0,if=none,format=raw -qtest stdio
  outl 0xcf8 0x8000181f
  outl 0xcfc 0xa044d79
  outl 0xcf8 0x80001802
  outl 0xcf8 0x80001804
  outl 0xcfc 0xb9045dff
  outl 0xcf8 0x8000180e
  outl 0xcfc 0xfb9465a
  outl 0xf85 0x9e1ea5c2
  write 0x9f002 0x1 0x04
  write 0x9f004 0x1 0x04
  write 0x9e040 0x1 0x04
  write 0x9e043 0x1 0x01
  write 0x9e048 0x1 0x10
  write 0x9e04c 0x1 0x01
  write 0x9e04e 0x1 0x6e
  write 0x104 0x1 0x01
  write 0x9e6e3 0x1 0x01
  write 0x9e6eb 0x1 0x04
  write 0x9e6ec 0x1 0x6e
  write 0x9f006 0x1 0x04
  write 0x9f008 0x1 0x04
  write 0x9f00a 0x1 0x04
  outl 0xf8f 0xc
  EOF

  === Stack Trace ===

  qemu-fuzz-i386: ../hw/block/virtio-blk.c:917: void 
virtio_blk_reset(VirtIODevice *): Assertion `!s->dataplane_started' failed.
  ==702068== ERROR: libFuzzer: deadly signal
  #0 0x55bd6fc9f311 in __sanitizer_print_stack_trace (fuzz-i386+0x2b16311)
  #1 0x55bd6fbe83d8 in fuzzer::PrintStackTrace() (fuzz-i386+0x2a5f3d8)
  #2 0x55bd6fbce413 in fuzzer::Fuzzer::CrashCallback() (fuzz-i386+0x2a45413)
  #3 0x7ff5241b813f  (/lib/x86_64-linux-gnu/libpthread.so.0+0x1413f)
  #4 0x7ff523feddb0 in __libc_signal_restore_set 
signal/../sysdeps/unix/sysv/linux/internal-signals.h:86:3
  #5 0x7ff523feddb0 in raise signal/../sysdeps/unix/sysv/linux/raise.c:48:3
  #6 0x7ff523fd7536 in abort stdlib/abort.c:79:7
  #7 0x7ff523fd740e in __assert_fail_base assert/assert.c:92:3
  #8 0x7ff523fe65b1 in __assert_fail assert/assert.c:101:3
  #9 0x55bd7116c435 in virtio_blk_reset hw/block/virtio-blk.c:917:5
  #10 0x55bd710c94a2 in virtio_reset hw/virtio/virtio.c:2001:9
  #11 0x55bd6ff0e0a5 in virtio_pci_reset hw/virtio/virtio-pci.c:1886:5
  #12 0x55bd6ff10686 in virtio_ioport_write hw/virtio/virtio-pci.c:339:13
  #13 0x55bd6ff10686 in virtio_pci_config_write hw/virtio/virtio-pci.c:456:9
  #14 0x55bd713fd025 in memory_region_write_accessor softmmu/memory.c:491:5
  #15 0x55bd713fca93 in access_with_adjusted_size softmmu/memory.c:552:18
  #16 0x55bd713fc2f0 in memory_region_dispatch_write softmmu/memory.c
  #17 0x55bd70e4bf36 in flatview_write_continue softmmu/physmem.c:2759:23
  #18 0x55bd70e41bbb in flatview_write softmmu/physmem.c:2799:14
  #19 0x55bd70e41bbb in address_space_write softmmu/physmem.c:2891:18
  #20 0x55bd71153462 in cpu_outl softmmu/ioport.c:80:5
  #21 0x55bd712d586e in qtest_process_command softmmu/qtest.c:483:13
  #22 0x55bd712d35bf in qtest_process_inbuf softmmu/qtest.c:797:9
  #23 0x55bd712d3315 in qtest_server_inproc_recv softmmu/qtest.c:904:9
  #24 0x55bd71910df8 in qtest_sendf tests/qtest/libqtest.c:438:5
  #25 0x55bd71911fae in qtest_out tests/qtest/libqtest.c:952:5
  #26 0x55bd71911fae in qtest_outl tests/qtest/libqtest.c:968:5
  #27 0x55bd6fcd1aa2 in op_out tests/qtest/fuzz/generic_fuzz.c:395:13
  #28 0x55bd6fcd04e9 in generic_fuzz tests/qtest/fuzz/generic_fuzz.c:680:17
  #29 0x55bd6fcc9723 in LLVMFuzzerTestOneInput tests/qtest/fuzz/fuzz.c:151:5

  OSS-Fuzz Report:
  https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28524

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



[Bug 1907497] Re: [OSS-Fuzz] Issue 28435 qemu:qemu-fuzz-i386-target-generic-fuzz-intel-hda: Stack-overflow in ldl_le_dma

2021-01-15 Thread Peter Maydell
** Tags added: fuzzer

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

Title:
  [OSS-Fuzz] Issue 28435 qemu:qemu-fuzz-i386-target-generic-fuzz-intel-
  hda: Stack-overflow in ldl_le_dma

Status in QEMU:
  New

Bug description:
   affects qemu

  === Reproducer (build with --enable-sanitizers) ===

  cat << EOF | ./qemu-system-i386 -machine q35 -nodefaults \
  -device intel-hda,id=hda0 -device hda-output,bus=hda0.0 \
  -device hda-micro,bus=hda0.0 -device hda-duplex,bus=hda0.0 \
  -qtest stdio
  outl 0xcf8 0x8804
  outw 0xcfc 0x
  write 0x0 0x1 0x12
  write 0x2 0x1 0x2f
  outl 0xcf8 0x8811
  outl 0xcfc 0x5a6a4406
  write 0x6a44005a 0x1 0x11
  write 0x6a44005c 0x1 0x3f
  write 0x6a442050 0x4 0x446a
  write 0x6a44204a 0x1 0xf3
  write 0x6a44204c 0x1 0xff
  writeq 0x6a44005a 0x17b3f0011
  write 0x6a442050 0x4 0x446a
  write 0x6a44204a 0x1 0xf3
  write 0x6a44204c 0x1 0xff
  EOF

  === Stack Trace ===
  ==411958==ERROR: AddressSanitizer: stack-overflow on address 0x7ffcaeb8bc88 
(pc 0x55c7c9dc1159 bp 0x7ffcaeb8c4d0 sp 0x7ffcaeb8bc90 T0)
  #0 0x55c7c9dc1159 in __asan_memcpy (u-system-i386+0x2a13159)
  #1 0x55c7cb2a457e in flatview_do_translate softmmu/physmem.c:513:12
  #2 0x55c7cb2bdab0 in flatview_translate softmmu/physmem.c:563:15
  #3 0x55c7cb2bdab0 in flatview_read softmmu/physmem.c:2861:10
  #4 0x55c7cb2bdab0 in address_space_read_full softmmu/physmem.c:2875:18
  #5 0x55c7caaec937 in dma_memory_rw_relaxed include/sysemu/dma.h:87:18
  #6 0x55c7caaec937 in dma_memory_rw include/sysemu/dma.h:110:12
  #7 0x55c7caaec937 in dma_memory_read include/sysemu/dma.h:116:12
  #8 0x55c7caaec937 in ldl_le_dma include/sysemu/dma.h:179:1
  #9 0x55c7caaec937 in ldl_le_pci_dma include/hw/pci/pci.h:816:1
  #10 0x55c7caaec937 in intel_hda_corb_run hw/audio/intel-hda.c:338:16
  #11 0x55c7cb2e7198 in memory_region_write_accessor softmmu/memory.c:491:5
  #12 0x55c7cb2e6bd3 in access_with_adjusted_size softmmu/memory.c:552:18
  #13 0x55c7cb2e646c in memory_region_dispatch_write softmmu/memory.c
  #14 0x55c7cb2c8445 in flatview_write_continue softmmu/physmem.c:2759:23
  #15 0x55c7cb2bdfb8 in flatview_write softmmu/physmem.c:2799:14
  #16 0x55c7cb2bdfb8 in address_space_write softmmu/physmem.c:2891:18
  #17 0x55c7caae2c54 in dma_memory_rw_relaxed include/sysemu/dma.h:87:18
  #18 0x55c7caae2c54 in dma_memory_rw include/sysemu/dma.h:110:12
  #19 0x55c7caae2c54 in dma_memory_write include/sysemu/dma.h:122:12
  #20 0x55c7caae2c54 in stl_le_dma include/sysemu/dma.h:179:1
  #21 0x55c7caae2c54 in stl_le_pci_dma include/hw/pci/pci.h:816:1
  #22 0x55c7caae2c54 in intel_hda_response hw/audio/intel-hda.c:370:5
  #23 0x55c7caaeca00 in intel_hda_corb_run hw/audio/intel-hda.c:342:9
  #24 0x55c7cb2e7198 in memory_region_write_accessor softmmu/memory.c:491:5
  ...

  OSS-Fuzz Report: https://bugs.chromium.org/p/oss-
  fuzz/issues/detail?id=28435

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



[Bug 1907909] Re: assertion failure in am53c974

2021-01-15 Thread Peter Maydell
** Tags added: fuzzer

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

Title:
  assertion failure in am53c974

Status in QEMU:
  New

Bug description:
  Hello,

  Using hypervisor fuzzer, hyfuzz, I found an assertion failure through
  am53c974 emulator.

  A malicious guest user/process could use this flaw to abort the QEMU
  process on the host, resulting in a denial of service.

  This was found in version 5.2.0 (master)

  
  qemu-system-i386: ../hw/scsi/esp.c:402: void esp_do_dma(ESPState *): 
Assertion `s->cmdlen <= sizeof(s->cmdbuf) && len <= sizeof(s->cmdbuf) - 
s->cmdlen' failed.

  #0  __GI_raise (sig=sig@entry=0x6) at ../sysdeps/unix/sysv/linux/raise.c:51
  51  ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
  [Current thread is 1 (Thread 0x7fdd25dc4700 (LWP 28983))]
  gdb-peda$ bt
  #0  0x7fdd3f8b5f47 in __GI_raise (sig=sig@entry=0x6) at 
../sysdeps/unix/sysv/linux/raise.c:51
  #1  0x7fdd3f8b78b1 in __GI_abort () at abort.c:79
  #2  0x7fdd3f8a742a in __assert_fail_base (fmt=0x7fdd3fa2ea38 "%s%s%s:%u: 
%s%sAssertion `%s' failed.\\n%n", assertion=assertion@entry=0x55b3e11a51c6 
"s->cmdlen <= sizeof(s->cmdbuf) && len <= sizeof(s->cmdbuf) - s->cmdlen", 
file=file@entry=0x55b3e11a4f73 "../hw/scsi/esp.c", line=line@entry=0x192, 
function=function@entry=0x55b3e11a520d "void esp_do_dma(ESPState *)") at 
assert.c:92
  #3  0x7fdd3f8a74a2 in __GI___assert_fail (assertion=0x55b3e11a51c6 
"s->cmdlen <= sizeof(s->cmdbuf) && len <= sizeof(s->cmdbuf) - s->cmdlen", 
file=0x55b3e11a4f73 "../hw/scsi/esp.c", line=0x192, function=0x55b3e11a520d 
"void esp_do_dma(ESPState *)") at assert.c:101
  #4  0x55b3e0941441 in esp_do_dma (s=0x55b3e49d1c88) at 
../hw/scsi/esp.c:401
  #5  0x55b3e0944261 in handle_ti (s=0x55b3e49d1c88) at ../hw/scsi/esp.c:549
  #6  0x55b3e093fdf9 in esp_dma_enable (s=0x55b3e49d1c88, irq=, level=)
  at ../hw/scsi/esp.c:79
  #7  0x55b3e0897930 in esp_pci_dma_write (pci=, 
saddr=, val=) at ../hw/scsi/esp-pci.c:83
  #8  0x55b3e0897930 in esp_pci_io_write (opaque=, 
addr=, val=0xcf, size=0x4) at ../hw/scsi/esp-pci.c:209
  #9  0x55b3e0e8f798 in memory_region_write_accessor (mr=, 
addr=, value=, size=, 
shift=, mask=, attrs=...)
  at ../softmmu/memory.c:491
  #10 0x55b3e0e8f58e in access_with_adjusted_size (addr=, 
value=, size=, access_size_min=, 
access_size_max=, access_fn=, mr=, 
attrs=...) at ../softmmu/memory.c:552
  #11 0x55b3e0e8f58e in memory_region_dispatch_write (mr=0x55b3e49d1b70, 
addr=, data=, op=, attrs=...) at 
../softmmu/memory.c:1501
  #12 0x55b3e0e21541 in address_space_stb (as=, 
addr=, val=0xffcf, attrs=..., result=0x0) at 
../memory_ldst.c.inc:382
  #13 0x7fdcd84a4a7f in code_gen_buffer ()
  #14 0x55b3e0e57da0 in cpu_tb_exec (cpu=0x55b3e3c33650, itb=)
  at ../accel/tcg/cpu-exec.c:178
  #15 0x55b3e0e589eb in cpu_loop_exec_tb (tb=, 
cpu=, last_tb=, tb_exit=) at ../accel/tcg/cpu-exec.c:658
  #16 0x55b3e0e589eb in cpu_exec (cpu=0x55b3e3c33650) at 
../accel/tcg/cpu-exec.c:771
  #17 0x55b3e0e87b9f in tcg_cpu_exec (cpu=) at 
../accel/tcg/tcg-cpus.c:243
  #18 0x55b3e0e87b9f in tcg_cpu_thread_fn (arg=0x55b3e3c33650) at 
../accel/tcg/tcg-cpus.c:427
  #19 0x55b3e115f775 in qemu_thread_start (args=) at 
../util/qemu-thread-posix.c:521
  #20 0x7fdd3fc6f6db in start_thread (arg=0x7fdd25dc4700) at 
pthread_create.c:463
  #21 0x7fdd3f998a3f in clone () at 
../sysdeps/unix/sysv/linux/x86_64/clone.S:95

  To reproduce the assertion failure, please run the QEMU with the
  following command line.

  
  $ ./qemu-system-i386 -m 512 -drive 
file=./hyfuzz.img,index=0,media=disk,format=raw -device am53c974,id=scsi 
-device scsi-hd,drive=SysDisk -drive id=SysDisk,if=none,file=./disk.img

  Please let me know if I can provide any further info.

  Thank you.

  - Cheolwoo, Myung (Seoul National University)

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



[Bug 1904954] Re: lan9118 bug peeked received message size not equal to actual received message size

2021-01-15 Thread Peter Maydell
Fix now in master: commit e7e29fdbbe07f.


** Changed in: qemu
   Status: In Progress => Fix Committed

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

Title:
  lan9118 bug peeked received message size not equal to actual received
  message size

Status in QEMU:
  Fix Committed

Bug description:
  peeked message size is not equal to read message size

  Bug in the code at line:
  https://github.com/qemu/qemu/blob/master/hw/net/lan9118.c#L1209

  s->tx_status_fifo_head should be s->rx_status_fifo_head

  Could also be a security bug, as the user could allocate a buffer of
  size peeked data smaller than the actual packet received, which could
  cause a buffer overflow.

  Thanks,

  Alfred

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



[Bug 1905444] Re: [OSS-Fuzz] Issue 27796 in oss-fuzz: qemu:qemu-fuzz-i386-target-generic-fuzz-xhci: Stack-overflow in address_space_stl_internal

2021-01-15 Thread Peter Maydell
** Tags added: fuzzer

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

Title:
  [OSS-Fuzz] Issue 27796 in oss-fuzz: qemu:qemu-fuzz-i386-target-
  generic-fuzz-xhci: Stack-overflow in address_space_stl_internal

Status in QEMU:
  New

Bug description:
   affects qemu

  OSS-Fuzz Report: https://bugs.chromium.org/p/oss-
  fuzz/issues/detail?id=27796

  === Reproducer (build with --enable-sanitizers) ===
  cat << EOF | ./qemu-system-i386 -display none  -machine accel=qtest, \
  -m 512M -machine q35 -nodefaults \
  -drive file=null-co://,if=none,format=raw,id=disk0 \
  -device qemu-xhci,id=xhci -device usb-tablet,bus=xhci.0 \
  -qtest-log none -qtest stdio
  outl 0xcf8 0x8803
  outw 0xcfc 0x5e46
  outl 0xcf8 0x8810
  outl 0xcfc 0xff5a5e46
  write 0xff5a5020 0x6 0x0b70
  outl 0xcf8 0x8893
  outb 0xcfc 0x93
  writel 0xff5a7000 0xff5a5020
  write 0xff5a700c 0x4 0x0c0c2e58
  write 0xff5a4040 0x4 0x00d26001
  write 0xff5a4044 0x4 0x030
  EOF

  === Stack Trace ===
  ==50473==ERROR: AddressSanitizer: stack-overflow on address 0x7ffe3ec97e28 
(pc 0x55e292eac159 bp 0x7ffe3ec98670 sp 0x7ffe3ec97e30 T0)
  #0 0x55e292eac159 in __asan_memcpy (u-system-i386+0x2a0e159)
  #1 0x55e2944bc04e in flatview_do_translate softmmu/physmem.c:513:12
  #2 0x55e2944dbe90 in flatview_translate softmmu/physmem.c:563:15
  #3 0x55e2944dbe90 in address_space_translate include/exec/memory.h:2362:12
  #4 0x55e2944dbe90 in address_space_stl_internal memory_ldst.c.inc:316:10
  #5 0x55e29393d2a0 in xhci_intr_update hw/usb/hcd-xhci.c:554:13
  #6 0x55e29393efb9 in xhci_runtime_write hw/usb/hcd-xhci.c:3032:9
  #7 0x55e294230428 in memory_region_write_accessor softmmu/memory.c:484:5
  #8 0x55e29422fe63 in access_with_adjusted_size softmmu/memory.c:545:18
  #9 0x55e29422f6fc in memory_region_dispatch_write softmmu/memory.c
  #10 0x55e2944dc03c in address_space_stl_internal memory_ldst.c.inc:319:13
  #11 0x55e29393d2a0 in xhci_intr_update hw/usb/hcd-xhci.c:554:13
  #12 0x55e29393efb9 in xhci_runtime_write hw/usb/hcd-xhci.c:3032:9
  #13 0x55e294230428 in memory_region_write_accessor softmmu/memory.c:484:5
  #14 0x55e29422fe63 in access_with_adjusted_size softmmu/memory.c:545:18
  #15 0x55e29422f6fc in memory_region_dispatch_write softmmu/memory.c
  #16 0x55e2944dc03c in address_space_stl_internal memory_ldst.c.inc:319:13
  #17 0x55e29393d2a0 in xhci_intr_update hw/usb/hcd-xhci.c:554:13
  #18 0x55e29393efb9 in xhci_runtime_write hw/usb/hcd-xhci.c:3032:9

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



[Bug 1907817] Re: qemu-aarch64 tcg assertion v5.2.0

2021-01-15 Thread Peter Maydell
Fix now in master as commit 6d3ef04893bde -- will be in next QEMU
release.


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

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

Title:
  qemu-aarch64 tcg assertion v5.2.0

Status in QEMU:
  Fix Committed

Bug description:
  After updating to 5.2 I am getting following assertion error:
  qemu-aarch64: ../tcg/tcg-op-gvec.c:54: check_size_align: Assertion `(maxsz & 
max_align) == 0' failed.

  I think it was introduced by commit:
  e2e7168a214b0ed98dc357bba96816486a289762

  Becasue before this change, in function simd_desc only maxsz % 8 == 0 was 
checked, but after this change qemu check for following:
   
  max_align = maxsz >= 16 ? 15 : 7;
  tcg_debug_assert((maxsz & max_align) == 0);  <--- here assertion happens

  in my case maxsz=56.

  
  Whole backtrace:
  #4  0x004000314770 in check_size_align (oprsz=56, maxsz=56, ofs=0) at 
../tcg/tcg-op-gvec.c:54
  #5  0x004000314950 in simd_desc (oprsz=56, maxsz=56, data=0) at 
../tcg/tcg-op-gvec.c:89
  #6  0x004000316270 in do_dup (vece=0, dofs=3144, oprsz=56, maxsz=56, 
in_32=0x0, in_64=0x0, in_c=0) at ../tcg/tcg-op-gvec.c:630
  #7  0x0040003164d0 in expand_clr (dofs=3144, maxsz=56) at 
../tcg/tcg-op-gvec.c:679
  #8  0x004000319bb0 in tcg_gen_gvec_mov (vece=3, dofs=3136, aofs=3136, 
oprsz=8, maxsz=64) at ../tcg/tcg-op-gvec.c:1538
  #9  0x004000200dc0 in clear_vec_high (s=0x40021a8180, is_q=false, rd=0) 
at ../target/arm/translate-a64.c:592
  #10 0x004000200e40 in write_fp_dreg (s=0x40021a8180, reg=0, v=0x1108) at 
../target/arm/translate-a64.c:600
  --Type  for more, q to quit, c to continue without paging--
  #11 0x004000200e90 in write_fp_sreg (s=0x40021a8180, reg=0, v=0x1060) at 
../target/arm/translate-a64.c:608
  #12 0x004000214210 in handle_fpfpcvt (s=0x40021a8180, rd=0, rn=0, 
opcode=2, itof=true, rmode=0, scale=64, sf=0, type=0)
  at ../target/arm/translate-a64.c:6988
  #13 0x004000214f90 in disas_fp_int_conv (s=0x40021a8180, insn=505544704) 
at ../target/arm/translate-a64.c:7299
  #14 0x004000215350 in disas_data_proc_fp (s=0x40021a8180, insn=505544704) 
at ../target/arm/translate-a64.c:7389
  #15 0x00400022aa70 in disas_data_proc_simd_fp (s=0x40021a8180, 
insn=505544704) at ../target/arm/translate-a64.c:14494
  #16 0x00400022af90 in disas_a64_insn (env=0x7fac59b6b490, s=0x40021a8180) 
at ../target/arm/translate-a64.c:14663
  #17 0x00400022b750 in aarch64_tr_translate_insn (dcbase=0x40021a8180, 
cpu=0x7fac59b63150) at ../target/arm/translate-a64.c:14823
  #18 0x0040002e8630 in translator_loop (ops=0x4000902e00 
, db=0x40021a8180, cpu=0x7fac59b63150, 
  tb=0x7fac3419c5c0, max_insns=512) at ../accel/tcg/translator.c:103
  #19 0x0040002e3a60 in gen_intermediate_code (cpu=0x7fac59b63150, 
tb=0x7fac3419c5c0, max_insns=512)
  at ../target/arm/translate.c:9283
  #20 0x0040002fed30 in tb_gen_code (cpu=0x7fac59b63150, pc=4458820, 
cs_base=0, flags=2148544819, cflags=-16777216)
  at ../accel/tcg/translate-all.c:1744
  #21 0x00400036a6e0 in tb_find (cpu=0x7fac59b63150, 
last_tb=0x7fac3419c400, tb_exit=0, cf_mask=0) at ../accel/tcg/cpu-exec.c:414
  --Type  for more, q to quit, c to continue without paging--
  #22 0x00400036b040 in cpu_exec (cpu=0x7fac59b63150) at 
../accel/tcg/cpu-exec.c:770
  #23 0x004000113a90 in cpu_loop (env=0x7fac59b6b490) at 
../linux-user/aarch64/cpu_loop.c:84
  #24 0x0040002fb8c0 in main (argc=2, argv=0x40021a8e68, envp=0x40021a8e80) 
at ../linux-user/main.c:864

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



[Bug 1909247] Re: QEMU: use after free vulnerability in esp_do_dma() in hw/scsi/esp.c

2021-01-15 Thread Peter Maydell
** Tags added: fuzzer

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

Title:
  QEMU: use after free vulnerability in esp_do_dma() in hw/scsi/esp.c

Status in QEMU:
  New

Bug description:
  A use-after-free vulnerability was found in the am53c974 SCSI host bus
  adapter emulation of QEMU. It could occur in the esp_do_dma() function
  in hw/scsi/esp.c while handling the 'Information Transfer' command
  (CMD_TI). A privileged guest user may abuse this flaw to crash the
  QEMU process on the host, resulting in a denial of service or
  potential code execution with the privileges of the QEMU process.

  This issue was reported by Cheolwoo Myung (Seoul National University).

  Original report:
  Using hypervisor fuzzer, hyfuzz, I found a use-after-free issue in
  am53c974 emulator of QEMU enabled ASan.

  It occurs while transferring information, as it does not check the
  buffer to be transferred.

  A malicious guest user/process could use this flaw to crash the QEMU
  process resulting in DoS scenario.

  To reproduce this issue, please run the QEMU with the following command
  line.

  # To enable ASan option, please set configuration with the following
  $ ./configure --target-list=i386-softmmu --disable-werror --enable-sanitizers
  $ make

  # To reproduce this issue, please run the QEMU process with the following 
command line
  $ ./qemu-system-i386 -m 512 -drive 
file=./hyfuzz.img,index=0,media=disk,format=raw \
  -device am53c974,id=scsi -device scsi-hd,drive=SysDisk \
  -drive id=SysDisk,if=none,file=./disk.img

  Please find attached the disk images to reproduce this issue.

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



[Bug 1901532] Re: Assertion failure `mr != NULL' failed through usb-ehci

2021-01-15 Thread Peter Maydell
** Tags added: fuzzer

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

Title:
  Assertion failure `mr != NULL' failed through usb-ehci

Status in QEMU:
  Confirmed

Bug description:
  Hello,

  Using hypervisor fuzzer, hyfuzz, I found an assertion failure through
  usb-ehci.

  This was found in version 5.0.1 (stable-5.0).

  

  qemu-system-i386: src/qemu-repro/exec.c:3581: address_space_unmap: Assertion 
`mr != NULL' failed.
  [1]14721 abort  src/qemu-repro/build/i386-softmmu/qemu-system-i386

  
  To reproduce the assertion failure, please run the QEMU with following 
command line.

  ```
  $ qemu-system-i386 -drive file=./hyfuzz.img,index=0,media=disk,format=raw -m 
512 -drive if=none,id=stick,file=./usbdisk.img -device usb-ehci,id=ehci -device 
usb-storage,bus=ehci.0,drive=stick
  ```

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



[Bug 1904652] Re: Assertion failure in usb-ohci

2021-01-15 Thread Peter Maydell
** Tags added: fuzzer

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

Title:
  Assertion failure in usb-ohci

Status in QEMU:
  New

Bug description:
  Hello,

  Using hypervisor fuzzer, hyfuzz, I found an assertion failure through
  usb-ohci.

  A malicious guest user/process could use this flaw to abort the QEMU
  process on the host, resulting in a denial of service.

  This was found in version 5.2.0 (master)

  

  ```

  Program terminated with signal SIGABRT, Aborted.

  #0  __GI_raise (sig=sig@entry=0x6) at ../sysdeps/unix/sysv/linux/raise.c:51
  51  ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
  [Current thread is 1 (Thread 0x7f34d0411440 (LWP 9418))]
  gdb-peda$ bt
  #0  0x7f34c8d4ef47 in __GI_raise (sig=sig@entry=0x6) at 
../sysdeps/unix/sysv/linux/raise.c:51
  #1  0x7f34c8d508b1 in __GI_abort () at abort.c:79
  #2  0x55d3a2081844 in ohci_frame_boundary (opaque=0x55d3a4ecdaf0) at 
../hw/usb/hcd-ohci.c:1297
  #3  0x55d3a25be155 in timerlist_run_timers (timer_list=0x55d3a3fd9840) at 
../util/qemu-timer.c:574
  #4  0x55d3a25beaba in qemu_clock_run_timers (type=QEMU_CLOCK_VIRTUAL) at 
../util/qemu-timer.c:588
  #5  0x55d3a25beaba in qemu_clock_run_all_timers () at 
../util/qemu-timer.c:670
  #6  0x55d3a25e69a1 in main_loop_wait (nonblocking=) at 
../util/main-loop.c:531
  #7  0x55d3a2433972 in qemu_main_loop () at ../softmmu/vl.c:1678
  #8  0x55d3a1d0969b in main (argc=, argc@entry=0x15, 
argv=,
  argv@entry=0x7ffc6de722a8, envp=) at ../softmmu/main.c:50
  #9  0x7f34c8d31b97 in __libc_start_main (main=
  0x55d3a1d09690 , argc=0x15, argv=0x7ffc6de722a8, init=, fini=, rtld_fini=, 
stack_end=0x7ffc6de72298) at ../csu/libc-start.c:310
  #10 0x55d3a1d095aa in _start ()
  ```

  To reproduce the assertion failure, please run the QEMU with the
  following command line.

  ```
  [Terminal 1]

  $ qemu-system-i386 -m 512 -drive
  file=./fs.img,index=1,media=disk,format=raw -drive
  file=./hyfuzz.img,index=0,media=disk,format=raw -drive
  if=none,id=stick,file=./usbdisk.img,format=raw -device pci-ohci,id=usb
  -device usb-storage,bus=usb.0,drive=stick

  [Terminal 2]

  $ ./repro_log ./fs.img ./pci-ohci

  ```

  Please let me know if I can provide any further info.
  -Cheolwoo, Myung (Seoul National University)

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



[Bug 1906693] Re: Assertion Failure in bdrv_co_write_req_prepare through megasas

2021-01-15 Thread Peter Maydell
** Tags added: fuzzer

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

Title:
  Assertion Failure in bdrv_co_write_req_prepare through megasas

Status in QEMU:
  New

Bug description:
   affects qemu
   subscribe h...@suse.com
   subscribe phi...@redhat.com
   subscribe stefa...@redhat.com
   subscribe kw...@redhat.com

  === Stack Trace ===
  qemu-fuzz-i386: block/io.c:1835: int bdrv_co_write_req_prepare(BdrvChild *, 
int64_t, uint64_t, BdrvTrackedRequest *, int): Assertion `child->perm & 
BLK_PERM_WRITE' failed.
  ==1505128== ERROR: libFuzzer: deadly signal
  #0 0x55a083b92cee in __sanitizer_print_stack_trace 
(qemu-fuzz-i386+0x793cee)
  #1 0x55a083b6c1d1 in fuzzer::PrintStackTrace() (qemu-fuzz-i386+0x76d1d1)
  #2 0x55a083b4f0d6 in fuzzer::Fuzzer::CrashCallback() (.part.0) 
(qemu-fuzz-i386+0x7500d6)
  #3 0x55a083b4f19b in fuzzer::Fuzzer::StaticCrashSignalCallback() 
(qemu-fuzz-i386+0x75019b)
  #4 0x7f8d24ed6a8f  (/lib64/libpthread.so.0+0x14a8f)
  #5 0x7f8d24d079e4 in raise (/lib64/libc.so.6+0x3c9e4)
  #6 0x7f8d24cf0894 in abort (/lib64/libc.so.6+0x25894)
  #7 0x7f8d24cf0768 in __assert_fail_base.cold (/lib64/libc.so.6+0x25768)
  #8 0x7f8d24cffe75 in __assert_fail (/lib64/libc.so.6+0x34e75)
  #9 0x55a08423763f in bdrv_co_write_req_prepare block/io.c:1835:13
  #10 0x55a0842343a8 in bdrv_aligned_pwritev block/io.c:1915:11
  #11 0x55a084233765 in bdrv_co_pwritev_part block/io.c:2104:11
  #12 0x55a084260d1a in blk_do_pwritev_part block/block-backend.c:1260:11
  #13 0x55a08426163e in blk_aio_write_entry block/block-backend.c:1476:17
  #14 0x55a0843b0d23 in coroutine_trampoline util/coroutine-ucontext.c:173:9
  #15 0x7f8d24d1d22f  (/lib64/libc.so.6+0x5222f)

  === Reproducer===
  cat << EOF | ./qemu-system-i386 -M q35 \
  -device megasas-gen2 -device scsi-cd,drive=null0 \
  -blockdev driver=null-co,read-zeroes=on,node-name=null0 \
  -monitor none -serial none -display none \
  -machine accel=qtest -m 64 -qtest stdio
  outl 0xcf8 0x80001804
  outl 0xcfc 0xff
  outl 0xcf8 0x8000181b
  outl 0xcfc 0x7052005
  write 0x5cc0 0x1 0x03
  write 0x5cc7 0x1 0x40
  write 0x5ce0 0x1 0x0a
  write 0x5cf3 0x1 0x01
  write 0x5cf7 0x1 0x40
  write 0x5cf8 0x1 0x0a
  write 0x5cff 0x1 0x05
  write 0x5d03 0x1 0x5b
  write 0x5d06 0x1 0x4f
  write 0x5d0b 0x1 0x01
  write 0x5d0f 0x1 0x40
  write 0x5d10 0x1 0x0a
  write 0x5d17 0x1 0x05
  write 0x5d1b 0x1 0x5b
  write 0x5d1e 0x1 0x4f
  write 0x5d23 0x1 0x01
  write 0x5d27 0x1 0x40
  write 0x5d28 0x1 0x0a
  write 0x5d2f 0x1 0x05
  write 0x5d33 0x1 0x5b
  write 0x5d36 0x1 0x4f
  write 0x5d3b 0x1 0x01
  write 0x5d3f 0x1 0x40
  write 0x5d40 0x1 0x0a
  write 0x5d47 0x1 0x05
  write 0x5d4b 0x1 0x5b
  write 0x5d4e 0x1 0x4f
  write 0x5d53 0x1 0x01
  write 0x5d57 0x1 0x40
  write 0x5d58 0x1 0x0a
  write 0x5d5f 0x1 0x05
  write 0x5d63 0x1 0x5b
  write 0x5d66 0x1 0x4f
  write 0x5d6b 0x1 0x01
  write 0x5d6f 0x1 0x40
  write 0x5d70 0x1 0x0a
  write 0x5d77 0x1 0x05
  write 0x5d7b 0x1 0x5b
  write 0x5d7e 0x1 0x4f
  write 0x5d83 0x1 0x01
  write 0x5d87 0x1 0x40
  write 0x5d88 0x1 0x0a
  write 0x5d8f 0x1 0x05
  write 0x5d93 0x1 0x5b
  write 0x5d96 0x1 0x4f
  write 0x5d9b 0x1 0x01
  write 0x5d9f 0x1 0x40
  write 0x5da0 0x1 0x0a
  write 0x5da7 0x1 0x05
  write 0x5dab 0x1 0x5b
  write 0x5dae 0x1 0x4f
  write 0x5db3 0x1 0x01
  write 0x5db7 0x1 0x40
  write 0x5db8 0x1 0x0a
  write 0x5dbf 0x1 0x05
  write 0x5dc3 0x1 0x5b
  write 0x5dc6 0x1 0x4f
  write 0x5dcb 0x1 0x01
  write 0x5dcf 0x1 0x40
  write 0x5dd0 0x1 0x0a
  write 0x5dd7 0x1 0x05
  write 0x5ddb 0x1 0x5b
  write 0x5dde 0x1 0x4f
  write 0x5de3 0x1 0x01
  write 0x5de7 0x1 0x40
  write 0x5de8 0x1 0x0a
  write 0x5def 0x1 0x05
  write 0x5df3 0x1 0x5b
  write 0x5df6 0x1 0x4f
  write 0x5dfb 0x1 0x01
  write 0x5dff 0x1 0x40
  write 0x5e00 0x1 0x0a
  write 0x5e07 0x1 0x05
  write 0x5e0b 0x1 0x5b
  write 0x5e0e 0x1 0x4f
  write 0x5e13 0x1 0x01
  write 0x5e17 0x1 0x40
  write 0x5e18 0x1 0x0a
  write 0x5e1f 0x1 0x05
  write 0x5e23 0x1 0x5b
  write 0x5e26 0x1 0x4f
  write 0x5e2b 0x1 0x01
  write 0x5e2f 0x1 0x40
  write 0x5e30 0x1 0x0a
  write 0x5e37 0x1 0x05
  write 0x5e3b 0x1 0x5b
  write 0x5e3e 0x1 0x4f
  write 0x5e43 0x1 0x01
  write 0x5e47 0x1 0x40
  write 0x5e48 0x1 0x0a
  write 0x5e4f 0x1 0x05
  write 0x5e53 0x1 0x5b
  write 0x5e56 0x1 0x4f
  write 0x5e5b 0x1 0x01
  write 0x5e5f 0x1 0x40
  write 0x5e60 0x1 0x0a
  write 0x5e67 0x1 0x05
  write 0x5e6b 0x1 0x5b
  write 0x5e6e 0x1 0x4f
  write 0x5e73 0x1 0x01
  write 0x5e77 0x1 0x40
  write 0x5e78 0x1 0x0a
  write 0x5e7f 0x1 0x05
  write 0x5e83 0x1 0x5b
  write 0x5e86 0x1 0x4f
  write 0x5e8b 0x1 0x01
  write 0x5e8f 0x1 0x40
  write 0x5e90 0x1 0x0a
  write 0x5e97 0x1 0x05
  write 0x5e9b 0x1 0x5b
  write 0x5e9e 0x1 0x4f
  write 0x5ea3 0x1 0x01
  write 0x5ea7 0x1 0x40
  write 0x5ea8 0x1 0x0a
  write 0x5eaf 0x1 0x05
  write 0x5eb3 0x1 0x5b
  write 

Re: [PULL 0/1] 9p security fix 2021-01-15

2021-01-15 Thread Peter Maydell
On Fri, 15 Jan 2021 at 09:05, Greg Kurz  wrote:
>
> The following changes since commit 7c79721606be11b5bc556449e5bcbc331ef6867d:
>
>   Merge remote-tracking branch 'remotes/rth-gitlab/tags/pull-tcg-20210113' 
> into staging (2021-01-14 09:54:29 +)
>
> are available in the Git repository at:
>
>   https://gitlab.com/gkurz/qemu.git tags/9p-next-2021-01-15
>
> for you to fetch changes up to 89fbea8737e8f7b954745a1ffc4238d377055305:
>
>   9pfs: Fully restart unreclaim loop (CVE-2021-20181) (2021-01-15 08:44:28 
> +0100)
>
> 
> Fix for CVE-2021-20181

Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/6.0
for any user-visible changes.

-- PMM



Re: Call for Google Summer of Code 2021 project ideas

2021-01-15 Thread Kashyap Chamarthy
On Thu, Jan 14, 2021 at 11:36:23AM -0500, John Snow wrote:
> On 1/14/21 7:29 AM, Markus Armbruster wrote:

[...]

> So I see two possible options for "not inventing a language":
> 
> 1. Raw QMP
> 2. The existing qmp-shell syntax, warts and all.
> 
> I don't see a tremendous problem with doing both; but we can start with raw
> QMP. The existing qmp-shell syntax is at least definitely very easy to write
> a new parser for, even if it's kind of ugly and insufficient. I still see
> value in designing a new TUI with the old syntax.
> 
> > The project then aims to build a tool that adds useful features over
> > "socat "READLINE,history=$HOME/.qmp_history,prompt=QMP>"
> > UNIX-CONNECT:/path/to/socket".
> > 
> > If it succeeds, you can still design and implement a "better" language,
> > and let users choose the one they prefer.  Or you could add features to
> > help with typing QMP.
> > 
> > > I don't
> > > think it's a blocker to have someone work on the TUI and asynchronous
> > > dispatch elements. I think even just keeping our current parsing but
> > > adding some of the features outlined in the proposal would be a big
> > > usability win.
> > 
> > I don't feel this particular itch, but I'm certainly not objecting to
> > anyone scratching.
> > 
> 
> It's something I'd like to see so that I can walk non-QEMU devs through
> interacting with QEMU at a low level for the purposes of debugging,
> reproducing problems, prototyping features, etc.
>
> I use qmp-shell all the time for debugging things myself, I find it easier
> to use than copy-pasting things directly into socat. I wouldn't mind the
> shell getting a little smarter to help me out -- the ability to see async
> events and reconnect on disconnect would already be a massive improvement to
> *my* quality of life.

As an infrequent user of `qmp-shell`, the async events stuff is really
beneficial to me too.  And, I'm happy to play the test guinea pig to
give the patchs a spin.  (I'm somewhat behind on the goings-on in this
area, very slowly catching up.)

> So much so that I spent a lot of time in December to write an async qmp
> library O:-)

Nice, I recall that you planned to use the 'asyncio' primitives from
Python 3.6.

-- 
/kashyap




Re: [PATCH v7 13/13] s390: Recognize confidential-guest-support option

2021-01-15 Thread Cornelia Huck
On Thu, 14 Jan 2021 10:58:11 +1100
David Gibson  wrote:

> At least some s390 cpu models support "Protected Virtualization" (PV),
> a mechanism to protect guests from eavesdropping by a compromised
> hypervisor.
> 
> This is similar in function to other mechanisms like AMD's SEV and
> POWER's PEF, which are controlled by the "confidential-guest-support"
> machine option.  s390 is a slightly special case, because we already
> supported PV, simply by using a CPU model with the required feature
> (S390_FEAT_UNPACK).
> 
> To integrate this with the option used by other platforms, we
> implement the following compromise:
> 
>  - When the confidential-guest-support option is set, s390 will
>recognize it, verify that the CPU can support PV (failing if not)
>and set virtio default options necessary for encrypted or protected
>guests, as on other platforms.  i.e. if confidential-guest-support
>is set, we will either create a guest capable of entering PV mode,
>or fail outright.
> 
>  - If confidential-guest-support is not set, guests might still be
>able to enter PV mode, if the CPU has the right model.  This may be
>a little surprising, but shouldn't actually be harmful.
> 
> To start a guest supporting Protected Virtualization using the new
> option use the command line arguments:
> -object s390-pv-guest,id=pv0 -machine confidential-guest-support=pv0
> 
> Signed-off-by: David Gibson 
> ---
>  docs/confidential-guest-support.txt |  3 ++
>  docs/system/s390x/protvirt.rst  | 19 ++---
>  hw/s390x/pv.c   | 62 +
>  include/hw/s390x/pv.h   |  1 +
>  target/s390x/kvm.c  |  3 ++
>  5 files changed, 82 insertions(+), 6 deletions(-)
> 

(...)

> +int s390_pv_init(ConfidentialGuestSupport *cgs, Error **errp)
> +{
> +if (!object_dynamic_cast(OBJECT(cgs), TYPE_S390_PV_GUEST)) {
> +return 0;
> +}
> +
> +if (!s390_has_feat(S390_FEAT_UNPACK)) {
> +error_setg(errp,
> +   "CPU model does not support Protected Virtualization");
> +return -1;
> +}
> +
> +cgs->ready = true;
> +
> +return 0;
> +}

Do we want to add a migration blocker here? If we keep the one that is
added when the guest transitions, we'll end up with two of them, but
that might be easier than trying to unify it.




Re: [PATCH] util/cacheflush: Fix error generated by clang

2021-01-15 Thread Richard Henderson
On 1/14/21 9:56 PM, Gan Qixin wrote:
> When compiling qemu-fuzz-i386 on aarch64 host, clang reported the following
> error:
> 
> ../util/cacheflush.c:38:44: error: value size does not match register size
> specified by the constraint and modifier [-Werror,-Wasm-operand-widths]
> asm volatile("mrs\t%0, ctr_el0" : "=r"(save_ctr_el0));
>^
> ../util/cacheflush.c:38:24: note: use constraint modifier "w"
> asm volatile("mrs\t%0, ctr_el0" : "=r"(save_ctr_el0));
>^~
>%w0
> 
> Modify the type of save_ctr_el0 to uint64_t to fix it.
> 
> Reported-by: Euler Robot 
> Signed-off-by: Gan Qixin 
> ---
> Cc: Richard Henderson 
> Cc: Paolo Bonzini 
> ---
>  util/cacheflush.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)

This is clang being overly-picky.  IMO it should have done the mrs into a
64-bit register, then truncated the value when storing to the 32-bit variable.
 Which is what GCC does.  Certainly the code as written only needs the low 20
bits of the result.

But your change will not really make any difference to the generated code,
except for 4 more bytes of storage, so:

Reviewed-by: Richard Henderson 


r~



Re: [PATCH] qtest/npcm7xx_pwm-test: Fix memleak in pwm_qom_get

2021-01-15 Thread Havard Skinnemoen via
+Hao Wu

On Fri, Jan 15, 2021 at 1:15 AM Philippe Mathieu-Daudé  wrote:
>
> On 1/15/21 8:56 AM, Gan Qixin wrote:
> > The pwm_qom_get function didn't free "response", which caused an indirect
> > memory leak. So use qobject_unref() to fix it.
> >
> > ASAN shows memory leak stack:
> >
> > Indirect leak of 7416 byte(s) in 18000 object(s) allocated from:
> > #0 0x7f96e2f79d4e in __interceptor_calloc (/lib64/libasan.so.5+0x112d4e)
> > #1 0x7f96e2d98a50 in g_malloc0 (/lib64/libglib-2.0.so.0+0x55a50)
> > #2 0x556313112180 in qdict_new ../qobject/qdict.c:30
> > #3 0x556313115bca in parse_object ../qobject/json-parser.c:318
> > #4 0x556313117810 in parse_value ../qobject/json-parser.c:546
> > #5 0x556313117bda in json_parser_parse ../qobject/json-parser.c:580
> > #6 0x55631310fe67 in json_message_process_token 
> > ../qobject/json-streamer.c:92
> > #7 0x5563131210b7 in json_lexer_feed_char ../qobject/json-lexer.c:313
> > #8 0x556313121662 in json_lexer_feed ../qobject/json-lexer.c:350
> > #9 0x5563131101e9 in json_message_parser_feed 
> > ../qobject/json-streamer.c:121
> > #10 0x5563130cb81e in qmp_fd_receive ../tests/qtest/libqtest.c:614
> > #11 0x5563130cba2b in qtest_qmp_receive_dict 
> > ../tests/qtest/libqtest.c:636
> > #12 0x5563130cb939 in qtest_qmp_receive ../tests/qtest/libqtest.c:624
> > #13 0x5563130cbe0d in qtest_vqmp ../tests/qtest/libqtest.c:715
> > #14 0x5563130cc40f in qtest_qmp ../tests/qtest/libqtest.c:756
> > #15 0x5563130c5623 in pwm_qom_get ../tests/qtest/npcm7xx_pwm-test.c:180
> > #16 0x5563130c595e in pwm_get_duty ../tests/qtest/npcm7xx_pwm-test.c:210
> > #17 0x5563130c7529 in test_toggle ../tests/qtest/npcm7xx_pwm-test.c:447
> >
> > Reported-by: Euler Robot 
> > Signed-off-by: Gan Qixin 
> > ---
> > Cc: Havard Skinnemoen 
> > Cc: Tyrone Ting 
> > Cc: Thomas Huth 
> > Cc: Laurent Vivier 
> > ---
> >  tests/qtest/npcm7xx_pwm-test.c | 5 -
> >  1 file changed, 4 insertions(+), 1 deletion(-)
>
> Reviewed-by: Philippe Mathieu-Daudé 

Reviewed-by: Havard Skinnemoen 

Thanks!



Re: [PATCH 2/3] tests/acceptance: Test the mpc8544ds machine

2021-01-15 Thread Willian Rampazzo
On Tue, Jan 12, 2021 at 1:44 PM Thomas Huth  wrote:
>
> We can use the "Stupid creek" image to test the mpc8544ds ppc machine.
>
> Signed-off-by: Thomas Huth 
> ---
>  tests/acceptance/machine_ppc.py | 17 +
>  1 file changed, 17 insertions(+)

Reviewed-by: Willian Rampazzo 




Re: [PATCH 3/3] tests/acceptance: Add a test for the virtex-ml507 ppc machine

2021-01-15 Thread Willian Rampazzo
On Tue, Jan 12, 2021 at 1:45 PM Thomas Huth  wrote:
>
> The "And a hippo new year" image from the QEMU advent calendar 2020
> can be used to test the virtex-ml507 ppc machine.
>
> Signed-off-by: Thomas Huth 
> ---
>  tests/acceptance/machine_ppc.py | 18 ++
>  1 file changed, 18 insertions(+)

Reviewed-by: Willian Rampazzo 




Re: [PATCH v1 7/7] spapr.c: consider CPU core online state before allowing unplug

2021-01-15 Thread Greg Kurz
On Thu, 14 Jan 2021 15:06:28 -0300
Daniel Henrique Barboza  wrote:

> The only restriction we have when unplugging CPUs is to forbid unplug of
> the boot cpu core. spapr_core_unplug_possible() does not contemplate the

I can't remember why this restriction was introduced in the first place...
This should be investigated and documented if the limitation still stands.

> possibility of some cores being offlined by the guest, meaning that we're
> rolling the dice regarding on whether we're unplugging the last online
> CPU core the guest has.
> 

Trying to unplug the last CPU is obviously something that deserves
special care. LoPAPR is quite explicit on the outcome : this should
terminate the partition.

13.7.4.1.1. Isolation of CPUs

The isolation of a CPU, in all cases, is preceded by the stop-self
RTAS function for all processor threads, and the OS insures that all
the CPU’s threads are in the RTAS stopped state prior to isolating the
CPU. Isolation of a processor that is not stopped produces unpredictable
results. The stopping of the last processor thread of a LPAR partition
effectively kills the partition, and at that point, ownership of all
partition resources reverts to the platform firmware.

R1-13.7.4.1.1-1. For the LRDR option: Prior to issuing the RTAS
set-indicator specifying isolate isolation-state of a CPU DR
connector type, all the CPU threads must be in the RTAS stopped
state.

R1-13.7.4.1.1-2. For the LRDR option: Stopping of the last processor
thread of a LPAR partition with the stop-self RTAS function, must kill
the partition, with ownership of all partition resources reverting to
the platform firmware.

This is clearly not how things work today : linux doesn't call
"stop-self" on the last vCPU and even if it did, QEMU doesn't
terminate the VM.

If there's a valid reason to not implement this PAPR behavior, I'd like
it to be documented.

> If we hit the jackpot, we're going to detach the core DRC and pulse the
> hotplug IRQ, but the guest OS will refuse to release the CPU. Our
> spapr_core_unplug() DRC release callback will never be called and the CPU
> core object will keep existing in QEMU. No error message will be sent
> to the user, but the CPU core wasn't unplugged from the guest.
> 
> If the guest OS onlines the CPU core again we won't be able to hotunplug it
> either. 'dmesg' inside the guest will report a failed attempt to offline an
> unknown CPU:
> 
> [  923.003994] pseries-hotplug-cpu: Failed to offline CPU , rc: -16
> 
> This is the result of stopping the DRC state transition in the middle in the
> first failed attempt.
> 

Yes, at this point only a machine reset can fix things up.

Given this is linux's choice not to call "stop-self" as it should do, I'm not
super fan of hardcoding this logic in QEMU, unless there are really good
reasons to do so.

> We can avoid this, and potentially other bad things from happening, if we
> avoid to attempt the unplug altogether in this scenario. Let's check for
> the online/offline state of the CPU cores in the guest before allowing
> the hotunplug, and forbid removing a CPU core if it's the last one online
> in the guest.
> 
> Reported-by: Xujun Ma 
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1911414
> Signed-off-by: Daniel Henrique Barboza 
> ---
>  hw/ppc/spapr.c | 39 ++-
>  1 file changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index a2f01c21aa..d269dcd102 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3709,9 +3709,16 @@ static void spapr_core_unplug(HotplugHandler 
> *hotplug_dev, DeviceState *dev)
>  static int spapr_core_unplug_possible(HotplugHandler *hotplug_dev, CPUCore 
> *cc,
>Error **errp)
>  {
> +CPUArchId *core_slot;
> +SpaprCpuCore *core;
> +PowerPCCPU *cpu;
> +CPUState *cs;
> +bool last_cpu_online = true;
>  int index;
>  
> -if (!spapr_find_cpu_slot(MACHINE(hotplug_dev), cc->core_id, )) {
> +core_slot = spapr_find_cpu_slot(MACHINE(hotplug_dev), cc->core_id,
> +);
> +if (!core_slot) {
>  error_setg(errp, "Unable to find CPU core with core-id: %d",
> cc->core_id);
>  return -1;
> @@ -3722,6 +3729,36 @@ static int spapr_core_unplug_possible(HotplugHandler 
> *hotplug_dev, CPUCore *cc,
>  return -1;
>  }
>  
> +/* Allow for any non-boot CPU core to be unplugged if already offline */
> +core = SPAPR_CPU_CORE(core_slot->cpu);
> +cs = CPU(core->threads[0]);
> +if (cs->halted) {
> +return 0;
> +}
> +
> +/*
> + * Do not allow core unplug if it's the last core online.
> + */
> +cpu = POWERPC_CPU(cs);
> +CPU_FOREACH(cs) {
> +PowerPCCPU *c = POWERPC_CPU(cs);
> +
> +if (c == cpu) {
> +continue;
> +}
> +
> +if (!cs->halted) {
> +last_cpu_online = false;
> +break;
> +  

Re: [PATCH] qtest/npcm7xx_pwm-test: Fix memleak in pwm_qom_get

2021-01-15 Thread Hao Wu via
On Fri, Jan 15, 2021 at 9:17 AM Havard Skinnemoen 
wrote:

> +Hao Wu
>
> On Fri, Jan 15, 2021 at 1:15 AM Philippe Mathieu-Daudé 
> wrote:
> >
> > On 1/15/21 8:56 AM, Gan Qixin wrote:
> > > The pwm_qom_get function didn't free "response", which caused an
> indirect
> > > memory leak. So use qobject_unref() to fix it.
> > >
> > > ASAN shows memory leak stack:
> > >
> > > Indirect leak of 7416 byte(s) in 18000 object(s) allocated from:
> > > #0 0x7f96e2f79d4e in __interceptor_calloc
> (/lib64/libasan.so.5+0x112d4e)
> > > #1 0x7f96e2d98a50 in g_malloc0 (/lib64/libglib-2.0.so.0+0x55a50)
> > > #2 0x556313112180 in qdict_new ../qobject/qdict.c:30
> > > #3 0x556313115bca in parse_object ../qobject/json-parser.c:318
> > > #4 0x556313117810 in parse_value ../qobject/json-parser.c:546
> > > #5 0x556313117bda in json_parser_parse ../qobject/json-parser.c:580
> > > #6 0x55631310fe67 in json_message_process_token
> ../qobject/json-streamer.c:92
> > > #7 0x5563131210b7 in json_lexer_feed_char
> ../qobject/json-lexer.c:313
> > > #8 0x556313121662 in json_lexer_feed ../qobject/json-lexer.c:350
> > > #9 0x5563131101e9 in json_message_parser_feed
> ../qobject/json-streamer.c:121
> > > #10 0x5563130cb81e in qmp_fd_receive ../tests/qtest/libqtest.c:614
> > > #11 0x5563130cba2b in qtest_qmp_receive_dict
> ../tests/qtest/libqtest.c:636
> > > #12 0x5563130cb939 in qtest_qmp_receive
> ../tests/qtest/libqtest.c:624
> > > #13 0x5563130cbe0d in qtest_vqmp ../tests/qtest/libqtest.c:715
> > > #14 0x5563130cc40f in qtest_qmp ../tests/qtest/libqtest.c:756
> > > #15 0x5563130c5623 in pwm_qom_get
> ../tests/qtest/npcm7xx_pwm-test.c:180
> > > #16 0x5563130c595e in pwm_get_duty
> ../tests/qtest/npcm7xx_pwm-test.c:210
> > > #17 0x5563130c7529 in test_toggle
> ../tests/qtest/npcm7xx_pwm-test.c:447
> > >
> > > Reported-by: Euler Robot 
> > > Signed-off-by: Gan Qixin 
> > > ---
> > > Cc: Havard Skinnemoen 
> > > Cc: Tyrone Ting 
> > > Cc: Thomas Huth 
> > > Cc: Laurent Vivier 
> > > ---
> > >  tests/qtest/npcm7xx_pwm-test.c | 5 -
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > Reviewed-by: Philippe Mathieu-Daudé 
>
> Reviewed-by: Havard Skinnemoen 
>
Reviewed-by: Hao Wu 

Thank you for finding this out!

>
> Thanks!
>


Re: [RFC PATCH v0 1/1] target/ppc: Support for H_RPT_INVALIDATE hcall

2021-01-15 Thread Greg Kurz
On Fri, 15 Jan 2021 14:01:28 +0530
Bharata B Rao  wrote:

> On Wed, Jan 13, 2021 at 05:22:56PM +0100, Greg Kurz wrote:
> > Hi Bharata,
> > 
> > On Wed,  6 Jan 2021 14:29:10 +0530
> > Bharata B Rao  wrote:
> > 
> > > If KVM_CAP_RPT_INVALIDATE KVM capability is enabled, then
> > > 
> > > - indicate the availability of H_RPT_INVALIDATE hcall to the guest via
> > >   ibm,hypertas-functions property.
> > > - Enable the hcall
> > > 
> > > Both the above are done only if the new sPAPR machine capability
> > > cap-rpt-invalidate is set.
> > > 
> > > Note: The KVM implementation of the hcall has been posted for upstream
> > > review here:
> > > https://lore.kernel.org/linuxppc-dev/20210105090557.2150104-1-bhar...@linux.ibm.com/T/#t
> > > 
> > > Update to linux-headers/linux/kvm.h here is temporary, will be
> > > done via header updates once the kernel change is accepted upstream.
> > > 
> > > Signed-off-by: Bharata B Rao 
> > > ---
> > 
> > Patch looks mostly fine. A few remarks below.
> > 
> > >  hw/ppc/spapr.c|  7 ++
> > >  hw/ppc/spapr_caps.c   | 49 +++
> > >  include/hw/ppc/spapr.h|  8 +--
> > >  linux-headers/linux/kvm.h |  1 +
> > >  target/ppc/kvm.c  | 12 ++
> > >  target/ppc/kvm_ppc.h  | 11 +
> > >  6 files changed, 86 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > index 489cefcb81..0228083800 100644
> > > --- a/hw/ppc/spapr.c
> > > +++ b/hw/ppc/spapr.c
> > > @@ -890,6 +890,11 @@ static void spapr_dt_rtas(SpaprMachineState *spapr, 
> > > void *fdt)
> > >  add_str(hypertas, "hcall-copy");
> > >  add_str(hypertas, "hcall-debug");
> > >  add_str(hypertas, "hcall-vphn");
> > > +if (kvm_enabled() &&
> > 
> > You shouldn't check KVM here. The capability is enough to decide if we
> > should expose "hcall-rpt-invalidate" or not. FWIW we won't even reach
> > this code when running with anything but KVM.
> 
> Correct, the capability itself can be only for KVM case.
> 
> > 
> > > +(spapr_get_cap(spapr, SPAPR_CAP_RPT_INVALIDATE) == 
> > > SPAPR_CAP_ON)) {
> > > +add_str(hypertas, "hcall-rpt-invalidate");
> > > +}
> > > +
> > >  add_str(qemu_hypertas, "hcall-memop1");
> > >  
> > >  if (!kvm_enabled() || kvmppc_spapr_use_multitce()) {
> > > @@ -2021,6 +2026,7 @@ static const VMStateDescription vmstate_spapr = {
> > >  _spapr_cap_ccf_assist,
> > >  _spapr_cap_fwnmi,
> > >  _spapr_fwnmi,
> > > +_spapr_cap_rpt_invalidate,
> > >  NULL
> > >  }
> > >  };
> > > @@ -4478,6 +4484,7 @@ static void spapr_machine_class_init(ObjectClass 
> > > *oc, void *data)
> > >  smc->default_caps.caps[SPAPR_CAP_LARGE_DECREMENTER] = SPAPR_CAP_ON;
> > >  smc->default_caps.caps[SPAPR_CAP_CCF_ASSIST] = SPAPR_CAP_ON;
> > >  smc->default_caps.caps[SPAPR_CAP_FWNMI] = SPAPR_CAP_ON;
> > > +smc->default_caps.caps[SPAPR_CAP_RPT_INVALIDATE] = SPAPR_CAP_OFF;
> > 
> > Any reason for not enabling this for the default machine type and
> > disabling it for existing machine types only ?
> 
> If this capability is enabled, then
> 
> 1. First level guest (L1) can off-load the TLB invalidations to the
> new hcall if the platform has disabled LPCR[GTSE].
> 
> 2. Nested guest (L2) will switch to this new hcall rather than using
> the old H_TLB_INVALIDATE hcall.
> 
> Case 2 is optional and case 1 makes sense only if LPCR[GTSE]=off.

I don't think this is relevant, as the importance of each case can change,
e.g. nested is gaining momentum.

> Hence I thought keeping it off by default and expecting the
> user to turn it on only if required would be correct.
> 

If the feature is an improvement, even for what is considered a corner
case now, and it doesn't do harm to setups that won't use it, then it
should be enabled IMHO.

> Please note that turning this capability ON will result in the
> new hcall being exposed to the guest. I hope this is the right
> usage of spapr-caps?
> 

That's perfectly fine and this is why we should set it to ON
for the default machine type only.

> > > diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
> > > index 73ce2bc951..8e27f8421f 100644
> > > --- a/target/ppc/kvm_ppc.h
> > > +++ b/target/ppc/kvm_ppc.h
> > > @@ -24,6 +24,7 @@ void kvmppc_enable_logical_ci_hcalls(void);
> > >  void kvmppc_enable_set_mode_hcall(void);
> > >  void kvmppc_enable_clear_ref_mod_hcalls(void);
> > >  void kvmppc_enable_h_page_init(void);
> > > +void kvmppc_enable_h_rpt_invalidate(void);
> > >  void kvmppc_set_papr(PowerPCCPU *cpu);
> > >  int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr);
> > >  void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);
> > > @@ -72,6 +73,7 @@ bool kvmppc_has_cap_nested_kvm_hv(void);
> > >  int kvmppc_set_cap_nested_kvm_hv(int enable);
> > >  int kvmppc_get_cap_large_decr(void);
> > >  int kvmppc_enable_cap_large_decr(PowerPCCPU *cpu, int enable);
> > > +int 

Re: Windows installer builds apparently broken since October?

2021-01-15 Thread Paolo Bonzini

On 15/01/21 16:57, Peter Maydell wrote:

On Fri, 15 Jan 2021 at 15:52, Stefan Weil  wrote:

I forgot to mention that some of the problems with the Meson build also
occur on macOS with Homebrew: they always happen when a software package
requires special compiler flags to find its include files or libraries,
but the Meson build does not use the result from pkg-config for them.


Yeah -- we fixed that in commit 3eacf70bb5a83e4 for gnutls, which
is the main one homebrew was running into. Is Windows having
problems with other libs too?

Paolo: did we come up with a generic solution for this or
do we really have to add entries to dependency lists all
over the build system for every library we use?


I will work on a fix in Meson (basically not using -Wl,--whole-archive 
and instead linking in individual objects).  I also have a very ugly way 
to do the same in QEMU but I don't think it's worth pursuing it except 
for a proof of concept.


Paolo




Re: [RFC PATCH 0/5] hw/block/nvme: support multi-path for ctrl/ns

2021-01-15 Thread Keith Busch
On Fri, Jan 15, 2021 at 02:57:45PM +0100, Klaus Jensen wrote:
> 
> As you already mentioned, the problem I see with this approach is that
> we have separate namespaces attached to separate controllers. So we are
> faking it to the max and if I/O starts going through the other
> controller we end up on a namespace that is unrelated (different data).
> Havoc ensues.
> 
> My approach looks a lot like yours, but I hacked around this by adding
> extra 'ctrl-0', 'ctrl-1', ..., link-parameters to the namespace device,
> replacing the bus. This works well because the namespace then just
> registers with multiple controllers. But adding more parameters like
> that just isnt nice, so I've been trying to figure out how to allow a
> parameter to be specified multiple times, so we could just do more
> 'ctrl'-parameters.
> 
> Alas, since I started thinking about namespace sharing I have been
> regretting that I didn't reverse the bus-mechanic for namespace
> attachment. It would align better with the theory of operation if it was
> the controllers that attached to namespaces, and not the other way
> around. So what I would actually really prefer, is that we had multiple
> 'ns' link parameters on the controller device.

Would this work better if we introduce a new device in the nvme hierarchy:
the nvme-subsystem? You could attach multi-path namespaces and
controllers to that, and namespaces you don't want shared can attach
directly to controllers like they do today. You could also auto-assign
cntlid, and you wouldn't need to duplicate serial numbers in your
parameters.



[PATCH v4 01/10] iotests.py: Assume a couple of variables as given

2021-01-15 Thread Max Reitz
There are a couple of environment variables that we fetch with
os.environ.get() without supplying a default.  Clearly they are required
and expected to be set by the ./check script (as evidenced by
execute_setup_common(), which checks for test_dir and
qemu_default_machine to be set, and aborts if they are not).

Using .get() this way has the disadvantage of returning an Optional[str]
type, which mypy will complain about when tests just assume these values
to be str.

Use [] instead, which raises a KeyError for environment variables that
are not set.  When this exception is raised, catch it and move the abort
code from execute_setup_common() there.

Drop the 'assert iotests.sock_dir is not None' from iotest 300, because
that sort of thing is precisely what this patch wants to prevent.

Signed-off-by: Max Reitz 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---
 tests/qemu-iotests/300|  1 -
 tests/qemu-iotests/iotests.py | 26 +-
 2 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/tests/qemu-iotests/300 b/tests/qemu-iotests/300
index 5b75121b84..b864a21d5e 100755
--- a/tests/qemu-iotests/300
+++ b/tests/qemu-iotests/300
@@ -27,7 +27,6 @@ import qemu
 
 BlockBitmapMapping = List[Dict[str, Union[str, List[Dict[str, str]
 
-assert iotests.sock_dir is not None
 mig_sock = os.path.join(iotests.sock_dir, 'mig_sock')
 
 
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index dcdcd0387f..52facb8e04 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -75,12 +75,20 @@ qemu_opts = os.environ.get('QEMU_OPTIONS', 
'').strip().split(' ')
 
 imgfmt = os.environ.get('IMGFMT', 'raw')
 imgproto = os.environ.get('IMGPROTO', 'file')
-test_dir = os.environ.get('TEST_DIR')
-sock_dir = os.environ.get('SOCK_DIR')
 output_dir = os.environ.get('OUTPUT_DIR', '.')
-cachemode = os.environ.get('CACHEMODE')
-aiomode = os.environ.get('AIOMODE')
-qemu_default_machine = os.environ.get('QEMU_DEFAULT_MACHINE')
+
+try:
+test_dir = os.environ['TEST_DIR']
+sock_dir = os.environ['SOCK_DIR']
+cachemode = os.environ['CACHEMODE']
+aiomode = os.environ['AIOMODE']
+qemu_default_machine = os.environ['QEMU_DEFAULT_MACHINE']
+except KeyError:
+# We are using these variables as proxies to indicate that we're
+# not being run via "check". There may be other things set up by
+# "check" that individual test cases rely on.
+sys.stderr.write('Please run this test via the "check" script\n')
+sys.exit(os.EX_USAGE)
 
 socket_scm_helper = os.environ.get('SOCKET_SCM_HELPER', 'socket_scm_helper')
 
@@ -1294,14 +1302,6 @@ def execute_setup_common(supported_fmts: Sequence[str] = 
(),
 """
 # Note: Python 3.6 and pylint do not like 'Collection' so use 'Sequence'.
 
-# We are using TEST_DIR and QEMU_DEFAULT_MACHINE as proxies to
-# indicate that we're not being run via "check". There may be
-# other things set up by "check" that individual test cases rely
-# on.
-if test_dir is None or qemu_default_machine is None:
-sys.stderr.write('Please run this test via the "check" script\n')
-sys.exit(os.EX_USAGE)
-
 debug = '-d' in sys.argv
 if debug:
 sys.argv.remove('-d')
-- 
2.29.2




[PATCH v4 00/10] iotests: Fix 129 and expand 297’s reach

2021-01-15 Thread Max Reitz
Cover letters:
v1: https://lists.nongnu.org/archive/html/qemu-block/2021-01/msg00254.html
v2: https://lists.nongnu.org/archive/html/qemu-block/2021-01/msg00296.html
v3: https://lists.nongnu.org/archive/html/qemu-block/2021-01/msg00371.html

git:
https://github.com/XanClic/qemu.git fix-129-2-v4
https://git.xanclic.moe/XanClic/qemu.git fix-129-2-v4


Hi,

See the v1 cover letter above for the main point of this series (it’s
just that all patch indices are shifted up by two).


The main change in v2 is the extension of iotest 297 to run pylint and
mypy not only on iotests.py, but on every Python file in the
qemu-iotests/ directory that isn’t part of a skip list.

The main changes in v3 are that 297 is rewritten in Python, that patch 1
is added (which helps tests to pass mypy scrutiny without having to
assert that vital variables such as iotests.test_dir are not None), and
that patch 10 is added (because I was already modifying 300 in patch 1,
so I thought i might as well).


Change in v4 (from v3):
- Patch 2:
  - Fix flake8 complaints (PEP8 violations)
  - Modify only a copy of os.environ, and pass that down to pylint and
mypy (instead of accidentally modifying os.environ itself)
  - s/text=True/universal_newlines=True/
(@text was added in Python 3.7, but qemu requires only 3.6)

- Patch 6:
  - Fix flake8 complaints (PEP8 violations); kept R-bs, it’s just a
question of indentation

- Patch 9:
  - Mention modification to 297 in the commit message

- Patch 10:
  - Mention modification to 297 in the commit message
  - s/PYTHONPATH/sys.path/
  - Fix flake8 complaints (PEP8 violations)


git-backport-diff of v3 <-> v4:

Key:
[] : patches are identical
[] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/10:[] [--] 'iotests.py: Assume a couple of variables as given'
002/10:[0007] [FC] 'iotests/297: Rewrite in Python and extend reach'
003/10:[] [--] 'iotests: Move try_remove to iotests.py'
004/10:[] [--] 'iotests/129: Remove test images in tearDown()'
005/10:[] [--] 'iotests/129: Do not check @busy'
006/10:[0004] [FC] 'iotests/129: Use throttle node'
007/10:[] [-C] 'iotests/129: Actually test a commit job'
008/10:[] [--] 'iotests/129: Limit mirror job's buffer size'
009/10:[] [--] 'iotests/129: Clean up pylint and mypy complaints'
010/10:[0006] [FC] 'iotests/300: Clean up pylint and mypy complaints'


Max Reitz (10):
  iotests.py: Assume a couple of variables as given
  iotests/297: Rewrite in Python and extend reach
  iotests: Move try_remove to iotests.py
  iotests/129: Remove test images in tearDown()
  iotests/129: Do not check @busy
  iotests/129: Use throttle node
  iotests/129: Actually test a commit job
  iotests/129: Limit mirror job's buffer size
  iotests/129: Clean up pylint and mypy complaints
  iotests/300: Clean up pylint and mypy complaints

 tests/qemu-iotests/124|   8 +--
 tests/qemu-iotests/129|  72 +-
 tests/qemu-iotests/297| 110 +++---
 tests/qemu-iotests/297.out|   5 +-
 tests/qemu-iotests/300|  19 --
 tests/qemu-iotests/iotests.py |  37 ++--
 6 files changed, 169 insertions(+), 82 deletions(-)

-- 
2.29.2




[PATCH v4 06/10] iotests/129: Use throttle node

2021-01-15 Thread Max Reitz
Throttling on the BB has not affected block jobs in a while, so it is
possible that one of the jobs in 129 finishes before the VM is stopped.
We can fix that by running the job from a throttle node.

Signed-off-by: Max Reitz 
Reviewed-by: Eric Blake 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---
 tests/qemu-iotests/129 | 37 +
 1 file changed, 13 insertions(+), 24 deletions(-)

diff --git a/tests/qemu-iotests/129 b/tests/qemu-iotests/129
index dd23bb2e5a..d40e2db24e 100755
--- a/tests/qemu-iotests/129
+++ b/tests/qemu-iotests/129
@@ -32,20 +32,18 @@ class TestStopWithBlockJob(iotests.QMPTestCase):
 iotests.qemu_img('create', '-f', iotests.imgfmt, self.test_img,
  "-b", self.base_img, '-F', iotests.imgfmt)
 iotests.qemu_io('-f', iotests.imgfmt, '-c', 'write -P0x5d 1M 128M', 
self.test_img)
-self.vm = iotests.VM().add_drive(self.test_img)
+self.vm = iotests.VM()
+self.vm.add_object('throttle-group,id=tg0,x-bps-total=1024')
+
+source_drive = 'driver=throttle,' \
+   'throttle-group=tg0,' \
+   f'file.driver={iotests.imgfmt},' \
+   f'file.file.filename={self.test_img}'
+
+self.vm.add_drive(None, source_drive)
 self.vm.launch()
 
 def tearDown(self):
-params = {"device": "drive0",
-  "bps": 0,
-  "bps_rd": 0,
-  "bps_wr": 0,
-  "iops": 0,
-  "iops_rd": 0,
-  "iops_wr": 0,
- }
-result = self.vm.qmp("block_set_io_throttle", conv_keys=False,
- **params)
 self.vm.shutdown()
 for img in (self.test_img, self.target_img, self.base_img):
 iotests.try_remove(img)
@@ -53,33 +51,24 @@ class TestStopWithBlockJob(iotests.QMPTestCase):
 def do_test_stop(self, cmd, **args):
 """Test 'stop' while block job is running on a throttled drive.
 The 'stop' command shouldn't drain the job"""
-params = {"device": "drive0",
-  "bps": 1024,
-  "bps_rd": 0,
-  "bps_wr": 0,
-  "iops": 0,
-  "iops_rd": 0,
-  "iops_wr": 0,
- }
-result = self.vm.qmp("block_set_io_throttle", conv_keys=False,
- **params)
-self.assert_qmp(result, 'return', {})
 result = self.vm.qmp(cmd, **args)
 self.assert_qmp(result, 'return', {})
+
 result = self.vm.qmp("stop")
 self.assert_qmp(result, 'return', {})
 result = self.vm.qmp("query-block-jobs")
+
 self.assert_qmp(result, 'return[0]/status', 'running')
 self.assert_qmp(result, 'return[0]/ready', False)
 
 def test_drive_mirror(self):
 self.do_test_stop("drive-mirror", device="drive0",
-  target=self.target_img,
+  target=self.target_img, format=iotests.imgfmt,
   sync="full")
 
 def test_drive_backup(self):
 self.do_test_stop("drive-backup", device="drive0",
-  target=self.target_img,
+  target=self.target_img, format=iotests.imgfmt,
   sync="full")
 
 def test_block_commit(self):
-- 
2.29.2




[PATCH v4 02/10] iotests/297: Rewrite in Python and extend reach

2021-01-15 Thread Max Reitz
Instead of checking iotests.py only, check all Python files in the
qemu-iotests/ directory.  Of course, most of them do not pass, so there
is an extensive skip list for now.  (The only files that do pass are
209, 254, 283, and iotests.py.)

(Alternatively, we could have the opposite, i.e. an explicit list of
files that we do want to check, but I think it is better to check files
by default.)

Unless started in debug mode (./check -d), the output has no information
on which files are tested, so we will not have a problem e.g. with
backports, where some files may be missing when compared to upstream.

Besides the technical rewrite, some more things are changed:

- For the pylint invocation, PYTHONPATH is adjusted.  This mirrors
  setting MYPYPATH for mypy.

- Also, MYPYPATH is now derived from PYTHONPATH, so that we include
  paths set by the environment.  Maybe at some point we want to let the
  check script add '../../python/' to PYTHONPATH so that iotests.py does
  not need to do that.

- Passing --notes=FIXME,XXX to pylint suppresses warnings for TODO
  comments.  TODO is fine, we do not need 297 to complain about such
  comments.

- The "Success" line from mypy's output is suppressed, because (A) it
  does not add useful information, and (B) it would leak information
  about the files having been tested to the reference output, which we
  decidedly do not want.

Suggested-by: Vladimir Sementsov-Ogievskiy 
Signed-off-by: Max Reitz 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---
 tests/qemu-iotests/297 | 110 +
 tests/qemu-iotests/297.out |   5 +-
 2 files changed, 90 insertions(+), 25 deletions(-)

diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index 5c5420712b..fa9e2cac78 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -1,4 +1,4 @@
-#!/usr/bin/env bash
+#!/usr/bin/env python3
 #
 # Copyright (C) 2020 Red Hat, Inc.
 #
@@ -15,30 +15,96 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see .
 
-seq=$(basename $0)
-echo "QA output created by $seq"
+import os
+import re
+import shutil
+import subprocess
+import sys
 
-status=1   # failure is the default!
+import iotests
 
-# get standard environment
-. ./common.rc
 
-if ! type -p "pylint-3" > /dev/null; then
-_notrun "pylint-3 not found"
-fi
-if ! type -p "mypy" > /dev/null; then
-_notrun "mypy not found"
-fi
+# TODO: Empty this list!
+SKIP_FILES = (
+'030', '040', '041', '044', '045', '055', '056', '057', '065', '093',
+'096', '118', '124', '129', '132', '136', '139', '147', '148', '149',
+'151', '152', '155', '163', '165', '169', '194', '196', '199', '202',
+'203', '205', '206', '207', '208', '210', '211', '212', '213', '216',
+'218', '219', '222', '224', '228', '234', '235', '236', '237', '238',
+'240', '242', '245', '246', '248', '255', '256', '257', '258', '260',
+'262', '264', '266', '274', '277', '280', '281', '295', '296', '298',
+'299', '300', '302', '303', '304', '307',
+'nbd-fault-injector.py', 'qcow2.py', 'qcow2_format.py', 'qed.py'
+)
 
-pylint-3 --score=n iotests.py
 
-MYPYPATH=../../python/ mypy --warn-unused-configs --disallow-subclassing-any \
---disallow-any-generics --disallow-incomplete-defs \
---disallow-untyped-decorators --no-implicit-optional \
---warn-redundant-casts --warn-unused-ignores \
---no-implicit-reexport iotests.py
+def is_python_file(filename):
+if not os.path.isfile(filename):
+return False
 
-# success, all done
-echo "*** done"
-rm -f $seq.full
-status=0
+if filename.endswith('.py'):
+return True
+
+with open(filename) as f:
+try:
+first_line = f.readline()
+return re.match('^#!.*python', first_line) is not None
+except UnicodeDecodeError:  # Ignore binary files
+return False
+
+
+def run_linters():
+files = [filename for filename in (set(os.listdir('.')) - set(SKIP_FILES))
+ if is_python_file(filename)]
+
+iotests.logger.debug('Files to be checked:')
+iotests.logger.debug(', '.join(sorted(files)))
+
+print('=== pylint ===')
+sys.stdout.flush()
+
+# Todo notes are fine, but fixme's or xxx's should probably just be
+# fixed (in tests, at least)
+env = os.environ.copy()
+try:
+env['PYTHONPATH'] += ':../../python/'
+except KeyError:
+env['PYTHONPATH'] = '../../python/'
+subprocess.run(('pylint-3', '--score=n', '--notes=FIXME,XXX', *files),
+   env=env, check=False)
+
+print('=== mypy ===')
+sys.stdout.flush()
+
+# We have to call mypy separately for each file.  Otherwise, it
+# will interpret all given files as belonging together (i.e., they
+# may not both define the same classes, etc.; most notably, they
+# must not both define the __main__ module).
+env['MYPYPATH'] = env['PYTHONPATH']
+for filename in 

[PATCH v4 10/10] iotests/300: Clean up pylint and mypy complaints

2021-01-15 Thread Max Reitz
And consequentially drop it from 297's skip list.

Signed-off-by: Max Reitz 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---
 tests/qemu-iotests/297 |  2 +-
 tests/qemu-iotests/300 | 18 +++---
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index d2d9292839..ce7f85cfe0 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -33,7 +33,7 @@ SKIP_FILES = (
 '218', '219', '222', '224', '228', '234', '235', '236', '237', '238',
 '240', '242', '245', '246', '248', '255', '256', '257', '258', '260',
 '262', '264', '266', '274', '277', '280', '281', '295', '296', '298',
-'299', '300', '302', '303', '304', '307',
+'299', '302', '303', '304', '307',
 'nbd-fault-injector.py', 'qcow2.py', 'qcow2_format.py', 'qed.py'
 )
 
diff --git a/tests/qemu-iotests/300 b/tests/qemu-iotests/300
index b864a21d5e..4115f90578 100755
--- a/tests/qemu-iotests/300
+++ b/tests/qemu-iotests/300
@@ -22,7 +22,11 @@ import os
 import random
 import re
 from typing import Dict, List, Optional, Union
+
 import iotests
+
+# Import qemu after iotests.py has amended sys.path
+# pylint: disable=wrong-import-order
 import qemu
 
 BlockBitmapMapping = List[Dict[str, Union[str, List[Dict[str, str]
@@ -110,10 +114,14 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
 If @msg is None, check that there has not been any error.
 """
 self.vm_b.shutdown()
+
+log = self.vm_b.get_log()
+assert log is not None  # Loaded after shutdown
+
 if msg is None:
-self.assertNotIn('qemu-system-', self.vm_b.get_log())
+self.assertNotIn('qemu-system-', log)
 else:
-self.assertIn(msg, self.vm_b.get_log())
+self.assertIn(msg, log)
 
 @staticmethod
 def mapping(node_name: str, node_alias: str,
@@ -445,9 +453,13 @@ class 
TestBlockBitmapMappingErrors(TestDirtyBitmapMigration):
 
 # Check for the error in the source's log
 self.vm_a.shutdown()
+
+log = self.vm_a.get_log()
+assert log is not None  # Loaded after shutdown
+
 self.assertIn(f"Cannot migrate bitmap '{name}' on node "
   f"'{self.src_node_name}': Name is longer than 255 bytes",
-  self.vm_a.get_log())
+  log)
 
 # Expect abnormal shutdown of the destination VM because of
 # the failed migration
-- 
2.29.2




[PATCH v4 07/10] iotests/129: Actually test a commit job

2021-01-15 Thread Max Reitz
Before this patch, test_block_commit() performs an active commit, which
under the hood is a mirror job.  If we want to test various different
block jobs, we should perhaps run an actual commit job instead.

Doing so requires adding an overlay above the source node before the
commit is done (and then specifying the source node as the top node for
the commit job).

Signed-off-by: Max Reitz 
Reviewed-by: Eric Blake 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---
 tests/qemu-iotests/129 | 27 +--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/129 b/tests/qemu-iotests/129
index d40e2db24e..104be6dded 100755
--- a/tests/qemu-iotests/129
+++ b/tests/qemu-iotests/129
@@ -26,6 +26,7 @@ class TestStopWithBlockJob(iotests.QMPTestCase):
 test_img = os.path.join(iotests.test_dir, 'test.img')
 target_img = os.path.join(iotests.test_dir, 'target.img')
 base_img = os.path.join(iotests.test_dir, 'base.img')
+overlay_img = os.path.join(iotests.test_dir, 'overlay.img')
 
 def setUp(self):
 iotests.qemu_img('create', '-f', iotests.imgfmt, self.base_img, "1G")
@@ -36,6 +37,7 @@ class TestStopWithBlockJob(iotests.QMPTestCase):
 self.vm.add_object('throttle-group,id=tg0,x-bps-total=1024')
 
 source_drive = 'driver=throttle,' \
+   'node-name=source,' \
'throttle-group=tg0,' \
f'file.driver={iotests.imgfmt},' \
f'file.file.filename={self.test_img}'
@@ -45,7 +47,8 @@ class TestStopWithBlockJob(iotests.QMPTestCase):
 
 def tearDown(self):
 self.vm.shutdown()
-for img in (self.test_img, self.target_img, self.base_img):
+for img in (self.test_img, self.target_img, self.base_img,
+self.overlay_img):
 iotests.try_remove(img)
 
 def do_test_stop(self, cmd, **args):
@@ -72,7 +75,27 @@ class TestStopWithBlockJob(iotests.QMPTestCase):
   sync="full")
 
 def test_block_commit(self):
-self.do_test_stop("block-commit", device="drive0")
+# Add overlay above the source node so that we actually use a
+# commit job instead of a mirror job
+
+iotests.qemu_img('create', '-f', iotests.imgfmt, self.overlay_img,
+ '1G')
+
+result = self.vm.qmp('blockdev-add', **{
+ 'node-name': 'overlay',
+ 'driver': iotests.imgfmt,
+ 'file': {
+ 'driver': 'file',
+ 'filename': self.overlay_img
+ }
+ })
+self.assert_qmp(result, 'return', {})
+
+result = self.vm.qmp('blockdev-snapshot',
+ node='source', overlay='overlay')
+self.assert_qmp(result, 'return', {})
+
+self.do_test_stop('block-commit', device='drive0', top_node='source')
 
 if __name__ == '__main__':
 iotests.main(supported_fmts=["qcow2"],
-- 
2.29.2




[PATCH v4 04/10] iotests/129: Remove test images in tearDown()

2021-01-15 Thread Max Reitz
Signed-off-by: Max Reitz 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Eric Blake 
Reviewed-by: Willian Rampazzo 
---
 tests/qemu-iotests/129 | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/qemu-iotests/129 b/tests/qemu-iotests/129
index 0e13244d85..2fc65ada6a 100755
--- a/tests/qemu-iotests/129
+++ b/tests/qemu-iotests/129
@@ -47,6 +47,8 @@ class TestStopWithBlockJob(iotests.QMPTestCase):
 result = self.vm.qmp("block_set_io_throttle", conv_keys=False,
  **params)
 self.vm.shutdown()
+for img in (self.test_img, self.target_img, self.base_img):
+iotests.try_remove(img)
 
 def do_test_stop(self, cmd, **args):
 """Test 'stop' while block job is running on a throttled drive.
-- 
2.29.2




[PATCH v4 09/10] iotests/129: Clean up pylint and mypy complaints

2021-01-15 Thread Max Reitz
And consequentially drop it from 297's skip list.

Signed-off-by: Max Reitz 
Reviewed-by: Willian Rampazzo 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---
 tests/qemu-iotests/129 | 4 ++--
 tests/qemu-iotests/297 | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/129 b/tests/qemu-iotests/129
index 80a5db521b..9a56217bf8 100755
--- a/tests/qemu-iotests/129
+++ b/tests/qemu-iotests/129
@@ -20,7 +20,6 @@
 
 import os
 import iotests
-import time
 
 class TestStopWithBlockJob(iotests.QMPTestCase):
 test_img = os.path.join(iotests.test_dir, 'test.img')
@@ -32,7 +31,8 @@ class TestStopWithBlockJob(iotests.QMPTestCase):
 iotests.qemu_img('create', '-f', iotests.imgfmt, self.base_img, "1G")
 iotests.qemu_img('create', '-f', iotests.imgfmt, self.test_img,
  "-b", self.base_img, '-F', iotests.imgfmt)
-iotests.qemu_io('-f', iotests.imgfmt, '-c', 'write -P0x5d 1M 128M', 
self.test_img)
+iotests.qemu_io('-f', iotests.imgfmt, '-c', 'write -P0x5d 1M 128M',
+self.test_img)
 self.vm = iotests.VM()
 self.vm.add_object('throttle-group,id=tg0,x-bps-total=1024')
 
diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index fa9e2cac78..d2d9292839 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -27,7 +27,7 @@ import iotests
 # TODO: Empty this list!
 SKIP_FILES = (
 '030', '040', '041', '044', '045', '055', '056', '057', '065', '093',
-'096', '118', '124', '129', '132', '136', '139', '147', '148', '149',
+'096', '118', '124', '132', '136', '139', '147', '148', '149',
 '151', '152', '155', '163', '165', '169', '194', '196', '199', '202',
 '203', '205', '206', '207', '208', '210', '211', '212', '213', '216',
 '218', '219', '222', '224', '228', '234', '235', '236', '237', '238',
-- 
2.29.2




[PATCH v4 03/10] iotests: Move try_remove to iotests.py

2021-01-15 Thread Max Reitz
Signed-off-by: Max Reitz 
Reviewed-by: Eric Blake 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---
 tests/qemu-iotests/124|  8 +---
 tests/qemu-iotests/iotests.py | 11 +++
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
index 3705cbb6b3..e40eeb50b9 100755
--- a/tests/qemu-iotests/124
+++ b/tests/qemu-iotests/124
@@ -22,6 +22,7 @@
 
 import os
 import iotests
+from iotests import try_remove
 
 
 def io_write_patterns(img, patterns):
@@ -29,13 +30,6 @@ def io_write_patterns(img, patterns):
 iotests.qemu_io('-c', 'write -P%s %s %s' % pattern, img)
 
 
-def try_remove(img):
-try:
-os.remove(img)
-except OSError:
-pass
-
-
 def transaction_action(action, **kwargs):
 return {
 'type': action,
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 52facb8e04..a69b4cdc4e 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -523,12 +523,15 @@ class FilePath:
 return False
 
 
+def try_remove(img):
+try:
+os.remove(img)
+except OSError:
+pass
+
 def file_path_remover():
 for path in reversed(file_path_remover.paths):
-try:
-os.remove(path)
-except OSError:
-pass
+try_remove(path)
 
 
 def file_path(*names, base_dir=test_dir):
-- 
2.29.2




Re: [RFC PATCH 0/5] hw/block/nvme: support multi-path for ctrl/ns

2021-01-15 Thread Klaus Jensen
On Jan 15 09:35, Keith Busch wrote:
> On Fri, Jan 15, 2021 at 02:57:45PM +0100, Klaus Jensen wrote:
> > 
> > As you already mentioned, the problem I see with this approach is that
> > we have separate namespaces attached to separate controllers. So we are
> > faking it to the max and if I/O starts going through the other
> > controller we end up on a namespace that is unrelated (different data).
> > Havoc ensues.
> > 
> > My approach looks a lot like yours, but I hacked around this by adding
> > extra 'ctrl-0', 'ctrl-1', ..., link-parameters to the namespace device,
> > replacing the bus. This works well because the namespace then just
> > registers with multiple controllers. But adding more parameters like
> > that just isnt nice, so I've been trying to figure out how to allow a
> > parameter to be specified multiple times, so we could just do more
> > 'ctrl'-parameters.
> > 
> > Alas, since I started thinking about namespace sharing I have been
> > regretting that I didn't reverse the bus-mechanic for namespace
> > attachment. It would align better with the theory of operation if it was
> > the controllers that attached to namespaces, and not the other way
> > around. So what I would actually really prefer, is that we had multiple
> > 'ns' link parameters on the controller device.
> 
> Would this work better if we introduce a new device in the nvme hierarchy:
> the nvme-subsystem? You could attach multi-path namespaces and
> controllers to that, and namespaces you don't want shared can attach
> directly to controllers like they do today. You could also auto-assign
> cntlid, and you wouldn't need to duplicate serial numbers in your
> parameters.

I kinda POC'ed that, but I think I tried to make it work with a bus and
walking it and all kinds of fancy stuff.

I think it can just be a 'link' parameter, so something like:

  -device nvme-subsys,id=subsys0
  -device nvme,id=nvme0,subsys=subsys0
  -device nvme,id=nvme1,subsys=subsys0
  -device nvme-ns,id=shared-ns1,nsid=1,subsys=subsys0
  -device nvme-ns,id=private-ns2,nsid=2,bus=nvme0

When a controller "registers" with the subsystem it attaches to all
namespaces known, and when a namespace attaches, it attaches to all
controllers known. We can even add a 'detached' bool parameter to the
namespace and keep controllers from attaching, but allowing for later
attachment.

Cool!

Question: NSIDs must be the same on each controller for shared
namespaces, but can private namespaces "share" nsid across controllers
in the subsystem?  I don't think the spec is clear on that point.


signature.asc
Description: PGP signature


[PATCH v4 05/10] iotests/129: Do not check @busy

2021-01-15 Thread Max Reitz
@busy is false when the job is paused, which happens all the time
because that is how jobs yield (e.g. for mirror at least since commit
565ac01f8d3).

Back when 129 was added (2015), perhaps there was no better way of
checking whether the job was still actually running.  Now we have the
@status field (as of 58b295ba52c, i.e. 2018), which can give us exactly
that information.

Signed-off-by: Max Reitz 
Reviewed-by: Eric Blake 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---
 tests/qemu-iotests/129 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/129 b/tests/qemu-iotests/129
index 2fc65ada6a..dd23bb2e5a 100755
--- a/tests/qemu-iotests/129
+++ b/tests/qemu-iotests/129
@@ -69,7 +69,7 @@ class TestStopWithBlockJob(iotests.QMPTestCase):
 result = self.vm.qmp("stop")
 self.assert_qmp(result, 'return', {})
 result = self.vm.qmp("query-block-jobs")
-self.assert_qmp(result, 'return[0]/busy', True)
+self.assert_qmp(result, 'return[0]/status', 'running')
 self.assert_qmp(result, 'return[0]/ready', False)
 
 def test_drive_mirror(self):
-- 
2.29.2




Re: [RFC PATCH 0/5] hw/block/nvme: support multi-path for ctrl/ns

2021-01-15 Thread Keith Busch
On Fri, Jan 15, 2021 at 06:47:20PM +0100, Klaus Jensen wrote:
> Cool!

I thought so too :)
 
> Question: NSIDs must be the same on each controller for shared
> namespaces, but can private namespaces "share" nsid across controllers
> in the subsystem?  I don't think the spec is clear on that point.

The namespace NSID has to be unique within the entire subsystem, whether
they're shared or private.



[PATCH v4 08/10] iotests/129: Limit mirror job's buffer size

2021-01-15 Thread Max Reitz
Issuing 'stop' on the VM drains all nodes.  If the mirror job has many
large requests in flight, this may lead to significant I/O that looks a
bit like 'stop' would make the job try to complete (which is what 129
should verify not to happen).

We can limit the I/O in flight by limiting the buffer size, so mirror
will make very little progress during the 'stop' drain.

(We do not need to do anything about commit, which has a buffer size of
512 kB by default; or backup, which goes cluster by cluster.  Once we
have asynchronous requests for backup, that will change, but then we can
fine-tune the backup job to only perform a single request on a very
small chunk, too.)

Signed-off-by: Max Reitz 
Reviewed-by: Eric Blake 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Willian Rampazzo 
---
 tests/qemu-iotests/129 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/129 b/tests/qemu-iotests/129
index 104be6dded..80a5db521b 100755
--- a/tests/qemu-iotests/129
+++ b/tests/qemu-iotests/129
@@ -67,7 +67,7 @@ class TestStopWithBlockJob(iotests.QMPTestCase):
 def test_drive_mirror(self):
 self.do_test_stop("drive-mirror", device="drive0",
   target=self.target_img, format=iotests.imgfmt,
-  sync="full")
+  sync="full", buf_size=65536)
 
 def test_drive_backup(self):
 self.do_test_stop("drive-backup", device="drive0",
-- 
2.29.2




[RFC PATCH] meson: Only install ROMs when building system emulation binaries

2021-01-15 Thread Philippe Mathieu-Daudé
It is pointless to install ROM blobs for user emulation.

Signed-off-by: Philippe Mathieu-Daudé 
---
RFC because I think it would be better to make the 'blobs'
option a 'feature' instead of a boolean so we can set it
as 'auto' and then in that case we could do something

  blobs = have_system

because currently ./configure still displays:

Install blobs: YES

which is confusing.
---
 meson.build | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 954152c90fe..273b8e6baa9 100644
--- a/meson.build
+++ b/meson.build
@@ -2254,7 +2254,9 @@
 
 subdir('scripts')
 subdir('tools')
-subdir('pc-bios')
+if have_system
+  subdir('pc-bios')
+endif
 subdir('docs')
 subdir('tests')
 if gtk.found()
-- 
2.26.2




Re: Windows installer builds apparently broken since October?

2021-01-15 Thread Philippe Mathieu-Daudé
On 1/15/21 4:34 PM, Stefan Weil wrote:
> Am 15.01.21 um 15:01 schrieb Peter Maydell:
> 
>> I was just trying to see what updates the qemu.nsi file needed for
>> the merge-all-the-manuals-into-one-place change, and I discovered
>> that it's been broken since October when we removed the Changelog file:
>>
>> File: "/tmp/qemu-test/src\Changelog" -> no files found.
>> Usage: File [/nonfatal] [/a] ([/r] [/x filespec [...]] filespec [...] |
>>     /oname=outfile one_file_only)
>> Error in script "../src/qemu.nsi" on line 119 -- aborting creation
>> process
>>
>> You can reproduce that with:
>>
>> make -C my-build-dir docker-image-fedora V=1 NETWORK=1
>> make -C my-build-dir docker-test-mingw@fedora J=8 NETWORK=1
>>
>> This used to be in CI (patchew ran this config) but it clearly can't
>> be being CI'd any more, or we'd have noticed.
>>
>> Stefan, I see you have more recent installer binaries on your
>> site than that -- do you have some local patches for this?
> 
> 
> Hello Peter,
> 
> although I have some local fixes (available for example in
> https://github.com/stweil/qemu/) I am still struggling with 5.2.0.
> 
> One problem which was recently discussed on the list is the directory
> structure of the installation (especially the location for BIOS and
> similar files) which still needs changes (which als require updates for
> qemu.nsi). I'd prefer a similar hierarchical structure for both Linux
> and Windows (instead of a flat one which does not work with the current
> code).
> 
> Other problems are caused by the new QEMU build system in my special
> build context (Debian cross build with Cygwin packages).
> 
> A third challenge comes from users who would like to see new features
> like zstd or braille which up to now were missing in my binaries.
> 
> As I am quite busy with other things, too, I am afraid that it will take
> some more weeks until I can send a set of patches to fix the most urgent
> issues.

Cc'ing Yonggang Luo who might help.

> 
> Removing Changelog from qemu.nsi is easy, but not nearly sufficient:
> https://github.com/stweil/qemu/commit/923c93a663e4e51231f6ea389c19c0a960fa9f99.
> 
> 
> Kind regards,
> 
> Stefan
> 
> 
> 




Re: [PATCH v2 07/25] tests/docker: fix sorting in package lists

2021-01-15 Thread Wainer dos Santos Moschetta



On 1/14/21 10:02 AM, Daniel P. Berrangé wrote:

This will make diffs in later patches clearer.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Daniel P. Berrangé 
---
  tests/docker/dockerfiles/centos7.docker   |  4 ++--
  tests/docker/dockerfiles/fedora.docker|  4 ++--
  tests/docker/dockerfiles/opensuse-leap.docker | 16 
  tests/docker/dockerfiles/ubuntu1804.docker|  4 ++--
  tests/docker/dockerfiles/ubuntu2004.docker|  8 +---
  5 files changed, 19 insertions(+), 17 deletions(-)



Reviewed-by: Wainer dos Santos Moschetta 




diff --git a/tests/docker/dockerfiles/centos7.docker 
b/tests/docker/dockerfiles/centos7.docker
index 66d805dec3..b2a4719284 100644
--- a/tests/docker/dockerfiles/centos7.docker
+++ b/tests/docker/dockerfiles/centos7.docker
@@ -5,13 +5,14 @@ RUN yum -y update
  
  # Please keep this list sorted alphabetically

  ENV PACKAGES \
+SDL2-devel \
  bzip2 \
  bzip2-devel \
  ccache \
  csnappy-devel \
  dbus-daemon \
-gcc-c++ \
  gcc \
+gcc-c++ \
  gettext \
  git \
  glib2-devel \
@@ -32,7 +33,6 @@ ENV PACKAGES \
  perl-Test-Harness \
  pixman-devel \
  python3 \
-SDL2-devel \
  spice-glib-devel \
  spice-server-devel \
  tar \
diff --git a/tests/docker/dockerfiles/fedora.docker 
b/tests/docker/dockerfiles/fedora.docker
index d9b764aea2..03b88f1cfe 100644
--- a/tests/docker/dockerfiles/fedora.docker
+++ b/tests/docker/dockerfiles/fedora.docker
@@ -2,6 +2,7 @@ FROM registry.fedoraproject.org/fedora:32
  
  # Please keep this list sorted alphabetically

  ENV PACKAGES \
+SDL2-devel \
  bc \
  brlapi-devel \
  bzip2 \
@@ -74,10 +75,10 @@ ENV PACKAGES \
  mingw64-pixman \
  mingw64-pkg-config \
  mingw64-SDL2 \
-nmap-ncat \
  ncurses-devel \
  nettle-devel \
  ninja-build \
+nmap-ncat \
  numactl-devel \
  perl \
  perl-Test-Harness \
@@ -91,7 +92,6 @@ ENV PACKAGES \
  python3-sphinx \
  python3-virtualenv \
  rdma-core-devel \
-SDL2-devel \
  snappy-devel \
  sparse \
  spice-server-devel \
diff --git a/tests/docker/dockerfiles/opensuse-leap.docker 
b/tests/docker/dockerfiles/opensuse-leap.docker
index e7dc14bf99..ed194125a7 100644
--- a/tests/docker/dockerfiles/opensuse-leap.docker
+++ b/tests/docker/dockerfiles/opensuse-leap.docker
@@ -8,46 +8,46 @@ ENV PACKAGES \
  cyrus-sasl-devel \
  gcc \
  gcc-c++ \
-mkisofs \
  gettext-runtime \
  git \
  glib2-devel \
  glusterfs-devel \
-libgnutls-devel \
  gtk3-devel \
+libSDL2-devel \
+libSDL2_image-devel \
  libaio-devel \
  libattr-devel \
  libcap-ng-devel \
  libepoxy-devel \
  libfdt-devel \
+libgnutls-devel \
  libiscsi-devel \
  libjpeg8-devel \
+libnuma-devel \
+libpixman-1-0-devel \
  libpmem-devel \
  libpng16-devel \
  librbd-devel \
  libseccomp-devel \
+libspice-server-devel \
  libssh-devel \
  lzo-devel \
  make \
-libSDL2_image-devel \
+mkisofs \
  ncurses-devel \
  ninja \
-libnuma-devel \
  perl \
-libpixman-1-0-devel \
  python3-base \
  python3-virtualenv \
  rdma-core-devel \
-libSDL2-devel \
  snappy-devel \
-libspice-server-devel \
  systemd-devel \
  systemtap-sdt-devel \
  tar \
  usbredir-devel \
  virglrenderer-devel \
-xen-devel \
  vte-devel \
+xen-devel \
  zlib-devel
  ENV QEMU_CONFIGURE_OPTS --python=/usr/bin/python3.6
  
diff --git a/tests/docker/dockerfiles/ubuntu1804.docker b/tests/docker/dockerfiles/ubuntu1804.docker

index 3534111637..58a373e205 100644
--- a/tests/docker/dockerfiles/ubuntu1804.docker
+++ b/tests/docker/dockerfiles/ubuntu1804.docker
@@ -45,9 +45,9 @@ ENV PACKAGES \
  libxen-dev \
  libzstd-dev \
  make \
-python3-yaml \
-python3-sphinx \
  ninja-build \
+python3-sphinx \
+python3-yaml \
  sparse \
  xfslibs-dev
  RUN apt-get update && \
diff --git a/tests/docker/dockerfiles/ubuntu2004.docker 
b/tests/docker/dockerfiles/ubuntu2004.docker
index 552b57f903..2bb7e2ab1e 100644
--- a/tests/docker/dockerfiles/ubuntu2004.docker
+++ b/tests/docker/dockerfiles/ubuntu2004.docker
@@ -1,7 +1,9 @@
  FROM docker.io/library/ubuntu:20.04
-ENV PACKAGES flex bison \
+ENV PACKAGES \
+bison \
  ccache \
  clang-10\
+flex \
  gcc \
  genisoimage \
  gettext \
@@ -60,8 +62,8 @@ ENV PACKAGES flex bison \
  sparse \
  tesseract-ocr \
  tesseract-ocr-eng \
-xfslibs-dev\
-vim
+vim \
+xfslibs-dev
  RUN apt-get update && \
  DEBIAN_FRONTEND=noninteractive apt-get -y install $PACKAGES
  RUN dpkg -l $PACKAGES | sort > /packages.txt





Re: About 'qemu-security' list subscription process

2021-01-15 Thread Daniel P . Berrangé
On Thu, Jan 14, 2021 at 07:33:32PM +0530, P J P wrote:
>   Hello,
> 
> * We have received quite a few subscription requests for the 'qemu-security'
>   list in the last few weeks. Majority of them are rejected because we could
>   not identify the user from merely their email-id.
> 
> * I have requested them to send a subscription request email with a 'Self
>   Introduction' to the list.
> 
> * However, some of the subscribers are familiar from the
>   qemu-devel/oss-security mailing lists. And some are corporate emails like
>   
> 
> * One of the request is pending (3+) votes/acks for OR against member
>   subscription.
> 
> How do we handle these requests?

I believe we want to keep the membership of qemu-security reasonably
small. Primarily people who can commit to helping with the initial
triage to identify which specific subsystem maintainers to pull in.
In addition major consumers of QEMU with whom we need to coordinate
choice of disclosure date for embargoed images.

There is obviously a danger to the project if we mistakenly allow
membership from someone who is not acting in interests in the QEMU
project, so I think the bar needs to be reasonably high. IOW ideally
there should be some web of trust whereby some existing member(s)
knows the person/entity who is requesting acces. Other cases would
have to be evaluated case-by-case basis.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|




Re: [PATCH v19 08/20] io: add qio_channel_readv_full_all_eof & qio_channel_readv_full_all helpers

2021-01-15 Thread Jag Raman



> On Jan 15, 2021, at 4:20 AM, Stefan Hajnoczi  wrote:
> 
> On Thu, Jan 14, 2021 at 01:24:37PM -0500, Jag Raman wrote:
>> 
>> 
>>> On Jan 14, 2021, at 1:00 PM, Daniel P. Berrangé  wrote:
>>> 
>>> On Thu, Jan 14, 2021 at 12:55:58PM -0500, Jag Raman wrote:
 
 
> On Jan 14, 2021, at 11:27 AM, Stefan Hajnoczi  wrote:
> 
> On Thu, Jan 14, 2021 at 10:40:03AM -0500, Jagannathan Raman wrote:
>> +int qio_channel_readv_full_all(QIOChannel *ioc,
>> +   const struct iovec *iov,
>> +   size_t niov,
>> +   int **fds, size_t *nfds,
>> +   Error **errp)
>> {
>> -int ret = qio_channel_readv_all_eof(ioc, iov, niov, errp);
>> +int ret = qio_channel_readv_full_all_eof(ioc, iov, niov, fds, nfds, 
>> errp);
>> 
>>   if (ret == 0) {
>> -ret = -1;
>>   error_setg(errp,
>>  "Unexpected end-of-file before all bytes were read");
> 
> qio_channel_readv_full_all_eof() can read file descriptors but no data
> and return 0.
> 
> Here that case is converted into an error and the file descriptors
> aren't closed, freed, and fds/nfds isn't cleared.
 
 That’s a valid point. I’m wondering if the fix for this case should be in
 qio_channel_readv_full_all_eof(), instead of here.
 
 qio_channel_readv_full_all_eof() should probably return error (-1) if the
 amount of data read does not match iov_size(). If the caller is only 
 expecting
 to read fds, and not any data, it would indicate that by setting iov to 
 NULL
 and/or setting niov=0. If the caller is setting these parameters, it means 
 it is
 expecting data.Does that sound good?
>>> 
>>> The API spec for the existing _eof() methods says:
>>> 
>>> * The function will wait for all requested data
>>> * to be read, yielding from the current coroutine
>>> * if required.
>>> *
>>> * If end-of-file occurs before any data is read,
>>> * no error is reported; otherwise, if it occurs
>>> * before all requested data has been read, an error
>>> * will be reported.
>>> 
>>> 
>>> IOW, return '0' is *only* valid if we've not read anything. I consider
>>> file descriptors to be something.
>>> 
>>> IOW, qio_channel_readv_full_all_eof must only return 0, if it didn't
>>> read any data and also didn't receive any file descriptors. So yeah,
>>> we must return -1 in the scenario Stefan describes
>> 
>> That makes sense to me. Reading “fds" is something, which is different
>> from our previous understanding. I thought data only meant iov, and not fds.
>> 
>> So the return values for qio_channel_readv_full_all_eof() would be:
>>  - ‘0’ only if EOF is reached without reading any fds and data.
>>  - ‘1’ if all data that the caller expects are read (even if the caller reads
>>fds exclusively, without any iovs)
>>  - ‘-1’ otherwise, considered as error
>> 
>> qio_channel_readv_full_all() would return:
>>  - ‘0’ if all the data that caller expects are read
>>  - ‘-1’ otherwise, considered as error
>> 
>> Hey Stefan,
>> 
>>Does this sound good to you?
> 
> The while (nlocal_iov > 0) loop only runs if the caller has requested to
> read at least some data, so the fds-only case doesn't work yet.
> 
> This suggests that no current QEMU code relies on the fds-only case.
> Therefore you could change the doc comment to clarify this instead of
> adding support for this case to the code.
> 
> But if you would to fully support the fds-only case that would be even
> better.
> 
> Stefan

We are working on sending the next revision out. We could handle the
fds-only case by altering the while loop condition to be:
((nlocal_iov > 0) || local_fds)

For reference, we would need to handle the following cases:
len < 0; !partial, !*nfds   => ret = -1;
len = 0; !partial, !*nfds   => ret = 0;
len < 0; partial, !*nfds=> ret = -1; errmsg;
len = 0; partial, !*nfds=> ret = -1; errmsg;
len < 0; partial, *nfds => ret = -1; errmsg, clearfds
len < 0; !partial, *nfds=> ret = -1; errmsg, clearfds
len = 0; partial, *nfds => ret = -1; errmsg, clearfds
len = 0; !partial, *nfds=> ret = -1; errmsg, clearfds
len = 0; !niov; (nfds && *nfds) => ret = 1 /* fds-only */
len > 0 => ret 1

Thank you!
--
Jag


RE: [for-6.0 v5 11/13] spapr: PEF: prevent migration

2021-01-15 Thread Ram Pai
On Thu, Jan 14, 2021 at 10:36:43AM +, Dr. David Alan Gilbert wrote:
> * Christian Borntraeger (borntrae...@de.ibm.com) wrote:
> > 
> > 
> > On 13.01.21 13:42, Dr. David Alan Gilbert wrote:
> > > * Cornelia Huck (coh...@redhat.com) wrote:
> > >> On Tue, 5 Jan 2021 12:41:25 -0800
> > >> Ram Pai  wrote:
> > >>
> > >>> On Tue, Jan 05, 2021 at 11:56:14AM +0100, Halil Pasic wrote:
> >  On Mon, 4 Jan 2021 10:40:26 -0800
> >  Ram Pai  wrote:
> > >>
> > > The main difference between my proposal and the other proposal is...
> > >
> > >   In my proposal the guest makes the compatibility decision and acts
> > >   accordingly.  In the other proposal QEMU makes the compatibility
> > >   decision and acts accordingly. I argue that QEMU cannot make a good
> > >   compatibility decision, because it wont know in advance, if the 
> > > guest
> > >   will or will-not switch-to-secure.
> > >   
> > 
> >  You have a point there when you say that QEMU does not know in advance,
> >  if the guest will or will-not switch-to-secure. I made that argument
> >  regarding VIRTIO_F_ACCESS_PLATFORM (iommu_platform) myself. My idea
> >  was to flip that property on demand when the conversion occurs. David
> >  explained to me that this is not possible for ppc, and that having the
> >  "securable-guest-memory" property (or whatever the name will be)
> >  specified is a strong indication, that the VM is intended to be used as
> >  a secure VM (thus it is OK to hurt the case where the guest does not
> >  try to transition). That argument applies here as well.  
> > >>>
> > >>> As suggested by Cornelia Huck, what if QEMU disabled the
> > >>> "securable-guest-memory" property if 'must-support-migrate' is enabled?
> > >>> Offcourse; this has to be done with a big fat warning stating
> > >>> "secure-guest-memory" feature is disabled on the machine.
> > >>> Doing so, will continue to support guest that do not try to transition.
> > >>> Guest that try to transition will fail and terminate themselves.
> > >>
> > >> Just to recap the s390x situation:
> > >>
> > >> - We currently offer a cpu feature that indicates secure execution to
> > >>   be available to the guest if the host supports it.
> > >> - When we introduce the secure object, we still need to support
> > >>   previous configurations and continue to offer the cpu feature, even
> > >>   if the secure object is not specified.
> > >> - As migration is currently not supported for secured guests, we add a
> > >>   blocker once the guest actually transitions. That means that
> > >>   transition fails if --only-migratable was specified on the command
> > >>   line. (Guests not transitioning will obviously not notice anything.)
> > >> - With the secure object, we will already fail starting QEMU if
> > >>   --only-migratable was specified.
> > >>
> > >> My suggestion is now that we don't even offer the cpu feature if
> > >> --only-migratable has been specified. For a guest that does not want to
> > >> transition to secure mode, nothing changes; a guest that wants to
> > >> transition to secure mode will notice that the feature is not available
> > >> and fail appropriately (or ultimately, when the ultravisor call fails).
> > >> We'd still fail starting QEMU for the secure object + --only-migratable
> > >> combination.
> > >>
> > >> Does that make sense?
> > > 
> > > It's a little unusual; I don't think we have any other cases where
> > > --only-migratable changes the behaviour; I think it normally only stops
> > > you doing something that would have made it unmigratable or causes
> > > an operation that would make it unmigratable to fail.
> > 
> > I would like to NOT block this feature with --only-migrateable. A guest
> > can startup unprotected (and then is is migrateable). the migration blocker
> > is really a dynamic aspect during runtime. 
> 
> But the point of --only-migratable is to turn things that would have
> blocked migration into failures, so that a VM started with
> --only-migratable is *always* migratable.

I believe, the proposed behavior, does follow the above rule. The
VM started with --only-migratable will always be migratable. Any
behavior; in the guest, to the contrary will disallow the behavior or
terminate the guest, but will never let the VM transition to a
non-migratable state.


RP



Re: [PATCH v4 10/10] iotests/300: Clean up pylint and mypy complaints

2021-01-15 Thread Willian Rampazzo
On Fri, Jan 15, 2021 at 2:43 PM Max Reitz  wrote:
>
> And consequentially drop it from 297's skip list.
>
> Signed-off-by: Max Reitz 
> Reviewed-by: Vladimir Sementsov-Ogievskiy 
> ---
>  tests/qemu-iotests/297 |  2 +-
>  tests/qemu-iotests/300 | 18 +++---
>  2 files changed, 16 insertions(+), 4 deletions(-)
>

Reviewed-by: Willian Rampazzo 




Re: [PATCH v4 07/10] iotests/129: Actually test a commit job

2021-01-15 Thread Willian Rampazzo
On Fri, Jan 15, 2021 at 2:43 PM Max Reitz  wrote:
>
> Before this patch, test_block_commit() performs an active commit, which
> under the hood is a mirror job.  If we want to test various different
> block jobs, we should perhaps run an actual commit job instead.
>
> Doing so requires adding an overlay above the source node before the
> commit is done (and then specifying the source node as the top node for
> the commit job).
>
> Signed-off-by: Max Reitz 
> Reviewed-by: Eric Blake 
> Reviewed-by: Vladimir Sementsov-Ogievskiy 
> ---
>  tests/qemu-iotests/129 | 27 +--
>  1 file changed, 25 insertions(+), 2 deletions(-)

Reviewed-by: Willian Rampazzo 




Re: [PATCH v4 06/10] iotests/129: Use throttle node

2021-01-15 Thread Willian Rampazzo
On Fri, Jan 15, 2021 at 2:43 PM Max Reitz  wrote:
>
> Throttling on the BB has not affected block jobs in a while, so it is
> possible that one of the jobs in 129 finishes before the VM is stopped.
> We can fix that by running the job from a throttle node.
>
> Signed-off-by: Max Reitz 
> Reviewed-by: Eric Blake 
> Reviewed-by: Vladimir Sementsov-Ogievskiy 
> ---
>  tests/qemu-iotests/129 | 37 +
>  1 file changed, 13 insertions(+), 24 deletions(-)

Reviewed-by: Willian Rampazzo 




Re: [PATCH v4 05/10] iotests/129: Do not check @busy

2021-01-15 Thread Willian Rampazzo
On Fri, Jan 15, 2021 at 2:43 PM Max Reitz  wrote:
>
> @busy is false when the job is paused, which happens all the time
> because that is how jobs yield (e.g. for mirror at least since commit
> 565ac01f8d3).
>
> Back when 129 was added (2015), perhaps there was no better way of
> checking whether the job was still actually running.  Now we have the
> @status field (as of 58b295ba52c, i.e. 2018), which can give us exactly
> that information.
>
> Signed-off-by: Max Reitz 
> Reviewed-by: Eric Blake 
> Reviewed-by: Vladimir Sementsov-Ogievskiy 
> ---
>  tests/qemu-iotests/129 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Reviewed-by: Willian Rampazzo 




Re: [PATCH v4 03/10] iotests: Move try_remove to iotests.py

2021-01-15 Thread Willian Rampazzo
On Fri, Jan 15, 2021 at 2:43 PM Max Reitz  wrote:
>
> Signed-off-by: Max Reitz 
> Reviewed-by: Eric Blake 
> Reviewed-by: Vladimir Sementsov-Ogievskiy 
> ---
>  tests/qemu-iotests/124|  8 +---
>  tests/qemu-iotests/iotests.py | 11 +++
>  2 files changed, 8 insertions(+), 11 deletions(-)
>

Reviewed-by: Willian Rampazzo 




Re: [PATCH 03/15] arc: Opcode definitions table

2021-01-15 Thread Cupertino Miranda
Hi Richard,

Sorry to take so long to get through the changes after your review.
I am still going through the improving process and waiting for some 
internal company approval to publish the generator of the TCG 
instruction definitions, as we have discussed.

Nevertheless, there are some questions. I will address them near the 
proper places and through the different patches.

Once again, thank you very much for your reviews. ;-)

On 12/1/20 8:22 PM, Richard Henderson wrote:
> On 11/11/20 10:17 AM, cupertinomira...@gmail.com wrote:
>> From: Claudiu Zissulescu 
>>
>> Signed-off-by: Claudiu Zissulescu 
>> ---
>>   target/arc/opcodes.def | 19976 +++
>>   1 file changed, 19976 insertions(+)
>>   create mode 100644 target/arc/opcodes.def
> 
> OMG.  20k lines.
> 
> I assume this is gnu binutils opcodes/arc-tbl.h?
> 
> You are the contributor there, so a re-license is fine.  It would be good to
> document the upstream location and revision, against some future re-sync.
> 
> That said, this format is less than ideal:
> 
>> +/* abs<.f> b,c 00100bbb0010FBBBCC001001.  */
>> +{ "abs", 0x202F0009, 0xF8FF003F, ARC_OPCODE_ARC600 | ARC_OPCODE_ARC700 | 
>> ARC_OPCODE_ARCv2EM | ARC_OPCODE_ARCv2HS, ARITH, NONE, { OPERAND_RB, 
>> OPERAND_RC }, { C_F }},
> 
> You've got the same information in two places
> (00100bbb0010FBBBCC001001) vs (0x202F0009, 0xF8FF003F, OPERAND_*).
> Moreover, "abs" as a string is not especially useful, and means that you have
> to deal with strings in the translator instead of C symbols or enumerators.
> 
> It would be relatively easy to generate a decodetree file from this input,
> which would simplify the translator.
> 
> At a bare minimum strip the quotes and wrap in a macro so that you can (1)
> define an enumerator and (2) put the entries into an array indexed by the
> enumerator.
> 

As you know, we reused the code from binutils to implement the decoder.
In that sense, we kindly request to allow us to do it through binutils 
development flow later on. We will change the tables in binutils
and those changes will also be mirrored to QEMU.
Is this Ok ?




Re: [PATCH 06/15] arc: TCG instruction definitions

2021-01-15 Thread Cupertino Miranda
>> +
>> +assert(ctx->insn.limm_p == 0 && !in_delay_slot);
>> +
>> +if (ctx->insn.limm_p == 0 && !in_delay_slot) {
>> +in_delay_slot = true;
>> +uint32_t cpc = ctx->cpc;
>> +uint32_t pcl = ctx->pcl;
>> +insn_t insn = ctx->insn;
>> +
>> +ctx->cpc = ctx->npc;
>> +ctx->pcl = ctx->cpc & 0xfffc;
>> +
>> +++ctx->ds;
>> +
>> +TCGLabel *do_not_set_bta_and_de = gen_new_label();
>> +tcg_gen_brcondi_i32(TCG_COND_NE, take_branch, 1, 
>> do_not_set_bta_and_de);
>> +/*
>> + * In case an exception should be raised during the execution
>> + * of delay slot, bta value is used to set erbta.
>> + */
>> +tcg_gen_mov_tl(cpu_bta, bta);
>> +/* We are in a delay slot */
>> +tcg_gen_mov_tl(cpu_DEf, take_branch);
>> +gen_set_label(do_not_set_bta_and_de);
>> +
>> +tcg_gen_movi_tl(cpu_is_delay_slot_instruction, 1);
>> +
>> +/* Set the pc to the next pc */
>> +tcg_gen_movi_tl(cpu_pc, ctx->npc);
>> +/* Necessary for the likely call to restore_state_to_opc() */
>> +tcg_gen_insn_start(ctx->npc);
> 
> Wow, this looks wrong.
> 
> It doesn't work with icount or single-stepping.  You need to be able to begin 
> a
> TB at any instruction, including a delay slot.
> 
> You should have a look at some of the other targets that handle this, e.g.
> openrisc or sparc.  Since you've got NPC already, for looping, sparc is
> probably the better match.
> 
> There should be no reason why you'd need to emit insn_start outside of
> arc_tr_insn_start.
> 

We are also able to start TB at any instruction, that is not what is 
being validated here. It is just verifying if a delayslot instruction 
does not have the delayslot flag set, and that the delayslot instruction 
does not have a limm.

The way we encode delayslot instructions is a little peculiar. When some 
instruction defines a delayslot, it calls this function and the 
instruction is decoded inline.
As far as I remember the change of instruction context, with 
tcg_gen_insns_start, allowed us to properly make gdb jump from branch 
instruction to delay slot instruction and then back.

The asser was used to guarantee that those conditions where never broken 
internally. The condition becomes irrelevant.


>> + ***
>> + * Statically inferred return function *
>> + ***
>> + */
>> +
>> +TCGv arc_gen_next_reg(const DisasCtxt *ctx, TCGv reg)
>> +{
>> +int i;
>> +for (i = 0; i < 64; i += 2) {
>> +if (reg == cpu_r[i]) {
>> +return cpu_r[i + 1];
>> +}
>> +}
>> +/* Check if REG is an odd register. */
>> +for (i = 1; i < 64; i += 2) {
>> +/* If so, that is unsanctioned. */
>> +if (reg == cpu_r[i]) {
>> +arc_gen_excp(ctx, EXCP_INST_ERROR, 0, 0);
>> +return NULL;
>> +}
>> +}
> 
> Wow, really?  Can't you grab this directly from the operand decoding?  Where
> surely we have already ensured that the relevant regnos are not odd.

Indeed, we can grab it from operand decoding. Please allow us to do this 
change once we rework binutils code.


Re: [PATCH 04/15] arc: TCG and decoder glue code and helpers

2021-01-15 Thread Cupertino Miranda
>> +void QEMU_NORETURN helper_halt(CPUARCState *env, uint32_t npc)
>> +{
>> +CPUState *cs = env_cpu(env);
>> +if (env->stat.Uf) {
>> +cs->exception_index = EXCP_PRIVILEGEV;
>> +env->causecode = 0;
>> +env->param = 0;
>> + /* Restore PC such that we point at the faulty instruction.  */
>> +env->eret = env->pc;
> 
> Any reason not to handle Uf at translate time?  Or at least create a single
> helper function for that here.  But it seems like translate will have to do a
> lot of priv checking anyway and will already have that handy.

Since we needed a helper anyway to deal with causecode and param, we 
thought it would be reasonable to do all in the helper.
We did not made a TCG access for causecode and param enviroment values.

> 
>> +void helper_enter(CPUARCState *env, uint32_t u6)
>> +{
>> +/* nothing to do? then bye-bye! */
>> +if (!u6) {
>> +return;
>> +}
>> +
>> +uint8_t regs   = u6 & 0x0f; /* u[3:0] determines registers to save 
>> */
>> +boolsave_fp= u6 & 0x10; /* u[4] indicates if fp must be saved  
>> */
>> +boolsave_blink = u6 & 0x20; /* u[5] indicates saving of blink  
>> */
>> +uint8_t stack_size = 4 * (regs + save_fp + save_blink);
>> +
>> +/* number of regs to be saved must be sane */
>> +check_enter_leave_nr_regs(env, regs, GETPC());
> 
> Both of these checks could be translate time.
> 
>> +/* this cannot be executed in a delay/execution slot */
>> +check_delay_or_execution_slot(env, GETPC());
> 
> As could this.
> 
>> +/* stack must be a multiple of 4 (32 bit aligned) */
>> +check_addr_is_word_aligned(env, CPU_SP(env) - stack_size, GETPC());
>> +
>> +uint32_t tmp_sp = CPU_SP(env);
>> +
>> +if (save_fp) {
>> +tmp_sp -= 4;
>> +cpu_stl_data(env, tmp_sp, CPU_FP(env));
>> +}
> 
> And what if these stores raise an exception?  I doubt you're going to get an
> exception at the correct pc.
> 
>> +void helper_leave(CPUARCState *env, uint32_t u7)
> 
> Similarly.  I think that both of these could be implemented entirely in
> translate, which is what
> 
>> +bool restore_fp= u7 & 0x10; /* u[4] indicates if fp must be saved  
>> */
>> +bool restore_blink = u7 & 0x20; /* u[5] indicates saving of blink  
>> */
>> +bool jump_to_blink = u7 & 0x40; /* u[6] should we jump to blink?   
>> */
> 
> these bits strongly imply.
> 

For lack of knowing better, it is unclear to me where to draw the line 
when choosing between a translate time (tcg) or helper implementation.
Your suggestions for carry/overflow computation are sharp and we should 
have never used an helper, however I wonder what would be the benefit of 
implementing enter and leave through TCG.

We have dealt with those exception issues by just changing SP in the end 
of the instruction implementation, when no exceptions can happen.

As far as I understand when an exception happens in the middle of the 
helper or even on a TCG implementation, it jumps out of that TB 
execution to deal with the exception. On rtie instead of it returning to 
the same tcg_ld or tcg_st where it actually triggered the exception it 
will re-decode the same instruction which triggered the exception, and 
re-attempts to execute it.
Is that the case in current TCG implementation, or did it improved and 
it is now able to return to previous execution flow (i.e translation 
block) ?


Re: [PATCH v2 08/25] tests/docker: fix mistakes in centos package lists

2021-01-15 Thread Wainer dos Santos Moschetta



On 1/14/21 10:02 AM, Daniel P. Berrangé wrote:

dbus-daemon doesn't exist in centos7, it is part of dbus.

snappy is used by QEMU, not csnappy.

mesa-libEGL-devel is not used in QEMU at all, but mesa-libgbm-devel is.

vte291-devel is required for GTK3, not vte-devel.

spice-glib-devel is not use in QEMU at all, but spice-protocol is.

librdmacm-devel is a virtual provides for compat, the actual package
used is rdma-core-devel.

There is no need to specifically refer to python36, we can just
use python3 as in other distros.

Signed-off-by: Daniel P. Berrangé 
---
  tests/docker/dockerfiles/centos7.docker | 11 +--
  tests/docker/dockerfiles/centos8.docker | 10 +-
  2 files changed, 10 insertions(+), 11 deletions(-)



Reviewed-by: Wainer dos Santos Moschetta 




diff --git a/tests/docker/dockerfiles/centos7.docker 
b/tests/docker/dockerfiles/centos7.docker
index b2a4719284..1eb3455144 100644
--- a/tests/docker/dockerfiles/centos7.docker
+++ b/tests/docker/dockerfiles/centos7.docker
@@ -9,8 +9,7 @@ ENV PACKAGES \
  bzip2 \
  bzip2-devel \
  ccache \
-csnappy-devel \
-dbus-daemon \
+dbus \
  gcc \
  gcc-c++ \
  gettext \
@@ -22,21 +21,21 @@ ENV PACKAGES \
  libepoxy-devel \
  libfdt-devel \
  libgcrypt-devel \
-librdmacm-devel \
  libzstd-devel \
  lzo-devel \
  make \
-mesa-libEGL-devel \
  mesa-libgbm-devel \
  nettle-devel \
  ninja-build \
  perl-Test-Harness \
  pixman-devel \
  python3 \
-spice-glib-devel \
+rdma-core-devel \
+snappy-devel \
+spice-protocol \
  spice-server-devel \
  tar \
-vte-devel \
+vte291-devel \
  xen-devel \
  zlib-devel
  RUN yum install -y $PACKAGES
diff --git a/tests/docker/dockerfiles/centos8.docker 
b/tests/docker/dockerfiles/centos8.docker
index a95350466a..b64ee7071d 100644
--- a/tests/docker/dockerfiles/centos8.docker
+++ b/tests/docker/dockerfiles/centos8.docker
@@ -19,16 +19,16 @@ ENV PACKAGES \
  libgcrypt-devel \
  lzo-devel \
  make \
-mesa-libEGL-devel \
-nmap-ncat \
+mesa-libgbm-devel \
  nettle-devel \
  ninja-build \
+nmap-ncat \
  perl-Test-Harness \
  pixman-devel \
-python36 \
+python3 \
  rdma-core-devel \
-spice-glib-devel \
-spice-server \
+spice-protocol \
+spice-server-devel \
  tar \
  zlib-devel
  





Re: [PATCH 05/15] arc: TCG instruction generator and hand-definitions

2021-01-15 Thread Cupertino Miranda
> On 11/11/20 10:17 AM, cupertinomira...@gmail.com wrote:
>> +/*
>> + * The macro to add boiler plate code for conditional execution.
>> + * It will add tcg_gen codes only if there is a condition to
>> + * be checked (ctx->insn.cc != 0). This macro assumes that there
>> + * is a "ctx" variable of type "DisasCtxt *" in context. Remember
>> + * to pair it with CC_EPILOGUE macro.
>> + */
>> +#define CC_PROLOGUE   \
>> +  TCGv cc = tcg_temp_local_new(); \
>> +  TCGLabel *done = gen_new_label();   \
>> +  do {\
>> +if (ctx->insn.cc) {   \
>> +arc_gen_verifyCCFlag(ctx, cc);\
>> +tcg_gen_brcondi_tl(TCG_COND_NE, cc, 1, done); \
>> +} \
>> +  } while (0)
>> +
>> +/*
>> + * The finishing counter part of CC_PROLUGE. This is supposed
>> + * to be put at the end of the function using it.
>> + */
>> +#define CC_EPILOGUE  \
>> +if (ctx->insn.cc) {  \
>> +gen_set_label(done); \
>> +}\
>> +tcg_temp_free(cc)
> 
> Why would this need to be boiler-plate?  Why would this not simply exist in
> exactly one location?
> 
> You don't need a tcg_temp_local_new, because the cc value is not used past the
> branch.  You should free the temp immediately after the branch.
> 

I wonder if what you thought was to move those macros to functions and 
call it when CC_PROLOGUE and CC_EPILOGUE are used.
I think the macros were choosen due to the sharing of the 'cc' and 
'done' variables in both CC_PROLOGUE AND CC_EPILOGUE.

>> +void gen_goto_tb(DisasContext *ctx, int n, TCGv dest)
>> +{
>> +tcg_gen_mov_tl(cpu_pc, dest);
>> +tcg_gen_andi_tl(cpu_pcl, dest, 0xfffc);
>> +if (ctx->base.singlestep_enabled) {
>> +gen_helper_debug(cpu_env);
>> +}
>> +tcg_gen_exit_tb(NULL, 0);
> 
> Missing else.  This is dead code for single-step.
Goes a little above my knowledge of QEMU internals to be honest.
Do you have a recommendation what we should be doing here ?

> 
>> +void arc_translate_init(void)
>> +{
>> +int i;
>> +static int init_not_done = 1;
>> +
>> +if (init_not_done == 0) {
>> +return;
>> +}
> 
> Useless.  This will only be called once.
> 
>> +#define ARC_REG_OFFS(x) offsetof(CPUARCState, x)
>> +
>> +#define NEW_ARC_REG(x) \
>> +tcg_global_mem_new_i32(cpu_env, offsetof(CPUARCState, x), #x)
>> +
>> +cpu_S1f = NEW_ARC_REG(macmod.S1);
>> +cpu_S2f = NEW_ARC_REG(macmod.S2);
>> +cpu_CSf = NEW_ARC_REG(macmod.CS);
>> +
>> +cpu_Zf  = NEW_ARC_REG(stat.Zf);
>> +cpu_Lf  = NEW_ARC_REG(stat.Lf);
>> +cpu_Nf  = NEW_ARC_REG(stat.Nf);
>> +cpu_Cf  = NEW_ARC_REG(stat.Cf);
>> +cpu_Vf  = NEW_ARC_REG(stat.Vf);
>> +cpu_Uf  = NEW_ARC_REG(stat.Uf);
>> +cpu_DEf = NEW_ARC_REG(stat.DEf);
>> +cpu_ESf = NEW_ARC_REG(stat.ESf);
>> +cpu_AEf = NEW_ARC_REG(stat.AEf);
>> +cpu_Hf  = NEW_ARC_REG(stat.Hf);
>> +cpu_IEf = NEW_ARC_REG(stat.IEf);
>> +cpu_Ef  = NEW_ARC_REG(stat.Ef);
>> +
>> +cpu_is_delay_slot_instruction = 
>> NEW_ARC_REG(stat.is_delay_slot_instruction);
>> +
>> +cpu_l1_Zf = NEW_ARC_REG(stat_l1.Zf);
>> +cpu_l1_Lf = NEW_ARC_REG(stat_l1.Lf);
>> +cpu_l1_Nf = NEW_ARC_REG(stat_l1.Nf);
>> +cpu_l1_Cf = NEW_ARC_REG(stat_l1.Cf);
>> +cpu_l1_Vf = NEW_ARC_REG(stat_l1.Vf);
>> +cpu_l1_Uf = NEW_ARC_REG(stat_l1.Uf);
>> +cpu_l1_DEf = NEW_ARC_REG(stat_l1.DEf);
>> +cpu_l1_AEf = NEW_ARC_REG(stat_l1.AEf);
>> +cpu_l1_Hf = NEW_ARC_REG(stat_l1.Hf);
>> +
>> +cpu_er_Zf = NEW_ARC_REG(stat_er.Zf);
>> +cpu_er_Lf = NEW_ARC_REG(stat_er.Lf);
>> +cpu_er_Nf = NEW_ARC_REG(stat_er.Nf);
>> +cpu_er_Cf = NEW_ARC_REG(stat_er.Cf);
>> +cpu_er_Vf = NEW_ARC_REG(stat_er.Vf);
>> +cpu_er_Uf = NEW_ARC_REG(stat_er.Uf);
>> +cpu_er_DEf = NEW_ARC_REG(stat_er.DEf);
>> +cpu_er_AEf = NEW_ARC_REG(stat_er.AEf);
>> +cpu_er_Hf = NEW_ARC_REG(stat_er.Hf);
>> +
>> +cpu_eret = NEW_ARC_REG(eret);
>> +cpu_erbta = NEW_ARC_REG(erbta);
>> +cpu_ecr = NEW_ARC_REG(ecr);
>> +cpu_efa = NEW_ARC_REG(efa);
>> +cpu_bta = NEW_ARC_REG(bta);
>> +cpu_lps = NEW_ARC_REG(lps);
>> +cpu_lpe = NEW_ARC_REG(lpe);
>> +cpu_pc = NEW_ARC_REG(pc);
>> +cpu_npc = NEW_ARC_REG(npc);
>> +
>> +cpu_bta_l1 = NEW_ARC_REG(bta_l1);
>> +cpu_bta_l2 = NEW_ARC_REG(bta_l2);
>> +
>> +cpu_intvec = NEW_ARC_REG(intvec);
>> +
>> +for (i = 0; i < 64; i++) {
>> +char name[16];
>> +
>> +sprintf(name, "r[%d]", i);
>> +
>> +cpu_r[i] = tcg_global_mem_new_i32(cpu_env,
>> +  ARC_REG_OFFS(r[i]),
>> +  strdup(name));
>> +}
>> +
>> +cpu_gp = cpu_r[26];
>> +cpu_fp = cpu_r[27];
>> +cpu_sp = cpu_r[28];
>> +cpu_ilink1 = cpu_r[29];
>> +

<    1   2   3   4   5   >