date:20200311

As it happens, I posted some cleanups for this last week:
https://patchew.org/QEMU/20200302175829.2183-1-richard.hender...@linaro.org/

Some of them have been queued to Peter's target-arm.next branch,
but that hasn't made it to master yet.

** Changed in: qemu
   Status: New => In Progress

** Changed in: qemu
 Assignee: (unassigned) => Richard Henderson (rth)

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

Title:
  ARM: tag bits cleared in FAR_EL1

Status in QEMU:
  In Progress

Bug description:
  The ARM Architecture Reference Manual provides the following for
  FAR_EL1:

  "For a Data Abort or Watchpoint exception, if address tagging is
  enabled for the address accessed by the data access that caused the
  exception, then this field includes the tag."

  However, I have found that the tag bits in FAR_EL1 are always clear,
  even if the tag bits were set in the original access.

  I can reproduce the problem on both 4.1.1 and master
  (6e8a73e911f066527e775e04b98f31ebd19db600).

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

[Bug 1867072] Re: ARM: tag bits cleared in FAR_EL1

Actually, I take that back: Peter has merged my TBI patch set,
and is included in 6e8a73e911f066.

Do you have a test case?

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

Title:
  ARM: tag bits cleared in FAR_EL1

Status in QEMU:
  In Progress

Bug description:
  The ARM Architecture Reference Manual provides the following for
  FAR_EL1:

  "For a Data Abort or Watchpoint exception, if address tagging is
  enabled for the address accessed by the data access that caused the
  exception, then this field includes the tag."

  However, I have found that the tag bits in FAR_EL1 are always clear,
  even if the tag bits were set in the original access.

  I can reproduce the problem on both 4.1.1 and master
  (6e8a73e911f066527e775e04b98f31ebd19db600).

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

Re: Re: [PATCH] mem-prealloc: initialize cond and mutex(Internet mail)

2020-03-11 Thread 陈蒙蒙

ok ,thanks Paolo

bauer
 
From: Paolo Bonzini
Date: 2020-03-12 02:34
To: bauerchen(陈蒙蒙); Igor Mammedov
CC: borntraeger; qemu-devel; qemu-s390x; mhartmay
Subject: Re: [PATCH] mem-prealloc: initialize cond and mutex(Internet mail)
On 10/03/20 08:06, bauerchen(陈蒙蒙) wrote:
> oh ,yes.Thanks
> I want to know if I submit a new fixed patch or just a patch fixed
> current problem?? 
> if a new fixed patch, maybe need a revert ?
 
Sorry I missed this message.  I have already sent a fixed patch, thanks!
 
Paolo
 
> 
> bauerchen(陈蒙蒙)
>
>  
> *From:* Igor Mammedov 
> *Date:* 2020-03-09 21:19
> *To:* bauerchen(陈蒙蒙) 
> *CC:* borntraeger ; pbonzini
> ; qemu-devel
> ; qemu-s390x
> ; mhartmay 
> *Subject:* Re: [PATCH] mem-prealloc: initialize cond and
> mutex(Internet mail)
> On Mon, 9 Mar 2020 11:16:10 +
> bauerchen(陈蒙蒙)  wrote:
>  
> > Thanks,  in fact,do_touch_pages is called just when vm starts up,
> but using init flag and Gonce maybe more elegant !
> > if needed,I can submit a new patch !
> > thanks very much!
>  
> it's called from os_mem_prealloc() -> touch_all_pages() which is called
> at least once per an instance of hotsmem backend. So if several backends
> are used then it should be called several times.
> The same applies when a hostmem backend is added during runtime
> (hotplug)
>  
>  
>

Re: [PATCH 02/16] accel/tcg: Add probe_access_flags

On 3/10/20 11:44 PM, Richard Henderson wrote:
> +int probe_access_flags(CPUArchState *env, target_ulong addr,
> +   MMUAccessType access_type, int mmu_idx,
> +   bool nonfault, void **phost, uintptr_t retaddr)
> +{
> +void *host;
> +int flags;
> +
> +flags = probe_access_internal(env, addr, 0, access_type, mmu_idx,
> +  nonfault, , retaddr);

Bug here.  Should have passed along phost to probe_access_internal instead of a
new local host variable.

I've sent a v2 for this patch, in-reply-to.  I'll not repost the whole patch
set until I've also addressed any review.

r~

[PATCH v2 02/16] accel/tcg: Add probe_access_flags

This new interface will allow targets to probe for a page
and then handle watchpoints themselves.  This will be most
useful for vector predicated memory operations, where one
page lookup can be used for many operations, and one test
can avoid many watchpoint checks.

Signed-off-by: Richard Henderson 
---
v2: Fix return of host pointer in softmmu probe_access_flags.
---
 include/exec/cpu-all.h  |  13 ++-
 include/exec/exec-all.h |  22 +
 accel/tcg/cputlb.c  | 177 
 accel/tcg/user-exec.c   |  36 +---
 4 files changed, 149 insertions(+), 99 deletions(-)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 49384bb66a..43ddcf024c 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -328,7 +328,18 @@ CPUArchState *cpu_copy(CPUArchState *env);
  | CPU_INTERRUPT_TGT_EXT_3   \
  | CPU_INTERRUPT_TGT_EXT_4)
 
-#if !defined(CONFIG_USER_ONLY)
+#ifdef CONFIG_USER_ONLY
+
+/*
+ * Allow some level of source compatibility with softmmu.  We do not
+ * support any of the more exotic features, so only invalid pages may
+ * be signaled by probe_access_flags().
+ */
+#define TLB_INVALID_MASK(1 << (TARGET_PAGE_BITS_MIN - 1))
+#define TLB_MMIO0
+#define TLB_WATCHPOINT  0
+
+#else
 
 /*
  * Flags stored in the low bits of the TLB virtual address.
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index d656a1f05c..8792bea07a 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -362,6 +362,28 @@ static inline void *probe_read(CPUArchState *env, 
target_ulong addr, int size,
 return probe_access(env, addr, size, MMU_DATA_LOAD, mmu_idx, retaddr);
 }
 
+/**
+ * probe_access_flags:
+ * @env: CPUArchState
+ * @addr: guest virtual address to look up
+ * @access_type: read, write or execute permission
+ * @mmu_idx: MMU index to use for lookup
+ * @nonfault: suppress the fault
+ * @phost: return value for host address
+ * @retaddr: return address for unwinding
+ *
+ * Similar to probe_access, loosely returning the TLB_FLAGS_MASK for
+ * the page, and storing the host address for RAM in @phost.
+ *
+ * If @nonfault is set, do not raise an exception but return TLB_INVALID_MASK.
+ * Do not handle watchpoints, but include TLB_WATCHPOINT in the returned flags.
+ * Do handle clean pages, so exclude TLB_NOTDIRY from the returned flags.
+ * For simplicity, all "mmio-like" flags are folded to TLB_MMIO.
+ */
+int probe_access_flags(CPUArchState *env, target_ulong addr,
+   MMUAccessType access_type, int mmu_idx,
+   bool nonfault, void **phost, uintptr_t retaddr);
+
 #define CODE_GEN_ALIGN   16 /* must be >= of the size of a icache line 
*/
 
 /* Estimated block size for TB allocation.  */
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index e3b5750c3b..bbe265ce28 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1231,86 +1231,16 @@ static void notdirty_write(CPUState *cpu, vaddr 
mem_vaddr, unsigned size,
 }
 }
 
-/*
- * Probe for whether the specified guest access is permitted. If it is not
- * permitted then an exception will be taken in the same way as if this
- * were a real access (and we will not return).
- * If the size is 0 or the page requires I/O access, returns NULL; otherwise,
- * returns the address of the host page similar to tlb_vaddr_to_host().
- */
-void *probe_access(CPUArchState *env, target_ulong addr, int size,
-   MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
+static int probe_access_internal(CPUArchState *env, target_ulong addr,
+ int fault_size, MMUAccessType access_type,
+ int mmu_idx, bool nonfault,
+ void **phost, uintptr_t retaddr)
 {
 uintptr_t index = tlb_index(env, mmu_idx, addr);
 CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr);
-target_ulong tlb_addr;
-size_t elt_ofs;
-int wp_access;
-
-g_assert(-(addr | TARGET_PAGE_MASK) >= size);
-
-switch (access_type) {
-case MMU_DATA_LOAD:
-elt_ofs = offsetof(CPUTLBEntry, addr_read);
-wp_access = BP_MEM_READ;
-break;
-case MMU_DATA_STORE:
-elt_ofs = offsetof(CPUTLBEntry, addr_write);
-wp_access = BP_MEM_WRITE;
-break;
-case MMU_INST_FETCH:
-elt_ofs = offsetof(CPUTLBEntry, addr_code);
-wp_access = BP_MEM_READ;
-break;
-default:
-g_assert_not_reached();
-}
-tlb_addr = tlb_read_ofs(entry, elt_ofs);
-
-if (unlikely(!tlb_hit(tlb_addr, addr))) {
-if (!victim_tlb_hit(env, mmu_idx, index, elt_ofs,
-addr & TARGET_PAGE_MASK)) {
-tlb_fill(env_cpu(env), addr, size, access_type, mmu_idx, retaddr);
-/* TLB resize via tlb_fill may have moved the entry. */
-index = tlb_index(env, mmu_idx, addr);
-entry = tlb_entry(env, mmu_idx, addr);
-

[Bug 1653577] Re: Ability to set umask for 9pfs

2020-03-11 Thread Launchpad Bug Tracker

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

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

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

Title:
  Ability to set umask for 9pfs

Status in QEMU:
  Expired

Bug description:
  We should be able to specify the umask for 9pfs so that files created
  by the guest can be accessed by other users on the host. Currently
  they're only accessible by the user running qemu (and of course,
  root).

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

Re: [PATCH v3 2/2] spapr: Enable virtio iommu_platform=on by default

2020-03-11 Thread Alexey Kardashevskiy




On 10/03/2020 21:43, Greg Kurz wrote:
> On Thu, 5 Mar 2020 12:59:03 +0100
> Greg Kurz  wrote:
> 
>> On Thu,  5 Mar 2020 15:30:09 +1100
>> David Gibson  wrote:
>>
>>> Traditionally, virtio devices don't do DMA by the usual path on the
>>> guest platform.  In particular they usually bypass any virtual IOMMU
>>> the guest has, using hypervisor magic to access untranslated guest
>>> physical addresses.
>>>
>>> There's now the optional iommu_platform flag which can tell virtio
>>> devices to use the platform's normal DMA path, including any IOMMUs.
>>> That flag was motiviated for the case of hardware virtio
>>> implementations, but there are other reasons to want it.
>>>
>>> Specifically, the fact that the virtio device doesn't use vIOMMU
>>> translation means that virtio devices are unsafe to pass to nested
>>> guests, or to use with VFIO userspace drivers inside the guest.  This
>>> is particularly noticeable on the pseries platform which *always* has
>>> a guest-visible vIOMMU.
>>>
>>> Not using the normal DMA path also causes difficulties for the guest
>>> side driver when using the upcoming POWER Secure VMs (a.k.a. PEF).
>>> While it's theoretically possible to handle this on the guest side,
>>> it's really fiddly.  Given the other problems with the non-translated
>>> virtio device, let's just enable vIOMMU translation for virtio devices
>>> by default in the pseries-5.0 (and later) machine types.
>>>
>>> This does mean the new machine type will no longer support guest
>>> kernels older than 4.8, unless they have support for the virtio
>>> IOMMU_PLATFORM flag backported (which some distro kernels like RHEL7
>>> do).
>>>
>>> Signed-off-by: David Gibson 
>>> ---
>>
>> The patch looks good but I'm not sure if we're quite ready to merge
>> it yet. With this applied, I get zero output on a virtio-serial based
>> console:
>>
>> ie.
>>   -chardev stdio,id=con0 -device virtio-serial -device 
>> virtconsole,chardev=con0 
>>
>> FYI, virtio-serial is a bit broken for spapr with iommu_platform=off already:
>>
>> (1) pressing a key in the console during SLOF or grub has no effect
>>
>> (2) the guest kernel boot stays stuck around quiesce
>>
>> These are regressions introduced by this SLOF update:
>>
>> a363e9ed8731f45674260932a340a0d81c4b0a6f is the first bad commit
>> commit a363e9ed8731f45674260932a340a0d81c4b0a6f
>> Author: Alexey Kardashevskiy 
>> Date:   Tue Dec 17 11:31:54 2019 +1100
>> pseries: Update SLOF firmware image
>>
>> A trivial fix was already posted on the SLOF list for (1) :
>>
>> https://patchwork.ozlabs.org/patch/1249338/
>>
>> (2) is still under investigation but the console is _at least_
>> functional until the guest OS takes control. This is no longer
>> the case with this patch.
>>
> 
> Some progress was made on the SLOF front:
> 
> https://patchwork.ozlabs.org/project/slof/list/?series=163314
> 
> With these series applied to SLOF, I can now boot a fedora31 guest
> with a virtio-serial console and iommu_platform=on... but now
> I'm trying out other virtio devices supported by SLOF and I'm
> running into issues around virtio-pci.disable-legacy as mentioned
> in some other mail...
> 
> It seems we may not be ready to merge this series yet.


fwiw I sent a pull request:

https://lore.kernel.org/qemu-devel/20200312041010.16229-1-...@ozlabs.ru/T/#u



> 
>>>  hw/ppc/spapr.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>> index 3cfc98ac61..5ef099536e 100644
>>> --- a/hw/ppc/spapr.c
>>> +++ b/hw/ppc/spapr.c
>>> @@ -4575,6 +4575,7 @@ static void 
>>> spapr_machine_latest_class_options(MachineClass *mc)
>>>   */
>>>  static GlobalProperty compat[] = {
>>>  { TYPE_VIRTIO_PCI, "disable-legacy", "on", },
>>> +{ TYPE_VIRTIO_DEVICE, "iommu_platform", "on", },
>>>  };
>>>  
>>>  mc->alias = "pseries";
>>> @@ -4622,6 +4623,7 @@ static void 
>>> spapr_machine_4_2_class_options(MachineClass *mc)
>>>  SpaprMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
>>>  static GlobalProperty compat[] = {
>>>  { TYPE_VIRTIO_PCI, "disable-legacy", "auto" },
>>> +{ TYPE_VIRTIO_DEVICE, "iommu_platform", "off", },
>>>  };
>>>  
>>>  spapr_machine_5_0_class_options(mc);
>>
> 

-- 
Alexey

[PULL SUBSYSTEM qemu-pseries] pseries: Update SLOF firmware image

2020-03-11 Thread Alexey Kardashevskiy

The following changes since commit 0ba04f78e7c37748db89885249a653b28c962bd9:

  ppc/spapr: Move GPRs setup to one place (2020-03-12 10:21:16 +1100)

are available in the Git repository at:

  g...@github.com:aik/qemu.git tags/qemu-slof-20200312

for you to fetch changes up to 8b1987b232d0da2df1f33bf9e3e25fcb1874b91d:

  pseries: Update SLOF firmware image (2020-03-12 15:04:40 +1100)


Alexey Kardashevskiy (1):
  pseries: Update SLOF firmware image

 pc-bios/README   |   2 +-
 pc-bios/slof.bin | Bin 968616 -> 968848 bytes
 roms/SLOF|   2 +-
 3 files changed, 2 insertions(+), 2 deletions(-)


*** Note: this is not for master, this is for pseries


This mainly fixes virtio-serial with and without
enabled iommu-platform.

The full list of changes is:

Alexey Kardashevskiy (3):
  llfw: Fix debug printf warnings
  virtio-serial: Close device completely
  version: update to 20200312

Cédric Le Goater (1):
  virtio: Fix typo in virtio_serial_init()

Greg Kurz (2):
  virtio-serial: Don't override some words
  virtio-serial: Rework shutdown sequence

Re: [PATCH] acpi: Add Windows ACPI Emulated Device Table (WAET)

2020-03-11 Thread Liran Alon




On 11/03/2020 22:24, Michael S. Tsirkin wrote:

Notice the process as documented in ./tests/qtest/bios-tables-test.c


Thanks for explicitly pointing me to that process.

I have followed the process described there (Both steps 1-3 and steps 4-7).

On step (6), I have noted that many existing ACPI tables don't have 
expected binaries for all the execution-matrix.
E.g. tests/data/acpi/pc/APIC.{bridge, ipmikcs, memhp, numamem} are all 
missing.

Similar missing files exists for FACP, FACS, HPET and MCFG.

I should add for WAET the expected binaries for all the execution-matrix 
right?
Is it just an existing issue that for the existing tables some of the 
expected binaries are missing? But the tests seems to pass.

Can you clarify this for me?

Thanks,
-Liran

Re: [PATCH qemu v8 3/3] spapr: Implement Open Firmware client interface

2020-03-11 Thread Alexey Kardashevskiy




On 11/03/2020 20:43, Paolo Bonzini wrote:
> On 10/03/20 06:07, Alexey Kardashevskiy wrote:
>> The PAPR platform which describes an OS environment that's presented by
>> a combination of a hypervisor and firmware. The features it specifies
>> require collaboration between the firmware and the hypervisor.
>>
>> Since the beginning, the runtime component of the firmware (RTAS) has
>> been implemented as a 20 byte shim which simply forwards it to
>> a hypercall implemented in qemu. The boot time firmware component is
>> SLOF - but a build that's specific to qemu, and has always needed to be
>> updated in sync with it. Even though we've managed to limit the amount
>> of runtime communication we need between qemu and SLOF, there's some,
>> and it has become increasingly awkward to handle as we've implemented
>> new features.
>>
>> This implements a boot time OF client interface (CI) which is
>> enabled by a new "x-vof" pseries machine option (stands for "Virtual Open
>> Firmware). When enabled, QEMU implements the custom H_OF_CLIENT hcall
>> which implements Open Firmware Client Interface (OF CI). This allows
>> using a smaller stateless firmware which does not have to manage
>> the device tree.
>>
>> The new "vof.bin" firmware image is included with source code under
>> pc-bios/. It also includes RTAS blob.
>>
>> This implements a handful of CI methods just to get -kernel/-initrd
>> working. In particular, this implements the device tree fetching and
>> simple memory allocator - "claim" (an OF CI memory allocator) and updates
>> "/memory@0/available" to report the client about available memory.
>>
>> This implements changing some device tree properties which we know how
>> to deal with, the rest is ignored. To allow changes, this skips
>> fdt_pack() when x-vof=on as not packing the blob leaves some room for
>> appending.
>>
>> In absence of SLOF, this assigns "phandles" to device tree nodes to make
>> device tree traversing work.
>>
>> When x-vof=on, this adds "/chosen" every time QEMU (re)builds a tree.
>>
>> This adds basic instances support which are managed by a hashmap
>> ihandle -> [phandle].
>>
>> Before the guest started, the used memory is:
>> 0..4000 - the initial firmware
>> 1..18 - stack
>>
>> This OF CI does not implement "interpret".
>>
>> With this basic support, this can only boot into kernel directly.
>> However this is just enough for the petitboot kernel and initradmdisk to
>> boot from any possible source. Note this requires reasonably recent guest
>> kernel with:
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=df5be5be8735
>>
>> Signed-off-by: Alexey Kardashevskiy 
> 
> Thank you very much Alexey!  At least, since it can run petitboot, it's
> not completely useless.
> 
> Acked-by: Paolo Bonzini 
> 


Cool, thanks! Have you tried it yourself or you are just taking my word
for it? :)


-- 
Alexey

Re: [PATCH v3 1/2] spapr: Disable legacy virtio devices for pseries-5.0 and later

On Wed, Mar 11, 2020 at 03:11:16AM -0400, Michael S. Tsirkin wrote:
> On Wed, Mar 11, 2020 at 11:58:57AM +1100, David Gibson wrote:
> > Note that several things that I believe are now in the PCIe spec, but
> > really derive more from PC legacy considerations, don't apply at all
> > for PAPR.  e.g. there's no meaningful distinction between integrated
> > and slotted devices, multiple independent host bridges is routine and
> > doesn't require any (virtual) hardware visible domain numbers.
> 
> Domain numbers are a Linux thing, not a PCIe thing. On x86 they come
> from ACPI segment numbers. As such they aren't usually hardware
> visible on x86, they are supplied by firmware.

Oh, ok.  I thought that at least on the standard IO 0xcf8 host bridge
controller the domain number was written into certain registers to
select the relevant root bus.

On POWER the domain numbers are arbitrarily assigned within Linux.
"Hardware" (well, the firmware/hypervisor) uses a different
identifier, called the BUID (generally a large, 64-bit pseudo-address)
in the device tree and hypercalls.

[As an aside, this means the use of domain numbers in libvirt XML is
complete bogosity]

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


signature.asc
Description: PGP signature

Re: Upstream QEMU guest support policy ? Re: [PATCH v3 0/2] spapr: Use vIOMMU translation for virtio by default

On Wed, Mar 11, 2020 at 03:33:59AM -0400, Michael S. Tsirkin wrote:
> On Wed, Mar 11, 2020 at 12:12:47PM +1100, David Gibson wrote:
> > I am wondering if we have to introduce an "svm=on" flag anyway.  It's
> > pretty ugly, since all it would be doing is changing defaults here and
> > there for compatibilty with a possible future SVM transition, but
> > maybe it's the best we can do :/.
> 
> Frankly I'm surprised there's no way for the hypervisor to block VM
> transition to secure mode. To me an inability to disable DRM looks like
> a security problem.

Uh.. I don't immediately see how it's a security problem, though I'm
certainly convinced it's a problem in other ways.

> Does not the ultravisor somehow allow
> enabling/disabling this functionality from the hypervisor?

Not at present, but as mentioned on the other thread, Paul and I came
up with a tentative plan to change that.

> It would be
> even better if the hypervisor could block the guest from poking at the
> ultravisor completely but I guess that would be too much to hope for.

Yeah, probably :/.

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


signature.asc
Description: PGP signature

Re: Upstream QEMU guest support policy ? Re: [PATCH v3 0/2] spapr: Use vIOMMU translation for virtio by default

On Wed, Mar 11, 2020 at 07:48:26AM -0400, Michael S. Tsirkin wrote:
> On Wed, Mar 11, 2020 at 10:01:27AM +, Daniel P. Berrangé wrote:
> > On Wed, Mar 11, 2020 at 12:12:47PM +1100, David Gibson wrote:
> > > On Tue, Mar 10, 2020 at 11:43:43AM +, Daniel P. Berrangé wrote:
> > > > On Thu, Mar 05, 2020 at 03:30:07PM +1100, David Gibson wrote:
> > > > > Upcoming Secure VM support for pSeries machines introduces some
> > > > > complications for virtio, since the transfer buffers need to be
> > > > > explicitly shared so that the hypervisor can access them.
> > > > > 
> > > > > While it's not strictly speaking dependent on it, the fact that virtio
> > > > > devices bypass normal platform IOMMU translation complicates the issue
> > > > > on the guest side.  Since there are some significan downsides to
> > > > > bypassing the vIOMMU anyway, let's just disable that.
> > > > > 
> > > > > There's already a flag to do this in virtio, just turn it on by
> > > > > default for forthcoming pseries machine types.
> > > > 
> > > > Breaking existing guest OS to support a new secure VM feature that
> > > > may not even be used/wanted doesn't seems like a sensible tradeoff
> > > > for default out of the box behaviour.
> > > > 
> > > > IOW, if Secure VM needs this, can we tie the change in virtio and
> > > > IOMMU defaults to the machine type flag that enables the use of
> > > > Secure VM.
> > > 
> > > There is no such flag.
> > > 
> > > In the POWER secure VM model, the secure mode option isn't something
> > > that's constructed in when the hypervisor builds the VM.  Instead the
> > > VM is started normally and transitions itself to secure mode by
> > > talking directly with the ultravisor (it then uses TPM shenannigans to
> > > safely get the keys to its real storage backend(s)).
> > 
> > This is pretty suprising to me. The ability to use secure VM mode surely
> > depends on host hardware features. We would need to be able to block the
> > use of this, in order to allow VMs to be live migrated to hosts which
> > lack the feature. Automatically & silently enabling a feature that
> > has a hardware dependancy is something we aim to avoid, unless the user
> > has opted in via some flag (such as -cpu host, or a -cpu $NAME, that
> > implies the feature).
> 
> That's something I don't know. Is migration supported in this mode?

Not at this stage, though there's plans for it later.

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


signature.asc
Description: PGP signature

Re: Upstream QEMU guest support policy ? Re: [PATCH v3 0/2] spapr: Use vIOMMU translation for virtio by default

On Wed, Mar 11, 2020 at 10:01:27AM +, Daniel P. Berrangé wrote:
65;5803;1c> On Wed, Mar 11, 2020 at 12:12:47PM +1100, David Gibson wrote:
> > On Tue, Mar 10, 2020 at 11:43:43AM +, Daniel P. Berrangé wrote:
> > > On Thu, Mar 05, 2020 at 03:30:07PM +1100, David Gibson wrote:
> > > > Upcoming Secure VM support for pSeries machines introduces some
> > > > complications for virtio, since the transfer buffers need to be
> > > > explicitly shared so that the hypervisor can access them.
> > > > 
> > > > While it's not strictly speaking dependent on it, the fact that virtio
> > > > devices bypass normal platform IOMMU translation complicates the issue
> > > > on the guest side.  Since there are some significan downsides to
> > > > bypassing the vIOMMU anyway, let's just disable that.
> > > > 
> > > > There's already a flag to do this in virtio, just turn it on by
> > > > default for forthcoming pseries machine types.
> > > 
> > > Breaking existing guest OS to support a new secure VM feature that
> > > may not even be used/wanted doesn't seems like a sensible tradeoff
> > > for default out of the box behaviour.
> > > 
> > > IOW, if Secure VM needs this, can we tie the change in virtio and
> > > IOMMU defaults to the machine type flag that enables the use of
> > > Secure VM.
> > 
> > There is no such flag.
> > 
> > In the POWER secure VM model, the secure mode option isn't something
> > that's constructed in when the hypervisor builds the VM.  Instead the
> > VM is started normally and transitions itself to secure mode by
> > talking directly with the ultravisor (it then uses TPM shenannigans to
> > safely get the keys to its real storage backend(s)).
> 
> This is pretty suprising to me. The ability to use secure VM mode surely
> depends on host hardware features. We would need to be able to block the
> use of this, in order to allow VMs to be live migrated to hosts which
> lack the feature. Automatically & silently enabling a feature that
> has a hardware dependancy is something we aim to avoid, unless the user
> has opted in via some flag (such as -cpu host, or a -cpu $NAME, that
> implies the feature).

That is an excellent point, which I had not previously considered.

I have confirmed that there is indeed not, at present, a way to
disable the secure transition.  But, it looks like it's not too late
to fix it.

I've discussed with Paul Mackerras, and early in the secure transition
apparently the UV makes a call to the HV, which is allowed to fail.

So, we're looking at adding another KVM capability for secure mode.
It will default to disabled, and until it is explicitly enabled, KVM
will always fail that call from the UV, effectively preventing guests
from going into secure mode.

We can then wire that up to a new spapr cap in qemu, which we can also
use to configure these virtio defaults.

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


signature.asc
Description: PGP signature

[Bug 1867072] [NEW] ARM: tag bits cleared in FAR_EL1

2020-03-11 Thread Peter Collingbourne

Public bug reported:

The ARM Architecture Reference Manual provides the following for
FAR_EL1:

"For a Data Abort or Watchpoint exception, if address tagging is enabled
for the address accessed by the data access that caused the exception,
then this field includes the tag."

However, I have found that the tag bits in FAR_EL1 are always clear,
even if the tag bits were set in the original access.

I can reproduce the problem on both 4.1.1 and master
(6e8a73e911f066527e775e04b98f31ebd19db600).

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  ARM: tag bits cleared in FAR_EL1

Status in QEMU:
  New

Bug description:
  The ARM Architecture Reference Manual provides the following for
  FAR_EL1:

  "For a Data Abort or Watchpoint exception, if address tagging is
  enabled for the address accessed by the data access that caused the
  exception, then this field includes the tag."

  However, I have found that the tag bits in FAR_EL1 are always clear,
  even if the tag bits were set in the original access.

  I can reproduce the problem on both 4.1.1 and master
  (6e8a73e911f066527e775e04b98f31ebd19db600).

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

[PATCH v3 2/2] target/arm: kvm: Handle DABT with no valid ISS

2020-03-11 Thread Beata Michalska

On ARMv7 & ARMv8 some load/store instructions might trigger a data abort
exception with no valid ISS info to be decoded. The lack of decode info
makes it at least tricky to emulate those instruction which is one of the
(many) reasons why KVM will not even try to do so.

Add support for handling those by requesting KVM to inject external
dabt into the quest.

Signed-off-by: Beata Michalska 
---
 target/arm/cpu.h |  3 ++
 target/arm/kvm.c | 81 
 target/arm/kvm32.c   | 26 +
 target/arm/kvm64.c   | 36 +++
 target/arm/kvm_arm.h | 22 ++
 5 files changed, 168 insertions(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 4ffd991..45fdd2e 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -560,6 +560,9 @@ typedef struct CPUARMState {
 uint64_t esr;
 } serror;
 
+uint8_t ext_dabt_pending:1; /* Request for injecting ext DABT */
+uint8_t ext_dabt_raised:1; /* Tracking/verifying injection of ext DABT */
+
 /* State of our input IRQ/FIQ/VIRQ/VFIQ lines */
 uint32_t irq_line_state;
 
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 85860e6..8b7b708 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -39,6 +39,7 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
 
 static bool cap_has_mp_state;
 static bool cap_has_inject_serror_esr;
+static bool cap_has_inject_ext_dabt;
 
 static ARMHostCPUFeatures arm_host_cpu_features;
 
@@ -244,6 +245,16 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
 ret = -EINVAL;
 }
 
+if (kvm_check_extension(s, KVM_CAP_ARM_NISV_TO_USER)) {
+if (kvm_vm_enable_cap(s, KVM_CAP_ARM_NISV_TO_USER, 0)) {
+warn_report("Failed to enable DABT NISV cap");
+} else {
+/* Set status for supporting the external dabt injection */
+cap_has_inject_ext_dabt = kvm_check_extension(s,
+KVM_CAP_ARM_INJECT_EXT_DABT);
+}
+}
+
 return ret;
 }
 
@@ -703,9 +714,20 @@ int kvm_put_vcpu_events(ARMCPU *cpu)
 events.exception.serror_esr = env->serror.esr;
 }
 
+if (cap_has_inject_ext_dabt) {
+events.exception.ext_dabt_pending = env->ext_dabt_pending;
+}
+
 ret = kvm_vcpu_ioctl(CPU(cpu), KVM_SET_VCPU_EVENTS, );
 if (ret) {
 error_report("failed to put vcpu events");
+} else if (env->ext_dabt_pending) {
+/*
+ * Mark that the external DABT has been injected,
+ * if one has been requested
+ */
+env->ext_dabt_raised = env->ext_dabt_pending;
+env->ext_dabt_pending = 0;
 }
 
 return ret;
@@ -737,6 +759,30 @@ int kvm_get_vcpu_events(ARMCPU *cpu)
 
 void kvm_arch_pre_run(CPUState *cs, struct kvm_run *run)
 {
+ARMCPU *cpu = ARM_CPU(cs);
+CPUARMState *env = >env;
+
+if (unlikely(env->ext_dabt_raised)) {
+/*
+ * Verifying that the ext DABT has been properly injected,
+ * otherwise risking indefinitely re-running the faulting instruction
+ * Covering a very narrow case for kernels 5.5..5.5.4
+ * when injected abort was misconfigured to be
+ * an IMPLEMENTATION DEFINED exception (for 32-bit EL1)
+ */
+if (!arm_feature(env, ARM_FEATURE_AARCH64) &&
+unlikely(kvm_arm_verify_ext_dabt_pending(cs))) {
+
+error_report("Data abort exception with no valid ISS generated by "
+   "guest memory access. KVM unable to emulate faulting "
+   "instruction. Failed to inject an external data abort "
+   "into the guest.");
+abort();
+   }
+   /* Clear the status */
+   env->ext_dabt_raised = 0;
+}
+
 }
 
 MemTxAttrs kvm_arch_post_run(CPUState *cs, struct kvm_run *run)
@@ -819,6 +865,11 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
 ret = EXCP_DEBUG;
 } /* otherwise return to guest */
 break;
+case KVM_EXIT_ARM_NISV:
+/* External DABT with no valid iss to decode */
+ret = kvm_arm_handle_dabt_nisv(cs, run->arm_nisv.esr_iss,
+   run->arm_nisv.fault_ipa);
+break;
 default:
 qemu_log_mask(LOG_UNIMP, "%s: un-handled exit reason %d\n",
   __func__, run->exit_reason);
@@ -953,3 +1004,33 @@ int kvm_arch_msi_data_to_gsi(uint32_t data)
 {
 return (data - 32) & 0x;
 }
+
+int kvm_arm_handle_dabt_nisv(CPUState *cs, uint64_t esr_iss,
+ uint64_t fault_ipa)
+{
+ARMCPU *cpu = ARM_CPU(cs);
+CPUARMState *env = >env;
+
+   /*
+* ISS [23:14] is invalid so there is a limited info
+* on what has just happened so the only *useful* thing that can
+* be retrieved from ISS is WnR & DFSC (though in some cases WnR
+* might be less of a value as well)
+*/
+
+/*
+ * Set pending ext dabt and trigger SET_EVENTS so that

[PATCH v3 1/2] target/arm: kvm: Inject events at the last stage of sync

2020-03-11 Thread Beata Michalska

KVM_SET_VCPU_EVENTS might actually lead to vcpu registers being modified.
As such this should be the last step of sync to avoid potential overwriting
of whatever changes KVM might have done.

Signed-off-by: Beata Michalska 
---
 target/arm/kvm32.c | 15 ++-
 target/arm/kvm64.c | 15 ++-
 2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/target/arm/kvm32.c b/target/arm/kvm32.c
index f703c4f..f271181 100644
--- a/target/arm/kvm32.c
+++ b/target/arm/kvm32.c
@@ -409,17 +409,22 @@ int kvm_arch_put_registers(CPUState *cs, int level)
 return ret;
 }
 
-ret = kvm_put_vcpu_events(cpu);
-if (ret) {
-return ret;
-}
-
 write_cpustate_to_list(cpu, true);
 
 if (!write_list_to_kvmstate(cpu, level)) {
 return EINVAL;
 }
 
+/*
+ * Setting VCPU events should be triggered after syncing the registers
+ * to avoid overwriting potential changes made by KVM upon calling
+ * KVM_SET_VCPU_EVENTS ioctl
+ */
+ret = kvm_put_vcpu_events(cpu);
+if (ret) {
+return ret;
+}
+
 kvm_arm_sync_mpstate_to_kvm(cpu);
 
 return ret;
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 93ba144..be5b31c 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -1094,17 +1094,22 @@ int kvm_arch_put_registers(CPUState *cs, int level)
 return ret;
 }
 
-ret = kvm_put_vcpu_events(cpu);
-if (ret) {
-return ret;
-}
-
 write_cpustate_to_list(cpu, true);
 
 if (!write_list_to_kvmstate(cpu, level)) {
 return -EINVAL;
 }
 
+   /*
+* Setting VCPU events should be triggered after syncing the registers
+* to avoid overwriting potential changes made by KVM upon calling
+* KVM_SET_VCPU_EVENTS ioctl
+*/
+ret = kvm_put_vcpu_events(cpu);
+if (ret) {
+return ret;
+}
+
 kvm_arm_sync_mpstate_to_kvm(cpu);
 
 return ret;
-- 
2.7.4

[PATCH v3 0/2] target/arm: kvm: Support for KVM DABT with no valid ISS

2020-03-11 Thread Beata Michalska

Some of the ARMv7 & ARMv8 load/store instructions might trigger a data abort
exception with no valid ISS info to be decoded. The lack of decode info
makes it at least tricky to emulate the instruction which is one of the
(many) reasons why KVM will not even try to do so.

So far, if a guest made an attempt to access memory outside the memory slot,
KVM reported vague ENOSYS. As a result QEMU exited with no useful information
being provided or even a clue on what has just happened.

ARM KVM introduced support for notifying of an attempt to execute
an instruction that resulted in dabt with no valid ISS decoding info.
This still leaves QEMU to handle the case, but at least now it gives more
control and a start point for more meaningful handling of such cases.

This patchset relies on KVM to insert the external data abort into the guest.

v3:
 - Fix setting KVM cap per vm not per vcpu
 - Simplifying the handler to bare minimum with no default logging to address
   the potential risk of overflooding the host (adding support for rate
   limiting the logs turned out to be bit too invasive to justify the little
   add-on value from logs in this particular case)
 - Adding handling KVM bug (for small range of affected kernels):
   little bit of trade-off between what's reasonable and what's effective:
   aborting qemu when running on buggy host kernel

v2:
- Improving/re-phrasing messaging
- Dropping messing around with forced sync (@see [PATCH v2 1/2])
  and PC alignment


Beata Michalska (2):
  target/arm: kvm: Inject events at the last stage of sync
  target/arm: kvm: Handle DABT with no valid ISS

 target/arm/cpu.h |  3 ++
 target/arm/kvm.c | 81 
 target/arm/kvm32.c   | 41 ++
 target/arm/kvm64.c   | 51 +
 target/arm/kvm_arm.h | 22 ++
 5 files changed, 188 insertions(+), 10 deletions(-)

-- 
2.7.4

Re: [PATCH v2 0/2] Fix MAP_SYNC support when host has older glibc version

2020-03-11 Thread no-reply

Patchew URL: 
https://patchew.org/QEMU/20200311232342.1614944-1-ehabk...@redhat.com/



Hi,

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

=== TEST SCRIPT BEGIN ===
#!/bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

PASS 1 fdc-test /x86_64/fdc/cmos
PASS 2 fdc-test /x86_64/fdc/no_media_on_start
PASS 3 fdc-test /x86_64/fdc/read_without_media
==9775==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 4 fdc-test /x86_64/fdc/media_change
PASS 5 fdc-test /x86_64/fdc/sense_interrupt
PASS 6 fdc-test /x86_64/fdc/relative_seek
---
PASS 32 test-opts-visitor /visitor/opts/range/beyond
PASS 33 test-opts-visitor /visitor/opts/dict/unvisited
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-coroutine -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="test-coroutine" 
==9823==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
==9823==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 
0x7ffda46dc000; bottom 0x7feed101; size: 0x000ed36cc000 (63676661760)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 1 test-coroutine /basic/no-dangling-access
---
PASS 11 test-aio /aio/event/wait
PASS 12 test-aio /aio/event/flush
PASS 13 test-aio /aio/event/wait/no-flush-cb
==9838==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 14 test-aio /aio/timer/schedule
PASS 15 test-aio /aio/coroutine/queue-chaining
PASS 16 test-aio /aio-gsource/flush
---
PASS 12 fdc-test /x86_64/fdc/read_no_dma_19
PASS 13 fdc-test /x86_64/fdc/fuzz-registers
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img 
tests/qtest/ide-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="ide-test" 
==9846==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 ide-test /x86_64/ide/identify
PASS 28 test-aio /aio-gsource/timer/schedule
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-aio-multithread -m=quick -k --tap < /dev/null | 
./scripts/tap-driver.pl --test-name="test-aio-multithread" 
==9855==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 test-aio-multithread /aio/multi/lifecycle
==9852==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 2 ide-test /x86_64/ide/flush
PASS 2 test-aio-multithread /aio/multi/schedule
==9872==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 3 ide-test /x86_64/ide/bmdma/simple_rw
==9883==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 3 test-aio-multithread /aio/multi/mutex/contended
PASS 4 ide-test /x86_64/ide/bmdma/trim
==9894==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 4 test-aio-multithread /aio/multi/mutex/handoff
PASS 5 test-aio-multithread /aio/multi/mutex/mcs
PASS 6 test-aio-multithread /aio/multi/mutex/pthread
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-throttle -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="test-throttle" 
==9911==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 test-throttle /throttle/leak_bucket
PASS 2 test-throttle /throttle/compute_wait
PASS 3 test-throttle /throttle/init
---
PASS 14 test-throttle /throttle/config/max
PASS 15 test-throttle /throttle/config/iops_size
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-thread-pool -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="test-thread-pool" 
==9915==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 test-thread-pool /thread-pool/submit
PASS 2 test-thread-pool /thread-pool/submit-aio
PASS 3 test-thread-pool /thread-pool/submit-co
PASS 4 test-thread-pool /thread-pool/submit-many
==9917==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 5 test-thread-pool /thread-pool/cancel
PASS 6 test-thread-pool /thread-pool/cancel-async
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 +

Re: [PATCH 0/2] Fix MAP_SYNC support when host has older glibc version

2020-03-11 Thread no-reply

Patchew URL: 
https://patchew.org/QEMU/20200311225130.1599619-1-ehabk...@redhat.com/



Hi,

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

=== TEST SCRIPT BEGIN ===
#!/bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

PASS 1 fdc-test /x86_64/fdc/cmos
PASS 2 fdc-test /x86_64/fdc/no_media_on_start
PASS 3 fdc-test /x86_64/fdc/read_without_media
==9860==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 4 fdc-test /x86_64/fdc/media_change
PASS 5 fdc-test /x86_64/fdc/sense_interrupt
PASS 6 fdc-test /x86_64/fdc/relative_seek
---
PASS 33 test-opts-visitor /visitor/opts/dict/unvisited
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-coroutine -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="test-coroutine" 
PASS 11 fdc-test /x86_64/fdc/read_no_dma_18
==9935==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 test-coroutine /basic/no-dangling-access
==9935==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 
0x7ffca8617000; bottom 0x7fe69b8b2000; size: 0x00160cd65000 (94704652288)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 2 test-coroutine /basic/lifecycle
---
PASS 12 fdc-test /x86_64/fdc/read_no_dma_19
PASS 13 fdc-test /x86_64/fdc/fuzz-registers
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img 
tests/qtest/ide-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="ide-test" 
==9958==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 ide-test /x86_64/ide/identify
PASS 14 test-aio /aio/timer/schedule
==9950==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 15 test-aio /aio/coroutine/queue-chaining
PASS 16 test-aio /aio-gsource/flush
PASS 17 test-aio /aio-gsource/bh/schedule
---
PASS 25 test-aio /aio-gsource/event/wait
PASS 26 test-aio /aio-gsource/event/flush
PASS 27 test-aio /aio-gsource/event/wait/no-flush-cb
==9964==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 2 ide-test /x86_64/ide/flush
==9970==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 3 ide-test /x86_64/ide/bmdma/simple_rw
PASS 28 test-aio /aio-gsource/timer/schedule
==9976==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-aio-multithread -m=quick -k --tap < /dev/null | 
./scripts/tap-driver.pl --test-name="test-aio-multithread" 
PASS 4 ide-test /x86_64/ide/bmdma/trim
==9983==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 test-aio-multithread /aio/multi/lifecycle
==9985==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 2 test-aio-multithread /aio/multi/schedule
PASS 3 test-aio-multithread /aio/multi/mutex/contended
PASS 4 test-aio-multithread /aio/multi/mutex/handoff
PASS 5 test-aio-multithread /aio/multi/mutex/mcs
==10017==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 6 test-aio-multithread /aio/multi/mutex/pthread
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-throttle -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="test-throttle" 
==10029==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 test-throttle /throttle/leak_bucket
PASS 2 test-throttle /throttle/compute_wait
PASS 3 test-throttle /throttle/init
---
PASS 14 test-throttle /throttle/config/max
PASS 15 test-throttle /throttle/config/iops_size
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-thread-pool -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="test-thread-pool" 
==10033==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 test-thread-pool /thread-pool/submit
PASS 2 test-thread-pool /thread-pool/submit-aio
PASS 3 test-thread-pool /thread-pool/submit-co
---
PASS 2 test-hbitmap /hbitmap/size/0
PASS 3 test-hbitmap /hbitmap/size/unaligned
PASS 4 test-hbitmap

[PATCH v2 1/2] Use -isystem for linux-headers dir

glibc and Linux-provided headers are known to generate macro
redefinition warnings when used together.  For example:
 and  duplicate some macro definitions.

We normally never see those warnings because GCC suppresses
warnings generated by system headers.  We carry our own copy of
Linux header files, though, and this makes those warnings not be
suppressed when glibc headers are included before Linux headers
(e.g. if  is included before ).

Use -isystem instead of -I for linux-headers.  This makes the
compiler treat our linux-headers directory the same way it treats
system-provided Linux headers, and suppress warnings generated by
them.

Signed-off-by: Eduardo Habkost 
---
Changes v1 -> v2:
* Use -isystem for $PWD/linux-headers too
  Reported-by: "Michael S. Tsirkin" 
---
 Makefile.target | 2 +-
 configure   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile.target b/Makefile.target
index 2d43dc586a..934a9f7431 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -12,7 +12,7 @@ endif
 
 $(call set-vpath, $(SRC_PATH):$(BUILD_DIR))
 ifdef CONFIG_LINUX
-QEMU_CFLAGS += -I../linux-headers
+QEMU_CFLAGS += -isystem ../linux-headers
 endif
 QEMU_CFLAGS += -iquote .. -iquote $(SRC_PATH)/target/$(TARGET_BASE_ARCH) 
-DNEED_CPU_H
 
diff --git a/configure b/configure
index cbf864bff1..bf5bf70600 100755
--- a/configure
+++ b/configure
@@ -899,7 +899,7 @@ Linux)
   linux="yes"
   linux_user="yes"
   kvm="yes"
-  QEMU_INCLUDES="-I\$(SRC_PATH)/linux-headers -I$PWD/linux-headers 
$QEMU_INCLUDES"
+  QEMU_INCLUDES="-isystem \$(SRC_PATH)/linux-headers -isystem 
$PWD/linux-headers $QEMU_INCLUDES"
   supported_os="yes"
   libudev="yes"
 ;;
-- 
2.24.1

[PATCH v2 2/2] mmap-alloc: Include osdep.h before checking CONFIG_LINUX

The CONFIG_LINUX check at the top of mmap-alloc.c never worked
because it was done before including osdep.h.

This means MAP_SYNC and MAP_SHARED_VALIDATE would always be set
to 0 at the beginning of the file.  Luckily, this didn't break
when using recent glibc versions (2.28+), because those macros
were redefined by glibc headers.

Move the CONFIG_LINUX check after the main include lines, so the
CONFIG_LINUX check works and we actually include .
This will make MAP_SYNC and MAP_SHARED_VALIDATE available even if
the host has an older glibc version.

Reported-by: Jingqi Liu 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Eduardo Habkost 
---
Changes v1 -> v2:
* (none)
---
 util/mmap-alloc.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
index 27dcccd8ec..7c2ce98eb0 100644
--- a/util/mmap-alloc.c
+++ b/util/mmap-alloc.c
@@ -9,6 +9,9 @@
  * This work is licensed under the terms of the GNU GPL, version 2 or
  * later.  See the COPYING file in the top-level directory.
  */
+#include "qemu/osdep.h"
+#include "qemu/mmap-alloc.h"
+#include "qemu/host-utils.h"
 
 #ifdef CONFIG_LINUX
 #include 
@@ -17,10 +20,6 @@
 #define MAP_SHARED_VALIDATE   0x0
 #endif /* CONFIG_LINUX */
 
-#include "qemu/osdep.h"
-#include "qemu/mmap-alloc.h"
-#include "qemu/host-utils.h"
-
 #define HUGETLBFS_MAGIC   0x958458f6
 
 #ifdef CONFIG_LINUX
-- 
2.24.1

[PATCH v2 0/2] Fix MAP_SYNC support when host has older glibc version

Changes v1 -> v2:
* Use -isystem for $PWD/linux-headers too
  Reported-by: "Michael S. Tsirkin" 

This is an alternative to the patch submitted at:

  From: Jingqi Liu 
  Subject: [PATCH] util: fix to get configuration macros in util/mmap-alloc.c
  Date: Thu,  5 Mar 2020 23:41:42 +0800
  Message-Id: <20200305154142.63070-1-jingqi@intel.com>

Before moving the osdep.h include to the top of the file, we had
to address warnings triggered when  was included
after  (done in patch 1/2).

Eduardo Habkost (2):
  Use -isystem for linux-headers dir
  mmap-alloc: Include osdep.h before checking CONFIG_LINUX

 Makefile.target   | 2 +-
 configure | 2 +-
 util/mmap-alloc.c | 7 +++
 3 files changed, 5 insertions(+), 6 deletions(-)

-- 
2.24.1

Re: [PATCH] acpi: Add Windows ACPI Emulated Device Table (WAET)

2020-03-11 Thread Liran Alon




On 11/03/2020 22:36, Michael S. Tsirkin wrote:

Thanks for the patch! Some questions/comments:

On Wed, Mar 11, 2020 at 07:08:26PM +0200, Liran Alon wrote:

From: Elad Gabay 

Microsoft introduced this ACPI table to avoid Windows guests performing
various workarounds for device erratas. As the virtual device emulated
by VMM may not have the errata.

Currently, WAET allows hypervisor to inform guest about two
specific behaviors: One for RTC and the other for ACPI PM Timer.

Support for WAET have been introduced since Windows Vista. This ACPI
table is also exposed by other hypervisors, such as VMware, by default.

This patch adds WAET ACPI Table to QEMU.

Could you add a bit more info? Why is this so useful we are adding this
by default? How does it change windows behaviour when present?
It changes behavior as documented in the WAET specification linked below 
(and the comments above the flags definitions).
Specifically for ACPI_WAET_PM_TIMER_GOOD (Which is the only bit we set), 
the guest performs only one read of ACPI PM Timer instead of multiple to 
obtain it's value.

Which improves performance as it removes unnecessary VMExits.



It also makes sure to introduce
the new ACPI table only for new machine-types.

OK and why is that?
As ACPI tables are guest-visible, we should make sure to not change it 
between machine-types.
For example, a change in ACPI tables may invalidate a Windows guest 
license activation (As platform have changed).
But this is just a good practice in general and in the past it was said 
by maintainers that this is one of the main reasons that ACPI and SMBIOS 
generation have moved from SeaBIOS to QEMU.



Signed-off-by: Elad Gabay 
Co-developed-by: Liran Alon 
Signed-off-by: Liran Alon 
---
  hw/i386/acpi-build.c| 18 ++
  hw/i386/pc_piix.c   |  2 ++
  hw/i386/pc_q35.c|  2 ++
  include/hw/acpi/acpi-defs.h | 25 +
  include/hw/i386/pc.h|  1 +
  5 files changed, 48 insertions(+)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 9c4e46fa7466..29f70741cd96 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2512,6 +2512,19 @@ build_dmar_q35(GArray *table_data, BIOSLinker *linker)
  build_header(linker, table_data, (void *)(table_data->data + dmar_start),
   "DMAR", table_data->len - dmar_start, 1, NULL, NULL);
  }
+
+static void
+build_waet(GArray *table_data, BIOSLinker *linker)
+{
+AcpiTableWaet *waet;
+
+waet = acpi_data_push(table_data, sizeof(*waet));

Can combine with the previous line.

Ok. Will do in v2.



+waet->emulated_device_flags = cpu_to_le32(ACPI_WAET_PM_TIMER_GOOD);
+
+build_header(linker, table_data,
+ (void *)waet, "WAET", sizeof(*waet), 1, NULL, NULL);
+}
+
  /*
   *   IVRS table as specified in AMD IOMMU Specification v2.62, Section 5.2
   *   accessible here 
https://urldefense.com/v3/__http://support.amd.com/TechDocs/48882_IOMMU.pdf__;!!GqivPVa7Brio!OAQpLo9QhdHiNDa_aRLR_ma1nWLZU1aQhDozYgUlrqBZiz1vKdZgg-lTDMIj_5g$
@@ -2859,6 +2872,11 @@ void acpi_build(AcpiBuildTables *tables, MachineState 
*machine)
machine->nvdimms_state, machine->ram_slots);
  }
  
+if (!pcmc->do_not_add_waet_acpi) {

+acpi_add_table(table_offsets, tables_blob);
+build_waet(tables_blob, tables->linker);
+}
+
  /* Add tables supplied by user (if any) */
  for (u = acpi_table_first(); u; u = acpi_table_next(u)) {
  unsigned len = acpi_table_len(u);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 9088db8fb601..2d11a8b50a9c 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -432,9 +432,11 @@ DEFINE_I440FX_MACHINE(v5_0, "pc-i440fx-5.0", NULL,
  
  static void pc_i440fx_4_2_machine_options(MachineClass *m)

  {
+PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
  pc_i440fx_5_0_machine_options(m);
  m->alias = NULL;
  m->is_default = false;
+pcmc->do_not_add_waet_acpi = true;
  compat_props_add(m->compat_props, hw_compat_4_2, hw_compat_4_2_len);
  compat_props_add(m->compat_props, pc_compat_4_2, pc_compat_4_2_len);
  }
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 84cf925cf43a..1e0a726b27a7 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -361,8 +361,10 @@ DEFINE_Q35_MACHINE(v5_0, "pc-q35-5.0", NULL,
  
  static void pc_q35_4_2_machine_options(MachineClass *m)

  {
+PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
  pc_q35_5_0_machine_options(m);
  m->alias = NULL;
+pcmc->do_not_add_waet_acpi = true;
  compat_props_add(m->compat_props, hw_compat_4_2, hw_compat_4_2_len);
  compat_props_add(m->compat_props, pc_compat_4_2, pc_compat_4_2_len);
  }
diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
index 57a3f58b0c9a..803c904471d5 100644
--- a/include/hw/acpi/acpi-defs.h
+++ b/include/hw/acpi/acpi-defs.h
@@ -634,4 +634,29 @@ struct AcpiIortRC {
  } QEMU_PACKED;
  typedef struct AcpiIortRC

Re: [PATCH 1/2] Use -isystem for linux-headers dir

On Wed, Mar 11, 2020 at 07:08:06PM -0400, Eduardo Habkost wrote:
> On Wed, Mar 11, 2020 at 07:05:45PM -0400, Michael S. Tsirkin wrote:
> > On Wed, Mar 11, 2020 at 06:51:29PM -0400, Eduardo Habkost wrote:
> > > glibc and Linux-provided headers are known to generate macro
> > > redefinition warnings when used together.  For example:
> > >  and  duplicate some macro definitions.
> > > 
> > > We normally never see those warnings because GCC suppresses
> > > warnings generated by system headers.  We carry our own copy of
> > > Linux header files, though, and this makes those warnings not be
> > > suppressed when glibc headers are included before Linux headers
> > > (e.g. if  is included before ).
> > > 
> > > Use -isystem instead of -I for linux-headers.  This makes the
> > > compiler treat our linux-headers directory the same way it treats
> > > system-provided Linux headers, and suppress warnings generated by
> > > them.
> > > 
> > > Signed-off-by: Eduardo Habkost 
> > > ---
> > >  Makefile.target | 2 +-
> > >  configure   | 2 +-
> > >  2 files changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/Makefile.target b/Makefile.target
> > > index 2d43dc586a..934a9f7431 100644
> > > --- a/Makefile.target
> > > +++ b/Makefile.target
> > > @@ -12,7 +12,7 @@ endif
> > >  
> > >  $(call set-vpath, $(SRC_PATH):$(BUILD_DIR))
> > >  ifdef CONFIG_LINUX
> > > -QEMU_CFLAGS += -I../linux-headers
> > > +QEMU_CFLAGS += -isystem ../linux-headers
> > >  endif
> > >  QEMU_CFLAGS += -iquote .. -iquote $(SRC_PATH)/target/$(TARGET_BASE_ARCH) 
> > > -DNEED_CPU_H
> > >  
> > > diff --git a/configure b/configure
> > > index cbf864bff1..04a2a7f2dd 100755
> > > --- a/configure
> > > +++ b/configure
> > > @@ -899,7 +899,7 @@ Linux)
> > >linux="yes"
> > >linux_user="yes"
> > >kvm="yes"
> > > -  QEMU_INCLUDES="-I\$(SRC_PATH)/linux-headers -I$PWD/linux-headers 
> > > $QEMU_INCLUDES"
> > > +  QEMU_INCLUDES="-isystem \$(SRC_PATH)/linux-headers 
> > > -I$PWD/linux-headers $QEMU_INCLUDES"
> > 
> > Shouldn't both be -isystem?
> 
> I haven't noticed we had both.
> 
> This line looks weird, does anybody know why we have
> $PWD/linux-headers here?

This is why:

commit a585140dd546ffb606ec506b362ab9decf1ab14e
Author: Alexey Kardashevskiy 
Date:   Wed May 29 23:30:43 2013 +1000

   qemu: fix out of tree cross compile

   The symlink to "asm" platform linux headers is made in the build tree by
   the configure script but gcc is not told to look for them there.

   The patch fixes this.

   Signed-off-by: Alexey Kardashevskiy 
   Signed-off-by: Michael Tokarev 

-- 
Eduardo

Re: [PATCH 1/2] Use -isystem for linux-headers dir

On Wed, Mar 11, 2020 at 07:05:45PM -0400, Michael S. Tsirkin wrote:
> On Wed, Mar 11, 2020 at 06:51:29PM -0400, Eduardo Habkost wrote:
> > glibc and Linux-provided headers are known to generate macro
> > redefinition warnings when used together.  For example:
> >  and  duplicate some macro definitions.
> > 
> > We normally never see those warnings because GCC suppresses
> > warnings generated by system headers.  We carry our own copy of
> > Linux header files, though, and this makes those warnings not be
> > suppressed when glibc headers are included before Linux headers
> > (e.g. if  is included before ).
> > 
> > Use -isystem instead of -I for linux-headers.  This makes the
> > compiler treat our linux-headers directory the same way it treats
> > system-provided Linux headers, and suppress warnings generated by
> > them.
> > 
> > Signed-off-by: Eduardo Habkost 
> > ---
> >  Makefile.target | 2 +-
> >  configure   | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Makefile.target b/Makefile.target
> > index 2d43dc586a..934a9f7431 100644
> > --- a/Makefile.target
> > +++ b/Makefile.target
> > @@ -12,7 +12,7 @@ endif
> >  
> >  $(call set-vpath, $(SRC_PATH):$(BUILD_DIR))
> >  ifdef CONFIG_LINUX
> > -QEMU_CFLAGS += -I../linux-headers
> > +QEMU_CFLAGS += -isystem ../linux-headers
> >  endif
> >  QEMU_CFLAGS += -iquote .. -iquote $(SRC_PATH)/target/$(TARGET_BASE_ARCH) 
> > -DNEED_CPU_H
> >  
> > diff --git a/configure b/configure
> > index cbf864bff1..04a2a7f2dd 100755
> > --- a/configure
> > +++ b/configure
> > @@ -899,7 +899,7 @@ Linux)
> >linux="yes"
> >linux_user="yes"
> >kvm="yes"
> > -  QEMU_INCLUDES="-I\$(SRC_PATH)/linux-headers -I$PWD/linux-headers 
> > $QEMU_INCLUDES"
> > +  QEMU_INCLUDES="-isystem \$(SRC_PATH)/linux-headers -I$PWD/linux-headers 
> > $QEMU_INCLUDES"
> 
> Shouldn't both be -isystem?

I haven't noticed we had both.

This line looks weird, does anybody know why we have
$PWD/linux-headers here?

-- 
Eduardo

Re: [PATCH 2/2] mmap-alloc: Include osdep.h before checking CONFIG_LINUX

2020-03-11 Thread Michael S. Tsirkin

On Wed, Mar 11, 2020 at 06:51:30PM -0400, Eduardo Habkost wrote:
> The CONFIG_LINUX check at the top of mmap-alloc.c never worked
> because it was done before including osdep.h.
> 
> This means MAP_SYNC and MAP_SHARED_VALIDATE would always be set
> to 0 at the beginning of the file.  Luckily, this didn't break
> when using recent glibc versions (2.28+), because those macros
> were redefined by glibc headers.
> 
> Move the CONFIG_LINUX check after the main include lines, so the
> CONFIG_LINUX check works and we actually include .
> This will make MAP_SYNC and MAP_SHARED_VALIDATE available even if
> the host has an older glibc version.
> 
> Reported-by: Jingqi Liu 
> Signed-off-by: Eduardo Habkost 

Makes sense, and I guess we should CC stable on this?

Reviewed-by: Michael S. Tsirkin 

> ---
>  util/mmap-alloc.c | 7 +++
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
> index 27dcccd8ec..7c2ce98eb0 100644
> --- a/util/mmap-alloc.c
> +++ b/util/mmap-alloc.c
> @@ -9,6 +9,9 @@
>   * This work is licensed under the terms of the GNU GPL, version 2 or
>   * later.  See the COPYING file in the top-level directory.
>   */
> +#include "qemu/osdep.h"
> +#include "qemu/mmap-alloc.h"
> +#include "qemu/host-utils.h"
>  
>  #ifdef CONFIG_LINUX
>  #include 
> @@ -17,10 +20,6 @@
>  #define MAP_SHARED_VALIDATE   0x0
>  #endif /* CONFIG_LINUX */
>  
> -#include "qemu/osdep.h"
> -#include "qemu/mmap-alloc.h"
> -#include "qemu/host-utils.h"
> -
>  #define HUGETLBFS_MAGIC   0x958458f6
>  
>  #ifdef CONFIG_LINUX
> -- 
> 2.24.1

Re: [PATCH 1/2] Use -isystem for linux-headers dir

2020-03-11 Thread Michael S. Tsirkin

On Wed, Mar 11, 2020 at 06:51:29PM -0400, Eduardo Habkost wrote:
> glibc and Linux-provided headers are known to generate macro
> redefinition warnings when used together.  For example:
>  and  duplicate some macro definitions.
> 
> We normally never see those warnings because GCC suppresses
> warnings generated by system headers.  We carry our own copy of
> Linux header files, though, and this makes those warnings not be
> suppressed when glibc headers are included before Linux headers
> (e.g. if  is included before ).
> 
> Use -isystem instead of -I for linux-headers.  This makes the
> compiler treat our linux-headers directory the same way it treats
> system-provided Linux headers, and suppress warnings generated by
> them.
> 
> Signed-off-by: Eduardo Habkost 
> ---
>  Makefile.target | 2 +-
>  configure   | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Makefile.target b/Makefile.target
> index 2d43dc586a..934a9f7431 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -12,7 +12,7 @@ endif
>  
>  $(call set-vpath, $(SRC_PATH):$(BUILD_DIR))
>  ifdef CONFIG_LINUX
> -QEMU_CFLAGS += -I../linux-headers
> +QEMU_CFLAGS += -isystem ../linux-headers
>  endif
>  QEMU_CFLAGS += -iquote .. -iquote $(SRC_PATH)/target/$(TARGET_BASE_ARCH) 
> -DNEED_CPU_H
>  
> diff --git a/configure b/configure
> index cbf864bff1..04a2a7f2dd 100755
> --- a/configure
> +++ b/configure
> @@ -899,7 +899,7 @@ Linux)
>linux="yes"
>linux_user="yes"
>kvm="yes"
> -  QEMU_INCLUDES="-I\$(SRC_PATH)/linux-headers -I$PWD/linux-headers 
> $QEMU_INCLUDES"
> +  QEMU_INCLUDES="-isystem \$(SRC_PATH)/linux-headers -I$PWD/linux-headers 
> $QEMU_INCLUDES"

Shouldn't both be -isystem?

>supported_os="yes"
>libudev="yes"
>  ;;
> -- 
> 2.24.1

Re: [PATCH v8 00/18] Add Allwinner H3 SoC and Orange Pi PC Machine

2020-03-11 Thread no-reply

Patchew URL: 
https://patchew.org/QEMU/20200311221854.30370-1-nieklinnenb...@gmail.com/



Hi,

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

=== TEST SCRIPT BEGIN ===
#!/bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

PASS 1 fdc-test /x86_64/fdc/cmos
PASS 2 fdc-test /x86_64/fdc/no_media_on_start
PASS 3 fdc-test /x86_64/fdc/read_without_media
==6199==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 4 fdc-test /x86_64/fdc/media_change
PASS 5 fdc-test /x86_64/fdc/sense_interrupt
PASS 6 fdc-test /x86_64/fdc/relative_seek
---
PASS 32 test-opts-visitor /visitor/opts/range/beyond
PASS 33 test-opts-visitor /visitor/opts/dict/unvisited
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-coroutine -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="test-coroutine" 
==6240==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
==6240==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 
0x7ffc6ef02000; bottom 0x7f6fe8dc6000; size: 0x008c8613c000 (603544862720)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 1 test-coroutine /basic/no-dangling-access
---
PASS 12 test-aio /aio/event/flush
PASS 13 test-aio /aio/event/wait/no-flush-cb
PASS 14 test-aio /aio/timer/schedule
==6255==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 15 test-aio /aio/coroutine/queue-chaining
PASS 16 test-aio /aio-gsource/flush
PASS 17 test-aio /aio-gsource/bh/schedule
---
PASS 12 fdc-test /x86_64/fdc/read_no_dma_19
PASS 13 fdc-test /x86_64/fdc/fuzz-registers
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img 
tests/qtest/ide-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="ide-test" 
==6263==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 28 test-aio /aio-gsource/timer/schedule
PASS 1 ide-test /x86_64/ide/identify
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-aio-multithread -m=quick -k --tap < /dev/null | 
./scripts/tap-driver.pl --test-name="test-aio-multithread" 
==6272==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 test-aio-multithread /aio/multi/lifecycle
==6269==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 2 test-aio-multithread /aio/multi/schedule
PASS 3 test-aio-multithread /aio/multi/mutex/contended
PASS 2 ide-test /x86_64/ide/flush
PASS 4 test-aio-multithread /aio/multi/mutex/handoff
==6299==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 3 ide-test /x86_64/ide/bmdma/simple_rw
==6310==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 5 test-aio-multithread /aio/multi/mutex/mcs
PASS 4 ide-test /x86_64/ide/bmdma/trim
==6321==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 6 test-aio-multithread /aio/multi/mutex/pthread
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-throttle -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="test-throttle" 
==6328==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 test-throttle /throttle/leak_bucket
PASS 2 test-throttle /throttle/compute_wait
PASS 3 test-throttle /throttle/init
---
PASS 14 test-throttle /throttle/config/max
PASS 15 test-throttle /throttle/config/iops_size
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-thread-pool -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="test-thread-pool" 
==6332==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 test-thread-pool /thread-pool/submit
PASS 2 test-thread-pool /thread-pool/submit-aio
PASS 3 test-thread-pool /thread-pool/submit-co
---
PASS 2 test-hbitmap /hbitmap/size/0
PASS 3 test-hbitmap /hbitmap/size/unaligned
PASS 4 test-hbitmap /hbitmap/iter/empty
==6402==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 5 test-hbitmap /hbitmap/iter/partial
PASS 6 test-hbitmap

[PATCH v7 09/13] hw/i386: Introduce apicid functions inside X86MachineState

Introduce model specific apicid functions inside X86MachineState.
These functions will be loaded from X86CPUDefinition.

Signed-off-by: Babu Moger 
Reviewed-by: Igor Mammedov 
Acked-by: Michael S. Tsirkin 
---
 hw/i386/x86.c |5 +
 include/hw/i386/x86.h |9 +
 2 files changed, 14 insertions(+)

diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 42834d2319..0a81ab5151 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -911,6 +911,11 @@ static void x86_machine_initfn(Object *obj)
 x86ms->smm = ON_OFF_AUTO_AUTO;
 x86ms->max_ram_below_4g = 0; /* use default */
 x86ms->smp_dies = 1;
+
+x86ms->apicid_from_cpu_idx = x86_apicid_from_cpu_idx;
+x86ms->topo_ids_from_apicid = x86_topo_ids_from_apicid;
+x86ms->apicid_from_topo_ids = x86_apicid_from_topo_ids;
+x86ms->apicid_pkg_offset = apicid_pkg_offset;
 }
 
 static void x86_machine_class_init(ObjectClass *oc, void *data)
diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
index 22babcb3bb..2643b57629 100644
--- a/include/hw/i386/x86.h
+++ b/include/hw/i386/x86.h
@@ -65,6 +65,15 @@ typedef struct {
 
 OnOffAuto smm;
 
+/* Apic id specific handlers */
+uint32_t (*apicid_from_cpu_idx)(X86CPUTopoInfo *topo_info,
+unsigned cpu_index);
+void (*topo_ids_from_apicid)(apic_id_t apicid, X86CPUTopoInfo *topo_info,
+ X86CPUTopoIDs *topo_ids);
+apic_id_t (*apicid_from_topo_ids)(X86CPUTopoInfo *topo_info,
+  const X86CPUTopoIDs *topo_ids);
+uint32_t (*apicid_pkg_offset)(X86CPUTopoInfo *topo_info);
+
 /*
  * Address space used by IOAPIC device. All IOAPIC interrupts
  * will be translated to MSI messages in the address space.

[PATCH v7 07/13] hw/386: Add EPYC mode topology decoding functions

These functions add support for building EPYC mode topology given the smp
details like numa nodes, cores, threads and sockets.

The new apic id decoding is mostly similar to current apic id decoding
except that it adds a new field node_id when numa configured. Removes all
the hardcoded values. Subsequent patches will use these functions to build
the topology.

Following functions are added.
apicid_llc_width_epyc
apicid_llc_offset_epyc
apicid_pkg_offset_epyc
apicid_from_topo_ids_epyc
x86_topo_ids_from_idx_epyc
x86_topo_ids_from_apicid_epyc
x86_apicid_from_cpu_idx_epyc

The topology details are available in Processor Programming Reference (PPR)
for AMD Family 17h Model 01h, Revision B1 Processors. The revision guides are
available from the bugzilla Link below.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537

Signed-off-by: Babu Moger 
Acked-by: Igor Mammedov 
Acked-by: Michael S. Tsirkin 
---
 include/hw/i386/topology.h |  100 
 1 file changed, 100 insertions(+)

diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h
index b9593b9905..07239f95f4 100644
--- a/include/hw/i386/topology.h
+++ b/include/hw/i386/topology.h
@@ -47,6 +47,7 @@ typedef uint32_t apic_id_t;
 
 typedef struct X86CPUTopoIDs {
 unsigned pkg_id;
+unsigned node_id;
 unsigned die_id;
 unsigned core_id;
 unsigned smt_id;
@@ -88,6 +89,11 @@ static inline unsigned apicid_die_width(X86CPUTopoInfo 
*topo_info)
 return apicid_bitwidth_for_count(topo_info->dies_per_pkg);
 }
 
+/* Bit width of the node_id field per socket */
+static inline unsigned apicid_node_width_epyc(X86CPUTopoInfo *topo_info)
+{
+return apicid_bitwidth_for_count(MAX(topo_info->nodes_per_pkg, 1));
+}
 /* Bit offset of the Core_ID field
  */
 static inline unsigned apicid_core_offset(X86CPUTopoInfo *topo_info)
@@ -108,6 +114,100 @@ static inline unsigned apicid_pkg_offset(X86CPUTopoInfo 
*topo_info)
 return apicid_die_offset(topo_info) + apicid_die_width(topo_info);
 }
 
+#define NODE_ID_OFFSET 3 /* Minimum node_id offset if numa configured */
+
+/*
+ * Bit offset of the node_id field
+ *
+ * Make sure nodes_per_pkg >  0 if numa configured else zero.
+ */
+static inline unsigned apicid_node_offset_epyc(X86CPUTopoInfo *topo_info)
+{
+unsigned offset = apicid_die_offset(topo_info) +
+  apicid_die_width(topo_info);
+
+if (topo_info->nodes_per_pkg) {
+return MAX(NODE_ID_OFFSET, offset);
+} else {
+return offset;
+}
+}
+
+/* Bit offset of the Pkg_ID (socket ID) field */
+static inline unsigned apicid_pkg_offset_epyc(X86CPUTopoInfo *topo_info)
+{
+return apicid_node_offset_epyc(topo_info) +
+   apicid_node_width_epyc(topo_info);
+}
+
+/*
+ * Make APIC ID for the CPU based on Pkg_ID, Core_ID, SMT_ID
+ *
+ * The caller must make sure core_id < nr_cores and smt_id < nr_threads.
+ */
+static inline apic_id_t
+x86_apicid_from_topo_ids_epyc(X86CPUTopoInfo *topo_info,
+  const X86CPUTopoIDs *topo_ids)
+{
+return (topo_ids->pkg_id  << apicid_pkg_offset_epyc(topo_info)) |
+   (topo_ids->node_id << apicid_node_offset_epyc(topo_info)) |
+   (topo_ids->die_id  << apicid_die_offset(topo_info)) |
+   (topo_ids->core_id << apicid_core_offset(topo_info)) |
+   topo_ids->smt_id;
+}
+
+static inline void x86_topo_ids_from_idx_epyc(X86CPUTopoInfo *topo_info,
+  unsigned cpu_index,
+  X86CPUTopoIDs *topo_ids)
+{
+unsigned nr_nodes = MAX(topo_info->nodes_per_pkg, 1);
+unsigned nr_dies = topo_info->dies_per_pkg;
+unsigned nr_cores = topo_info->cores_per_die;
+unsigned nr_threads = topo_info->threads_per_core;
+unsigned cores_per_node = DIV_ROUND_UP((nr_dies * nr_cores * nr_threads),
+nr_nodes);
+
+topo_ids->pkg_id = cpu_index / (nr_dies * nr_cores * nr_threads);
+topo_ids->node_id = (cpu_index / cores_per_node) % nr_nodes;
+topo_ids->die_id = cpu_index / (nr_cores * nr_threads) % nr_dies;
+topo_ids->core_id = cpu_index / nr_threads % nr_cores;
+topo_ids->smt_id = cpu_index % nr_threads;
+}
+
+/*
+ * Calculate thread/core/package IDs for a specific topology,
+ * based on APIC ID
+ */
+static inline void x86_topo_ids_from_apicid_epyc(apic_id_t apicid,
+X86CPUTopoInfo *topo_info,
+X86CPUTopoIDs *topo_ids)
+{
+topo_ids->smt_id = apicid &
+~(0xUL << apicid_smt_width(topo_info));
+topo_ids->core_id =
+(apicid >> apicid_core_offset(topo_info)) &
+~(0xUL << apicid_core_width(topo_info));
+topo_ids->die_id =
+(apicid >> apicid_die_offset(topo_info)) &
+~(0xUL << apicid_die_width(topo_info));
+topo_ids->node_id =
+(apicid >>

[PATCH v7 10/13] i386: Introduce use_epyc_apic_id_encoding in X86CPUDefinition

Add a boolean variable use_epyc_apic_id_encoding in X86CPUDefinition.
This will be set if this cpu model needs to use new EPYC based
apic id encoding.

Override the handlers with EPYC based handlers if use_epyc_apic_id_encoding
is set. This will be done in x86_cpus_init.

Signed-off-by: Babu Moger 
---
 target/i386/cpu.c |   16 
 target/i386/cpu.h |1 +
 2 files changed, 17 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 7361a53166..1e4400df7a 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1614,6 +1614,10 @@ typedef struct X86CPUDefinition {
 FeatureWordArray features;
 const char *model_id;
 CPUCaches *cache_info;
+
+/* Use AMD EPYC encoding for apic id */
+bool use_epyc_apic_id_encoding;
+
 /*
  * Definitions for alternative versions of CPU model.
  * List is terminated by item with version == 0.
@@ -1655,6 +1659,18 @@ static const X86CPUVersionDefinition 
*x86_cpu_def_get_versions(X86CPUDefinition
 return def->versions ?: default_version_list;
 }
 
+bool cpu_x86_use_epyc_apic_id_encoding(const char *cpu_type)
+{
+X86CPUClass *xcc = X86_CPU_CLASS(object_class_by_name(cpu_type));
+
+assert(xcc);
+if (xcc->model && xcc->model->cpudef) {
+return xcc->model->cpudef->use_epyc_apic_id_encoding;
+} else {
+return false;
+}
+}
+
 static CPUCaches epyc_cache_info = {
 .l1d_cache = &(CPUCacheInfo) {
 .type = DATA_CACHE,
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 7e9e963d78..6e522fcd34 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1897,6 +1897,7 @@ void cpu_clear_apic_feature(CPUX86State *env);
 void host_cpuid(uint32_t function, uint32_t count,
 uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx);
 void host_vendor_fms(char *vendor, int *family, int *model, int *stepping);
+bool cpu_x86_use_epyc_apic_id_encoding(const char *cpu_type);
 
 /* helper.c */
 bool x86_cpu_tlb_fill(CPUState *cs, vaddr address, int size,

Re: [PATCH RESEND v2] block/nvme: introduce PMR support from NVMe 1.4 spec

2020-03-11 Thread Andrzej Jakowski

On 3/11/20 2:20 AM, Stefan Hajnoczi wrote:
> Please try:
> 
>   $ git grep pmem
> 
> backends/hostmem-file.c is the backend that can be used and the
> pmem_persist() API can be used to flush writes.

I've reworked this patch into hostmem-file type of backend.
>From simple tests in virtual machine: writing to PMR region
and then reading from it after VM power cycle I have observed that
there is no persistency.

I guess that persistent behavior can be achieved if memory backend file
resides on actual persistent memory in VMM. I haven't found mechanism to
persist memory backend file when it resides in the file system on block
storage. My original mmap + msync based solution worked well there.
I believe that main problem with mmap was with "ifdef _WIN32" that made it 
platform specific and w/o it patchew CI complained. 
Is there a way that I could rework mmap + msync solution so it would fit
into qemu design?

[PATCH v7 04/13] hw/i386: Remove unnecessary initialization in x86_cpu_new

The function pc_cpu_pre_plug takes care of initialization of CPUX86State.
So, remove the initialization here.

Suggested-by: Igor Mammedov 
Signed-off-by: Babu Moger 
Reviewed-by: Igor Mammedov 
Acked-by: Michael S. Tsirkin 
---
 hw/i386/x86.c |4 
 1 file changed, 4 deletions(-)

diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index dbbff46a4b..84d1944a34 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -103,13 +103,9 @@ void x86_cpu_new(X86MachineState *x86ms, int64_t apic_id, 
Error **errp)
 {
 Object *cpu = NULL;
 Error *local_err = NULL;
-CPUX86State *env = NULL;
 
 cpu = object_new(MACHINE(x86ms)->cpu_type);
 
-env = _CPU(cpu)->env;
-env->nr_dies = x86ms->smp_dies;
-
 object_property_set_uint(cpu, apic_id, "apic-id", _err);
 object_property_set_bool(cpu, true, "realized", _err);

[PATCH v7 13/13] i386: Fix pkg_id offset for EPYC cpu models

If the system is numa configured the pkg_offset needs
to be adjusted for EPYC cpu models. Fix it calling the
model specific handler.

Signed-off-by: Babu Moger 
Reviewed-by: Igor Mammedov 
Acked-by: Michael S. Tsirkin 
---
 hw/i386/pc.c  |1 +
 target/i386/cpu.c |4 ++--
 target/i386/cpu.h |1 +
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 2d7d611184..ab6da19bab 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1526,6 +1526,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
 
 env->nr_dies = x86ms->smp_dies;
 env->nr_nodes = topo_info.nodes_per_pkg;
+env->pkg_offset = x86ms->apicid_pkg_offset(_info);
 
 /*
  * If APIC ID is not set,
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 9ffaf5e2a0..c58ac38d29 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5610,7 +5610,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
 *ecx |= CPUID_TOPOLOGY_LEVEL_SMT;
 break;
 case 1:
-*eax = apicid_pkg_offset(_info);
+*eax = env->pkg_offset;
 *ebx = cs->nr_cores * cs->nr_threads;
 *ecx |= CPUID_TOPOLOGY_LEVEL_CORE;
 break;
@@ -5644,7 +5644,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
 *ecx |= CPUID_TOPOLOGY_LEVEL_CORE;
 break;
 case 2:
-*eax = apicid_pkg_offset(_info);
+*eax = env->pkg_offset;
 *ebx = env->nr_dies * cs->nr_cores * cs->nr_threads;
 *ecx |= CPUID_TOPOLOGY_LEVEL_DIE;
 break;
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 6e522fcd34..92872d2b7a 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1610,6 +1610,7 @@ typedef struct CPUX86State {
 
 unsigned nr_dies;
 unsigned nr_nodes;
+unsigned pkg_offset;
 } CPUX86State;
 
 struct kvm_msrs;

[PATCH v7 08/13] target/i386: Cleanup and use the EPYC mode topology functions

Use the new functions from topology.h and delete the unused code. Given the
sockets, nodes, cores and threads, the new functions generate apic id for EPYC
mode. Removes all the hardcoded values.

Signed-off-by: Babu Moger 
Acked-by: Michael S. Tsirkin 
Acked-by: Igor Mammedov 
---
 target/i386/cpu.c |  161 +++--
 1 file changed, 34 insertions(+), 127 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 2e5be37b21..7361a53166 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -338,68 +338,15 @@ static void encode_cache_cpuid8006(CPUCacheInfo *l2,
 }
 }
 
-/*
- * Definitions used for building CPUID Leaf 0x801D and 0x801E
- * Please refer to the AMD64 Architecture Programmer’s Manual Volume 3.
- * Define the constants to build the cpu topology. Right now, TOPOEXT
- * feature is enabled only on EPYC. So, these constants are based on
- * EPYC supported configurations. We may need to handle the cases if
- * these values change in future.
- */
-/* Maximum core complexes in a node */
-#define MAX_CCX 2
-/* Maximum cores in a core complex */
-#define MAX_CORES_IN_CCX 4
-/* Maximum cores in a node */
-#define MAX_CORES_IN_NODE 8
-/* Maximum nodes in a socket */
-#define MAX_NODES_PER_SOCKET 4
-
-/*
- * Figure out the number of nodes required to build this config.
- * Max cores in a node is 8
- */
-static int nodes_in_socket(int nr_cores)
-{
-int nodes;
-
-nodes = DIV_ROUND_UP(nr_cores, MAX_CORES_IN_NODE);
-
-   /* Hardware does not support config with 3 nodes, return 4 in that case */
-return (nodes == 3) ? 4 : nodes;
-}
-
-/*
- * Decide the number of cores in a core complex with the given nr_cores using
- * following set constants MAX_CCX, MAX_CORES_IN_CCX, MAX_CORES_IN_NODE and
- * MAX_NODES_PER_SOCKET. Maintain symmetry as much as possible
- * L3 cache is shared across all cores in a core complex. So, this will also
- * tell us how many cores are sharing the L3 cache.
- */
-static int cores_in_core_complex(int nr_cores)
-{
-int nodes;
-
-/* Check if we can fit all the cores in one core complex */
-if (nr_cores <= MAX_CORES_IN_CCX) {
-return nr_cores;
-}
-/* Get the number of nodes required to build this config */
-nodes = nodes_in_socket(nr_cores);
-
-/*
- * Divide the cores accros all the core complexes
- * Return rounded up value
- */
-return DIV_ROUND_UP(nr_cores, nodes * MAX_CCX);
-}
-
 /* Encode cache info for CPUID[801D] */
-static void encode_cache_cpuid801d(CPUCacheInfo *cache, CPUState *cs,
-uint32_t *eax, uint32_t *ebx,
-uint32_t *ecx, uint32_t *edx)
+static void encode_cache_cpuid801d(CPUCacheInfo *cache,
+   X86CPUTopoInfo *topo_info,
+   uint32_t *eax, uint32_t *ebx,
+   uint32_t *ecx, uint32_t *edx)
 {
 uint32_t l3_cores;
+unsigned nodes = MAX(topo_info->nodes_per_pkg, 1);
+
 assert(cache->size == cache->line_size * cache->associativity *
   cache->partitions * cache->sets);
 
@@ -408,10 +355,13 @@ static void encode_cache_cpuid801d(CPUCacheInfo 
*cache, CPUState *cs,
 
 /* L3 is shared among multiple cores */
 if (cache->level == 3) {
-l3_cores = cores_in_core_complex(cs->nr_cores);
-*eax |= ((l3_cores * cs->nr_threads) - 1) << 14;
+l3_cores = DIV_ROUND_UP((topo_info->dies_per_pkg *
+ topo_info->cores_per_die *
+ topo_info->threads_per_core),
+ nodes);
+*eax |= (l3_cores - 1) << 14;
 } else {
-*eax |= ((cs->nr_threads - 1) << 14);
+*eax |= ((topo_info->threads_per_core - 1) << 14);
 }
 
 assert(cache->line_size > 0);
@@ -431,55 +381,17 @@ static void encode_cache_cpuid801d(CPUCacheInfo 
*cache, CPUState *cs,
(cache->complex_indexing ? CACHE_COMPLEX_IDX : 0);
 }
 
-/* Data structure to hold the configuration info for a given core index */
-struct core_topology {
-/* core complex id of the current core index */
-int ccx_id;
-/*
- * Adjusted core index for this core in the topology
- * This can be 0,1,2,3 with max 4 cores in a core complex
- */
-int core_id;
-/* Node id for this core index */
-int node_id;
-/* Number of nodes in this config */
-int num_nodes;
-};
-
-/*
- * Build the configuration closely match the EPYC hardware. Using the EPYC
- * hardware configuration values (MAX_CCX, MAX_CORES_IN_CCX, MAX_CORES_IN_NODE)
- * right now. This could change in future.
- * nr_cores : Total number of cores in the config
- * core_id  : Core index of the current CPU
- * topo : Data structure to hold all the config info for this core index
- */
-static void build_core_topology(int nr_cores, int core_id,
-

[PATCH v7 03/13] machine: Add SMP Sockets in CpuTopology

Store the  smp sockets in CpuTopology. The socket information required to
build the apic id in EPYC mode. Right now socket information is not passed
to down when decoding the apic id. Add the socket information here.

Signed-off-by: Babu Moger 
Reviewed-by: Eduardo Habkost 
Reviewed-by: Igor Mammedov 
Acked-by: Michael S. Tsirkin 
---
 hw/core/machine.c   |1 +
 hw/i386/pc.c|1 +
 include/hw/boards.h |2 ++
 softmmu/vl.c|1 +
 4 files changed, 5 insertions(+)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 4778bc6b08..b958cd1b99 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -757,6 +757,7 @@ static void smp_parse(MachineState *ms, QemuOpts *opts)
 ms->smp.cpus = cpus;
 ms->smp.cores = cores;
 ms->smp.threads = threads;
+ms->smp.sockets = sockets;
 }
 
 if (ms->smp.cpus > 1) {
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 662abb549d..05e7f1090f 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -781,6 +781,7 @@ void pc_smp_parse(MachineState *ms, QemuOpts *opts)
 ms->smp.cpus = cpus;
 ms->smp.cores = cores;
 ms->smp.threads = threads;
+ms->smp.sockets = sockets;
 x86ms->smp_dies = dies;
 }
 
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 9bc42dfb22..d01056286a 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -236,12 +236,14 @@ typedef struct DeviceMemoryState {
  * @cpus: the number of present logical processors on the machine
  * @cores: the number of cores in one package
  * @threads: the number of threads in one core
+ * @sockets: the number of sockets on the machine
  * @max_cpus: the maximum number of logical processors on the machine
  */
 typedef struct CpuTopology {
 unsigned int cpus;
 unsigned int cores;
 unsigned int threads;
+unsigned int sockets;
 unsigned int max_cpus;
 } CpuTopology;
 
diff --git a/softmmu/vl.c b/softmmu/vl.c
index ff2685dff8..dadb798ac7 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -3935,6 +3935,7 @@ void qemu_init(int argc, char **argv, char **envp)
 current_machine->smp.max_cpus = machine_class->default_cpus;
 current_machine->smp.cores = 1;
 current_machine->smp.threads = 1;
+current_machine->smp.sockets = 1;
 
 machine_class->smp_parse(current_machine,
 qemu_opts_find(qemu_find_opts("smp-opts"), NULL));

[PATCH v7 12/13] target/i386: Enable new apic id encoding for EPYC based cpus models

The APIC ID is decoded based on the sequence sockets->dies->cores->threads.
This works fine for most standard AMD and other vendors' configurations,
but this decoding sequence does not follow that of AMD's APIC ID enumeration
strictly. In some cases this can cause CPU topology inconsistency.

When booting a guest VM, the kernel tries to validate the topology, and finds
it inconsistent with the enumeration of EPYC cpu models. The more details are
in the bug https://bugzilla.redhat.com/show_bug.cgi?id=1728166.

To fix the problem we need to build the topology as per the Processor
Programming Reference (PPR) for AMD Family 17h Model 01h, Revision B1
Processors. The documentation is available from the bugzilla Link below.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
It is also available at
https://www.amd.com/system/files/TechDocs/55570-B1_PUB.zip

Here is the text from the PPR.
Operating systems are expected to use Core::X86::Cpuid::SizeId[ApicIdSize], the
number of least significant bits in the Initial APIC ID that indicate core ID
within a processor, in constructing per-core CPUID masks.
Core::X86::Cpuid::SizeId[ApicIdSize] determines the maximum number of cores
(MNC) that the processor could theoretically support, not the actual number of
cores that are actually implemented or enabled on the processor, as indicated
by Core::X86::Cpuid::SizeId[NC].
Each Core::X86::Apic::ApicId[ApicId] register is preset as follows:
• ApicId[6] = Socket ID.
• ApicId[5:4] = Node ID.
• ApicId[3] = Logical CCX L3 complex ID
• ApicId[2:0]= (SMT) ? {LogicalCoreID[1:0],ThreadId} : {1'b0,LogicalCoreID[1:0]}

The new apic id encoding is enabled for EPYC and EPYC-Rome models.

Signed-off-by: Babu Moger 
Acked-by: Michael S. Tsirkin 
Acked-by: Igor Mammedov 
---
 target/i386/cpu.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 1e4400df7a..9ffaf5e2a0 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3925,6 +3925,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
 .xlevel = 0x801E,
 .model_id = "AMD EPYC Processor",
 .cache_info = _cache_info,
+.use_epyc_apic_id_encoding = 1,
 .versions = (X86CPUVersionDefinition[]) {
 { .version = 1 },
 {
@@ -4052,6 +4053,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
 .xlevel = 0x801E,
 .model_id = "AMD EPYC-Rome Processor",
 .cache_info = _rome_cache_info,
+.use_epyc_apic_id_encoding = 1,
 },
 };

[PATCH v7 11/13] hw/i386: Move arch_id decode inside x86_cpus_init

Apicid calculation depends on knowing the total number of numa nodes
for EPYC cpu models. Right now, we are calculating the arch_id while
parsing the numa(parse_numa). At this time, it is not known how many
total numa nodes are configured in the system.

Move the arch_id calculation inside x86_cpus_init. At this time, smp
parse is already completed and numa node information is available.

Override the handlers if use_epyc_apic_id_encoding is enabled in
cpu model definition.

Also replace the calling convention to use handlers from
X86MachineState.

Signed-off-by: Babu Moger 
---
 hw/i386/pc.c  |6 +++---
 hw/i386/x86.c |   37 ++---
 2 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 98ee763f68..2d7d611184 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1580,14 +1580,14 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
 topo_ids.die_id = cpu->die_id;
 topo_ids.core_id = cpu->core_id;
 topo_ids.smt_id = cpu->thread_id;
-cpu->apic_id = x86_apicid_from_topo_ids(_info, _ids);
+cpu->apic_id = x86ms->apicid_from_topo_ids(_info, _ids);
 }
 
 cpu_slot = pc_find_cpu_slot(MACHINE(pcms), cpu->apic_id, );
 if (!cpu_slot) {
 MachineState *ms = MACHINE(pcms);
 
-x86_topo_ids_from_apicid(cpu->apic_id, _info, _ids);
+x86ms->topo_ids_from_apicid(cpu->apic_id, _info, _ids);
 error_setg(errp,
 "Invalid CPU [socket: %u, die: %u, core: %u, thread: %u] with"
 " APIC ID %" PRIu32 ", valid index range 0:%d",
@@ -1608,7 +1608,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
 /* TODO: move socket_id/core_id/thread_id checks into x86_cpu_realizefn()
  * once -smp refactoring is complete and there will be CPU private
  * CPUState::nr_cores and CPUState::nr_threads fields instead of globals */
-x86_topo_ids_from_apicid(cpu->apic_id, _info, _ids);
+x86ms->topo_ids_from_apicid(cpu->apic_id, _info, _ids);
 if (cpu->socket_id != -1 && cpu->socket_id != topo_ids.pkg_id) {
 error_setg(errp, "property socket-id: %u doesn't match set apic-id:"
 " 0x%x (socket-id: %u)", cpu->socket_id, cpu->apic_id,
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 0a81ab5151..023dce1dbd 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -68,6 +68,22 @@ inline void init_topo_info(X86CPUTopoInfo *topo_info,
 topo_info->threads_per_core = ms->smp.threads;
 }
 
+/*
+ * Set up with the new EPYC topology handlers
+ *
+ * AMD uses different apic id encoding for EPYC based cpus. Override
+ * the default topo handlers with EPYC encoding handlers.
+ */
+static void x86_set_epyc_topo_handlers(MachineState *machine)
+{
+X86MachineState *x86ms = X86_MACHINE(machine);
+
+x86ms->apicid_from_cpu_idx = x86_apicid_from_cpu_idx_epyc;
+x86ms->topo_ids_from_apicid = x86_topo_ids_from_apicid_epyc;
+x86ms->apicid_from_topo_ids = x86_apicid_from_topo_ids_epyc;
+x86ms->apicid_pkg_offset = apicid_pkg_offset_epyc;
+}
+
 /*
  * Calculates initial APIC ID for a specific CPU index
  *
@@ -86,7 +102,7 @@ uint32_t x86_cpu_apic_id_from_index(X86MachineState *x86ms,
 
 init_topo_info(_info, x86ms);
 
-correct_id = x86_apicid_from_cpu_idx(_info, cpu_index);
+correct_id = x86ms->apicid_from_cpu_idx(_info, cpu_index);
 if (x86mc->compat_apic_id_mode) {
 if (cpu_index != correct_id && !warned && !qtest_enabled()) {
 error_report("APIC IDs set in compatibility mode, "
@@ -121,6 +137,11 @@ void x86_cpus_init(X86MachineState *x86ms, int 
default_cpu_version)
 MachineState *ms = MACHINE(x86ms);
 MachineClass *mc = MACHINE_GET_CLASS(x86ms);
 
+/* Check for apicid encoding */
+if (cpu_x86_use_epyc_apic_id_encoding(ms->cpu_type)) {
+x86_set_epyc_topo_handlers(ms);
+}
+
 x86_cpu_set_default_version(default_cpu_version);
 
 /*
@@ -134,6 +155,12 @@ void x86_cpus_init(X86MachineState *x86ms, int 
default_cpu_version)
 x86ms->apic_id_limit = x86_cpu_apic_id_from_index(x86ms,
   ms->smp.max_cpus - 1) + 
1;
 possible_cpus = mc->possible_cpu_arch_ids(ms);
+
+for (i = 0; i < ms->smp.cpus; i++) {
+ms->possible_cpus->cpus[i].arch_id =
+x86_cpu_apic_id_from_index(x86ms, i);
+}
+
 for (i = 0; i < ms->smp.cpus; i++) {
 x86_cpu_new(x86ms, possible_cpus->cpus[i].arch_id, _fatal);
 }
@@ -158,8 +185,7 @@ int64_t x86_get_default_cpu_node_id(const MachineState *ms, 
int idx)
init_topo_info(_info, x86ms);
 
assert(idx < ms->possible_cpus->len);
-   x86_topo_ids_from_apicid(ms->possible_cpus->cpus[idx].arch_id,
-_info, _ids);
+   x86_topo_ids_from_idx(_info, idx, _ids);
return topo_ids.pkg_id % ms->numa_state->num_nodes;
 }
 
@@ -190,10 +216,7 @@ const CPUArchIdList 
*x86_possible_cpu_arch_ids(MachineState *ms)

[PATCH v7 02/13] hw/i386: Consolidate topology functions

Now that we have all the parameters in X86CPUTopoInfo, we can just
pass the structure to calculate the offsets and width.

Signed-off-by: Babu Moger 
Reviewed-by: Igor Mammedov 
Acked-by: Michael S. Tsirkin 
---
 include/hw/i386/topology.h |   68 +--
 target/i386/cpu.c  |   23 +++
 tests/test-x86-cpuid.c |   69 +++-
 3 files changed, 75 insertions(+), 85 deletions(-)

diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h
index 7ea507f376..ba52d49079 100644
--- a/include/hw/i386/topology.h
+++ b/include/hw/i386/topology.h
@@ -69,56 +69,42 @@ static unsigned apicid_bitwidth_for_count(unsigned count)
 
 /* Bit width of the SMT_ID (thread ID) field on the APIC ID
  */
-static inline unsigned apicid_smt_width(unsigned nr_dies,
-unsigned nr_cores,
-unsigned nr_threads)
+static inline unsigned apicid_smt_width(X86CPUTopoInfo *topo_info)
 {
-return apicid_bitwidth_for_count(nr_threads);
+return apicid_bitwidth_for_count(topo_info->threads_per_core);
 }
 
 /* Bit width of the Core_ID field
  */
-static inline unsigned apicid_core_width(unsigned nr_dies,
- unsigned nr_cores,
- unsigned nr_threads)
+static inline unsigned apicid_core_width(X86CPUTopoInfo *topo_info)
 {
-return apicid_bitwidth_for_count(nr_cores);
+return apicid_bitwidth_for_count(topo_info->cores_per_die);
 }
 
 /* Bit width of the Die_ID field */
-static inline unsigned apicid_die_width(unsigned nr_dies,
-unsigned nr_cores,
-unsigned nr_threads)
+static inline unsigned apicid_die_width(X86CPUTopoInfo *topo_info)
 {
-return apicid_bitwidth_for_count(nr_dies);
+return apicid_bitwidth_for_count(topo_info->dies_per_pkg);
 }
 
 /* Bit offset of the Core_ID field
  */
-static inline unsigned apicid_core_offset(unsigned nr_dies,
-  unsigned nr_cores,
-  unsigned nr_threads)
+static inline unsigned apicid_core_offset(X86CPUTopoInfo *topo_info)
 {
-return apicid_smt_width(nr_dies, nr_cores, nr_threads);
+return apicid_smt_width(topo_info);
 }
 
 /* Bit offset of the Die_ID field */
-static inline unsigned apicid_die_offset(unsigned nr_dies,
-  unsigned nr_cores,
-   unsigned nr_threads)
+static inline unsigned apicid_die_offset(X86CPUTopoInfo *topo_info)
 {
-return apicid_core_offset(nr_dies, nr_cores, nr_threads) +
-   apicid_core_width(nr_dies, nr_cores, nr_threads);
+return apicid_core_offset(topo_info) + apicid_core_width(topo_info);
 }
 
 /* Bit offset of the Pkg_ID (socket ID) field
  */
-static inline unsigned apicid_pkg_offset(unsigned nr_dies,
- unsigned nr_cores,
- unsigned nr_threads)
+static inline unsigned apicid_pkg_offset(X86CPUTopoInfo *topo_info)
 {
-return apicid_die_offset(nr_dies, nr_cores, nr_threads) +
-   apicid_die_width(nr_dies, nr_cores, nr_threads);
+return apicid_die_offset(topo_info) + apicid_die_width(topo_info);
 }
 
 /* Make APIC ID for the CPU based on Pkg_ID, Core_ID, SMT_ID
@@ -128,16 +114,9 @@ static inline unsigned apicid_pkg_offset(unsigned nr_dies,
 static inline apic_id_t apicid_from_topo_ids(X86CPUTopoInfo *topo_info,
  const X86CPUTopoIDs *topo_ids)
 {
-unsigned nr_dies = topo_info->dies_per_pkg;
-unsigned nr_cores = topo_info->cores_per_die;
-unsigned nr_threads = topo_info->threads_per_core;
-
-return (topo_ids->pkg_id  <<
-   apicid_pkg_offset(nr_dies, nr_cores, nr_threads)) |
-   (topo_ids->die_id  <<
-   apicid_die_offset(nr_dies, nr_cores, nr_threads)) |
-   (topo_ids->core_id <<
-   apicid_core_offset(nr_dies, nr_cores, nr_threads)) |
+return (topo_ids->pkg_id  << apicid_pkg_offset(topo_info)) |
+   (topo_ids->die_id  << apicid_die_offset(topo_info)) |
+   (topo_ids->core_id << apicid_core_offset(topo_info)) |
topo_ids->smt_id;
 }
 
@@ -165,20 +144,15 @@ static inline void x86_topo_ids_from_apicid(apic_id_t 
apicid,
 X86CPUTopoInfo *topo_info,
 X86CPUTopoIDs *topo_ids)
 {
-unsigned nr_dies = topo_info->dies_per_pkg;
-unsigned nr_cores = topo_info->cores_per_die;
-unsigned nr_threads = topo_info->threads_per_core;
-
 topo_ids->smt_id = apicid &
-~(0xUL << apicid_smt_width(nr_dies, nr_cores, nr_threads));
+~(0xUL << apicid_smt_width(topo_info));
 topo_ids->core_id =
-

[PATCH v7 05/13] hw/i386: Update structures to save the number of nodes per package

Update structures X86CPUTopoIDs and CPUX86State to hold the number of
nodes per package. This is required to build EPYC mode topology.

Signed-off-by: Babu Moger 
Reviewed-by: Igor Mammedov 
Acked-by: Michael S. Tsirkin 
---
 hw/i386/pc.c   |1 +
 hw/i386/x86.c  |1 +
 include/hw/i386/topology.h |1 +
 target/i386/cpu.c  |1 +
 target/i386/cpu.h  |1 +
 tests/test-x86-cpuid.c |   40 
 6 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 05e7f1090f..ee89fcd1c3 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1525,6 +1525,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
 init_topo_info(_info, x86ms);
 
 env->nr_dies = x86ms->smp_dies;
+env->nr_nodes = topo_info.nodes_per_pkg;
 
 /*
  * If APIC ID is not set,
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 84d1944a34..42834d2319 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -62,6 +62,7 @@ inline void init_topo_info(X86CPUTopoInfo *topo_info,
 {
 MachineState *ms = MACHINE(x86ms);
 
+topo_info->nodes_per_pkg = ms->numa_state->num_nodes / ms->smp.sockets;
 topo_info->dies_per_pkg = x86ms->smp_dies;
 topo_info->cores_per_die = ms->smp.cores;
 topo_info->threads_per_core = ms->smp.threads;
diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h
index ba52d49079..04f01e2a09 100644
--- a/include/hw/i386/topology.h
+++ b/include/hw/i386/topology.h
@@ -53,6 +53,7 @@ typedef struct X86CPUTopoIDs {
 } X86CPUTopoIDs;
 
 typedef struct X86CPUTopoInfo {
+unsigned nodes_per_pkg;
 unsigned dies_per_pkg;
 unsigned cores_per_die;
 unsigned threads_per_core;
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 6df3127fd7..2e5be37b21 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6957,6 +6957,7 @@ static void x86_cpu_initfn(Object *obj)
 FeatureWord w;
 
 env->nr_dies = 1;
+env->nr_nodes = 1;
 cpu_set_cpustate_pointers(cpu);
 
 object_property_add(obj, "family", "int",
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 68b186d258..7e9e963d78 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1609,6 +1609,7 @@ typedef struct CPUX86State {
 TPRAccess tpr_access_type;
 
 unsigned nr_dies;
+unsigned nr_nodes;
 } CPUX86State;
 
 struct kvm_msrs;
diff --git a/tests/test-x86-cpuid.c b/tests/test-x86-cpuid.c
index bfabc0403a..049030a50e 100644
--- a/tests/test-x86-cpuid.c
+++ b/tests/test-x86-cpuid.c
@@ -31,12 +31,12 @@ static void test_topo_bits(void)
 X86CPUTopoInfo topo_info = {0};
 
 /* simple tests for 1 thread per core, 1 core per die, 1 die per package */
-topo_info = (X86CPUTopoInfo) {1, 1, 1};
+topo_info = (X86CPUTopoInfo) {0, 1, 1, 1};
 g_assert_cmpuint(apicid_smt_width(_info), ==, 0);
 g_assert_cmpuint(apicid_core_width(_info), ==, 0);
 g_assert_cmpuint(apicid_die_width(_info), ==, 0);
 
-topo_info = (X86CPUTopoInfo) {1, 1, 1};
+topo_info = (X86CPUTopoInfo) {0, 1, 1, 1};
 g_assert_cmpuint(x86_apicid_from_cpu_idx(_info, 0), ==, 0);
 g_assert_cmpuint(x86_apicid_from_cpu_idx(_info, 1), ==, 1);
 g_assert_cmpuint(x86_apicid_from_cpu_idx(_info, 2), ==, 2);
@@ -45,39 +45,39 @@ static void test_topo_bits(void)
 
 /* Test field width calculation for multiple values
  */
-topo_info = (X86CPUTopoInfo) {1, 1, 2};
+topo_info = (X86CPUTopoInfo) {0, 1, 1, 2};
 g_assert_cmpuint(apicid_smt_width(_info), ==, 1);
-topo_info = (X86CPUTopoInfo) {1, 1, 3};
+topo_info = (X86CPUTopoInfo) {0, 1, 1, 3};
 g_assert_cmpuint(apicid_smt_width(_info), ==, 2);
-topo_info = (X86CPUTopoInfo) {1, 1, 4};
+topo_info = (X86CPUTopoInfo) {0, 1, 1, 4};
 g_assert_cmpuint(apicid_smt_width(_info), ==, 2);
 
-topo_info = (X86CPUTopoInfo) {1, 1, 14};
+topo_info = (X86CPUTopoInfo) {0, 1, 1, 14};
 g_assert_cmpuint(apicid_smt_width(_info), ==, 4);
-topo_info = (X86CPUTopoInfo) {1, 1, 15};
+topo_info = (X86CPUTopoInfo) {0, 1, 1, 15};
 g_assert_cmpuint(apicid_smt_width(_info), ==, 4);
-topo_info = (X86CPUTopoInfo) {1, 1, 16};
+topo_info = (X86CPUTopoInfo) {0, 1, 1, 16};
 g_assert_cmpuint(apicid_smt_width(_info), ==, 4);
-topo_info = (X86CPUTopoInfo) {1, 1, 17};
+topo_info = (X86CPUTopoInfo) {0, 1, 1, 17};
 g_assert_cmpuint(apicid_smt_width(_info), ==, 5);
 
 
-topo_info = (X86CPUTopoInfo) {1, 30, 2};
+topo_info = (X86CPUTopoInfo) {0, 1, 30, 2};
 g_assert_cmpuint(apicid_core_width(_info), ==, 5);
-topo_info = (X86CPUTopoInfo) {1, 31, 2};
+topo_info = (X86CPUTopoInfo) {0, 1, 31, 2};
 g_assert_cmpuint(apicid_core_width(_info), ==, 5);
-topo_info = (X86CPUTopoInfo) {1, 32, 2};
+topo_info = (X86CPUTopoInfo) {0, 1, 32, 2};
 g_assert_cmpuint(apicid_core_width(_info), ==, 5);
-topo_info = (X86CPUTopoInfo) {1, 33, 2};
+topo_info = (X86CPUTopoInfo) {0, 1, 33, 2};

[PATCH v7 06/13] hw/i386: Rename apicid_from_topo_ids to x86_apicid_from_topo_ids

For consistency rename apicid_from_topo_ids to x86_apicid_from_topo_ids.
No functional change.

Signed-off-by: Babu Moger 
Reviewed-by: Igor Mammedov 
Acked-by: Michael S. Tsirkin 
---
 hw/i386/pc.c   |2 +-
 include/hw/i386/topology.h |6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index ee89fcd1c3..98ee763f68 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1580,7 +1580,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
 topo_ids.die_id = cpu->die_id;
 topo_ids.core_id = cpu->core_id;
 topo_ids.smt_id = cpu->thread_id;
-cpu->apic_id = apicid_from_topo_ids(_info, _ids);
+cpu->apic_id = x86_apicid_from_topo_ids(_info, _ids);
 }
 
 cpu_slot = pc_find_cpu_slot(MACHINE(pcms), cpu->apic_id, );
diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h
index 04f01e2a09..b9593b9905 100644
--- a/include/hw/i386/topology.h
+++ b/include/hw/i386/topology.h
@@ -112,8 +112,8 @@ static inline unsigned apicid_pkg_offset(X86CPUTopoInfo 
*topo_info)
  *
  * The caller must make sure core_id < nr_cores and smt_id < nr_threads.
  */
-static inline apic_id_t apicid_from_topo_ids(X86CPUTopoInfo *topo_info,
- const X86CPUTopoIDs *topo_ids)
+static inline apic_id_t x86_apicid_from_topo_ids(X86CPUTopoInfo *topo_info,
+ const X86CPUTopoIDs *topo_ids)
 {
 return (topo_ids->pkg_id  << apicid_pkg_offset(topo_info)) |
(topo_ids->die_id  << apicid_die_offset(topo_info)) |
@@ -165,7 +165,7 @@ static inline apic_id_t 
x86_apicid_from_cpu_idx(X86CPUTopoInfo *topo_info,
 {
 X86CPUTopoIDs topo_ids;
 x86_topo_ids_from_idx(topo_info, cpu_index, _ids);
-return apicid_from_topo_ids(topo_info, _ids);
+return x86_apicid_from_topo_ids(topo_info, _ids);
 }
 
 #endif /* HW_I386_TOPOLOGY_H */

[PATCH v7 00/13] APIC ID fixes for AMD EPYC CPU model

This series fixes APIC ID encoding problem reported on AMD EPYC cpu models.
https://bugzilla.redhat.com/show_bug.cgi?id=1728166

Currently, the APIC ID is decoded based on the sequence
sockets->dies->cores->threads. This works for most standard AMD and other
vendors' configurations, but this decoding sequence does not follow that of
AMD's APIC ID enumeration strictly. In some cases this can cause CPU topology
inconsistency. When booting a guest VM, the kernel tries to validate the
topology, and finds it inconsistent with the enumeration of EPYC cpu models.

To fix the problem we need to build the topology as per the Processor
Programming Reference (PPR) for AMD Family 17h Model 01h, Revision B1
Processors. The documentation is available from the bugzilla Link below.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537

Here is the text from the PPR.
Operating systems are expected to use Core::X86::Cpuid::SizeId[ApicIdSize], the
number of least significant bits in the Initial APIC ID that indicate core ID
within a processor, in constructing per-core CPUID masks.
Core::X86::Cpuid::SizeId[ApicIdSize] determines the maximum number of cores
(MNC) that the processor could theoretically support, not the actual number of
cores that are actually implemented or enabled on the processor, as indicated
by Core::X86::Cpuid::SizeId[NC].
Each Core::X86::Apic::ApicId[ApicId] register is preset as follows:
• ApicId[6] = Socket ID.
• ApicId[5:4] = Node ID.
• ApicId[3] = Logical CCX L3 complex ID
• ApicId[2:0]= (SMT) ? {LogicalCoreID[1:0],ThreadId} : {1'b0,LogicalCoreID[1:0]}

v7:
Generated the patches on top of git://github.com/ehabkost/qemu.git (x86-next).
Changes from v6.
1. Added new function x86_set_epyc_topo_handlers to override the apic id
encoding handlers.
2. Separated the code to set use_epyc_apic_id_encoding and added as a new patch
as it looked more logical.
3. Fixed minor typos.

v6:

https://lore.kernel.org/qemu-devel/158389385028.22020.7608244627303132902.st...@naples-babu.amd.com/
Generated the patches on top of git://github.com/ehabkost/qemu.git (x86-next).
Changes from v5.
1. Eduardo has already queued couple of patches, submitting the rest here.
2. Major change is how the EPYC mode apic id encoding handlers are loaded.
Added a boolean variable use_epyc_apic_id_encoding in X86CPUDefinition.
The variable is will be used to tell if we need to use EPYC mode encoding.
3. Eduardo reported bysectability problem with x86 unit test code.
Quashed the patches in 1 and 2 to resolve it. Problem was change in calling
conventions of topology related functions.
4. Also set the use_epyc_apic_id_encoding for EPYC-Rome. This model is
added recently to the cpu table.

v5:

https://lore.kernel.org/qemu-devel/158326531474.40452.11433722850425537745.st...@naples-babu.amd.com/
Generated the patches on top of git://github.com/ehabkost/qemu.git (x86-next).
Changes from v4.
1. Re-arranged the patches 2 and 4 as suggested by Igor.
2. Kept the apicid handler functions inside X86MachineState as discussed.
These handlers are loaded from X86CPUDefinitions.
3. Removed unnecessary X86CPUstate initialization from x86_cpu_new. Suggested
by Igor.
4. And other minor changes related to patch format.

v4:

https://lore.kernel.org/qemu-devel/158161767653.48948.10578064482878399556.st...@naples-babu.amd.com/
Changes from v3.
1. Moved the arch_id calculation inside the function x86_cpus_init. With this
change,
we dont need to change common numa code.(suggested by Igor)
2. Introduced the model specific handlers inside X86CPUDefinitions.
These handlers are loaded into X86MachineState during the init.
3. Removed llc_id from x86CPU.
4. Removed init_apicid_fn hanlder from MachineClass. Kept all the code changes
inside the x86.
5. Added new handler function apicid_pkg_offset for pkg_offset calculation.
6. And some Other minor changes.

v3:

https://lore.kernel.org/qemu-devel/157541968844.46157.17994918142533791313.st...@naples-babu.amd.com/

1. Consolidated the topology information in structure X86CPUTopoInfo.
2. Changed the ccx_id to llc_id as commented by upstream.
3. Generalized the apic id decoding. It is mostly similar to current apic id
except that it adds new field llc_id when numa configured. Removes all the
hardcoded values.
4. Removed the earlier parse_numa split. And moved the numa node
initialization
inside the numa_complete_configuration. This is bit cleaner as commented
by
Eduardo.
5. Added new function init_apicid_fn inside machine_class structure. This
will be used to update the apic id handler specific to cpu model.
6. Updated the cpuid unit tests.
7. TODO : Need to figure out how to dynamically update the handlers using cpu
models.
I might some guidance on that.

v2:

https://lore.kernel.org/qemu-devel/156779689013.21957.1631551572950676212.stgit@localhost.localdomain/
1. Introduced the new property

[PATCH 2/2] mmap-alloc: Include osdep.h before checking CONFIG_LINUX

The CONFIG_LINUX check at the top of mmap-alloc.c never worked
because it was done before including osdep.h.

This means MAP_SYNC and MAP_SHARED_VALIDATE would always be set
to 0 at the beginning of the file.  Luckily, this didn't break
when using recent glibc versions (2.28+), because those macros
were redefined by glibc headers.

Move the CONFIG_LINUX check after the main include lines, so the
CONFIG_LINUX check works and we actually include .
This will make MAP_SYNC and MAP_SHARED_VALIDATE available even if
the host has an older glibc version.

Reported-by: Jingqi Liu 
Signed-off-by: Eduardo Habkost 
---
 util/mmap-alloc.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
index 27dcccd8ec..7c2ce98eb0 100644
--- a/util/mmap-alloc.c
+++ b/util/mmap-alloc.c
@@ -9,6 +9,9 @@
  * This work is licensed under the terms of the GNU GPL, version 2 or
  * later.  See the COPYING file in the top-level directory.
  */
+#include "qemu/osdep.h"
+#include "qemu/mmap-alloc.h"
+#include "qemu/host-utils.h"
 
 #ifdef CONFIG_LINUX
 #include 
@@ -17,10 +20,6 @@
 #define MAP_SHARED_VALIDATE   0x0
 #endif /* CONFIG_LINUX */
 
-#include "qemu/osdep.h"
-#include "qemu/mmap-alloc.h"
-#include "qemu/host-utils.h"
-
 #define HUGETLBFS_MAGIC   0x958458f6
 
 #ifdef CONFIG_LINUX
-- 
2.24.1

[PATCH v7 01/13] hw/i386: Introduce X86CPUTopoInfo to contain topology info

This is an effort to re-arrange few data structure for better readability.

1. Add X86CPUTopoInfo which will have all the topology informations
   required to build the cpu topology. There is no functional changes.

2. Introduce init_topo_info to initialize X86CPUTopoInfo members from
   X86MachineState.

3. Update x86 unit tests for new calling convention with parameter 
X86CPUTopoInfo

There is no functional changes.

Signed-off-by: Babu Moger 
---
 hw/i386/pc.c   |   12 ++--
 hw/i386/x86.c  |   32 
 include/hw/i386/topology.h |   38 --
 include/hw/i386/x86.h  |3 +++
 tests/test-x86-cpuid.c |   43 ---
 5 files changed, 81 insertions(+), 47 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index f52e84b2ba..662abb549d 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1513,6 +1513,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
 X86MachineState *x86ms = X86_MACHINE(pcms);
 unsigned int smp_cores = ms->smp.cores;
 unsigned int smp_threads = ms->smp.threads;
+X86CPUTopoInfo topo_info;
 
 if(!object_dynamic_cast(OBJECT(cpu), ms->cpu_type)) {
 error_setg(errp, "Invalid CPU type, expected cpu type: '%s'",
@@ -1520,6 +1521,8 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
 return;
 }
 
+init_topo_info(_info, x86ms);
+
 env->nr_dies = x86ms->smp_dies;
 
 /*
@@ -1575,16 +1578,14 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
 topo_ids.die_id = cpu->die_id;
 topo_ids.core_id = cpu->core_id;
 topo_ids.smt_id = cpu->thread_id;
-cpu->apic_id = apicid_from_topo_ids(x86ms->smp_dies, smp_cores,
-smp_threads, _ids);
+cpu->apic_id = apicid_from_topo_ids(_info, _ids);
 }
 
 cpu_slot = pc_find_cpu_slot(MACHINE(pcms), cpu->apic_id, );
 if (!cpu_slot) {
 MachineState *ms = MACHINE(pcms);
 
-x86_topo_ids_from_apicid(cpu->apic_id, x86ms->smp_dies,
- smp_cores, smp_threads, _ids);
+x86_topo_ids_from_apicid(cpu->apic_id, _info, _ids);
 error_setg(errp,
 "Invalid CPU [socket: %u, die: %u, core: %u, thread: %u] with"
 " APIC ID %" PRIu32 ", valid index range 0:%d",
@@ -1605,8 +1606,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
 /* TODO: move socket_id/core_id/thread_id checks into x86_cpu_realizefn()
  * once -smp refactoring is complete and there will be CPU private
  * CPUState::nr_cores and CPUState::nr_threads fields instead of globals */
-x86_topo_ids_from_apicid(cpu->apic_id, x86ms->smp_dies,
- smp_cores, smp_threads, _ids);
+x86_topo_ids_from_apicid(cpu->apic_id, _info, _ids);
 if (cpu->socket_id != -1 && cpu->socket_id != topo_ids.pkg_id) {
 error_setg(errp, "property socket-id: %u doesn't match set apic-id:"
 " 0x%x (socket-id: %u)", cpu->socket_id, cpu->apic_id,
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 322fb6abbc..dbbff46a4b 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -57,6 +57,16 @@
 /* Physical Address of PVH entry point read from kernel ELF NOTE */
 static size_t pvh_start_addr;
 
+inline void init_topo_info(X86CPUTopoInfo *topo_info,
+   const X86MachineState *x86ms)
+{
+MachineState *ms = MACHINE(x86ms);
+
+topo_info->dies_per_pkg = x86ms->smp_dies;
+topo_info->cores_per_die = ms->smp.cores;
+topo_info->threads_per_core = ms->smp.threads;
+}
+
 /*
  * Calculates initial APIC ID for a specific CPU index
  *
@@ -68,13 +78,14 @@ static size_t pvh_start_addr;
 uint32_t x86_cpu_apic_id_from_index(X86MachineState *x86ms,
 unsigned int cpu_index)
 {
-MachineState *ms = MACHINE(x86ms);
 X86MachineClass *x86mc = X86_MACHINE_GET_CLASS(x86ms);
+X86CPUTopoInfo topo_info;
 uint32_t correct_id;
 static bool warned;
 
-correct_id = x86_apicid_from_cpu_idx(x86ms->smp_dies, ms->smp.cores,
- ms->smp.threads, cpu_index);
+init_topo_info(_info, x86ms);
+
+correct_id = x86_apicid_from_cpu_idx(_info, cpu_index);
 if (x86mc->compat_apic_id_mode) {
 if (cpu_index != correct_id && !warned && !qtest_enabled()) {
 error_report("APIC IDs set in compatibility mode, "
@@ -145,19 +156,22 @@ int64_t x86_get_default_cpu_node_id(const MachineState 
*ms, int idx)
 {
X86CPUTopoIDs topo_ids;
X86MachineState *x86ms = X86_MACHINE(ms);
+   X86CPUTopoInfo topo_info;
+
+   init_topo_info(_info, x86ms);
 
assert(idx < ms->possible_cpus->len);
x86_topo_ids_from_apicid(ms->possible_cpus->cpus[idx].arch_id,
-x86ms->smp_dies, ms->smp.cores,
-ms->smp.threads, _ids);
+_info,

[PATCH 0/2] Fix MAP_SYNC support when host has older glibc version

This is an alternative to the patch submitted at:

  From: Jingqi Liu 
  Subject: [PATCH] util: fix to get configuration macros in util/mmap-alloc.c
  Date: Thu,  5 Mar 2020 23:41:42 +0800
  Message-Id: <20200305154142.63070-1-jingqi@intel.com>

Before moving the osdep.h include to the top of the file, we had
to address warnings triggered when  was included
after  (done in patch 1/2).

Eduardo Habkost (2):
  Use -isystem for linux-headers dir
  mmap-alloc: Include osdep.h before checking CONFIG_LINUX

 Makefile.target   | 2 +-
 configure | 2 +-
 util/mmap-alloc.c | 7 +++
 3 files changed, 5 insertions(+), 6 deletions(-)

-- 
2.24.1

[PATCH 1/2] Use -isystem for linux-headers dir