Re: [RFC QEMU PATCH v3 1/1] xen: Use gsi instead of irq for mapping pirq

[Chen, Jiqian]

On 2023/12/11 23:33, Roger Pau Monné wrote:
> On Mon, Dec 11, 2023 at 12:52:40AM +0800, Jiqian Chen wrote:
>> In PVH dom0, it uses the linux local interrupt mechanism,
>> when it allocs irq for a gsi, it is dynamic, and follow
>> the principle of applying first, distributing first. And
>> the irq number is alloced from small to large, but the
>> applying gsi number is not, may gsi 38 comes before gsi
>> 28, that causes the irq number is not equal with the gsi
>> number. And when passthrough a device, qemu wants to use
>> gsi to map pirq, xen_pt_realize->xc_physdev_map_pirq, but
>> the gsi number is got from file
>> /sys/bus/pci/devices//irq in current code, so it
>> will fail when mapping.
>>
>> Use real gsi number read from gsi sysfs.
>>
>> Co-developed-by: Huang Rui 
>> Signed-off-by: Jiqian Chen 
>> ---
>>  hw/xen/xen-host-pci-device.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/xen/xen-host-pci-device.c b/hw/xen/xen-host-pci-device.c
>> index 8c6e9a1716..e270ac2631 100644
>> --- a/hw/xen/xen-host-pci-device.c
>> +++ b/hw/xen/xen-host-pci-device.c
>> @@ -364,7 +364,7 @@ void xen_host_pci_device_get(XenHostPCIDevice *d, 
>> uint16_t domain,
>>  }
>>  d->device_id = v;
>>  
>> -xen_host_pci_get_dec_value(d, "irq", , errp);
>> +xen_host_pci_get_dec_value(d, "gsi", , errp);
> 
> Don't you need to fallthrough to use the irq number on failure?
> Otherwise passthrough won't work on older Linux versions that don't
> expose the gsi node.
You are right, I will use the irq if there isn't a gsi sysfs, in next version. 
Thank you.

> 
> Thanks, Roger.

-- 
Best regards,
Jiqian Chen.

Re: [RFC] string-output-visitor: show structs as ""

[Markus Armbruster]

Stefan Hajnoczi  writes:

> StringOutputVisitor crashes when it visits a struct because
> ->start_struct() is NULL.
>
> Show "" instead of crashing. This is necessary because the
> virtio-blk-pci iothread-vq-mapping parameter that I'd like to introduce
> soon is a list of IOThreadMapping structs.
>
> Cc: Markus Armbruster 
> Signed-off-by: Stefan Hajnoczi 
> ---
> Can we do better?
>
> I am unfamiliar with StringOutputVisitor, so I wasn't sure how to
> proceed. Is the format or at least the intended use of
> StringOutputVisitor's output defined somewhere?

Bwuahahahaha!

SCNR

> Does it need to be a
> single line or can the output be multiple lines?

I'm afraid we need to review its users to be sure.

> Or maybe I shouldn't introduce a qdev property with IOThreadMappingList
> as its type in
> https://lore.kernel.org/qemu-devel/zuopifxiiwfq5...@redhat.com/?

QOM initially supported only scalar properties.

I think maintaining this restriction will lead to awkward interfaces.
Lists are clearly useful.  And then we'll likely want list of struct,
not multiple lists.

Lifting the restriction will take some work.

On the string visitors, see also my musings in

Subject: Re: [PATCH v2 1/2] qdev: add IOThreadVirtQueueMappingList property 
type 
Date: Mon, 11 Dec 2023 16:32:06 +0100
Message-ID: <87msugah6x@pond.sub.org>

> ---
>  include/qapi/string-output-visitor.h |  6 +++---
>  qapi/string-output-visitor.c | 14 ++
>  2 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/include/qapi/string-output-visitor.h 
> b/include/qapi/string-output-visitor.h
> index 268dfe9986..762fe3f705 100644
> --- a/include/qapi/string-output-visitor.h
> +++ b/include/qapi/string-output-visitor.h
> @@ -26,9 +26,9 @@ typedef struct StringOutputVisitor StringOutputVisitor;
>   * If everything else succeeds, pass @result to visit_complete() to
>   * collect the result of the visit.
>   *
> - * The string output visitor does not implement support for visiting
> - * QAPI structs, alternates, null, or arbitrary QTypes.  It also
> - * requires a non-null list argument to visit_start_list().
> + * The string output visitor does not implement support for alternates, null,
> + * or arbitrary QTypes.  It also requires a non-null list argument to
> + * visit_start_list().

Mention output for structs is information-free?

>   */
>  Visitor *string_output_visitor_new(bool human, char **result);
>  
> diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
> index c0cb72dbe4..363dac00fe 100644
> --- a/qapi/string-output-visitor.c
> +++ b/qapi/string-output-visitor.c
> @@ -292,6 +292,18 @@ static bool print_type_null(Visitor *v, const char 
> *name, QNull **obj,
>  return true;
>  }
>  
> +static bool start_struct(Visitor *v, const char *name, void **obj,
> + size_t size, Error **errp)
> +{
> +return true;
> +}
> +
> +static void end_struct(Visitor *v, void **obj)
> +{
> +StringOutputVisitor *sov = to_sov(v);
> +string_output_set(sov, g_strdup(""));

TODO comment?

> +}
> +
>  static bool
>  start_list(Visitor *v, const char *name, GenericList **list, size_t size,
> Error **errp)
> @@ -379,6 +391,8 @@ Visitor *string_output_visitor_new(bool human, char 
> **result)
>  v->visitor.type_str = print_type_str;
>  v->visitor.type_number = print_type_number;
>  v->visitor.type_null = print_type_null;
> +v->visitor.start_struct = start_struct;
> +v->visitor.end_struct = end_struct;
>  v->visitor.start_list = start_list;
>  v->visitor.next_list = next_list;
>  v->visitor.end_list = end_list;

Re: [PATCH v9 0/9] Unified CPU type check

[Gavin Shan]

Hi Phil,

On 12/4/23 10:47, Gavin Shan wrote:

This series bases on Phil's repository because the prepatory commits
have been queued to the branch.

   https://gitlab.com/philmd/qemu.git (branch: cpus-next)

There are two places where the user specified CPU type is checked to see
if it's supported or allowed by the board: machine_run_board_init() and
mc->init(). We don't have to maintain two duplicate sets of logic. This
series intends to move the check to machine_run_board_init() so that we
have unified CPU type check.

This series can be checked out from:

   g...@github.com:gwshan/qemu.git (branch: kvm/cpu-type)

PATCH[1-4] refactors and improves the logic to validate CPU type in
machine_run_board_init()
PATCH[5-9] validates the CPU type in machine_run_board_init() for the
individual boards

v6: https://lists.nongnu.org/archive/html/qemu-arm/2023-11/msg00768.html
v7: https://lists.nongnu.org/archive/html/qemu-arm/2023-11/msg01045.html
v8: https://lists.nongnu.org/archive/html/qemu-arm/2023-11/msg01168.html



Ping to see if there is a chance to queue it up before the Chrismas? :)

Thanks,
Gavin

Re: [PATCH 0/3] Support for Zoned UFS

[Jeuk Kim]

I've already done all the ufs related review.


If the SCSI maintainers approve this patchset, I'll put it in my tree 
and create a pull request.



Thank you,

Jeuk


On 12/8/2023 3:09 PM, Daejun Park wrote:

This patch enables zoned support for UFS devices.
By applying this patch, a QEMU run can use parameters to configure whether
each LU of each UFS is zoned, and the capacity, size, and max open zones.
Zoned UFS is implemented by referencing ZBC2.
(https://www.t10.org/members/w_zbc2.htm)

Daejun Park (3):
   hw/ufs: Support for Zoned UFS
   hw/scsi: add mode sense support for zbc device
   tests/qtest: Add tests for Zoned UFS

  hw/scsi/scsi-disk.c|  13 +-
  hw/ufs/lu.c| 616 +
  hw/ufs/ufs.c   |   6 +-
  hw/ufs/ufs.h   |  32 +++
  include/block/ufs.h|  31 +++
  tests/qtest/ufs-test.c | 178 
  6 files changed, 870 insertions(+), 6 deletions(-)

Re: [PATCH 1/3] hw/ufs: Support for Zoned UFS

[Jeuk Kim]

On 12/8/2023 3:13 PM, Daejun Park wrote:

This patch enables zoned ufs support.

By setting the LU parameter, each LU can be a host-managed zoned device.
This patch manages the zone condition and write pointer of each zone for
a zoned LU. It supports the report zones and reset write pointer commands
for Zoned LUs.

Signed-off-by: Daejun Park 


Reviewed-by: Jeuk Kim 

Re: [PATCH 3/3] tests/qtest: Add tests for Zoned UFS

[Jeuk Kim]

On 12/8/2023 3:22 PM, Daejun Park wrote:

This patch includes the following tests
   Test VPD page and report zones
   Test write and unaligned write error

Signed-off-by: Daejun Park 


Reviewed-by: Jeuk Kim 

RE: [PATCH for-9.0 03/10] vfio/container: Initialize VFIOIOMMUOps under vfio_init_container()

[Duan, Zhenzhong]

>-Original Message-
>From: Cédric Le Goater 
>Subject: Re: [PATCH for-9.0 03/10] vfio/container: Initialize VFIOIOMMUOps
>under vfio_init_container()
>
>On 12/11/23 06:59, Duan, Zhenzhong wrote:
>>
>>
>>> -Original Message-
>>> From: Cédric Le Goater 
>>> Sent: Friday, December 8, 2023 4:46 PM
>>> Subject: [PATCH for-9.0 03/10] vfio/container: Initialize VFIOIOMMUOps
>>> under vfio_init_container()
>>>
>>> vfio_init_container() already defines the IOMMU type of the container.
>>> Do the same for the VFIOIOMMUOps struct. This prepares ground for the
>>> following patches that will deduce the associated VFIOIOMMUOps struct
>>>from the IOMMU type.
>>>
>>> Signed-off-by: Cédric Le Goater 
>>> ---
>>> hw/vfio/container.c | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
>>> index
>>>
>afcfe8048805c58291d1104ff0ef20bdc457f99c..f4a0434a5239bfb6a17b91c8
>>> 879cb98e686afccc 100644
>>> --- a/hw/vfio/container.c
>>> +++ b/hw/vfio/container.c
>>> @@ -370,7 +370,7 @@ static int vfio_get_iommu_type(VFIOContainer
>>> *container,
>>> }
>>>
>>> static int vfio_init_container(VFIOContainer *container, int group_fd,
>>> -   Error **errp)
>>> +   VFIOAddressSpace *space, Error **errp)
>>> {
>>>  int iommu_type, ret;
>>>
>>> @@ -401,6 +401,7 @@ static int vfio_init_container(VFIOContainer
>>> *container, int group_fd,
>>>  }
>>>
>>>  container->iommu_type = iommu_type;
>>> +vfio_container_init(>bcontainer, space, _legacy_ops);
>>
>> Reviewed-by: Zhenzhong Duan 
>>
>> Not related to this patch, not clear if it's deserved to rename
>> vfio_container_init as vfio_bcontainer_init to distinguish
>> with vfio_init_container.
>
>I agree, the vfio_container_init() and vfio_init_container() names
>are confusing. I would keep vfio_container_init() because it is
>consistent with all routines handling 'VFIOContainerBase *' ops.

Oh, yes, that's better.

>
>I would be tempted to rename vfio_init_container() to vfio_set_iommu() ?

Sounds good.

Thanks
Zhenzhong

>
>Also, I will introduce a vfio_connect_setup() helper in v2 doing the
>assert as the other routines.
>
>Thanks,
>
>C.
>
>
>
>
>
>>
>> Thanks
>> Zhenzhong
>>
>>>  return 0;
>>> }
>>>
>>> @@ -583,9 +584,8 @@ static int vfio_connect_container(VFIOGroup
>*group,
>>> AddressSpace *as,
>>>  container = g_malloc0(sizeof(*container));
>>>  container->fd = fd;
>>>  bcontainer = >bcontainer;
>>> -vfio_container_init(bcontainer, space, _legacy_ops);
>>>
>>> -ret = vfio_init_container(container, group->fd, errp);
>>> +ret = vfio_init_container(container, group->fd, space, errp);
>>>  if (ret) {
>>>  goto free_container_exit;
>>>  }
>>> --
>>> 2.43.0
>>

Re: [PATCH RFC v2 00/12] virtio-net: add support for SR-IOV emulation

[Jason Wang]

On Mon, Dec 11, 2023 at 4:29 PM Akihiko Odaki  wrote:
>
> On 2023/12/11 16:26, Jason Wang wrote:
> > On Mon, Dec 11, 2023 at 1:30 PM Akihiko Odaki  
> > wrote:
> >>
> >> On 2023/12/11 11:52, Jason Wang wrote:
> >>> On Sun, Dec 10, 2023 at 12:06 PM Akihiko Odaki  
> >>> wrote:
> 
>  Introduction
>  
> 
>  This series is based on the RFC series submitted by Yui Washizu[1].
>  See also [2] for the context.
> 
>  This series enables SR-IOV emulation for virtio-net. It is useful
>  to test SR-IOV support on the guest, or to expose several vDPA devices
>  in a VM. vDPA devices can also provide L2 switching feature for
>  offloading though it is out of scope to allow the guest to configure
>  such a feature.
> 
>  The PF side code resides in virtio-pci. The VF side code resides in
>  the PCI common infrastructure, but it is restricted to work only for
>  virtio-net-pci because of lack of validation.
> 
>  User Interface
>  --
> 
>  A user can configure a SR-IOV capable virtio-net device by adding
>  virtio-net-pci functions to a bus. Below is a command line example:
>  -netdev user,id=n -netdev user,id=o
>  -netdev user,id=p -netdev user,id=q
>  -device pcie-root-port,id=b
>  -device virtio-net-pci,bus=b,addr=0x0.0x3,netdev=q,sriov-pf=f
>  -device virtio-net-pci,bus=b,addr=0x0.0x2,netdev=p,sriov-pf=f
>  -device virtio-net-pci,bus=b,addr=0x0.0x1,netdev=o,sriov-pf=f
>  -device virtio-net-pci,bus=b,addr=0x0.0x0,netdev=n,id=f
> 
>  The VFs specify the paired PF with "sriov-pf" property. The PF must be
>  added after all VFs. It is user's responsibility to ensure that VFs have
>  function numbers larger than one of the PF, and the function numbers
>  have a consistent stride.
> >>>
> >>> This seems not user friendly. Any reason we can't just allow user to
> >>> specify the stride here?
> >>
> >> It should be possible to assign addr automatically without requiring
> >> user to specify the stride. I'll try that in the next version.
> >>
> >>>
> >>> Btw, I vaguely remember qemu allows the params to be accepted as a
> >>> list. If this is true, we can accept a list of netdev here?
> >>
> >> Yes, rocker does that. But the problem is not just about getting
> >> parameters needed for VFs, which I forgot to mention in the cover letter
> >> and will explain below.
> >>
> >>>
> 
>  Keeping VF instances
>  
> 
>  A problem with SR-IOV emulation is that it needs to hotplug the VFs as
>  the guest requests. Previously, this behavior was implemented by
>  realizing and unrealizing VFs at runtime. However, this strategy does
>  not work well for the proposed virtio-net emulation; in this proposal,
>  device options passed in the command line must be maintained as VFs
>  are hotplugged, but they are consumed when the machine starts and not
>  available after that, which makes realizing VFs at runtime impossible.
> >>>
> >>> Could we store the device options in the PF?
> >>
> >> I wrote it's to store the device options, but the problem is actually
> >> more about realizing VFs at runtime instead of at the initialization time.
> >>
> >> Realizing VFs at runtime have two major problems. One is that it delays
> >> the validations of options; invalid options will be noticed when the
> >> guest requests to realize VFs.
> >
> > If PCI spec allows the failure when creating VF, then it should not be
> > a problem.
>
> I doubt the spec cares such a failure at all. VF enablement should
> always work for a real hardware. It's neither user-friendly to tell
> configuration errors at runtime.

I'm not sure which options we should care about? Did you mean netdev
options or the virtio-net specific ones?

If VF stick to the same options as PF (except for the SRIOV), it
should be validated during the PF initialization.

>
> >
> >> netdevs also warn that they are not used
> >> at initialization time, not knowing that they will be used by VFs later.
> >
> > We could invent things to calm down this false positive.
> >
> >> References to other QEMU objects in the option may also die before VFs
> >> are realized.
> >
> > Is there any other thing than netdev we need to consider?
>
> You will also want to set a distinct mac for each VF. Other properties
> does not matter much in my opinion.

Qemu doesn't check mac duplication now. So it's up to the mgmt layer.

>
> >
> >>
> >> The other problem is that QEMU cannot interact with the unrealized VFs.
> >> For example, if you type "device_add virtio-net-pci,id=vf,sriov-pf=pf"
> >> in HMP, you will expect "device_del vf" works, but it's hard to
> >> implement such behaviors with unrealized VFs.
> >
> > I think hotplug can only be done at PF level if we do that.
>
> Assuming you mean to let netdev and mac accept arrays, yes.
>
> >
> >>
> >> I was first going to 

Re: [PATCH v7 1/5] ebpf: Added eBPF map update through mmap.

[Jason Wang]

On Mon, Dec 11, 2023 at 10:07 PM Akihiko Odaki  wrote:
>
> On 2023/12/11 22:48, Yuri Benditovich wrote:
> > Akihiko,
> > This series was already discussed several months ago.
> > I'd suggest to postpone commenting on it and resume them after merging.
>
> I found a pull request:
> https://lore.kernel.org/all/20230908064507.14596-14-jasow...@redhat.com/
>
> Strangely patches from that series seem missed although earlier patches
> are on the tree. Jason, can you tell what's going on?

It should be my bad. eBPF RSS series were in V1 of the PULL request
but missed accidentally in V2.


>
> In any case, I wrote comments to the patch series. Andrew, can you check
> them? They are mostly nitpicks, but I think you may have a look at
> DEFINE_PROP_ARRAY(); it may make it easier to implement the libvirt side.
>
> I also forgot to say that properties should not have underscores;
> ebpf_rss_fds should be ebpf-rss-fds. See:
> https://gitlab.com/qemu-project/qemu/-/blob/master/include/qom/object.h?ref_type=heads#L1013
>
> The series needs to be rebased too, but probably it's better off to wait
> Jason to figure out the current situation of the series. Once it gets
> all sorted out, I'll rebase my series on top of it and ask for review.

Great, the pull request will be soon when 8.2 is released.

Thanks

>
> Regards,
> Akihiko Odaki
>

Re: [PATCH v7 5/5] ebpf: Updated eBPF program and skeleton.

[Jason Wang]

On Mon, Dec 11, 2023 at 7:51 PM Yuri Benditovich
 wrote:
>
> Hello Jason,
> Can you please let us know what happens with this series?

It should be my bad, it is in V1 of the pull request but missed
accidentally in V2 of the pull.

I've merged it here,

https://gitlab.com/jasowang/qemu.git

Please check if it's correct.

Thanks

>
> Thanks
> Yuri
>
> On Fri, Sep 8, 2023 at 9:43 AM Jason Wang  wrote:
>>
>> On Mon, Sep 4, 2023 at 7:23 PM Andrew Melnichenko  wrote:
>> >
>> > Hi Jason,
>> > According to our previous conversation, I've added checks to the meson 
>> > script.
>> > Please confirm that everything is correct
>>
>> I've queued this series.
>>
>> Thanks
>>

Re: QEMU developers fortnightly conference call for agenda for 2023-12-12

[Zhao Liu]

Hi Philippe,

On Mon, Dec 11, 2023 at 08:04:28PM +0100, Philippe Mathieu-Daudé wrote:
> Date: Mon, 11 Dec 2023 20:04:28 +0100
> From: Philippe Mathieu-Daudé 
> Subject: Re: QEMU developers fortnightly conference call for agenda for
>  2023-12-12
> 
> Hi Zhao,
> 

[snip]

> 
> FYI I have your series tagged for review (for generic QOM /
> machine [*]) but we need feedback from the x86 maintainers too,
> and eventually from riscv/arm too, since they might end up
> using your API.
> 
> [*] That said, I unlikely will have time the next 2 weeks.

Thanks for your help!

Best Regards,
Zhao

> 
> Regards,
> 
> Phil.
> 
> > Thanks,
> > Zhao
> 

Re: [PATCH 2/2] hw/usb/hcd-xhci.c: allow unaligned access to Capability Registers

[Tomoyuki Hirose]

Thanks for comment.

On Mon, Dec 11, 2023 at 10:57 PM Peter Maydell  wrote:
> We should definitely look at fixing the unaligned access
> stuff, but the linked bug report is not trying to do an
> unaligned access -- it wants to do a 2-byte read from offset 2,
> which is aligned. The capability registers in the xHCI spec
> are also all at offsets and sizes that mean that a natural
> read of them is not unaligned.

Shouldn't I link this bug report?
Or is it not appropriate to allow unaligned access?

thanks,
Tomoyuki HIROSE

Re: [PATCH 22/24] exec/cpu-all: Restrict inclusion of 'exec/user/guest-base.h'

[Richard Henderson]

On 12/11/23 13:19, Philippe Mathieu-Daudé wrote:

Declare 'have_guest_base' in "exec/user/guest-base.h".
Very few files require this header, so explicitly include
it there instead of "exec/cpu-all.h" which is used in many
source files.
Assert this user-specific header is only included from user
emulation.

Signed-off-by: Philippe Mathieu-Daudé
---
  include/exec/cpu-all.h | 3 ---
  include/exec/user/guest-base.h | 6 ++
  bsd-user/main.c| 1 +
  linux-user/elfload.c   | 1 +
  linux-user/main.c  | 1 +
  5 files changed, 9 insertions(+), 3 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [PATCH 24/24] target: Restrict 'sysemu/reset.h' to system emulation

[gaosong]

在 2023/12/12 上午5:20, Philippe Mathieu-Daudé 写道:

vCPU "reset" is only possible with system emulation.

Signed-off-by: Philippe Mathieu-Daudé 
---
  target/i386/cpu.c  | 2 +-
  target/loongarch/cpu.c | 2 ++
  2 files changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Song Gao 

Thanks.
Song Gao

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index dfb96217ad..17b6962d43 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -24,7 +24,6 @@
  #include "qemu/hw-version.h"
  #include "cpu.h"
  #include "tcg/helper-tcg.h"
-#include "sysemu/reset.h"
  #include "sysemu/hvf.h"
  #include "hvf/hvf-i386.h"
  #include "kvm/kvm_i386.h"
@@ -37,6 +36,7 @@
  #include "hw/qdev-properties.h"
  #include "hw/i386/topology.h"
  #ifndef CONFIG_USER_ONLY
+#include "sysemu/reset.h"
  #include "qapi/qapi-commands-machine-target.h"
  #include "exec/address-spaces.h"
  #include "hw/boards.h"
diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index fc075952e6..b26187dfde 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -17,7 +17,9 @@
  #include "internals.h"
  #include "fpu/softfloat-helpers.h"
  #include "cpu-csr.h"
+#ifndef CONFIG_USER_ONLY
  #include "sysemu/reset.h"
+#endif
  #include "tcg/tcg.h"
  #include "vec.h"
  

Re: [PATCH 20/24] exec: Declare abi_ptr type in its own 'tcg/abi_ptr.h' header

[Richard Henderson]

On 12/11/23 13:19, Philippe Mathieu-Daudé wrote:

The abi_ptr type is declared in "exec/cpu_ldst.h" with all
the load/store helpers. Some source files requiring abi_ptr
type don't need the load/store helpers. In order to simplify,
create a new "tcg/abi_ptr.h" header.

Signed-off-by: Philippe Mathieu-Daudé 
---
  include/exec/cpu_ldst.h   | 17 +++--
  include/exec/exec-all.h   |  1 +
  include/exec/translator.h |  5 -
  include/tcg/abi_ptr.h | 32 
  accel/tcg/cputlb.c|  1 +
  5 files changed, 41 insertions(+), 15 deletions(-)
  create mode 100644 include/tcg/abi_ptr.h


I'm unconvinced about this being tcg related, per se.
Leave it in include/exec/, with exec/user/abitypes.h.

With that,
Reviewed-by: Richard Henderson 


r~

[PATCH v2] vfio: Include libgen.h for basename API

[Khem Raj]

Glibc has two implementation one based on POSIX which is used when
libgen.h is included and second implementation is GNU implementation
which is used when string.h is included. The functions are no identical
in behavior. Musl C library does not implement the GNU version, but it
has provided a declaration in string.h but this has been corrected in
latest musl [1] which exposes places where it was being used from
string.h to error out especially when -Wimplicit-function-declaration is
treated as error.

| ../qemu-8.1.2/hw/vfio/pci.c:3030:18: error: call to undeclared function 
'basename'; ISO C99 and later do not support implicit function declarations 
[-Wimplicit-function-declaration]
|  3030 | group_name = basename(group_path);

clang-17 treats this warning as error by default

[1] 
https://git.musl-libc.org/cgit/musl/commit/?id=725e17ed6dff4d0cd22487bb64470881e86a92e7

Signed-off-by: Khem Raj 
---
v2: Add missing link for [1]

 hw/vfio/pci.c  | 1 +
 hw/vfio/platform.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index c62c02f7b6..f043c93b9e 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -19,6 +19,7 @@
  */
 
 #include "qemu/osdep.h"
+#include 
 #include 
 #include 
 
diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index 8e3d4ac458..a835ab03be 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -16,6 +16,7 @@
 
 #include "qemu/osdep.h"
 #include "qapi/error.h"
+#include 
 #include 
 #include 
 
-- 
2.43.0

[PATCH] vfio: Include libgen.h for basename API

[Khem Raj]

Glibc has two implementation one based on POSIX which is used when
libgen.h is included and second implementation is GNU implementation
which is used when string.h is included. The functions are no identical
in behavior. Musl C library does not implement the GNU version, but it
has provided a declaration in string.h but this has been corrected in
latest musl [1] which exposes places where it was being used from
string.h to error out especially when -Wimplicit-function-declaration is
treated as error.

| ../qemu-8.1.2/hw/vfio/pci.c:3030:18: error: call to undeclared function 
'basename'; ISO C99 and later do not support implicit function declarations 
[-Wimplicit-function-declaration]
|  3030 | group_name = basename(group_path);

clang-17 treats this warning as error by default

Signed-off-by: Khem Raj 
---
 hw/vfio/pci.c  | 1 +
 hw/vfio/platform.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index c62c02f7b6..f043c93b9e 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -19,6 +19,7 @@
  */
 
 #include "qemu/osdep.h"
+#include 
 #include 
 #include 
 
diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index 8e3d4ac458..a835ab03be 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -16,6 +16,7 @@
 
 #include "qemu/osdep.h"
 #include "qapi/error.h"
+#include 
 #include 
 #include 
 
-- 
2.43.0

[PATCH v4 2/4] tcg: Make tb_cflags() usable from target-agnostic code

[Ilya Leoshkevich]

Currently tb_cflags() is defined in exec-all.h, which is not usable
from target-agnostic code. Move it to translation-block.h, which is.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Ilya Leoshkevich 
Reviewed-by: Richard Henderson 
---
 include/exec/exec-all.h  | 6 --
 include/exec/translation-block.h | 6 ++
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index d9323ccfe6b..dca0a3a4040 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -460,12 +460,6 @@ int probe_access_full_mmu(CPUArchState *env, vaddr addr, 
int size,
 
 #endif
 
-/* Hide the qatomic_read to make code a little easier on the eyes */
-static inline uint32_t tb_cflags(const TranslationBlock *tb)
-{
-return qatomic_read(>cflags);
-}
-
 static inline tb_page_addr_t tb_page_addr0(const TranslationBlock *tb)
 {
 #ifdef CONFIG_USER_ONLY
diff --git a/include/exec/translation-block.h b/include/exec/translation-block.h
index e2b26e16da1..48211c890a7 100644
--- a/include/exec/translation-block.h
+++ b/include/exec/translation-block.h
@@ -145,4 +145,10 @@ struct TranslationBlock {
 /* The alignment given to TranslationBlock during allocation. */
 #define CODE_GEN_ALIGN  16
 
+/* Hide the qatomic_read to make code a little easier on the eyes */
+static inline uint32_t tb_cflags(const TranslationBlock *tb)
+{
+return qatomic_read(>cflags);
+}
+
 #endif /* EXEC_TRANSLATION_BLOCK_H */
-- 
2.43.0

[PATCH v4 4/4] accel/tcg: Move perf and debuginfo support to tcg

[Ilya Leoshkevich]

tcg/ should not depend on accel/tcg/, but perf and debuginfo
support provided by the latter are being used by tcg/tcg.c.

Since that's the only user, move both to tcg/.

Suggested-by: Philippe Mathieu-Daudé 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Ilya Leoshkevich 
Reviewed-by: Richard Henderson 
---
 accel/tcg/meson.build  | 2 --
 accel/tcg/translate-all.c  | 2 +-
 hw/core/loader.c   | 2 +-
 {accel => include}/tcg/debuginfo.h | 4 ++--
 {accel => include}/tcg/perf.h  | 4 ++--
 linux-user/elfload.c   | 2 +-
 linux-user/exit.c  | 2 +-
 linux-user/main.c  | 2 +-
 system/vl.c| 2 +-
 {accel/tcg => tcg}/debuginfo.c | 3 +--
 tcg/meson.build| 3 +++
 {accel/tcg => tcg}/perf.c  | 7 +++
 tcg/tcg.c  | 2 +-
 13 files changed, 18 insertions(+), 19 deletions(-)
 rename {accel => include}/tcg/debuginfo.h (96%)
 rename {accel => include}/tcg/perf.h (95%)
 rename {accel/tcg => tcg}/debuginfo.c (98%)
 rename {accel/tcg => tcg}/perf.c (99%)

diff --git a/accel/tcg/meson.build b/accel/tcg/meson.build
index 8783edd06ee..a7cb724edb2 100644
--- a/accel/tcg/meson.build
+++ b/accel/tcg/meson.build
@@ -16,8 +16,6 @@ tcg_ss.add(when: 'CONFIG_SYSTEM_ONLY', if_false: 
files('user-exec-stub.c'))
 if get_option('plugins')
   tcg_ss.add(files('plugin-gen.c'))
 endif
-tcg_ss.add(when: libdw, if_true: files('debuginfo.c'))
-tcg_ss.add(when: 'CONFIG_LINUX', if_true: files('perf.c'))
 specific_ss.add_all(when: 'CONFIG_TCG', if_true: tcg_ss)
 
 specific_ss.add(when: ['CONFIG_SYSTEM_ONLY', 'CONFIG_TCG'], if_true: files(
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 79a88f5fb75..3c1ce69ff36 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -63,7 +63,7 @@
 #include "tb-context.h"
 #include "internal-common.h"
 #include "internal-target.h"
-#include "perf.h"
+#include "tcg/perf.h"
 #include "tcg/insn-start-words.h"
 
 TBContext tb_ctx;
diff --git a/hw/core/loader.c b/hw/core/loader.c
index e7a9b3775bb..b8e52f3fb0f 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -62,7 +62,7 @@
 #include "hw/boards.h"
 #include "qemu/cutils.h"
 #include "sysemu/runstate.h"
-#include "accel/tcg/debuginfo.h"
+#include "tcg/debuginfo.h"
 
 #include 
 
diff --git a/accel/tcg/debuginfo.h b/include/tcg/debuginfo.h
similarity index 96%
rename from accel/tcg/debuginfo.h
rename to include/tcg/debuginfo.h
index f064e1c144b..858535b5da5 100644
--- a/accel/tcg/debuginfo.h
+++ b/include/tcg/debuginfo.h
@@ -4,8 +4,8 @@
  * SPDX-License-Identifier: GPL-2.0-or-later
  */
 
-#ifndef ACCEL_TCG_DEBUGINFO_H
-#define ACCEL_TCG_DEBUGINFO_H
+#ifndef TCG_DEBUGINFO_H
+#define TCG_DEBUGINFO_H
 
 #include "qemu/bitops.h"
 
diff --git a/accel/tcg/perf.h b/include/tcg/perf.h
similarity index 95%
rename from accel/tcg/perf.h
rename to include/tcg/perf.h
index f92dd52c699..c96b5920a3f 100644
--- a/accel/tcg/perf.h
+++ b/include/tcg/perf.h
@@ -4,8 +4,8 @@
  * SPDX-License-Identifier: GPL-2.0-or-later
  */
 
-#ifndef ACCEL_TCG_PERF_H
-#define ACCEL_TCG_PERF_H
+#ifndef TCG_PERF_H
+#define TCG_PERF_H
 
 #if defined(CONFIG_TCG) && defined(CONFIG_LINUX)
 /* Start writing perf-.map. */
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 11c45ab2934..a43e6114ac4 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -23,7 +23,7 @@
 #include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "target_signal.h"
-#include "accel/tcg/debuginfo.h"
+#include "tcg/debuginfo.h"
 
 #ifdef TARGET_ARM
 #include "target/arm/cpu-features.h"
diff --git a/linux-user/exit.c b/linux-user/exit.c
index 50266314e0a..1ff8fe4f072 100644
--- a/linux-user/exit.c
+++ b/linux-user/exit.c
@@ -17,7 +17,7 @@
  *  along with this program; if not, see .
  */
 #include "qemu/osdep.h"
-#include "accel/tcg/perf.h"
+#include "tcg/perf.h"
 #include "gdbstub/syscalls.h"
 #include "qemu.h"
 #include "user-internals.h"
diff --git a/linux-user/main.c b/linux-user/main.c
index 84691d707b2..b0b270b8be3 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -55,7 +55,7 @@
 #include "signal-common.h"
 #include "loader.h"
 #include "user-mmap.h"
-#include "accel/tcg/perf.h"
+#include "tcg/perf.h"
 
 #ifdef CONFIG_SEMIHOSTING
 #include "semihosting/semihost.h"
diff --git a/system/vl.c b/system/vl.c
index 2bcd9efb9a6..56baa1c81f2 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -96,7 +96,7 @@
 #endif
 #include "sysemu/qtest.h"
 #ifdef CONFIG_TCG
-#include "accel/tcg/perf.h"
+#include "tcg/perf.h"
 #endif
 
 #include "disas/disas.h"
diff --git a/accel/tcg/debuginfo.c b/tcg/debuginfo.c
similarity index 98%
rename from accel/tcg/debuginfo.c
rename to tcg/debuginfo.c
index 71c66d04d12..3753f7ef67c 100644
--- a/accel/tcg/debuginfo.c
+++ b/tcg/debuginfo.c
@@ -6,11 +6,10 @@
 
 #include "qemu/osdep.h"
 #include "qemu/lockable.h"
+#include "tcg/debuginfo.h"
 
 #include 

[PATCH v4 3/4] accel/tcg: Remove #ifdef TARGET_I386 from perf.c

[Ilya Leoshkevich]

Preparation for moving perf.c to tcg/.

This affects only profiling guest code, which has code in a non-0 based
segment, e.g., 16-bit code, which is not particularly important.

Suggested-by: Richard Henderson 
Signed-off-by: Ilya Leoshkevich 
Reviewed-by: Alex Bennée 
Reviewed-by: Richard Henderson 
---
 accel/tcg/perf.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/accel/tcg/perf.c b/accel/tcg/perf.c
index ba75c1bbe45..68a46b1b524 100644
--- a/accel/tcg/perf.c
+++ b/accel/tcg/perf.c
@@ -337,10 +337,6 @@ void perf_report_code(uint64_t guest_pc, TranslationBlock 
*tb,
 q[insn].address = gen_insn_data[insn * start_words + 0];
 if (tb_cflags(tb) & CF_PCREL) {
 q[insn].address |= (guest_pc & qemu_target_page_mask());
-} else {
-#if defined(TARGET_I386)
-q[insn].address -= tb->cs_base;
-#endif
 }
 q[insn].flags = DEBUGINFO_SYMBOL | (jitdump ? DEBUGINFO_LINE : 0);
 }
-- 
2.43.0

[PATCH v4 1/4] accel/tcg: Make use of qemu_target_page_mask() in perf.c

[Ilya Leoshkevich]

Stop using TARGET_PAGE_MASK in order to make perf.c more
target-agnostic.

Signed-off-by: Ilya Leoshkevich 
---
 accel/tcg/perf.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/accel/tcg/perf.c b/accel/tcg/perf.c
index cd1aa99a7ee..ba75c1bbe45 100644
--- a/accel/tcg/perf.c
+++ b/accel/tcg/perf.c
@@ -10,6 +10,7 @@
 
 #include "qemu/osdep.h"
 #include "elf.h"
+#include "exec/target_page.h"
 #include "exec/exec-all.h"
 #include "qemu/timer.h"
 #include "tcg/tcg.h"
@@ -335,7 +336,7 @@ void perf_report_code(uint64_t guest_pc, TranslationBlock 
*tb,
 /* FIXME: This replicates the restore_state_to_opc() logic. */
 q[insn].address = gen_insn_data[insn * start_words + 0];
 if (tb_cflags(tb) & CF_PCREL) {
-q[insn].address |= (guest_pc & TARGET_PAGE_MASK);
+q[insn].address |= (guest_pc & qemu_target_page_mask());
 } else {
 #if defined(TARGET_I386)
 q[insn].address -= tb->cs_base;
-- 
2.43.0

[PATCH v4 0/4] accel/tcg: Move perf and debuginfo support to tcg

[Ilya Leoshkevich]

Based-on: 20231211212003.21686-1-phi...@linaro.org

v3 -> v4: Rebase on top of Philippe's series.
  Move perf.h and debuginfo.h (Richard).

v2: https://patchew.org/QEMU/20230630234230.596193-1-...@linux.ibm.com/
v2 -> v3: Rebased.
  This series was lost and forgotten until Philippe reminded me
  about it.

v1: https://lists.gnu.org/archive/html/qemu-devel/2023-06/msg07037.html
v1 -> v2: Move qemu_target_page_mask() hunk to patch 1.
  Fix typos.

Hi,

This series is a follow-up to discussion in [1]; the goal is to build
perf and debuginfo support only one time.

Best regards,
Ilya

[1] https://lists.gnu.org/archive/html/qemu-devel/2023-06/msg06510.html

Ilya Leoshkevich (4):
  accel/tcg: Make use of qemu_target_page_mask() in perf.c
  tcg: Make tb_cflags() usable from target-agnostic code
  accel/tcg: Remove #ifdef TARGET_I386 from perf.c
  accel/tcg: Move perf and debuginfo support to tcg

 accel/tcg/meson.build  |  2 --
 accel/tcg/translate-all.c  |  2 +-
 hw/core/loader.c   |  2 +-
 include/exec/exec-all.h|  6 --
 include/exec/translation-block.h   |  6 ++
 {accel => include}/tcg/debuginfo.h |  4 ++--
 {accel => include}/tcg/perf.h  |  4 ++--
 linux-user/elfload.c   |  2 +-
 linux-user/exit.c  |  2 +-
 linux-user/main.c  |  2 +-
 system/vl.c|  2 +-
 {accel/tcg => tcg}/debuginfo.c |  3 +--
 tcg/meson.build|  3 +++
 {accel/tcg => tcg}/perf.c  | 14 +-
 tcg/tcg.c  |  2 +-
 15 files changed, 26 insertions(+), 30 deletions(-)
 rename {accel => include}/tcg/debuginfo.h (96%)
 rename {accel => include}/tcg/perf.h (95%)
 rename {accel/tcg => tcg}/debuginfo.c (98%)
 rename {accel/tcg => tcg}/perf.c (97%)

-- 
2.43.0

Re: [PATCH] xen: fix condition for enabling the Xen accelerator

[Stefano Stabellini]

On Sat, 9 Dec 2023, Paolo Bonzini wrote:
> A misspelled condition in xen_native.h is hiding a bug in the enablement of
> Xen for qemu-system-aarch64.  The bug becomes apparent when building for
> Xen 4.18.
> 
> While the i386 emulator provides the xenpv machine type for multiple 
> architectures,
> and therefore can be compiled with Xen enabled even when the host is Arm, the
> opposite is not true: qemu-system-aarch64 can only be compiled with Xen 
> support
> enabled when the host is Arm.
> 
> Expand the computation of accelerator_targets['CONFIG_XEN'] similar to what is
> already there for KVM, and fix xen_native.h.
> 
> Cc: Stefano Stabellini 
> Cc: Richard W.M. Jones 
> Cc: Daniel P. Berrangé 
> Reported-by: Michael Young 
> Supersedes: 
> <277e21fc78b75ec459efc7f5fde628a0222c63b0.1701989261.git.m.a.yo...@durham.ac.uk>
> Signed-off-by: Paolo Bonzini 

Reviewed-by: Stefano Stabellini 


> ---
>  include/hw/xen/xen_native.h |  2 +-
>  meson.build | 17 ++---
>  2 files changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/include/hw/xen/xen_native.h b/include/hw/xen/xen_native.h
> index 6f09c48823b..1a5ad693a4d 100644
> --- a/include/hw/xen/xen_native.h
> +++ b/include/hw/xen/xen_native.h
> @@ -532,7 +532,7 @@ static inline int 
> xendevicemodel_set_irq_level(xendevicemodel_handle *dmod,
>  }
>  #endif
>  
> -#if CONFIG_XEN_CTRL_INTERFACE_VERSION <= 41700
> +#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 41700
>  #define GUEST_VIRTIO_MMIO_BASE   xen_mk_ullong(0x0200)
>  #define GUEST_VIRTIO_MMIO_SIZE   xen_mk_ullong(0x0010)
>  #define GUEST_VIRTIO_MMIO_SPI_FIRST   33
> diff --git a/meson.build b/meson.build
> index ec01f8b138a..67f4ede8aea 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -123,21 +123,24 @@ if get_option('kvm').allowed() and targetos == 'linux'
>kvm_targets_c = '"' + '" ,"'.join(kvm_targets) + '"'
>  endif
>  config_host_data.set('CONFIG_KVM_TARGETS', kvm_targets_c)
> -
>  accelerator_targets = { 'CONFIG_KVM': kvm_targets }
>  
> +if cpu in ['x86', 'x86_64']
> +  xen_targets = ['i386-softmmu', 'x86_64-softmmu']
> +elif cpu in ['arm', 'aarch64']
> +  # i386 emulator provides xenpv machine type for multiple architectures
> +  xen_targets = ['i386-softmmu', 'x86_64-softmmu', 'aarch64-softmmu']
> +else
> +  xen_targets = []
> +endif
> +accelerator_targets += { 'CONFIG_XEN': xen_targets }
> +
>  if cpu in ['aarch64']
>accelerator_targets += {
>  'CONFIG_HVF': ['aarch64-softmmu']
>}
>  endif
>  
> -if cpu in ['x86', 'x86_64', 'arm', 'aarch64']
> -  # i386 emulator provides xenpv machine type for multiple architectures
> -  accelerator_targets += {
> -'CONFIG_XEN': ['i386-softmmu', 'x86_64-softmmu', 'aarch64-softmmu'],
> -  }
> -endif
>  if cpu in ['x86', 'x86_64']
>accelerator_targets += {
>  'CONFIG_HVF': ['x86_64-softmmu'],
> -- 
> 2.43.0
> 
> 

Re: [PATCH 19/24] exec/user: Do not include 'cpu.h' in 'abitypes.h'

[Richard Henderson]

On 12/11/23 13:19, Philippe Mathieu-Daudé wrote:

First, "exec/user/abitypes.h" is missing the following
includes (they are included by "cpu.h"):
  - "exec/target_long.h"
  - "exec/cpu-all.h"
  - "exec/tswap.h"
Second, it only requires the definitions from "cpu-param.h",
not the huge "cpu.h".

In order to avoid "cpu.h", pick the minimum required headers.

Assert this user-specific header is only included from user
emulation.

Signed-off-by: Philippe Mathieu-Daudé 


Why is cpu-all.h required?


r~

Re: [PATCH 18/24] accel/tcg: Un-inline retaddr helpers to 'user-retaddr.h'

[Richard Henderson]

On 12/11/23 13:19, Philippe Mathieu-Daudé wrote:

set_helper_retaddr() is only used in accel/tcg/user-exec.c.

clear_helper_retaddr() is only used in accel/tcg/user-exec.c
and accel/tcg/user-exec.c.

No need to expose their definitions to all user-emulation
files including "exec/cpu_ldst.h", move them to a new
"user-retaddr.h" header (restricted to accel/tcg/).

Signed-off-by: Philippe Mathieu-Daudé 


Reviewed-by: Richard Henderson 


r~

Re: [PATCH 15/24] exec/cpu-all: Remove unused headers

[Richard Henderson]

On 12/11/23 13:19, Philippe Mathieu-Daudé wrote:

Nothing is required from the "qemu/thread.h" and
"hw/core/cpu.h" headers.

Signed-off-by: Philippe Mathieu-Daudé 
---
  include/exec/cpu-all.h | 2 --
  1 file changed, 2 deletions(-)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 9a7b5737d3..b1e293a08f 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -22,8 +22,6 @@
  #include "exec/cpu-common.h"
  #include "exec/memory.h"
  #include "exec/tswap.h"
-#include "qemu/thread.h"
-#include "hw/core/cpu.h"


While thread.h is fine, I'm not sure removing hw/core/cpu.h from cpu-all.h is a good idea, 
and would explain the rather surprising changes to add core/cpu.h to other files.



r~

Re: [PATCH 16/24] exec/cpu-all: Reduce 'qemu/rcu.h' header inclusion

[Richard Henderson]

On 12/11/23 13:19, Philippe Mathieu-Daudé wrote:

"exec/cpu-all.h" doesn't need definitions from "qemu/rcu.h",
however "exec/ram_addr.h" does.

Signed-off-by: Philippe Mathieu-Daudé 
---
  include/exec/cpu-all.h  | 1 -
  include/exec/ram_addr.h | 1 +
  2 files changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Richard Henderson 


r~

Re: [PATCH 12/24] target/i386: Include missing 'exec/exec-all.h' header

[Richard Henderson]

On 12/11/23 13:19, Philippe Mathieu-Daudé wrote:

The XRSTOR instruction ends calling tlb_flush(), declared
in "exec/exec-all.h".

Signed-off-by: Philippe Mathieu-Daudé 
---
  target/i386/tcg/fpu_helper.c | 1 +
  1 file changed, 1 insertion(+)


Reviewed-by: Richard Henderson 


r~

Re: [PATCH 06/24] semihosting/guestfd: Remove unused 'semihosting/uaccess.h' header

[Richard Henderson]

On 12/11/23 13:19, Philippe Mathieu-Daudé wrote:

Nothing in guestfd.c requires "semihosting/uaccess.h".

Signed-off-by: Philippe Mathieu-Daudé 
---
  semihosting/guestfd.c | 1 -
  1 file changed, 1 deletion(-)

diff --git a/semihosting/guestfd.c b/semihosting/guestfd.c
index 955c2efbd0..fd7e609790 100644
--- a/semihosting/guestfd.c
+++ b/semihosting/guestfd.c
@@ -15,7 +15,6 @@
  #ifdef CONFIG_USER_ONLY
  #include "qemu.h"
  #else
-#include "semihosting/uaccess.h"


Then I would be surprised if qemu.h is required either -- that's where uaccess is done for 
user-only.



r~

Re: [PATCH 05/24] semihosting/uaccess: Avoid including 'cpu.h'

[Richard Henderson]

On 12/11/23 13:19, Philippe Mathieu-Daudé wrote:

"semihosting/uaccess.h" only requires declarations
from "exec/cpu-defs.h". Avoid including the huge "cpu.h".

Signed-off-by: Philippe Mathieu-Daudé 
---
  include/semihosting/uaccess.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Richard Henderson 


r~

Re: [PATCH 04/24] accel: Include missing 'exec/cpu_ldst.h' header

[Richard Henderson]

On 12/11/23 13:19, Philippe Mathieu-Daudé wrote:

Theses files call cpu_ldl_code() which is declared
in "exec/cpu_ldst.h".

Signed-off-by: Philippe Mathieu-Daudé 
---
  accel/tcg/translator.c| 1 +
  target/hexagon/translate.c| 1 +
  target/microblaze/cpu.c   | 1 +
  target/microblaze/translate.c | 1 +
  target/nios2/translate.c  | 1 +
  5 files changed, 5 insertions(+)


Reviewed-by: Richard Henderson 


r~

Re: [PATCH 03/24] target: Define TCG_GUEST_DEFAULT_MO in 'cpu-param.h'

[Richard Henderson]

On 12/11/23 13:19, Philippe Mathieu-Daudé wrote:

accel/tcg/ files requires the following definitions:

   - TARGET_LONG_BITS
   - TARGET_PAGE_BITS
   - TARGET_PHYS_ADDR_SPACE_BITS
   - TCG_GUEST_DEFAULT_MO

The first 3 are defined in "cpu-param.h". The last one
in "cpu.h", with a bunch of definitions irrelevant for
TCG. By moving the TCG_GUEST_DEFAULT_MO definition to
"cpu-param.h", we can simplify various accel/tcg includes.

Signed-off-by: Philippe Mathieu-Daudé
---


Reviewed-by: Richard Henderson 


r~

Re: [PATCH 02/24] exec: Expose 'target_page.h' API to user emulation

[Richard Henderson]

On 12/11/23 13:19, Philippe Mathieu-Daudé wrote:

User-only objects might benefit from the "exec/target_page.h"
API, which allows to build some objects once for all targets.

Signed-off-by: Philippe Mathieu-Daudé
---
  meson.build  |  2 +-
  page-target.c| 43 +++
  system/physmem.c | 35 ---
  3 files changed, 44 insertions(+), 36 deletions(-)
  create mode 100644 page-target.c


Reviewed-by: Richard Henderson 


r~

Re: [PATCH 24/24] target: Restrict 'sysemu/reset.h' to system emulation

[Warner Losh]

On Mon, Dec 11, 2023 at 2:23 PM Philippe Mathieu-Daudé 
wrote:

> vCPU "reset" is only possible with system emulation.
>
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  target/i386/cpu.c  | 2 +-
>  target/loongarch/cpu.c | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
>

Reviewed-by: Warner Losh 

Re: [PATCH 14/24] gdbstub: Include missing 'hw/core/cpu.h' header

[Warner Losh]

On Mon, Dec 11, 2023 at 2:22 PM Philippe Mathieu-Daudé 
wrote:

> Functions such gdb_get_cpu_pid() dereference CPUState so
> require the structure declaration from "hw/core/cpu.h":
>
>   static uint32_t gdb_get_cpu_pid(CPUState *cpu)
>   {
> ...
> return cpu->cluster_index + 1;
>   }
>
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  gdbstub/gdbstub.c | 1 +
>  1 file changed, 1 insertion(+)
>

Reviewed-by: Warner Losh 

Re: [PATCH 02/24] exec: Expose 'target_page.h' API to user emulation

[Warner Losh]

On Mon, Dec 11, 2023 at 2:20 PM Philippe Mathieu-Daudé 
wrote:

> User-only objects might benefit from the "exec/target_page.h"
> API, which allows to build some objects once for all targets.
>
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  meson.build  |  2 +-
>  page-target.c| 43 +++
>  system/physmem.c | 35 ---
>  3 files changed, 44 insertions(+), 36 deletions(-)
>  create mode 100644 page-target.c
>

Reviewed-by: Warner Losh 

Re: [PATCH 01/24] exec: Include 'cpu.h' before validating CPUArchState placement

[Warner Losh]

On Mon, Dec 11, 2023 at 2:53 PM Warner Losh  wrote:

>
>
> On Mon, Dec 11, 2023 at 2:20 PM Philippe Mathieu-Daudé 
> wrote:
>
>> CPUArchState 'env' field is defined within the ArchCPU structure,
>> so we need to include each target "cpu.h" header which defines it.
>>
>> Signed-off-by: Philippe Mathieu-Daudé 
>> ---
>>  include/exec/cpu-all.h | 9 +
>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>
>
> Signed-off-by: Warner Losh 
>

Brain f I meant:

Reviewed-by: Warner Losh 

Re: [PATCH 01/24] exec: Include 'cpu.h' before validating CPUArchState placement

[Warner Losh]

On Mon, Dec 11, 2023 at 2:20 PM Philippe Mathieu-Daudé 
wrote:

> CPUArchState 'env' field is defined within the ArchCPU structure,
> so we need to include each target "cpu.h" header which defines it.
>
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  include/exec/cpu-all.h | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
>

Signed-off-by: Warner Losh 

Re: [PULL 20/20] tracing: install trace events file only if necessary

[Carlos Santos]

On Mon, Dec 11, 2023 at 4:58 PM Stefan Hajnoczi  wrote:
>
> On Wed, Dec 06, 2023 at 07:26:01AM -0300, Carlos Santos wrote:
> > On Thu, Apr 20, 2023 at 9:10 AM Stefan Hajnoczi  wrote:
> > >
> > > From: Carlos Santos 
> > >
> > > It is not useful when configuring with --enable-trace-backends=nop.
> > >
> > > Signed-off-by: Carlos Santos 
> > > Signed-off-by: Stefan Hajnoczi 
> > > Message-Id: <20230408010410.281263-1-casan...@redhat.com>
> > > ---
> > >  trace/meson.build | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/trace/meson.build b/trace/meson.build
> > > index 8e80be895c..30b1d942eb 100644
> > > --- a/trace/meson.build
> > > +++ b/trace/meson.build
> > > @@ -64,7 +64,7 @@ trace_events_all = custom_target('trace-events-all',
> > >   input: trace_events_files,
> > >   command: [ 'cat', '@INPUT@' ],
> > >   capture: true,
> > > - install: true,
> > > + install: get_option('trace_backends') 
> > > != [ 'nop' ],
> > >   install_dir: qemu_datadir)
> > >
> > >  if 'ust' in get_option('trace_backends')
> > > --
> > > 2.39.2
> > >
> >
> > Hello,
> >
> > I still don't see this in the master branch. Is there something
> > preventing it from being applied?
>
> Hi Carlos,
> Apologies, I dropped this patch when respinning the pull request after
> the CI test failures caused by the zoned patches.
>
> Your patch has been merged on my tracing tree again and will make it
> into qemu.git/master when the development window opens again after the
> QEMU 8.2.0 release (hopefully next Tuesday).
>
> Stefan

Great. Thanks!

-- 
Carlos Santos
Senior Software Maintenance Engineer
Red Hat
casan...@redhat.comT: +55-11-3534-6186

Re: [PATCH v2 0/3] Virtio dmabuf improvements

[Stefan Hajnoczi]

On Thu, 7 Dec 2023 at 09:55, Albert Esteve  wrote:
>
> v1: https://www.mail-archive.com/qemu-devel@nongnu.org/msg1005257.html
> v1 -> v2:
>   - Solved an unitialized uuid value on vhost-user source
>   - Changed cleanup strategy, and traverse all objects in the
> table to remove them instead.

Please update the vhost-user specification
(docs/interop/vhost-user.rst) so people implementing front-ends and
back-ends are aware that only the back-end that added a shared
resource can remove it.

Acked-by: Stefan Hajnoczi 

>
> Various improvements for the virtio-dmabuf module.
> This patch includes:
>
> - Check for ownership before allowing a vhost device
>   to remove an object from the table.
> - Properly cleanup shared resources if a vhost device
>   object gets cleaned up.
> - Rename virtio dmabuf functions to `virtio_dmabuf_*`
>
> Albert Esteve (3):
>   hw/virtio: check owner for removing objects
>   hw/virtio: cleanup shared resources
>   hw/virtio: rename virtio dmabuf API
>
>  hw/display/virtio-dmabuf.c| 36 ---
>  hw/virtio/vhost-user.c| 31 ++---
>  hw/virtio/vhost.c |  3 ++
>  include/hw/virtio/virtio-dmabuf.h | 43 ++---
>  tests/unit/test-virtio-dmabuf.c   | 77 ++-
>  5 files changed, 138 insertions(+), 52 deletions(-)
>
> --
> 2.43.0
>

Re: [PATCH 20/24] exec: Declare abi_ptr type in its own 'tcg/abi_ptr.h' header

[Philippe Mathieu-Daudé]

On 11/12/23 22:19, Philippe Mathieu-Daudé wrote:

The abi_ptr type is declared in "exec/cpu_ldst.h" with all
the load/store helpers. Some source files requiring abi_ptr
type don't need the load/store helpers. In order to simplify,
create a new "tcg/abi_ptr.h" header.

Signed-off-by: Philippe Mathieu-Daudé 
---
  include/exec/cpu_ldst.h   | 17 +++--
  include/exec/exec-all.h   |  1 +
  include/exec/translator.h |  5 -
  include/tcg/abi_ptr.h | 32 
  accel/tcg/cputlb.c|  1 +
  5 files changed, 41 insertions(+), 15 deletions(-)
  create mode 100644 include/tcg/abi_ptr.h

diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
index 25e7239cc5..c69f02b699 100644
--- a/include/exec/cpu_ldst.h
+++ b/include/exec/cpu_ldst.h
@@ -65,18 +65,9 @@
  #include "exec/memopidx.h"
  #include "qemu/int128.h"
  #include "cpu.h"
+#include "tcg/abi_ptr.h"
  
  #if defined(CONFIG_USER_ONLY)

-/* sparc32plus has 64bit long but 32bit space address
- * this can make bad result with g2h() and h2g()
- */
-#if TARGET_VIRT_ADDR_SPACE_BITS <= 32
-typedef uint32_t abi_ptr;
-#define TARGET_ABI_FMT_ptr "%x"
-#else
-typedef uint64_t abi_ptr;
-#define TARGET_ABI_FMT_ptr "%"PRIx64
-#endif
  
  #ifndef TARGET_TAGGED_ADDRESSES

  static inline abi_ptr cpu_untagged_addr(CPUState *cs, abi_ptr x)
@@ -120,10 +111,8 @@ static inline bool guest_range_valid_untagged(abi_ulong 
start, abi_ulong len)
  assert(h2g_valid(x)); \
  h2g_nocheck(x); \
  })
-#else
-typedef target_ulong abi_ptr;
-#define TARGET_ABI_FMT_ptr TARGET_FMT_lx
-#endif
+
+#endif /* CONFIG_USER_ONLY */
  
  uint32_t cpu_ldub_data(CPUArchState *env, abi_ptr ptr);

  int cpu_ldsb_data(CPUArchState *env, abi_ptr ptr);




diff --git a/include/tcg/abi_ptr.h b/include/tcg/abi_ptr.h
new file mode 100644
index 00..415e31cabb
--- /dev/null
+++ b/include/tcg/abi_ptr.h
@@ -0,0 +1,32 @@
+/*
+ * TCG abi_ptr type
+ *
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+#ifndef TCG_ABI_PTR_H
+#define TCG_ABI_PTR_H
+
+#include "cpu-param.h"
+
+#if defined(CONFIG_USER_ONLY)
+/* sparc32plus has 64bit long but 32bit space address


Forgot to squash:

-- >8 --
@@ -11,3 +11,4 @@
 #if defined(CONFIG_USER_ONLY)
-/* sparc32plus has 64bit long but 32bit space address
+/*
+ * sparc32plus has 64bit long but 32bit space address
  * this can make bad result with g2h() and h2g()
---


+ * this can make bad result with g2h() and h2g()
+ */
+#if TARGET_VIRT_ADDR_SPACE_BITS <= 32
+typedef uint32_t abi_ptr;
+#define TARGET_ABI_FMT_ptr "%x"
+#else
+typedef uint64_t abi_ptr;
+#define TARGET_ABI_FMT_ptr "%"PRIx64
+#endif
+
+#else /* !CONFIG_USER_ONLY */
+
+#include "exec/target_long.h"
+
+typedef target_ulong abi_ptr;
+#define TARGET_ABI_FMT_ptr TARGET_FMT_lx
+
+#endif /* !CONFIG_USER_ONLY */
+
+#endif

[PATCH 21/24] exec/cpu_ldst: Avoid including 'cpu.h'

[Philippe Mathieu-Daudé]

"exec/cpu_ldst.h" doesn't need to huge "cpu.h" header,
but simply:

 - exec/cpu-defs.h
 - exec/tlb-common.h
 - exec/user/abitypes.h
 - exec/user/guest-base.h

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/exec/cpu_ldst.h | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
index c69f02b699..a115553ee8 100644
--- a/include/exec/cpu_ldst.h
+++ b/include/exec/cpu_ldst.h
@@ -64,11 +64,15 @@
 
 #include "exec/memopidx.h"
 #include "qemu/int128.h"
-#include "cpu.h"
+#include "exec/cpu-defs.h"
+#include "exec/tlb-common.h"
 #include "tcg/abi_ptr.h"
 
 #if defined(CONFIG_USER_ONLY)
 
+#include "exec/user/abitypes.h"
+#include "exec/user/guest-base.h"
+
 #ifndef TARGET_TAGGED_ADDRESSES
 static inline abi_ptr cpu_untagged_addr(CPUState *cs, abi_ptr x)
 {
-- 
2.41.0

[PATCH 15/24] exec/cpu-all: Remove unused headers

[Philippe Mathieu-Daudé]

Nothing is required from the "qemu/thread.h" and
"hw/core/cpu.h" headers.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/exec/cpu-all.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 9a7b5737d3..b1e293a08f 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -22,8 +22,6 @@
 #include "exec/cpu-common.h"
 #include "exec/memory.h"
 #include "exec/tswap.h"
-#include "qemu/thread.h"
-#include "hw/core/cpu.h"
 #include "qemu/rcu.h"
 
 /* some important defines:
-- 
2.41.0

Re: [PATCH v2 07/20] util/dsa: Implement DSA device start and stop logic.

[Fabiano Rosas]

Hao Xiang  writes:

> * DSA device open and close.
> * DSA group contains multiple DSA devices.
> * DSA group configure/start/stop/clean.
>
> Signed-off-by: Hao Xiang 
> Signed-off-by: Bryan Zhang 
> ---
>  include/qemu/dsa.h |  49 +++
>  util/dsa.c | 338 +
>  util/meson.build   |   1 +
>  3 files changed, 388 insertions(+)
>  create mode 100644 include/qemu/dsa.h
>  create mode 100644 util/dsa.c
>
> diff --git a/include/qemu/dsa.h b/include/qemu/dsa.h
> new file mode 100644
> index 00..30246b507e
> --- /dev/null
> +++ b/include/qemu/dsa.h
> @@ -0,0 +1,49 @@
> +#ifndef QEMU_DSA_H
> +#define QEMU_DSA_H
> +
> +#include "qemu/thread.h"
> +#include "qemu/queue.h"
> +
> +#ifdef CONFIG_DSA_OPT
> +
> +#pragma GCC push_options
> +#pragma GCC target("enqcmd")
> +
> +#include 
> +#include "x86intrin.h"
> +
> +#endif
> +
> +/**
> + * @brief Initializes DSA devices.
> + *
> + * @param dsa_parameter A list of DSA device path from migration parameter.

This code seems pretty generic, let's decouple this doc from migration.

> + * @return int Zero if successful, otherwise non zero.
> + */
> +int dsa_init(const char *dsa_parameter);
> +
> +/**
> + * @brief Start logic to enable using DSA.
> + */
> +void dsa_start(void);
> +
> +/**
> + * @brief Stop logic to clean up DSA by halting the device group and 
> cleaning up
> + * the completion thread.

"Stop the device group and the completion thread"

The mention of "clean/cleaning up" makes this confusing because of
dsa_cleanup() below.

> + */
> +void dsa_stop(void);
> +
> +/**
> + * @brief Clean up system resources created for DSA offloading.
> + *This function is called during QEMU process teardown.

This is not called during QEMU process teardown. It's called at the end
of migration AFAICS. Maybe just leave this sentence out.

> + */
> +void dsa_cleanup(void);
> +
> +/**
> + * @brief Check if DSA is running.
> + *
> + * @return True if DSA is running, otherwise false.
> + */
> +bool dsa_is_running(void);
> +
> +#endif
> \ No newline at end of file
> diff --git a/util/dsa.c b/util/dsa.c
> new file mode 100644
> index 00..8edaa892ec
> --- /dev/null
> +++ b/util/dsa.c
> @@ -0,0 +1,338 @@
> +/*
> + * Use Intel Data Streaming Accelerator to offload certain background
> + * operations.
> + *
> + * Copyright (c) 2023 Hao Xiang 
> + *Bryan Zhang 
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to 
> deal
> + * in the Software without restriction, including without limitation the 
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/queue.h"
> +#include "qemu/memalign.h"
> +#include "qemu/lockable.h"
> +#include "qemu/cutils.h"
> +#include "qemu/dsa.h"
> +#include "qemu/bswap.h"
> +#include "qemu/error-report.h"
> +#include "qemu/rcu.h"
> +
> +#ifdef CONFIG_DSA_OPT
> +
> +#pragma GCC push_options
> +#pragma GCC target("enqcmd")
> +
> +#include 
> +#include "x86intrin.h"
> +
> +#define DSA_WQ_SIZE 4096
> +#define MAX_DSA_DEVICES 16
> +
> +typedef QSIMPLEQ_HEAD(dsa_task_queue, buffer_zero_batch_task) dsa_task_queue;
> +
> +struct dsa_device {
> +void *work_queue;
> +};
> +
> +struct dsa_device_group {
> +struct dsa_device *dsa_devices;
> +int num_dsa_devices;
> +uint32_t index;
> +bool running;
> +QemuMutex task_queue_lock;
> +QemuCond task_queue_cond;
> +dsa_task_queue task_queue;
> +};
> +
> +uint64_t max_retry_count;
> +static struct dsa_device_group dsa_group;
> +
> +
> +/**
> + * @brief This function opens a DSA device's work queue and
> + *maps the DSA device memory into the current process.
> + *
> + * @param dsa_wq_path A pointer to the DSA device work queue's file path.
> + * @return A pointer to the mapped memory.
> + */
> +static void *
> +map_dsa_device(const char *dsa_wq_path)
> +{
> +void *dsa_device;
> +int fd;
> +
> +fd = open(dsa_wq_path, O_RDWR);
> +if (fd < 0) {
> +fprintf(stderr, "open %s failed with errno = 

[PATCH 19/24] exec/user: Do not include 'cpu.h' in 'abitypes.h'

[Philippe Mathieu-Daudé]

First, "exec/user/abitypes.h" is missing the following
includes (they are included by "cpu.h"):
 - "exec/target_long.h"
 - "exec/cpu-all.h"
 - "exec/tswap.h"
Second, it only requires the definitions from "cpu-param.h",
not the huge "cpu.h".

In order to avoid "cpu.h", pick the minimum required headers.

Assert this user-specific header is only included from user
emulation.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/exec/user/abitypes.h | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/exec/user/abitypes.h b/include/exec/user/abitypes.h
index 6178453d94..1a8cd1ac74 100644
--- a/include/exec/user/abitypes.h
+++ b/include/exec/user/abitypes.h
@@ -1,7 +1,14 @@
 #ifndef EXEC_USER_ABITYPES_H
 #define EXEC_USER_ABITYPES_H
 
-#include "cpu.h"
+#ifndef CONFIG_USER_ONLY
+#error Cannot include this header from system emulation
+#endif
+
+#include "cpu-param.h"
+#include "exec/target_long.h"
+#include "exec/cpu-all.h"
+#include "exec/tswap.h"
 
 #ifdef TARGET_ABI32
 #define TARGET_ABI_BITS 32
-- 
2.41.0

[PATCH 23/24] exec/cpu-all: Extract page-protection definitions to page-prot-common.h

[Philippe Mathieu-Daudé]

Extract page-protection definitions from "exec/cpu-all.h"
to "exec/page-prot-common.h".

The list of files requiring the new header was generated
using:

$ git grep -wE \
  'PAGE_(READ|WRITE|EXEC|BITS|VALID|ANON|RESERVED|TARGET_.|PASSTHROUGH)'

Signed-off-by: Philippe Mathieu-Daudé 
---
 bsd-user/bsd-mem.h   |  1 +
 bsd-user/qemu.h  |  1 +
 include/exec/cpu-all.h   | 35 +
 include/exec/page-prot-common.h  | 38 
 include/semihosting/uaccess.h|  1 +
 target/arm/cpu.h |  1 +
 target/ppc/internal.h|  1 +
 target/ppc/mmu-radix64.h |  2 ++
 accel/tcg/cputlb.c   |  1 +
 accel/tcg/tb-maint.c |  1 +
 accel/tcg/user-exec.c|  1 +
 bsd-user/mmap.c  |  1 +
 bsd-user/signal.c|  1 +
 cpu-target.c |  1 +
 hw/ppc/ppc440_bamboo.c   |  1 +
 hw/ppc/sam460ex.c|  1 +
 hw/ppc/virtex_ml507.c|  1 +
 linux-user/arm/cpu_loop.c|  1 +
 linux-user/elfload.c |  1 +
 linux-user/mmap.c|  1 +
 linux-user/nios2/cpu_loop.c  |  1 +
 linux-user/signal.c  |  1 +
 linux-user/syscall.c |  1 +
 system/physmem.c |  1 +
 target/alpha/helper.c|  1 +
 target/arm/ptw.c |  1 +
 target/arm/tcg/m_helper.c|  1 +
 target/arm/tcg/mte_helper.c  |  1 +
 target/arm/tcg/sve_helper.c  |  1 +
 target/avr/helper.c  |  1 +
 target/cris/mmu.c|  1 +
 target/hppa/mem_helper.c |  1 +
 target/hppa/translate.c  |  1 +
 target/i386/tcg/sysemu/excp_helper.c |  1 +
 target/loongarch/tlb_helper.c|  1 +
 target/m68k/helper.c |  1 +
 target/microblaze/helper.c   |  1 +
 target/microblaze/mmu.c  |  1 +
 target/mips/sysemu/physaddr.c|  1 +
 target/mips/tcg/sysemu/tlb_helper.c  |  1 +
 target/nios2/helper.c|  1 +
 target/nios2/mmu.c   |  1 +
 target/openrisc/mmu.c|  1 +
 target/ppc/mmu-hash32.c  |  1 +
 target/ppc/mmu-hash64.c  |  1 +
 target/ppc/mmu-radix64.c |  1 +
 target/ppc/mmu_common.c  |  1 +
 target/ppc/mmu_helper.c  |  1 +
 target/riscv/cpu_helper.c|  1 +
 target/riscv/pmp.c   |  1 +
 target/riscv/vector_helper.c |  1 +
 target/rx/cpu.c  |  1 +
 target/s390x/mmu_helper.c|  1 +
 target/s390x/tcg/mem_helper.c|  1 +
 target/sh4/helper.c  |  1 +
 target/sparc/ldst_helper.c   |  1 +
 target/sparc/mmu_helper.c|  1 +
 target/tricore/helper.c  |  1 +
 target/xtensa/mmu_helper.c   |  1 +
 target/xtensa/op_helper.c|  1 +
 60 files changed, 98 insertions(+), 34 deletions(-)
 create mode 100644 include/exec/page-prot-common.h

diff --git a/bsd-user/bsd-mem.h b/bsd-user/bsd-mem.h
index 21d9bab889..f95472bcab 100644
--- a/bsd-user/bsd-mem.h
+++ b/bsd-user/bsd-mem.h
@@ -56,6 +56,7 @@
 #include 
 
 #include "qemu-bsd.h"
+#include "exec/page-prot-common.h"
 
 extern struct bsd_shm_regions bsd_shm_regions[];
 extern abi_ulong target_brk;
diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
index dc842fffa7..1e38d2cb66 100644
--- a/bsd-user/qemu.h
+++ b/bsd-user/qemu.h
@@ -36,6 +36,7 @@ extern char **environ;
 #include "target_os_signal.h"
 #include "target.h"
 #include "exec/gdbstub.h"
+#include "exec/page-prot-common.h"
 #include "qemu/clang-tsa.h"
 
 #include "qemu-os.h"
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 5b75d04ff6..881288a4de 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -19,6 +19,7 @@
 #ifndef CPU_ALL_H
 #define CPU_ALL_H
 
+#include "exec/page-prot-common.h"
 #include "exec/cpu-common.h"
 #include "exec/memory.h"
 #include "exec/tswap.h"
@@ -165,40 +166,6 @@ extern const TargetPageBits target_page;
 
 #define TARGET_PAGE_ALIGN(addr) ROUND_UP((addr), TARGET_PAGE_SIZE)
 
-/* same as PROT_xxx */
-#define PAGE_READ  0x0001
-#define PAGE_WRITE 0x0002
-#define PAGE_EXEC  0x0004
-#define PAGE_BITS  (PAGE_READ | PAGE_WRITE | PAGE_EXEC)
-#define PAGE_VALID 0x0008
-/*
- * Original state of the write flag (used when tracking self-modifying code)
- */
-#define PAGE_WRITE_ORG 0x0010
-/*
- * Invalidate the TLB entry immediately, helpful for s390x
- * Low-Address-Protection. Used with PAGE_WRITE in tlb_set_page_with_attrs()
- */
-#define PAGE_WRITE_INV 0x0020
-/* For use with page_set_flags: page is being replaced; target_data cleared. */
-#define PAGE_RESET 0x0040
-/* For linux-user, indicates that the page is MAP_ANON. */
-#define PAGE_ANON  0x0080
-
-#if defined(CONFIG_BSD) && 

[PATCH 22/24] exec/cpu-all: Restrict inclusion of 'exec/user/guest-base.h'

[Philippe Mathieu-Daudé]

Declare 'have_guest_base' in "exec/user/guest-base.h".
Very few files require this header, so explicitly include
it there instead of "exec/cpu-all.h" which is used in many
source files.
Assert this user-specific header is only included from user
emulation.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/exec/cpu-all.h | 3 ---
 include/exec/user/guest-base.h | 6 ++
 bsd-user/main.c| 1 +
 linux-user/elfload.c   | 1 +
 linux-user/main.c  | 1 +
 5 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 2d568ae4f0..5b75d04ff6 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -74,9 +74,6 @@
 
 #if defined(CONFIG_USER_ONLY)
 #include "exec/user/abitypes.h"
-#include "exec/user/guest-base.h"
-
-extern bool have_guest_base;
 
 /*
  * If non-zero, the guest virtual address space is a contiguous subset
diff --git a/include/exec/user/guest-base.h b/include/exec/user/guest-base.h
index afe2ab7fbb..cf40151360 100644
--- a/include/exec/user/guest-base.h
+++ b/include/exec/user/guest-base.h
@@ -7,6 +7,12 @@
 #ifndef EXEC_USER_GUEST_BASE_H
 #define EXEC_USER_GUEST_BASE_H
 
+#ifndef CONFIG_USER_ONLY
+#error Cannot include this header from system emulation
+#endif
+
 extern uintptr_t guest_base;
 
+extern bool have_guest_base;
+
 #endif
diff --git a/bsd-user/main.c b/bsd-user/main.c
index e6014f517e..c331d727e1 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -36,6 +36,7 @@
 #include "qemu/help_option.h"
 #include "qemu/module.h"
 #include "exec/exec-all.h"
+#include "exec/user/guest-base.h"
 #include "tcg/startup.h"
 #include "qemu/timer.h"
 #include "qemu/envlist.h"
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index cf9e74468b..f9e9ac1760 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -6,6 +6,7 @@
 #include 
 
 #include "qemu.h"
+#include "exec/user/guest-base.h"
 #include "user-internals.h"
 #include "signal-common.h"
 #include "loader.h"
diff --git a/linux-user/main.c b/linux-user/main.c
index 0cdaf30d34..84691d707b 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -38,6 +38,7 @@
 #include "qemu/help_option.h"
 #include "qemu/module.h"
 #include "qemu/plugin.h"
+#include "exec/user/guest-base.h"
 #include "exec/exec-all.h"
 #include "exec/gdbstub.h"
 #include "gdbstub/user.h"
-- 
2.41.0

[PATCH 24/24] target: Restrict 'sysemu/reset.h' to system emulation

[Philippe Mathieu-Daudé]

vCPU "reset" is only possible with system emulation.

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/i386/cpu.c  | 2 +-
 target/loongarch/cpu.c | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index dfb96217ad..17b6962d43 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -24,7 +24,6 @@
 #include "qemu/hw-version.h"
 #include "cpu.h"
 #include "tcg/helper-tcg.h"
-#include "sysemu/reset.h"
 #include "sysemu/hvf.h"
 #include "hvf/hvf-i386.h"
 #include "kvm/kvm_i386.h"
@@ -37,6 +36,7 @@
 #include "hw/qdev-properties.h"
 #include "hw/i386/topology.h"
 #ifndef CONFIG_USER_ONLY
+#include "sysemu/reset.h"
 #include "qapi/qapi-commands-machine-target.h"
 #include "exec/address-spaces.h"
 #include "hw/boards.h"
diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index fc075952e6..b26187dfde 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -17,7 +17,9 @@
 #include "internals.h"
 #include "fpu/softfloat-helpers.h"
 #include "cpu-csr.h"
+#ifndef CONFIG_USER_ONLY
 #include "sysemu/reset.h"
+#endif
 #include "tcg/tcg.h"
 #include "vec.h"
 
-- 
2.41.0

Re: [PATCH 03/24] target: Define TCG_GUEST_DEFAULT_MO in 'cpu-param.h'

[Philippe Mathieu-Daudé]

On 11/12/23 22:19, Philippe Mathieu-Daudé wrote:

accel/tcg/ files requires the following definitions:

   - TARGET_LONG_BITS
   - TARGET_PAGE_BITS
   - TARGET_PHYS_ADDR_SPACE_BITS
   - TCG_GUEST_DEFAULT_MO

The first 3 are defined in "cpu-param.h". The last one
in "cpu.h", with a bunch of definitions irrelevant for
TCG. By moving the TCG_GUEST_DEFAULT_MO definition to
"cpu-param.h", we can simplify various accel/tcg includes.

Signed-off-by: Philippe Mathieu-Daudé 
---
  target/alpha/cpu-param.h  |  3 +++
  target/alpha/cpu.h|  3 ---
  target/arm/cpu-param.h|  8 +---
  target/arm/cpu.h  |  3 ---
  target/avr/cpu-param.h|  2 ++
  target/avr/cpu.h  |  2 --
  target/hppa/cpu-param.h   |  6 ++
  target/hppa/cpu.h |  6 --
  target/i386/cpu-param.h   |  3 +++
  target/i386/cpu.h |  3 ---
  target/loongarch/cpu-param.h  |  2 ++
  target/loongarch/cpu.h|  2 --
  target/microblaze/cpu-param.h |  3 +++
  target/microblaze/cpu.h   |  3 ---
  target/mips/cpu-param.h   |  2 ++
  target/mips/cpu.h |  2 --
  target/openrisc/cpu-param.h   |  2 ++
  target/openrisc/cpu.h |  2 --
  target/ppc/cpu-param.h|  2 ++
  target/ppc/cpu.h  |  2 --
  target/riscv/cpu-param.h  |  2 ++
  target/riscv/cpu.h|  2 --
  target/s390x/cpu-param.h  |  6 ++
  target/s390x/cpu.h|  3 ---
  target/sparc/cpu-param.h  | 23 +++
  target/sparc/cpu.h| 23 ---
  target/xtensa/cpu-param.h |  3 +++
  target/xtensa/cpu.h   |  3 ---
  28 files changed, 64 insertions(+), 62 deletions(-)




diff --git a/target/hppa/cpu-param.h b/target/hppa/cpu-param.h
index bb3d7ef6f7..4548103a18 100644
--- a/target/hppa/cpu-param.h
+++ b/target/hppa/cpu-param.h
@@ -21,4 +21,10 @@
  
  #define TARGET_PAGE_BITS 12
  
+/* PA-RISC 1.x processors have a strong memory model.  */

+/* ??? While we do not yet implement PA-RISC 2.0, those processors have
+   a weak memory model, but with TLB bits that force ordering on a per-page
+   basis.  It's probably easier to fall back to a strong memory model.  */


Forgot to squash:

-- >8 --
@@ -24,5 +24,7 @@
 /* PA-RISC 1.x processors have a strong memory model.  */
-/* ??? While we do not yet implement PA-RISC 2.0, those processors have
-   a weak memory model, but with TLB bits that force ordering on a per-page
-   basis.  It's probably easier to fall back to a strong memory model.  */
+/*
+ * ??? While we do not yet implement PA-RISC 2.0, those processors have
+ * a weak memory model, but with TLB bits that force ordering on a per-page
+ * basis.  It's probably easier to fall back to a strong memory model.
+ */
---


+#define TCG_GUEST_DEFAULT_MOTCG_MO_ALL
+
  #endif
diff --git a/target/hppa/cpu.h b/target/hppa/cpu.h
index 8be45c69c9..6b10ab20ba 100644
--- a/target/hppa/cpu.h
+++ b/target/hppa/cpu.h
@@ -25,12 +25,6 @@
  #include "qemu/cpu-float.h"
  #include "qemu/interval-tree.h"
  
-/* PA-RISC 1.x processors have a strong memory model.  */

-/* ??? While we do not yet implement PA-RISC 2.0, those processors have
-   a weak memory model, but with TLB bits that force ordering on a per-page
-   basis.  It's probably easier to fall back to a strong memory model.  */
-#define TCG_GUEST_DEFAULT_MOTCG_MO_ALL
-
  #define MMU_ABS_W_IDX 6
  #define MMU_ABS_IDX   7
  #define MMU_KERNEL_IDX8

[PATCH 04/24] accel: Include missing 'exec/cpu_ldst.h' header

[Philippe Mathieu-Daudé]

Theses files call cpu_ldl_code() which is declared
in "exec/cpu_ldst.h".

Signed-off-by: Philippe Mathieu-Daudé 
---
 accel/tcg/translator.c| 1 +
 target/hexagon/translate.c| 1 +
 target/microblaze/cpu.c   | 1 +
 target/microblaze/translate.c | 1 +
 target/nios2/translate.c  | 1 +
 5 files changed, 5 insertions(+)

diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index 38c34009a5..5b019a0852 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -12,6 +12,7 @@
 #include "qemu/error-report.h"
 #include "exec/exec-all.h"
 #include "exec/translator.h"
+#include "exec/cpu_ldst.h"
 #include "exec/plugin-gen.h"
 #include "tcg/tcg-op-common.h"
 #include "internal-target.h"
diff --git a/target/hexagon/translate.c b/target/hexagon/translate.c
index 666c061180..744dc4fc3f 100644
--- a/target/hexagon/translate.c
+++ b/target/hexagon/translate.c
@@ -23,6 +23,7 @@
 #include "exec/helper-gen.h"
 #include "exec/helper-proto.h"
 #include "exec/translation-block.h"
+#include "exec/cpu_ldst.h"
 #include "exec/log.h"
 #include "internal.h"
 #include "attribs.h"
diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
index bbb3335cad..91d7cd9061 100644
--- a/target/microblaze/cpu.c
+++ b/target/microblaze/cpu.c
@@ -28,6 +28,7 @@
 #include "qemu/module.h"
 #include "hw/qdev-properties.h"
 #include "exec/exec-all.h"
+#include "exec/cpu_ldst.h"
 #include "exec/gdbstub.h"
 #include "fpu/softfloat-helpers.h"
 #include "tcg/tcg.h"
diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index 49bfb4a0ea..1c9a364c2b 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -22,6 +22,7 @@
 #include "cpu.h"
 #include "disas/disas.h"
 #include "exec/exec-all.h"
+#include "exec/cpu_ldst.h"
 #include "tcg/tcg-op.h"
 #include "exec/helper-proto.h"
 #include "exec/helper-gen.h"
diff --git a/target/nios2/translate.c b/target/nios2/translate.c
index e806623594..a74eb6909f 100644
--- a/target/nios2/translate.c
+++ b/target/nios2/translate.c
@@ -25,6 +25,7 @@
 #include "cpu.h"
 #include "tcg/tcg-op.h"
 #include "exec/exec-all.h"
+#include "exec/cpu_ldst.h"
 #include "disas/disas.h"
 #include "exec/helper-proto.h"
 #include "exec/helper-gen.h"
-- 
2.41.0

[PATCH 16/24] exec/cpu-all: Reduce 'qemu/rcu.h' header inclusion

[Philippe Mathieu-Daudé]

"exec/cpu-all.h" doesn't need definitions from "qemu/rcu.h",
however "exec/ram_addr.h" does.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/exec/cpu-all.h  | 1 -
 include/exec/ram_addr.h | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index b1e293a08f..2d568ae4f0 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -22,7 +22,6 @@
 #include "exec/cpu-common.h"
 #include "exec/memory.h"
 #include "exec/tswap.h"
-#include "qemu/rcu.h"
 
 /* some important defines:
  *
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 90676093f5..aab7d6c57c 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -25,6 +25,7 @@
 #include "sysemu/tcg.h"
 #include "exec/ramlist.h"
 #include "exec/ramblock.h"
+#include "qemu/rcu.h"
 
 extern uint64_t total_dirty_pages;
 
-- 
2.41.0

[PATCH 18/24] accel/tcg: Un-inline retaddr helpers to 'user-retaddr.h'

[Philippe Mathieu-Daudé]

set_helper_retaddr() is only used in accel/tcg/user-exec.c.

clear_helper_retaddr() is only used in accel/tcg/user-exec.c
and accel/tcg/user-exec.c.

No need to expose their definitions to all user-emulation
files including "exec/cpu_ldst.h", move them to a new
"user-retaddr.h" header (restricted to accel/tcg/).

Signed-off-by: Philippe Mathieu-Daudé 
---
 accel/tcg/user-retaddr.h | 28 
 include/exec/cpu_ldst.h  | 28 ++--
 accel/tcg/cpu-exec.c |  3 +++
 accel/tcg/user-exec.c|  1 +
 4 files changed, 34 insertions(+), 26 deletions(-)
 create mode 100644 accel/tcg/user-retaddr.h

diff --git a/accel/tcg/user-retaddr.h b/accel/tcg/user-retaddr.h
new file mode 100644
index 00..e0f57e1994
--- /dev/null
+++ b/accel/tcg/user-retaddr.h
@@ -0,0 +1,28 @@
+#ifndef ACCEL_TCG_USER_RETADDR_H
+#define ACCEL_TCG_USER_RETADDR_H
+
+#include "qemu/atomic.h"
+
+extern __thread uintptr_t helper_retaddr;
+
+static inline void set_helper_retaddr(uintptr_t ra)
+{
+helper_retaddr = ra;
+/*
+ * Ensure that this write is visible to the SIGSEGV handler that
+ * may be invoked due to a subsequent invalid memory operation.
+ */
+signal_barrier();
+}
+
+static inline void clear_helper_retaddr(void)
+{
+/*
+ * Ensure that previous memory operations have succeeded before
+ * removing the data visible to the signal handler.
+ */
+signal_barrier();
+helper_retaddr = 0;
+}
+
+#endif
diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
index 6061e33ac9..25e7239cc5 100644
--- a/include/exec/cpu_ldst.h
+++ b/include/exec/cpu_ldst.h
@@ -300,31 +300,7 @@ Int128 cpu_atomic_cmpxchgo_be_mmu(CPUArchState *env, 
abi_ptr addr,
   Int128 cmpv, Int128 newv,
   MemOpIdx oi, uintptr_t retaddr);
 
-#if defined(CONFIG_USER_ONLY)
-
-extern __thread uintptr_t helper_retaddr;
-
-static inline void set_helper_retaddr(uintptr_t ra)
-{
-helper_retaddr = ra;
-/*
- * Ensure that this write is visible to the SIGSEGV handler that
- * may be invoked due to a subsequent invalid memory operation.
- */
-signal_barrier();
-}
-
-static inline void clear_helper_retaddr(void)
-{
-/*
- * Ensure that previous memory operations have succeeded before
- * removing the data visible to the signal handler.
- */
-signal_barrier();
-helper_retaddr = 0;
-}
-
-#else
+#if !defined(CONFIG_USER_ONLY)
 
 #include "tcg/oversized-guest.h"
 
@@ -376,7 +352,7 @@ static inline CPUTLBEntry *tlb_entry(CPUState *cpu, 
uintptr_t mmu_idx,
 return >neg.tlb.f[mmu_idx].table[tlb_index(cpu, mmu_idx, addr)];
 }
 
-#endif /* defined(CONFIG_USER_ONLY) */
+#endif /* !defined(CONFIG_USER_ONLY) */
 
 #if TARGET_BIG_ENDIAN
 # define cpu_lduw_datacpu_lduw_be_data
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index c938eb96f8..e591992d0c 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -44,6 +44,9 @@
 #include "tb-context.h"
 #include "internal-common.h"
 #include "internal-target.h"
+#if defined(CONFIG_USER_ONLY)
+#include "user-retaddr.h"
+#endif
 
 /* -icount align implementation. */
 
diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index 68b252cb8e..2575f0842f 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -31,6 +31,7 @@
 #include "tcg/tcg-ldst.h"
 #include "internal-common.h"
 #include "internal-target.h"
+#include "user-retaddr.h"
 
 __thread uintptr_t helper_retaddr;
 
-- 
2.41.0

[PATCH 20/24] exec: Declare abi_ptr type in its own 'tcg/abi_ptr.h' header

[Philippe Mathieu-Daudé]

The abi_ptr type is declared in "exec/cpu_ldst.h" with all
the load/store helpers. Some source files requiring abi_ptr
type don't need the load/store helpers. In order to simplify,
create a new "tcg/abi_ptr.h" header.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/exec/cpu_ldst.h   | 17 +++--
 include/exec/exec-all.h   |  1 +
 include/exec/translator.h |  5 -
 include/tcg/abi_ptr.h | 32 
 accel/tcg/cputlb.c|  1 +
 5 files changed, 41 insertions(+), 15 deletions(-)
 create mode 100644 include/tcg/abi_ptr.h

diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
index 25e7239cc5..c69f02b699 100644
--- a/include/exec/cpu_ldst.h
+++ b/include/exec/cpu_ldst.h
@@ -65,18 +65,9 @@
 #include "exec/memopidx.h"
 #include "qemu/int128.h"
 #include "cpu.h"
+#include "tcg/abi_ptr.h"
 
 #if defined(CONFIG_USER_ONLY)
-/* sparc32plus has 64bit long but 32bit space address
- * this can make bad result with g2h() and h2g()
- */
-#if TARGET_VIRT_ADDR_SPACE_BITS <= 32
-typedef uint32_t abi_ptr;
-#define TARGET_ABI_FMT_ptr "%x"
-#else
-typedef uint64_t abi_ptr;
-#define TARGET_ABI_FMT_ptr "%"PRIx64
-#endif
 
 #ifndef TARGET_TAGGED_ADDRESSES
 static inline abi_ptr cpu_untagged_addr(CPUState *cs, abi_ptr x)
@@ -120,10 +111,8 @@ static inline bool guest_range_valid_untagged(abi_ulong 
start, abi_ulong len)
 assert(h2g_valid(x)); \
 h2g_nocheck(x); \
 })
-#else
-typedef target_ulong abi_ptr;
-#define TARGET_ABI_FMT_ptr TARGET_FMT_lx
-#endif
+
+#endif /* CONFIG_USER_ONLY */
 
 uint32_t cpu_ldub_data(CPUArchState *env, abi_ptr ptr);
 int cpu_ldsb_data(CPUArchState *env, abi_ptr ptr);
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index df3d93a2e2..6a634c0889 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -22,6 +22,7 @@
 
 #include "cpu.h"
 #if defined(CONFIG_USER_ONLY)
+#include "tcg/abi_ptr.h"
 #include "exec/cpu_ldst.h"
 #endif
 #include "exec/translation-block.h"
diff --git a/include/exec/translator.h b/include/exec/translator.h
index 6d3f59d095..16d2449292 100644
--- a/include/exec/translator.h
+++ b/include/exec/translator.h
@@ -19,7 +19,10 @@
  */
 
 #include "qemu/bswap.h"
-#include "exec/cpu_ldst.h" /* for abi_ptr */
+#include "exec/cpu-common.h"
+#include "exec/cpu-defs.h"
+#include "cpu.h"
+#include "tcg/abi_ptr.h"
 
 /**
  * gen_intermediate_code
diff --git a/include/tcg/abi_ptr.h b/include/tcg/abi_ptr.h
new file mode 100644
index 00..415e31cabb
--- /dev/null
+++ b/include/tcg/abi_ptr.h
@@ -0,0 +1,32 @@
+/*
+ * TCG abi_ptr type
+ *
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+#ifndef TCG_ABI_PTR_H
+#define TCG_ABI_PTR_H
+
+#include "cpu-param.h"
+
+#if defined(CONFIG_USER_ONLY)
+/* sparc32plus has 64bit long but 32bit space address
+ * this can make bad result with g2h() and h2g()
+ */
+#if TARGET_VIRT_ADDR_SPACE_BITS <= 32
+typedef uint32_t abi_ptr;
+#define TARGET_ABI_FMT_ptr "%x"
+#else
+typedef uint64_t abi_ptr;
+#define TARGET_ABI_FMT_ptr "%"PRIx64
+#endif
+
+#else /* !CONFIG_USER_ONLY */
+
+#include "exec/target_long.h"
+
+typedef target_ulong abi_ptr;
+#define TARGET_ABI_FMT_ptr TARGET_FMT_lx
+
+#endif /* !CONFIG_USER_ONLY */
+
+#endif
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index db3f93fda9..c4500d3261 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -41,6 +41,7 @@
 #ifdef CONFIG_PLUGIN
 #include "qemu/plugin-memory.h"
 #endif
+#include "tcg/abi_ptr.h"
 #include "tcg/tcg-ldst.h"
 #include "tcg/oversized-guest.h"
 
-- 
2.41.0

[PATCH 02/24] exec: Expose 'target_page.h' API to user emulation

[Philippe Mathieu-Daudé]

User-only objects might benefit from the "exec/target_page.h"
API, which allows to build some objects once for all targets.

Signed-off-by: Philippe Mathieu-Daudé 
---
 meson.build  |  2 +-
 page-target.c| 43 +++
 system/physmem.c | 35 ---
 3 files changed, 44 insertions(+), 36 deletions(-)
 create mode 100644 page-target.c

diff --git a/meson.build b/meson.build
index d2c4c2adb3..5fdc4ef8db 100644
--- a/meson.build
+++ b/meson.build
@@ -3488,7 +3488,7 @@ if get_option('b_lto')
   pagevary = declare_dependency(link_with: pagevary)
 endif
 common_ss.add(pagevary)
-specific_ss.add(files('page-vary-target.c'))
+specific_ss.add(files('page-target.c', 'page-vary-target.c'))
 
 subdir('backends')
 subdir('disas')
diff --git a/page-target.c b/page-target.c
new file mode 100644
index 00..d286e2d58b
--- /dev/null
+++ b/page-target.c
@@ -0,0 +1,43 @@
+/*
+ * QEMU page values getters (target independent)
+ *
+ *  Copyright (c) 2003 Fabrice Bellard
+ *
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "exec/target_page.h"
+#include "exec/cpu-defs.h"
+#include "exec/cpu-all.h"
+
+size_t qemu_target_page_size(void)
+{
+return TARGET_PAGE_SIZE;
+}
+
+int qemu_target_page_mask(void)
+{
+return TARGET_PAGE_MASK;
+}
+
+int qemu_target_page_bits(void)
+{
+return TARGET_PAGE_BITS;
+}
+
+int qemu_target_page_bits_min(void)
+{
+return TARGET_PAGE_BITS_MIN;
+}
+
+/* Convert target pages to MiB (2**20). */
+size_t qemu_target_pages_to_MiB(size_t pages)
+{
+int page_bits = TARGET_PAGE_BITS;
+
+/* So far, the largest (non-huge) page size is 64k, i.e. 16 bits. */
+g_assert(page_bits < 20);
+
+return pages >> (20 - page_bits);
+}
diff --git a/system/physmem.c b/system/physmem.c
index a63853a7bc..4bdb3d0592 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -3422,41 +3422,6 @@ int cpu_memory_rw_debug(CPUState *cpu, vaddr addr,
 return 0;
 }
 
-/*
- * Allows code that needs to deal with migration bitmaps etc to still be built
- * target independent.
- */
-size_t qemu_target_page_size(void)
-{
-return TARGET_PAGE_SIZE;
-}
-
-int qemu_target_page_mask(void)
-{
-return TARGET_PAGE_MASK;
-}
-
-int qemu_target_page_bits(void)
-{
-return TARGET_PAGE_BITS;
-}
-
-int qemu_target_page_bits_min(void)
-{
-return TARGET_PAGE_BITS_MIN;
-}
-
-/* Convert target pages to MiB (2**20). */
-size_t qemu_target_pages_to_MiB(size_t pages)
-{
-int page_bits = TARGET_PAGE_BITS;
-
-/* So far, the largest (non-huge) page size is 64k, i.e. 16 bits. */
-g_assert(page_bits < 20);
-
-return pages >> (20 - page_bits);
-}
-
 bool cpu_physical_memory_is_io(hwaddr phys_addr)
 {
 MemoryRegion*mr;
-- 
2.41.0

[PATCH 09/24] hw/ppc/spapr_hcall: Remove unused 'exec/exec-all.h' included header

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/ppc/spapr_hcall.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 522a2396c7..fcefd1d1c7 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -8,7 +8,6 @@
 #include "qemu/main-loop.h"
 #include "qemu/module.h"
 #include "qemu/error-report.h"
-#include "exec/exec-all.h"
 #include "exec/tb-flush.h"
 #include "helper_regs.h"
 #include "hw/ppc/ppc.h"
-- 
2.41.0

[PATCH 01/24] exec: Include 'cpu.h' before validating CPUArchState placement

[Philippe Mathieu-Daudé]

CPUArchState 'env' field is defined within the ArchCPU structure,
so we need to include each target "cpu.h" header which defines it.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/exec/cpu-all.h | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 5340907cfd..9a7b5737d3 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -411,10 +411,6 @@ static inline bool tlb_hit(uint64_t tlb_addr, vaddr addr)
 /* accel/tcg/cpu-exec.c */
 int cpu_exec(CPUState *cpu);
 
-/* Validate correct placement of CPUArchState. */
-QEMU_BUILD_BUG_ON(offsetof(ArchCPU, parent_obj) != 0);
-QEMU_BUILD_BUG_ON(offsetof(ArchCPU, env) != sizeof(CPUState));
-
 /**
  * env_archcpu(env)
  * @env: The architecture environment
@@ -437,4 +433,9 @@ static inline CPUState *env_cpu(CPUArchState *env)
 return (void *)env - sizeof(CPUState);
 }
 
+/* Validate correct placement of CPUArchState. */
+#include "cpu.h"
+QEMU_BUILD_BUG_ON(offsetof(ArchCPU, parent_obj) != 0);
+QEMU_BUILD_BUG_ON(offsetof(ArchCPU, env) != sizeof(CPUState));
+
 #endif /* CPU_ALL_H */
-- 
2.41.0

[PATCH 03/24] target: Define TCG_GUEST_DEFAULT_MO in 'cpu-param.h'

[Philippe Mathieu-Daudé]

accel/tcg/ files requires the following definitions:

  - TARGET_LONG_BITS
  - TARGET_PAGE_BITS
  - TARGET_PHYS_ADDR_SPACE_BITS
  - TCG_GUEST_DEFAULT_MO

The first 3 are defined in "cpu-param.h". The last one
in "cpu.h", with a bunch of definitions irrelevant for
TCG. By moving the TCG_GUEST_DEFAULT_MO definition to
"cpu-param.h", we can simplify various accel/tcg includes.

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/alpha/cpu-param.h  |  3 +++
 target/alpha/cpu.h|  3 ---
 target/arm/cpu-param.h|  8 +---
 target/arm/cpu.h  |  3 ---
 target/avr/cpu-param.h|  2 ++
 target/avr/cpu.h  |  2 --
 target/hppa/cpu-param.h   |  6 ++
 target/hppa/cpu.h |  6 --
 target/i386/cpu-param.h   |  3 +++
 target/i386/cpu.h |  3 ---
 target/loongarch/cpu-param.h  |  2 ++
 target/loongarch/cpu.h|  2 --
 target/microblaze/cpu-param.h |  3 +++
 target/microblaze/cpu.h   |  3 ---
 target/mips/cpu-param.h   |  2 ++
 target/mips/cpu.h |  2 --
 target/openrisc/cpu-param.h   |  2 ++
 target/openrisc/cpu.h |  2 --
 target/ppc/cpu-param.h|  2 ++
 target/ppc/cpu.h  |  2 --
 target/riscv/cpu-param.h  |  2 ++
 target/riscv/cpu.h|  2 --
 target/s390x/cpu-param.h  |  6 ++
 target/s390x/cpu.h|  3 ---
 target/sparc/cpu-param.h  | 23 +++
 target/sparc/cpu.h| 23 ---
 target/xtensa/cpu-param.h |  3 +++
 target/xtensa/cpu.h   |  3 ---
 28 files changed, 64 insertions(+), 62 deletions(-)

diff --git a/target/alpha/cpu-param.h b/target/alpha/cpu-param.h
index 68c46f7998..419e454702 100644
--- a/target/alpha/cpu-param.h
+++ b/target/alpha/cpu-param.h
@@ -15,4 +15,7 @@
 #define TARGET_PHYS_ADDR_SPACE_BITS  44
 #define TARGET_VIRT_ADDR_SPACE_BITS  (30 + TARGET_PAGE_BITS)
 
+/* Alpha processors have a weak memory model */
+#define TCG_GUEST_DEFAULT_MO  (0)
+
 #endif
diff --git a/target/alpha/cpu.h b/target/alpha/cpu.h
index d672e911dd..5d9aa09ed9 100644
--- a/target/alpha/cpu.h
+++ b/target/alpha/cpu.h
@@ -24,9 +24,6 @@
 #include "exec/cpu-defs.h"
 #include "qemu/cpu-float.h"
 
-/* Alpha processors have a weak memory model */
-#define TCG_GUEST_DEFAULT_MO  (0)
-
 #define ICACHE_LINE_SIZE 32
 #define DCACHE_LINE_SIZE 32
 
diff --git a/target/arm/cpu-param.h b/target/arm/cpu-param.h
index f9b462a98f..59a5f9e480 100644
--- a/target/arm/cpu-param.h
+++ b/target/arm/cpu-param.h
@@ -23,14 +23,16 @@
 # ifdef TARGET_AARCH64
 #  define TARGET_TAGGED_ADDRESSES
 # endif
-#else
+#else /* !CONFIG_USER_ONLY */
 /*
  * ARMv7 and later CPUs have 4K pages minimum, but ARMv5 and v6
  * have to support 1K tiny pages.
  */
 # define TARGET_PAGE_BITS_VARY
 # define TARGET_PAGE_BITS_MIN  10
-
-#endif
+#endif /* !CONFIG_USER_ONLY */
+
+/* ARM processors have a weak memory model */
+#define TCG_GUEST_DEFAULT_MO  (0)
 
 #endif
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index a0282e0d28..ea5c8660dc 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -27,9 +27,6 @@
 #include "exec/cpu-defs.h"
 #include "qapi/qapi-types-common.h"
 
-/* ARM processors have a weak memory model */
-#define TCG_GUEST_DEFAULT_MO  (0)
-
 #ifdef TARGET_AARCH64
 #define KVM_HAVE_MCE_INJECTION 1
 #endif
diff --git a/target/avr/cpu-param.h b/target/avr/cpu-param.h
index 9a92bc74fc..93c2f470d0 100644
--- a/target/avr/cpu-param.h
+++ b/target/avr/cpu-param.h
@@ -32,4 +32,6 @@
 #define TARGET_PHYS_ADDR_SPACE_BITS 24
 #define TARGET_VIRT_ADDR_SPACE_BITS 24
 
+#define TCG_GUEST_DEFAULT_MO 0
+
 #endif
diff --git a/target/avr/cpu.h b/target/avr/cpu.h
index 7960c5c57a..02a787b4f3 100644
--- a/target/avr/cpu.h
+++ b/target/avr/cpu.h
@@ -30,8 +30,6 @@
 
 #define CPU_RESOLVING_TYPE TYPE_AVR_CPU
 
-#define TCG_GUEST_DEFAULT_MO 0
-
 /*
  * AVR has two memory spaces, data & code.
  * e.g. both have 0 address
diff --git a/target/hppa/cpu-param.h b/target/hppa/cpu-param.h
index bb3d7ef6f7..4548103a18 100644
--- a/target/hppa/cpu-param.h
+++ b/target/hppa/cpu-param.h
@@ -21,4 +21,10 @@
 
 #define TARGET_PAGE_BITS 12
 
+/* PA-RISC 1.x processors have a strong memory model.  */
+/* ??? While we do not yet implement PA-RISC 2.0, those processors have
+   a weak memory model, but with TLB bits that force ordering on a per-page
+   basis.  It's probably easier to fall back to a strong memory model.  */
+#define TCG_GUEST_DEFAULT_MOTCG_MO_ALL
+
 #endif
diff --git a/target/hppa/cpu.h b/target/hppa/cpu.h
index 8be45c69c9..6b10ab20ba 100644
--- a/target/hppa/cpu.h
+++ b/target/hppa/cpu.h
@@ -25,12 +25,6 @@
 #include "qemu/cpu-float.h"
 #include "qemu/interval-tree.h"
 
-/* PA-RISC 1.x processors have a strong memory model.  */
-/* ??? While we do not yet implement PA-RISC 2.0, those processors have
-   a weak memory model, but with TLB bits that force ordering on a per-page
-   basis.  It's probably easier to fall back to a strong 

[PATCH 14/24] gdbstub: Include missing 'hw/core/cpu.h' header

[Philippe Mathieu-Daudé]

Functions such gdb_get_cpu_pid() dereference CPUState so
require the structure declaration from "hw/core/cpu.h":

  static uint32_t gdb_get_cpu_pid(CPUState *cpu)
  {
...
return cpu->cluster_index + 1;
  }

Signed-off-by: Philippe Mathieu-Daudé 
---
 gdbstub/gdbstub.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 46d752bbc2..034a4ac211 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -37,6 +37,7 @@
 #include "hw/cpu/cluster.h"
 #include "hw/boards.h"
 #endif
+#include "hw/core/cpu.h"
 
 #include "sysemu/hw_accel.h"
 #include "sysemu/runstate.h"
-- 
2.41.0

[PATCH 07/24] host/load-extract: Include missing 'qemu/atomic.h' and 'qemu/int128.h'

[Philippe Mathieu-Daudé]

int128_make128(), int128_getlo() and int128_urshift() are
declared in "qemu/int128.h". qatomic_read__nocheck() is
declared in "qemu/atomic.h".

Signed-off-by: Philippe Mathieu-Daudé 
---
 host/include/generic/host/load-extract-al16-al8.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/host/include/generic/host/load-extract-al16-al8.h 
b/host/include/generic/host/load-extract-al16-al8.h
index d95556130f..6b47339b57 100644
--- a/host/include/generic/host/load-extract-al16-al8.h
+++ b/host/include/generic/host/load-extract-al16-al8.h
@@ -8,6 +8,9 @@
 #ifndef HOST_LOAD_EXTRACT_AL16_AL8_H
 #define HOST_LOAD_EXTRACT_AL16_AL8_H
 
+#include "qemu/atomic.h"
+#include "qemu/int128.h"
+
 /**
  * load_atom_extract_al16_or_al8:
  * @pv: host address
-- 
2.41.0

[PATCH 17/24] target/ppc/excp_helper: Avoid 'abi_ptr' in system emulation

[Philippe Mathieu-Daudé]

'abi_ptr' is a user specific type. The system emulation
equivalent is 'target_ulong'. Use it in ppc_ldl_code()
to emphasis this is not an user emulation function.

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/ppc/excp_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index a42743a3e0..3d7c9bbf1a 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -142,7 +142,7 @@ static inline bool insn_need_byteswap(CPUArchState *env)
 return !!(env->msr & ((target_ulong)1 << MSR_LE));
 }
 
-static uint32_t ppc_ldl_code(CPUArchState *env, abi_ptr addr)
+static uint32_t ppc_ldl_code(CPUArchState *env, target_ulong addr)
 {
 uint32_t insn = cpu_ldl_code(env, addr);
 
-- 
2.41.0

Re: [PATCH 00/24] exec: Rework of various headers (user focused)

[Philippe Mathieu-Daudé]

(Forgot to Cc rev.ng folks)

On 11/12/23 22:19, Philippe Mathieu-Daudé wrote:

Hi,

These patches are extracted from a bigger work where
"exec/{exec,cpu,translate}-all.h" are split in various
specific APIs. This helped:
   - differenciate/build:
   . user VS system
   . target-specific VS generic
 which is necessary for heterogeneous build
   - reduced header pressure
   - clarify APIs

This series is focused on user (vs system) cleanups.
More useful changes will come after.

Regards,

Phil.

Philippe Mathieu-Daudé (24):
   exec: Include 'cpu.h' before validating CPUArchState placement
   exec: Expose 'target_page.h' API to user emulation
   target: Define TCG_GUEST_DEFAULT_MO in 'cpu-param.h'
   accel: Include missing 'exec/cpu_ldst.h' header
   semihosting/uaccess: Avoid including 'cpu.h'
   semihosting/guestfd: Remove unused 'semihosting/uaccess.h' header
   host/load-extract: Include missing 'qemu/atomic.h' and 'qemu/int128.h'
   host/atomic128: Include missing 'qemu/atomic.h' header
   hw/ppc/spapr_hcall: Remove unused 'exec/exec-all.h' included header
   hw/misc/mips_itu: Remove unnecessary 'exec/exec-all.h' header
   hw/s390x/ipl: Remove unused 'exec/exec-all.h' included header
   target/i386: Include missing 'exec/exec-all.h' header
   accel/tcg: Include missing 'hw/core/cpu.h' header
   gdbstub: Include missing 'hw/core/cpu.h' header
   exec/cpu-all: Remove unused headers
   exec/cpu-all: Reduce 'qemu/rcu.h' header inclusion
   target/ppc/excp_helper: Avoid 'abi_ptr' in system emulation
   accel/tcg: Un-inline retaddr helpers to 'user-retaddr.h'
   exec/user: Do not include 'cpu.h' in 'abitypes.h'
   exec: Declare abi_ptr type in its own 'tcg/abi_ptr.h' header
   exec/cpu_ldst: Avoid including 'cpu.h'
   exec/cpu-all: Restrict inclusion of 'exec/user/guest-base.h'
   exec/cpu-all: Extract page-protection definitions to
 page-prot-common.h
   target: Restrict 'sysemu/reset.h' to system emulation

[PATCH 12/24] target/i386: Include missing 'exec/exec-all.h' header

[Philippe Mathieu-Daudé]

The XRSTOR instruction ends calling tlb_flush(), declared
in "exec/exec-all.h".

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/i386/tcg/fpu_helper.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/i386/tcg/fpu_helper.c b/target/i386/tcg/fpu_helper.c
index 4430d3d380..3bb018fbae 100644
--- a/target/i386/tcg/fpu_helper.c
+++ b/target/i386/tcg/fpu_helper.c
@@ -21,6 +21,7 @@
 #include 
 #include "cpu.h"
 #include "tcg-cpu.h"
+#include "exec/exec-all.h"
 #include "exec/cpu_ldst.h"
 #include "exec/helper-proto.h"
 #include "fpu/softfloat.h"
-- 
2.41.0

[PATCH 13/24] accel/tcg: Include missing 'hw/core/cpu.h' header

[Philippe Mathieu-Daudé]

tcg_cpu_init_cflags() accesses CPUState fields, so requires
"hw/core/cpu.h" to get its structure definition.

Signed-off-by: Philippe Mathieu-Daudé 
---
 accel/tcg/tcg-accel-ops.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
index 1b57290682..58806e2d7f 100644
--- a/accel/tcg/tcg-accel-ops.c
+++ b/accel/tcg/tcg-accel-ops.c
@@ -37,6 +37,8 @@
 #include "exec/tb-flush.h"
 #include "exec/gdbstub.h"
 
+#include "hw/core/cpu.h"
+
 #include "tcg-accel-ops.h"
 #include "tcg-accel-ops-mttcg.h"
 #include "tcg-accel-ops-rr.h"
-- 
2.41.0

[PATCH 11/24] hw/s390x/ipl: Remove unused 'exec/exec-all.h' included header

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/s390x/ipl.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 515dcf51b5..62182d81a0 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -35,7 +35,6 @@
 #include "qemu/cutils.h"
 #include "qemu/option.h"
 #include "standard-headers/linux/virtio_ids.h"
-#include "exec/exec-all.h"
 
 #define KERN_IMAGE_START0x01UL
 #define LINUX_MAGIC_ADDR0x010008UL
-- 
2.41.0

[PATCH 10/24] hw/misc/mips_itu: Remove unnecessary 'exec/exec-all.h' header

[Philippe Mathieu-Daudé]

mips_itu.c only requires declarations from "hw/core/cpu.h"
and "cpu.h". Avoid including the huge "exec/exec-all.h" header.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/misc/mips_itu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/misc/mips_itu.c b/hw/misc/mips_itu.c
index 5a83ccc4e8..37aea0e737 100644
--- a/hw/misc/mips_itu.c
+++ b/hw/misc/mips_itu.c
@@ -22,9 +22,10 @@
 #include "qemu/log.h"
 #include "qemu/module.h"
 #include "qapi/error.h"
-#include "exec/exec-all.h"
+#include "hw/core/cpu.h"
 #include "hw/misc/mips_itu.h"
 #include "hw/qdev-properties.h"
+#include "target/mips/cpu.h"
 
 #define ITC_TAG_ADDRSPACE_SZ (ITC_ADDRESSMAP_NUM * 8)
 /* Initialize as 4kB area to fit all 32 cells with default 128B grain.
-- 
2.41.0

[PATCH 06/24] semihosting/guestfd: Remove unused 'semihosting/uaccess.h' header

[Philippe Mathieu-Daudé]

Nothing in guestfd.c requires "semihosting/uaccess.h".

Signed-off-by: Philippe Mathieu-Daudé 
---
 semihosting/guestfd.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/semihosting/guestfd.c b/semihosting/guestfd.c
index 955c2efbd0..fd7e609790 100644
--- a/semihosting/guestfd.c
+++ b/semihosting/guestfd.c
@@ -15,7 +15,6 @@
 #ifdef CONFIG_USER_ONLY
 #include "qemu.h"
 #else
-#include "semihosting/uaccess.h"
 #include CONFIG_DEVICES
 #endif
 
-- 
2.41.0

[PATCH 05/24] semihosting/uaccess: Avoid including 'cpu.h'

[Philippe Mathieu-Daudé]

"semihosting/uaccess.h" only requires declarations
from "exec/cpu-defs.h". Avoid including the huge "cpu.h".

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/semihosting/uaccess.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/semihosting/uaccess.h b/include/semihosting/uaccess.h
index 3963eafc3e..6c8835fbcb 100644
--- a/include/semihosting/uaccess.h
+++ b/include/semihosting/uaccess.h
@@ -14,7 +14,7 @@
 #error Cannot include semihosting/uaccess.h from user emulation
 #endif
 
-#include "cpu.h"
+#include "exec/cpu-defs.h"
 
 #define get_user_u64(val, addr) \
 ({ uint64_t val_ = 0;   \
-- 
2.41.0

[PATCH 08/24] host/atomic128: Include missing 'qemu/atomic.h' header

[Philippe Mathieu-Daudé]

qatomic_cmpxchg__nocheck(), qatomic_read__nocheck(),
qatomic_set__nocheck() are defined in "qemu/atomic.h".
Include it in order to avoid:

  In file included from include/exec/helper-proto.h:10:
  In file included from include/exec/helper-proto-common.h:10:
  In file included from include/qemu/atomic128.h:61:
  In file included from host/include/aarch64/host/atomic128-cas.h:16:
  host/include/generic/host/atomic128-cas.h:23:11: error: call to undeclared 
function 'qatomic_cmpxchg__nocheck'; ISO C99 and later do not support implicit 
function declarations [-Wimplicit-function-declaration]
r.i = qatomic_cmpxchg__nocheck(ptr_align, c.i, n.i);
  ^

Signed-off-by: Philippe Mathieu-Daudé 
---
 host/include/generic/host/atomic128-cas.h  | 2 ++
 host/include/generic/host/atomic128-ldst.h | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/host/include/generic/host/atomic128-cas.h 
b/host/include/generic/host/atomic128-cas.h
index 6b40cc2271..4824f14659 100644
--- a/host/include/generic/host/atomic128-cas.h
+++ b/host/include/generic/host/atomic128-cas.h
@@ -11,6 +11,8 @@
 #ifndef HOST_ATOMIC128_CAS_H
 #define HOST_ATOMIC128_CAS_H
 
+#include "qemu/atomic.h"
+
 #if defined(CONFIG_ATOMIC128)
 static inline Int128 ATTRIBUTE_ATOMIC128_OPT
 atomic16_cmpxchg(Int128 *ptr, Int128 cmp, Int128 new)
diff --git a/host/include/generic/host/atomic128-ldst.h 
b/host/include/generic/host/atomic128-ldst.h
index 691e6a8531..12e4aca2da 100644
--- a/host/include/generic/host/atomic128-ldst.h
+++ b/host/include/generic/host/atomic128-ldst.h
@@ -11,6 +11,8 @@
 #ifndef HOST_ATOMIC128_LDST_H
 #define HOST_ATOMIC128_LDST_H
 
+#include "qemu/atomic.h"
+
 #if defined(CONFIG_ATOMIC128)
 # define HAVE_ATOMIC128_RO 1
 # define HAVE_ATOMIC128_RW 1
-- 
2.41.0

[PATCH 00/24] exec: Rework of various headers (user focused)

[Philippe Mathieu-Daudé]

Hi,

These patches are extracted from a bigger work where
"exec/{exec,cpu,translate}-all.h" are split in various
specific APIs. This helped:
  - differenciate/build:
  . user VS system
  . target-specific VS generic
which is necessary for heterogeneous build
  - reduced header pressure
  - clarify APIs

This series is focused on user (vs system) cleanups.
More useful changes will come after.

Regards,

Phil.

Philippe Mathieu-Daudé (24):
  exec: Include 'cpu.h' before validating CPUArchState placement
  exec: Expose 'target_page.h' API to user emulation
  target: Define TCG_GUEST_DEFAULT_MO in 'cpu-param.h'
  accel: Include missing 'exec/cpu_ldst.h' header
  semihosting/uaccess: Avoid including 'cpu.h'
  semihosting/guestfd: Remove unused 'semihosting/uaccess.h' header
  host/load-extract: Include missing 'qemu/atomic.h' and 'qemu/int128.h'
  host/atomic128: Include missing 'qemu/atomic.h' header
  hw/ppc/spapr_hcall: Remove unused 'exec/exec-all.h' included header
  hw/misc/mips_itu: Remove unnecessary 'exec/exec-all.h' header
  hw/s390x/ipl: Remove unused 'exec/exec-all.h' included header
  target/i386: Include missing 'exec/exec-all.h' header
  accel/tcg: Include missing 'hw/core/cpu.h' header
  gdbstub: Include missing 'hw/core/cpu.h' header
  exec/cpu-all: Remove unused headers
  exec/cpu-all: Reduce 'qemu/rcu.h' header inclusion
  target/ppc/excp_helper: Avoid 'abi_ptr' in system emulation
  accel/tcg: Un-inline retaddr helpers to 'user-retaddr.h'
  exec/user: Do not include 'cpu.h' in 'abitypes.h'
  exec: Declare abi_ptr type in its own 'tcg/abi_ptr.h' header
  exec/cpu_ldst: Avoid including 'cpu.h'
  exec/cpu-all: Restrict inclusion of 'exec/user/guest-base.h'
  exec/cpu-all: Extract page-protection definitions to
page-prot-common.h
  target: Restrict 'sysemu/reset.h' to system emulation

 meson.build   |  2 +-
 accel/tcg/user-retaddr.h  | 28 ++
 bsd-user/bsd-mem.h|  1 +
 bsd-user/qemu.h   |  1 +
 host/include/generic/host/atomic128-cas.h |  2 +
 host/include/generic/host/atomic128-ldst.h|  2 +
 .../generic/host/load-extract-al16-al8.h  |  3 ++
 include/exec/cpu-all.h| 50 +++---
 include/exec/cpu_ldst.h   | 51 ---
 include/exec/exec-all.h   |  1 +
 include/exec/page-prot-common.h   | 38 ++
 include/exec/ram_addr.h   |  1 +
 include/exec/translator.h |  5 +-
 include/exec/user/abitypes.h  |  9 +++-
 include/exec/user/guest-base.h|  6 +++
 include/semihosting/uaccess.h |  3 +-
 include/tcg/abi_ptr.h | 32 
 target/alpha/cpu-param.h  |  3 ++
 target/alpha/cpu.h|  3 --
 target/arm/cpu-param.h|  8 +--
 target/arm/cpu.h  |  4 +-
 target/avr/cpu-param.h|  2 +
 target/avr/cpu.h  |  2 -
 target/hppa/cpu-param.h   |  6 +++
 target/hppa/cpu.h |  6 ---
 target/i386/cpu-param.h   |  3 ++
 target/i386/cpu.h |  3 --
 target/loongarch/cpu-param.h  |  2 +
 target/loongarch/cpu.h|  2 -
 target/microblaze/cpu-param.h |  3 ++
 target/microblaze/cpu.h   |  3 --
 target/mips/cpu-param.h   |  2 +
 target/mips/cpu.h |  2 -
 target/openrisc/cpu-param.h   |  2 +
 target/openrisc/cpu.h |  2 -
 target/ppc/cpu-param.h|  2 +
 target/ppc/cpu.h  |  2 -
 target/ppc/internal.h |  1 +
 target/ppc/mmu-radix64.h  |  2 +
 target/riscv/cpu-param.h  |  2 +
 target/riscv/cpu.h|  2 -
 target/s390x/cpu-param.h  |  6 +++
 target/s390x/cpu.h|  3 --
 target/sparc/cpu-param.h  | 23 +
 target/sparc/cpu.h| 23 -
 target/xtensa/cpu-param.h |  3 ++
 target/xtensa/cpu.h   |  3 --
 accel/tcg/cpu-exec.c  |  3 ++
 accel/tcg/cputlb.c|  2 +
 accel/tcg/tb-maint.c  |  1 +
 accel/tcg/tcg-accel-ops.c |  2 +
 accel/tcg/translator.c|  1 +
 accel/tcg/user-exec.c |  2 +
 bsd-user/main.c   |  1 +
 bsd-user/mmap.c   |  1 +
 bsd-user/signal.c |  1 +
 cpu-target.c  

Re: [PULL 20/20] tracing: install trace events file only if necessary

[Stefan Hajnoczi]

On Wed, Dec 06, 2023 at 07:26:01AM -0300, Carlos Santos wrote:
> On Thu, Apr 20, 2023 at 9:10 AM Stefan Hajnoczi  wrote:
> >
> > From: Carlos Santos 
> >
> > It is not useful when configuring with --enable-trace-backends=nop.
> >
> > Signed-off-by: Carlos Santos 
> > Signed-off-by: Stefan Hajnoczi 
> > Message-Id: <20230408010410.281263-1-casan...@redhat.com>
> > ---
> >  trace/meson.build | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/trace/meson.build b/trace/meson.build
> > index 8e80be895c..30b1d942eb 100644
> > --- a/trace/meson.build
> > +++ b/trace/meson.build
> > @@ -64,7 +64,7 @@ trace_events_all = custom_target('trace-events-all',
> >   input: trace_events_files,
> >   command: [ 'cat', '@INPUT@' ],
> >   capture: true,
> > - install: true,
> > + install: get_option('trace_backends') != 
> > [ 'nop' ],
> >   install_dir: qemu_datadir)
> >
> >  if 'ust' in get_option('trace_backends')
> > --
> > 2.39.2
> >
> 
> Hello,
> 
> I still don't see this in the master branch. Is there something
> preventing it from being applied?

Hi Carlos,
Apologies, I dropped this patch when respinning the pull request after
the CI test failures caused by the zoned patches.

Your patch has been merged on my tracing tree again and will make it
into qemu.git/master when the development window opens again after the
QEMU 8.2.0 release (hopefully next Tuesday).

Stefan

> 
> -- 
> Carlos Santos
> Senior Software Maintenance Engineer
> Red Hat
> casan...@redhat.comT: +55-11-3534-6186
> 


signature.asc
Description: PGP signature

[RFC] string-output-visitor: show structs as ""

[Stefan Hajnoczi]

StringOutputVisitor crashes when it visits a struct because
->start_struct() is NULL.

Show "" instead of crashing. This is necessary because the
virtio-blk-pci iothread-vq-mapping parameter that I'd like to introduce
soon is a list of IOThreadMapping structs.

Cc: Markus Armbruster 
Signed-off-by: Stefan Hajnoczi 
---
Can we do better?

I am unfamiliar with StringOutputVisitor, so I wasn't sure how to
proceed. Is the format or at least the intended use of
StringOutputVisitor's output defined somewhere? Does it need to be a
single line or can the output be multiple lines?

Or maybe I shouldn't introduce a qdev property with IOThreadMappingList
as its type in
https://lore.kernel.org/qemu-devel/zuopifxiiwfq5...@redhat.com/?
---
 include/qapi/string-output-visitor.h |  6 +++---
 qapi/string-output-visitor.c | 14 ++
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/include/qapi/string-output-visitor.h 
b/include/qapi/string-output-visitor.h
index 268dfe9986..762fe3f705 100644
--- a/include/qapi/string-output-visitor.h
+++ b/include/qapi/string-output-visitor.h
@@ -26,9 +26,9 @@ typedef struct StringOutputVisitor StringOutputVisitor;
  * If everything else succeeds, pass @result to visit_complete() to
  * collect the result of the visit.
  *
- * The string output visitor does not implement support for visiting
- * QAPI structs, alternates, null, or arbitrary QTypes.  It also
- * requires a non-null list argument to visit_start_list().
+ * The string output visitor does not implement support for alternates, null,
+ * or arbitrary QTypes.  It also requires a non-null list argument to
+ * visit_start_list().
  */
 Visitor *string_output_visitor_new(bool human, char **result);
 
diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
index c0cb72dbe4..363dac00fe 100644
--- a/qapi/string-output-visitor.c
+++ b/qapi/string-output-visitor.c
@@ -292,6 +292,18 @@ static bool print_type_null(Visitor *v, const char *name, 
QNull **obj,
 return true;
 }
 
+static bool start_struct(Visitor *v, const char *name, void **obj,
+ size_t size, Error **errp)
+{
+return true;
+}
+
+static void end_struct(Visitor *v, void **obj)
+{
+StringOutputVisitor *sov = to_sov(v);
+string_output_set(sov, g_strdup(""));
+}
+
 static bool
 start_list(Visitor *v, const char *name, GenericList **list, size_t size,
Error **errp)
@@ -379,6 +391,8 @@ Visitor *string_output_visitor_new(bool human, char 
**result)
 v->visitor.type_str = print_type_str;
 v->visitor.type_number = print_type_number;
 v->visitor.type_null = print_type_null;
+v->visitor.start_struct = start_struct;
+v->visitor.end_struct = end_struct;
 v->visitor.start_list = start_list;
 v->visitor.next_list = next_list;
 v->visitor.end_list = end_list;
-- 
2.43.0

Re: [PATCH v8 00/19] virtio-net RSS/hash report fixes and improvements

[Yuri Benditovich]

I'm adding also Yan

On Mon, Dec 11, 2023 at 9:51 PM Yuri Benditovich <
yuri.benditov...@daynix.com> wrote:

> Hi Michael,
> Sure, I've reviewed that also, there was a fruitful discussion
> till the series rеаched its final form.
> At the beginning of September we've got the response from Jason that the
> series is queued upstream so we were calm and switched to libvirt part ))
>
> Seems like a misunderstanding, let's wait for Jason response.
>
> Thanks,
> Yuri
>
>
>
>
> On Mon, Dec 11, 2023 at 5:43 PM Michael S. Tsirkin  wrote:
>
>> On Mon, Dec 11, 2023 at 02:34:56PM +0200, Yuri Benditovich wrote:
>> > https://lists.gnu.org/archive/html/qemu-devel/2023-08/msg05859.html
>>
>> It's from August, I think it's fair to say it's not going upstream
>> unless there's some activity. Yuri did you review that series then?
>> Care to ack?
>>
>> --
>> MST
>>
>>

Re: [PATCH v8 00/19] virtio-net RSS/hash report fixes and improvements

[Yuri Benditovich]

Hi Michael,
Sure, I've reviewed that also, there was a fruitful discussion
till the series rеаched its final form.
At the beginning of September we've got the response from Jason that the
series is queued upstream so we were calm and switched to libvirt part ))

Seems like a misunderstanding, let's wait for Jason response.

Thanks,
Yuri




On Mon, Dec 11, 2023 at 5:43 PM Michael S. Tsirkin  wrote:

> On Mon, Dec 11, 2023 at 02:34:56PM +0200, Yuri Benditovich wrote:
> > https://lists.gnu.org/archive/html/qemu-devel/2023-08/msg05859.html
>
> It's from August, I think it's fair to say it's not going upstream
> unless there's some activity. Yuri did you review that series then?
> Care to ack?
>
> --
> MST
>
>

Re: [PATCH v2 12/20] migration/multifd: Add new migration option for multifd DSA offloading.

[Fabiano Rosas]

Hao Xiang  writes:

> Intel DSA offloading is an optional feature that turns on if
> proper hardware and software stack is available. To turn on
> DSA offloading in multifd live migration:
>
> multifd-dsa-accel="[dsa_dev_path1] ] [dsa_dev_path2] ... [dsa_dev_pathX]"
>
> This feature is turned off by default.

This patch breaks make check:

 43/357 qemu:qtest+qtest-x86_64 / qtest-x86_64/test-hmp   ERROR 
  0.52s
 79/357 qemu:qtest+qtest-x86_64 / qtest-x86_64/migration-test ERROR 
  3.59s
167/357 qemu:qtest+qtest-x86_64 / qtest-x86_64/qmp-cmd-test ERROR   
3.68s

Make sure you run make check before posting. Ideally also run the series
through the Gitlab CI on your personal fork.

> Signed-off-by: Hao Xiang 
> ---
>  migration/migration-hmp-cmds.c |  8 
>  migration/options.c| 28 
>  migration/options.h|  1 +
>  qapi/migration.json| 17 ++---
>  scripts/meson-buildoptions.sh  |  6 +++---
>  5 files changed, 54 insertions(+), 6 deletions(-)
>
> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
> index 86ae832176..d9451744dd 100644
> --- a/migration/migration-hmp-cmds.c
> +++ b/migration/migration-hmp-cmds.c
> @@ -353,6 +353,9 @@ void hmp_info_migrate_parameters(Monitor *mon, const 
> QDict *qdict)
>  monitor_printf(mon, "%s: '%s'\n",
>  MigrationParameter_str(MIGRATION_PARAMETER_TLS_AUTHZ),
>  params->tls_authz);
> +monitor_printf(mon, "%s: %s\n",

Use '%s' here.

> +MigrationParameter_str(MIGRATION_PARAMETER_MULTIFD_DSA_ACCEL),
> +params->multifd_dsa_accel);
>  
>  if (params->has_block_bitmap_mapping) {
>  const BitmapMigrationNodeAliasList *bmnal;
> @@ -615,6 +618,11 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict 
> *qdict)
>  p->has_block_incremental = true;
>  visit_type_bool(v, param, >block_incremental, );
>  break;
> +case MIGRATION_PARAMETER_MULTIFD_DSA_ACCEL:
> +p->multifd_dsa_accel = g_new0(StrOrNull, 1);
> +p->multifd_dsa_accel->type = QTYPE_QSTRING;
> +visit_type_str(v, param, >multifd_dsa_accel->u.s, );
> +break;
>  case MIGRATION_PARAMETER_MULTIFD_CHANNELS:
>  p->has_multifd_channels = true;
>  visit_type_uint8(v, param, >multifd_channels, );
> diff --git a/migration/options.c b/migration/options.c
> index 97d121d4d7..6e424b5d63 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -179,6 +179,8 @@ Property migration_properties[] = {
>  DEFINE_PROP_MIG_MODE("mode", MigrationState,
>parameters.mode,
>MIG_MODE_NORMAL),
> +DEFINE_PROP_STRING("multifd-dsa-accel", MigrationState,
> +   parameters.multifd_dsa_accel),
>  
>  /* Migration capabilities */
>  DEFINE_PROP_MIG_CAP("x-xbzrle", MIGRATION_CAPABILITY_XBZRLE),
> @@ -901,6 +903,13 @@ const char *migrate_tls_creds(void)
>  return s->parameters.tls_creds;
>  }
>  
> +const char *migrate_multifd_dsa_accel(void)
> +{
> +MigrationState *s = migrate_get_current();
> +
> +return s->parameters.multifd_dsa_accel;
> +}
> +
>  const char *migrate_tls_hostname(void)
>  {
>  MigrationState *s = migrate_get_current();
> @@ -1025,6 +1034,7 @@ MigrationParameters *qmp_query_migrate_parameters(Error 
> **errp)
>  params->vcpu_dirty_limit = s->parameters.vcpu_dirty_limit;
>  params->has_mode = true;
>  params->mode = s->parameters.mode;
> +params->multifd_dsa_accel = s->parameters.multifd_dsa_accel;
>  
>  return params;
>  }
> @@ -1033,6 +1043,7 @@ void migrate_params_init(MigrationParameters *params)
>  {
>  params->tls_hostname = g_strdup("");
>  params->tls_creds = g_strdup("");
> +params->multifd_dsa_accel = g_strdup("");
>  
>  /* Set has_* up only for parameter checks */
>  params->has_compress_level = true;
> @@ -1362,6 +1373,11 @@ static void 
> migrate_params_test_apply(MigrateSetParameters *params,
>  if (params->has_mode) {
>  dest->mode = params->mode;
>  }
> +
> +if (params->multifd_dsa_accel) {
> +assert(params->multifd_dsa_accel->type == QTYPE_QSTRING);
> +dest->multifd_dsa_accel = params->multifd_dsa_accel->u.s;
> +}
>  }
>  
>  static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
> @@ -1506,6 +1522,12 @@ static void migrate_params_apply(MigrateSetParameters 
> *params, Error **errp)
>  if (params->has_mode) {
>  s->parameters.mode = params->mode;
>  }
> +
> +if (params->multifd_dsa_accel) {
> +g_free(s->parameters.multifd_dsa_accel);
> +assert(params->multifd_dsa_accel->type == QTYPE_QSTRING);
> +s->parameters.multifd_dsa_accel = 
> g_strdup(params->multifd_dsa_accel->u.s);
> +}
>  }
>  
>  void qmp_migrate_set_parameters(MigrateSetParameters *params, Error 

Re: QEMU developers fortnightly conference call for agenda for 2023-12-12

[Philippe Mathieu-Daudé]

Hi Zhao,

On 11/12/23 14:29, Zhao Liu wrote:

Hi Juan,

On Tue, Dec 05, 2023 at 01:47:52PM +, Juan Quintela wrote:

Date: Tue, 05 Dec 2023 13:47:52 +
From: Juan Quintela 
Subject: QEMU developers fortnightly conference call for agenda for
  2023-12-12

Hi If you have any topics for the last qemu conference call of the year,
feel free to answer to this email. Later, Juan.

QEMU developers fortnightly conference call
Tuesday 2023-12-12 ⋅ 15:00 – 16:00
Central European Time - Madrid

Location
https://meet.jit.si/kvmcallmeeting  
https://www.google.com/url?q=https%3A%2F%2Fmeet.jit.si%2Fkvmcallmeeting=D=170221602000=AOvVaw1xrpPSmMRu9niy1trqCKrA



I want to talk about the RFC about QOM topology:
https://lore.kernel.org/qemu-devel/20231130144203.2307629-1-zhao1@linux.intel.com/

We would like to receive the initial feedback from the community on this
direction to see if we are on the right track.


FYI I have your series tagged for review (for generic QOM /
machine [*]) but we need feedback from the x86 maintainers too,
and eventually from riscv/arm too, since they might end up
using your API.

[*] That said, I unlikely will have time the next 2 weeks.

Regards,

Phil.


Thanks,
Zhao

Re: [PATCH 18/21] target/arm/kvm: Init cap_has_inject_serror_esr in kvm_arch_init

[Richard Henderson]

On 12/11/23 10:43, Philippe Mathieu-Daudé wrote:

On 11/12/23 18:09, Richard Henderson wrote:

On 11/24/23 03:54, Philippe Mathieu-Daudé wrote:

On 23/11/23 05:42, Richard Henderson wrote:

There is no need to do this in kvm_arch_init_vcpu per vcpu.
Inline kvm_arm_init_serror_injection rather than keep separate.

Signed-off-by: Richard Henderson 
---
  target/arm/kvm_arm.h |  8 
  target/arm/kvm.c | 13 -
  2 files changed, 4 insertions(+), 17 deletions(-)




@@ -562,6 +556,10 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
  cap_has_mp_state = kvm_check_extension(s, KVM_CAP_MP_STATE);
+    /* Check whether user space can specify guest syndrome value */
+    cap_has_inject_serror_esr =
+    kvm_check_extension(s, KVM_CAP_ARM_INJECT_SERROR_ESR);
+
  if (ms->smp.cpus > 256 &&
  !kvm_check_extension(s, KVM_CAP_ARM_IRQ_LINE_LAYOUT_2)) {
  error_report("Using more than 256 vcpus requires a host kernel "
@@ -1948,9 +1946,6 @@ int kvm_arch_init_vcpu(CPUState *cs)
  }
  cpu->mp_affinity = mpidr & ARM64_AFFINITY_MASK;
-    /* Check whether user space can specify guest syndrome value */
-    kvm_arm_init_serror_injection(cs);
-
  return kvm_arm_init_cpreg_list(cpu);
  }



Just checking, in a heterogeneous setup we still want to keep
these 2 calls per-vCPU, right?


There is no hetrogeneous kvm -- every vcpu must match the host cpu.


So big.LITTLE will never be a KVM thing?


Not as far as I'm aware.  There are even issues *running* on big.LITTLE hosts -- you must 
use taskset to limit qemu to one cpu type, either big or little.



r~

Re: [PATCH v3 4/4] accel/tcg: Move perf and debuginfo support to tcg

[Richard Henderson]

On 12/7/23 16:35, Ilya Leoshkevich wrote:

--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -63,7 +63,7 @@
  #include "tb-context.h"
  #include "internal-common.h"
  #include "internal-target.h"
-#include "perf.h"
+#include "tcg/perf.h"
  #include "tcg/insn-start-words.h"


Because this header is used outside of tcg/, the header should be 
include/tcg/perf.h.


diff --git a/hw/core/loader.c b/hw/core/loader.c
index e7a9b3775bb..b8e52f3fb0f 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -62,7 +62,7 @@
  #include "hw/boards.h"
  #include "qemu/cutils.h"
  #include "sysemu/runstate.h"
-#include "accel/tcg/debuginfo.h"
+#include "tcg/debuginfo.h"
  
  #include 
  
diff --git a/linux-user/elfload.c b/linux-user/elfload.c

index cf9e74468b1..62120c76151 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -21,7 +21,7 @@
  #include "qapi/error.h"
  #include "qemu/error-report.h"
  #include "target_signal.h"
-#include "accel/tcg/debuginfo.h"
+#include "tcg/debuginfo.h"
  
  #ifdef TARGET_ARM

  #include "target/arm/cpu-features.h"


Likewise.

Otherwise,
Reviewed-by: Richard Henderson 

r~

Re: [PATCH 03/40] vdpa: probe descriptor group index for data vqs

[Eugenio Perez Martin]

On Thu, Dec 7, 2023 at 7:53 PM Si-Wei Liu  wrote:
>
> Getting it ahead at initialization time instead of start time allows
> decision making independent of device status, while reducing failure
> possibility in starting device or during migration.
>
> Adding function vhost_vdpa_probe_desc_group() for that end. This
> function will be used to probe the descriptor group for data vqs.
>
> Signed-off-by: Si-Wei Liu 
> ---
>  net/vhost-vdpa.c | 89 
> 
>  1 file changed, 89 insertions(+)
>
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index 887c329..0cf3147 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -1688,6 +1688,95 @@ out:
>  return r;
>  }
>
> +static int vhost_vdpa_probe_desc_group(int device_fd, uint64_t features,
> +   int vq_index, int64_t *desc_grpidx,
> +   Error **errp)
> +{
> +uint64_t backend_features;
> +int64_t vq_group, desc_group;
> +uint8_t saved_status = 0;
> +uint8_t status = 0;
> +int r;
> +
> +ERRP_GUARD();
> +
> +r = ioctl(device_fd, VHOST_GET_BACKEND_FEATURES, _features);
> +if (unlikely(r < 0)) {
> +error_setg_errno(errp, errno, "Cannot get vdpa backend_features");
> +return r;
> +}
> +
> +if (!(backend_features & BIT_ULL(VHOST_BACKEND_F_IOTLB_ASID))) {
> +return 0;
> +}
> +
> +if (!(backend_features & BIT_ULL(VHOST_BACKEND_F_DESC_ASID))) {
> +return 0;
> +}
> +
> +r = ioctl(device_fd, VHOST_VDPA_GET_STATUS, _status);
> +if (unlikely(r)) {
> +error_setg_errno(errp, -r, "Cannot get device status");
> +goto out;

Nit, we could return here directly, can't we?

> +}
> +
> +r = ioctl(device_fd, VHOST_VDPA_SET_STATUS, );
> +if (unlikely(r)) {
> +error_setg_errno(errp, -r, "Cannot reset device");
> +goto out;
> +}
> +
> +r = ioctl(device_fd, VHOST_SET_FEATURES, );
> +if (unlikely(r)) {
> +error_setg_errno(errp, errno, "Cannot set features");

missing goto out?

> +}
> +
> +status = VIRTIO_CONFIG_S_ACKNOWLEDGE |
> + VIRTIO_CONFIG_S_DRIVER |
> + VIRTIO_CONFIG_S_FEATURES_OK;
> +
> +r = ioctl(device_fd, VHOST_VDPA_SET_STATUS, );
> +if (unlikely(r)) {
> +error_setg_errno(errp, -r, "Cannot set device status");
> +goto out;
> +}
> +
> +vq_group = vhost_vdpa_get_vring_group(device_fd, vq_index, errp);
> +if (unlikely(vq_group < 0)) {
> +if (vq_group != -ENOTSUP) {
> +r = vq_group;
> +goto out;
> +}
> +
> +/*
> + * The kernel report VHOST_BACKEND_F_IOTLB_ASID if the vdpa frontend
> + * support ASID even if the parent driver does not.
> + */
> +error_free(*errp);
> +*errp = NULL;
> +r = 0;
> +goto out;
> +}
> +
> +desc_group = vhost_vdpa_get_vring_desc_group(device_fd, vq_index,
> + errp);
> +if (unlikely(desc_group < 0)) {
> +r = desc_group;
> +goto out;
> +} else if (desc_group != vq_group) {
> +*desc_grpidx = desc_group;
> +}
> +r = 1;
> +
> +out:
> +status = 0;
> +ioctl(device_fd, VHOST_VDPA_SET_STATUS, );
> +if (saved_status) {
> +ioctl(device_fd, VHOST_VDPA_SET_STATUS, _status);
> +}
> +return r;
> +}
> +

It is invalid to add static functions without a caller, I think the
compiler will complain about this.

>  static NetClientState *net_vhost_vdpa_init(NetClientState *peer,
> const char *device,
> const char *name,
> --
> 1.8.3.1
>
>

Re: QEMU developers fortnightly conference call for agenda for 2023-12-12

[Philippe Mathieu-Daudé]

Cc'ing Brian (this could be a good place to discuss Hexagon semihosting)

On 5/12/23 14:47, Juan Quintela wrote:

QEMU developers fortnightly conference call

Hi
If you have any topics for the last qemu conference call of the year, 
feel free to answer to this email.


Later, Juan.

QEMU developers fortnightly conference call
Tuesday 2023-12-12 ⋅ 15:00 – 16:00 (Central European Time - Madrid)
If you need call details, please contact me: quint...@redhat.com 




Location

https://meet.jit.si/kvmcallmeeting
View map 

Re: [PATCH v3 3/4] accel/tcg: Remove #ifdef TARGET_I386 from perf.c

[Richard Henderson]

On 12/7/23 16:35, Ilya Leoshkevich wrote:

Preparation for moving perf.c to tcg/.

This affects only profiling guest code, which has code in a non-0 based
segment, e.g., 16-bit code, which is not particularly important.

Suggested-by: Richard Henderson 
Signed-off-by: Ilya Leoshkevich 
---
  accel/tcg/perf.c | 4 
  1 file changed, 4 deletions(-)




Reviewed-by: Richard Henderson 


r~

Re: [PATCH v3 2/4] tcg: Make tb_cflags() usable from target-agnostic code

[Richard Henderson]

On 12/7/23 16:35, Ilya Leoshkevich wrote:

Currently tb_cflags() is defined in exec-all.h, which is not usable
from target-agnostic code. Move it to translation-block.h, which is.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Ilya Leoshkevich 
---
  include/exec/exec-all.h  | 6 --
  include/exec/translation-block.h | 6 ++
  2 files changed, 6 insertions(+), 6 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [PATCH 20/21] target/arm/kvm: Unexport and tidy kvm_arm_sync_mpstate_to_{kvm, qemu}

[Philippe Mathieu-Daudé]

On 11/12/23 15:34, Peter Maydell wrote:

On Fri, 24 Nov 2023 at 12:06, Philippe Mathieu-Daudé  wrote:


On 23/11/23 05:42, Richard Henderson wrote:

Drop fprintfs and actually use the return values in the callers.

Signed-off-by: Richard Henderson 
---
   target/arm/kvm_arm.h | 20 
   target/arm/kvm.c | 23 ++-
   2 files changed, 6 insertions(+), 37 deletions(-)




   /*
* Sync the KVM MP_STATE into QEMU
*/
-int kvm_arm_sync_mpstate_to_qemu(ARMCPU *cpu)
+static int kvm_arm_sync_mpstate_to_qemu(ARMCPU *cpu)
   {
   if (cap_has_mp_state) {
   struct kvm_mp_state mp_state;
   int ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MP_STATE, _state);
   if (ret) {
-fprintf(stderr, "%s: failed to get MP_STATE %d/%s\n",
-__func__, ret, strerror(-ret));
-abort();


I suppose if this abort() had fired, we'd have reworked that code...
Maybe mention its removal? Otherwise,


Well, it's a "KVM has failed in a way that's fatal for the VM"
kind of error. It's OK to drop the abort() here because since
7191f24c7fcf we will catch error returns from these arch-specific
functions in the accel/kvm generic code. When this was written
before that commit then if we didn't detect and print something
here we'd just have silently dropped the error, I think.

I added a brief note to the commit message to that effect.


Thank you.

Re: [PATCH 18/21] target/arm/kvm: Init cap_has_inject_serror_esr in kvm_arch_init

[Philippe Mathieu-Daudé]

On 11/12/23 18:09, Richard Henderson wrote:

On 11/24/23 03:54, Philippe Mathieu-Daudé wrote:

On 23/11/23 05:42, Richard Henderson wrote:

There is no need to do this in kvm_arch_init_vcpu per vcpu.
Inline kvm_arm_init_serror_injection rather than keep separate.

Signed-off-by: Richard Henderson 
---
  target/arm/kvm_arm.h |  8 
  target/arm/kvm.c | 13 -
  2 files changed, 4 insertions(+), 17 deletions(-)




@@ -562,6 +556,10 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
  cap_has_mp_state = kvm_check_extension(s, KVM_CAP_MP_STATE);
+    /* Check whether user space can specify guest syndrome value */
+    cap_has_inject_serror_esr =
+    kvm_check_extension(s, KVM_CAP_ARM_INJECT_SERROR_ESR);
+
  if (ms->smp.cpus > 256 &&
  !kvm_check_extension(s, KVM_CAP_ARM_IRQ_LINE_LAYOUT_2)) {
  error_report("Using more than 256 vcpus requires a host 
kernel "

@@ -1948,9 +1946,6 @@ int kvm_arch_init_vcpu(CPUState *cs)
  }
  cpu->mp_affinity = mpidr & ARM64_AFFINITY_MASK;
-    /* Check whether user space can specify guest syndrome value */
-    kvm_arm_init_serror_injection(cs);
-
  return kvm_arm_init_cpreg_list(cpu);
  }



Just checking, in a heterogeneous setup we still want to keep
these 2 calls per-vCPU, right?


There is no hetrogeneous kvm -- every vcpu must match the host cpu.


So big.LITTLE will never be a KVM thing?

Re: [PATCH 00/40] vdpa-net: improve migration downtime through descriptor ASID and persistent IOTLB

[Eugenio Perez Martin]

On Thu, Dec 7, 2023 at 7:50 PM Si-Wei Liu  wrote:
>
> This patch series contain several enhancements to SVQ live migration downtime
> for vDPA-net hardware device, specifically on mlx5_vdpa. Currently it is based
> off of Eugenio's RFC v2 .load_setup series [1] to utilize the shared facility
> and reduce frictions in merging or duplicating code if at all possible.
>
> It's stacked up in particular order as below, as the optimization for one on
> the top has to depend on others on the bottom. Here's a breakdown for what
> each part does respectively:
>
> Patch #  |  Feature / optimization
> -V---
> 35 - 40  | trace events
> 34   | migrate_cancel bug fix
> 21 - 33  | (Un)map batching at stop-n-copy to further optimize LM down time
> 11 - 20  | persistent IOTLB [3] to improve LM down time
> 02 - 10  | SVQ descriptor ASID [2] to optimize SVQ switching
> 01   | dependent linux headers
>  V
>

Hi Si-Wei,

Thanks for the series, I think it contains great additions for the
live migration solution!

It is pretty large though. Do you think it would be feasible to split
out the fixes and the tracing patches in a separated series? That
would allow the reviews to focus on the downtime reduction. I think I
acked all of them.

Maybe we can even create a third series for the vring asid? I think we
should note an increase on performance using svq so it justifies by
itself too.

> Let's first define 2 sources of downtime that this work is concerned with:
>
> * SVQ switching downtime (Downtime #1): downtime at the start of migration.
>   Time spent on teardown and setup for SVQ mode switching, and this downtime
>   is regarded as the maxium time for an individual vdpa-net device.
>   No memory transfer is involved during SVQ switching, hence no .
>
> * LM downtime (Downtime #2): aggregated downtime for all vdpa-net devices on
>   resource teardown and setup in the last stop-n-copy phase on source host.
>
> With each part of the optimizations applied bottom up, the effective outcome
> in terms of down time (in seconds) performance can be observed in this table:
>
>
> |Downtime #1|Downtime #2
> +---+---
> Baseline QEMU   | 20s ~ 30s |20s
> |   |
> Iterative map   |   |
> at destination[1]   |5s |20s
> |   |
> SVQ descriptor  |   |
> ASID [2]|2s | 5s
> |   |
> |   |
> persistent IOTLB|2s | 2s
>   [3]   |   |
> |   |
> (Un)map batching|   |
> at stop-n-copy  |  1.7s |   1.5s
> before switchover   |   |
>
> (VM config: 128GB mem, 2 mlx5_vdpa devices, each w/ 4 data vqs)
>

Thanks for all the profiling, it looks promising!

> Please find the details regarding each enhancement on the commit log.
>
> Thanks,
> -Siwei
>
>
> [1] [RFC PATCH v2 00/10] Map memory at destination .load_setup in vDPA-net 
> migration
> https://lists.nongnu.org/archive/html/qemu-devel/2023-11/msg05711.html
> [2] VHOST_BACKEND_F_DESC_ASID
> https://lore.kernel.org/virtualization/20231018171456.1624030-2-dtatu...@nvidia.com/
> [3] VHOST_BACKEND_F_IOTLB_PERSIST
> https://lore.kernel.org/virtualization/1698304480-18463-1-git-send-email-si-wei@oracle.com/
>
> ---
>
> Si-Wei Liu (40):
>   linux-headers: add vhost_types.h and vhost.h
>   vdpa: add vhost_vdpa_get_vring_desc_group
>   vdpa: probe descriptor group index for data vqs
>   vdpa: piggyback desc_group index when probing isolated cvq
>   vdpa: populate desc_group from net_vhost_vdpa_init
>   vhost: make svq work with gpa without iova translation
>   vdpa: move around vhost_vdpa_set_address_space_id
>   vdpa: add back vhost_vdpa_net_first_nc_vdpa
>   vdpa: no repeat setting shadow_data
>   vdpa: assign svq descriptors a separate ASID when possible
>   vdpa: factor out vhost_vdpa_last_dev
>   vdpa: check map_thread_enabled before join maps thread
>   vdpa: ref counting VhostVDPAShared
>   vdpa: convert iova_tree to ref count based
>   vdpa: add svq_switching and flush_map to header
>   vdpa: indicate SVQ switching via flag
>   vdpa: judge if map can be kept across reset
>   vdpa: unregister listener on last dev cleanup
>   vdpa: should avoid map flushing with persistent iotlb
>   vdpa: avoid mapping flush across reset
>   vdpa: vhost_vdpa_dma_batch_end_once rename
>   vdpa: factor out vhost_vdpa_map_batch_begin
>   vdpa: vhost_vdpa_dma_batch_begin_once rename
>   vdpa: factor out vhost_vdpa_dma_batch_end
>   vdpa: add asid to dma_batch_once API
>   vdpa: return int for dma_batch_once API
>   vdpa: add asid to all dma_batch 

Re: [PATCH 40/40] vdpa: add trace event for vhost_vdpa_net_load_mq

[Eugenio Perez Martin]

On Thu, Dec 7, 2023 at 7:51 PM Si-Wei Liu  wrote:
>
> For better debuggability and observability.
>
> Signed-off-by: Si-Wei Liu 

Reviewed-by: Eugenio Pérez 

> ---
>  net/trace-events | 1 +
>  net/vhost-vdpa.c | 2 ++
>  2 files changed, 3 insertions(+)
>
> diff --git a/net/trace-events b/net/trace-events
> index be087e6..c128cc4 100644
> --- a/net/trace-events
> +++ b/net/trace-events
> @@ -30,3 +30,4 @@ vhost_vdpa_net_data_eval_flush(void *s, int qindex, int 
> svq_switch, bool svq_flu
>  vhost_vdpa_net_cvq_eval_flush(void *s, int qindex, int svq_switch, bool 
> svq_flush) "vhost_vdpa: %p qp: %d svq_switch: %d flush_map: %d"
>  vhost_vdpa_net_load_cmd(void *s, uint8_t class, uint8_t cmd, int data_num, 
> int data_size) "vdpa state: %p class: %u cmd: %u sg_num: %d size: %d"
>  vhost_vdpa_net_load_cmd_retval(void *s, uint8_t class, uint8_t cmd, int r) 
> "vdpa state: %p class: %u cmd: %u retval: %d"
> +vhost_vdpa_net_load_mq(void *s, int ncurqps) "vdpa state: %p current_qpairs: 
> %d"
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index 61da8b4..17b8d01 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -1109,6 +1109,8 @@ static int vhost_vdpa_net_load_mq(VhostVDPAState *s,
>  return 0;
>  }
>
> +trace_vhost_vdpa_net_load_mq(s, n->curr_queue_pairs);
> +
>  mq.virtqueue_pairs = cpu_to_le16(n->curr_queue_pairs);
>  const struct iovec data = {
>  .iov_base = ,
> --
> 1.8.3.1
>

Re: [PATCH 39/40] vdpa: add trace events for vhost_vdpa_net_load_cmd

[Eugenio Perez Martin]

On Thu, Dec 7, 2023 at 7:51 PM Si-Wei Liu  wrote:
>
> For better debuggability and observability.
>
> Signed-off-by: Si-Wei Liu 

Reviewed-by: Eugenio Pérez 

> ---
>  net/trace-events | 2 ++
>  net/vhost-vdpa.c | 2 ++
>  2 files changed, 4 insertions(+)
>
> diff --git a/net/trace-events b/net/trace-events
> index d650c71..be087e6 100644
> --- a/net/trace-events
> +++ b/net/trace-events
> @@ -28,3 +28,5 @@ colo_filter_rewriter_conn_offset(uint32_t offset) ": 
> offset=%u"
>  vhost_vdpa_set_address_space_id(void *v, unsigned vq_group, unsigned 
> asid_num) "vhost_vdpa: %p vq_group: %u asid: %u"
>  vhost_vdpa_net_data_eval_flush(void *s, int qindex, int svq_switch, bool 
> svq_flush) "vhost_vdpa: %p qp: %d svq_switch: %d flush_map: %d"
>  vhost_vdpa_net_cvq_eval_flush(void *s, int qindex, int svq_switch, bool 
> svq_flush) "vhost_vdpa: %p qp: %d svq_switch: %d flush_map: %d"
> +vhost_vdpa_net_load_cmd(void *s, uint8_t class, uint8_t cmd, int data_num, 
> int data_size) "vdpa state: %p class: %u cmd: %u sg_num: %d size: %d"
> +vhost_vdpa_net_load_cmd_retval(void *s, uint8_t class, uint8_t cmd, int r) 
> "vdpa state: %p class: %u cmd: %u retval: %d"
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index a0bd8cd..61da8b4 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -885,6 +885,7 @@ static ssize_t vhost_vdpa_net_load_cmd(VhostVDPAState *s,
>
>  assert(data_size < vhost_vdpa_net_cvq_cmd_page_len() - sizeof(ctrl));
>  cmd_size = sizeof(ctrl) + data_size;
> +trace_vhost_vdpa_net_load_cmd(s, class, cmd, data_num, data_size);
>  if (vhost_svq_available_slots(svq) < 2 ||
>  iov_size(out_cursor, 1) < cmd_size) {
>  /*
> @@ -916,6 +917,7 @@ static ssize_t vhost_vdpa_net_load_cmd(VhostVDPAState *s,
>
>  r = vhost_vdpa_net_cvq_add(s, , 1, , 1);
>  if (unlikely(r < 0)) {
> +trace_vhost_vdpa_net_load_cmd_retval(s, class, cmd, r);
>  return r;
>  }
>
> --
> 1.8.3.1
>

Re: [PATCH 37/40] vdpa: add vhost_vdpa_set_dev_vring_base trace for svq mode

[Eugenio Perez Martin]

On Thu, Dec 7, 2023 at 7:51 PM Si-Wei Liu  wrote:
>
> For better debuggability and observability.
>
> Signed-off-by: Si-Wei Liu 

Reviewed-by: Eugenio Pérez 

> ---
>  hw/virtio/trace-events | 2 +-
>  hw/virtio/vhost-vdpa.c | 5 -
>  2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> index a8d3321..5085607 100644
> --- a/hw/virtio/trace-events
> +++ b/hw/virtio/trace-events
> @@ -57,7 +57,7 @@ vhost_vdpa_dev_start(void *dev, bool started) "dev: %p 
> started: %d"
>  vhost_vdpa_set_log_base(void *dev, uint64_t base, unsigned long long size, 
> int refcnt, int fd, void *log) "dev: %p base: 0x%"PRIx64" size: %llu refcnt: 
> %d fd: %d log: %p"
>  vhost_vdpa_set_vring_addr(void *dev, unsigned int index, unsigned int flags, 
> uint64_t desc_user_addr, uint64_t used_user_addr, uint64_t avail_user_addr, 
> uint64_t log_guest_addr) "dev: %p index: %u flags: 0x%x desc_user_addr: 
> 0x%"PRIx64" used_user_addr: 0x%"PRIx64" avail_user_addr: 0x%"PRIx64" 
> log_guest_addr: 0x%"PRIx64
>  vhost_vdpa_set_vring_num(void *dev, unsigned int index, unsigned int num) 
> "dev: %p index: %u num: %u"
> -vhost_vdpa_set_vring_base(void *dev, unsigned int index, unsigned int num) 
> "dev: %p index: %u num: %u"
> +vhost_vdpa_set_dev_vring_base(void *dev, unsigned int index, unsigned int 
> num, bool svq) "dev: %p index: %u num: %u svq: %d"
>  vhost_vdpa_get_vring_base(void *dev, unsigned int index, unsigned int num, 
> bool svq) "dev: %p index: %u num: %u svq: %d"
>  vhost_vdpa_set_vring_kick(void *dev, unsigned int index, int fd) "dev: %p 
> index: %u fd: %d"
>  vhost_vdpa_set_vring_call(void *dev, unsigned int index, int fd) "dev: %p 
> index: %u fd: %d"
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index d66936f..ff4f218 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -1043,7 +1043,10 @@ static int vhost_vdpa_get_config(struct vhost_dev 
> *dev, uint8_t *config,
>  static int vhost_vdpa_set_dev_vring_base(struct vhost_dev *dev,
>   struct vhost_vring_state *ring)
>  {
> -trace_vhost_vdpa_set_vring_base(dev, ring->index, ring->num);
> +struct vhost_vdpa *v = dev->opaque;
> +
> +trace_vhost_vdpa_set_dev_vring_base(dev, ring->index, ring->num,
> +v->shadow_vqs_enabled);
>  return vhost_vdpa_call(dev, VHOST_SET_VRING_BASE, ring);
>  }
>
> --
> 1.8.3.1
>

Re: [PATCH 36/40] vdpa: add vhost_vdpa_get_vring_base trace for svq mode

[Eugenio Perez Martin]

On Thu, Dec 7, 2023 at 7:51 PM Si-Wei Liu  wrote:
>
> For better debuggability and observability.
>
> Signed-off-by: Si-Wei Liu 

Reviewed-by: Eugenio Pérez 

> ---
>  hw/virtio/trace-events | 2 +-
>  hw/virtio/vhost-vdpa.c | 3 ++-
>  2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> index 196f32f..a8d3321 100644
> --- a/hw/virtio/trace-events
> +++ b/hw/virtio/trace-events
> @@ -58,7 +58,7 @@ vhost_vdpa_set_log_base(void *dev, uint64_t base, unsigned 
> long long size, int r
>  vhost_vdpa_set_vring_addr(void *dev, unsigned int index, unsigned int flags, 
> uint64_t desc_user_addr, uint64_t used_user_addr, uint64_t avail_user_addr, 
> uint64_t log_guest_addr) "dev: %p index: %u flags: 0x%x desc_user_addr: 
> 0x%"PRIx64" used_user_addr: 0x%"PRIx64" avail_user_addr: 0x%"PRIx64" 
> log_guest_addr: 0x%"PRIx64
>  vhost_vdpa_set_vring_num(void *dev, unsigned int index, unsigned int num) 
> "dev: %p index: %u num: %u"
>  vhost_vdpa_set_vring_base(void *dev, unsigned int index, unsigned int num) 
> "dev: %p index: %u num: %u"
> -vhost_vdpa_get_vring_base(void *dev, unsigned int index, unsigned int num) 
> "dev: %p index: %u num: %u"
> +vhost_vdpa_get_vring_base(void *dev, unsigned int index, unsigned int num, 
> bool svq) "dev: %p index: %u num: %u svq: %d"
>  vhost_vdpa_set_vring_kick(void *dev, unsigned int index, int fd) "dev: %p 
> index: %u fd: %d"
>  vhost_vdpa_set_vring_call(void *dev, unsigned int index, int fd) "dev: %p 
> index: %u fd: %d"
>  vhost_vdpa_get_features(void *dev, uint64_t features) "dev: %p features: 
> 0x%"PRIx64
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index 8ba390d..d66936f 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -1607,6 +1607,7 @@ static int vhost_vdpa_get_vring_base(struct vhost_dev 
> *dev,
>
>  if (v->shadow_vqs_enabled) {
>  ring->num = virtio_queue_get_last_avail_idx(dev->vdev, ring->index);
> +trace_vhost_vdpa_get_vring_base(dev, ring->index, ring->num, true);
>  return 0;
>  }
>
> @@ -1619,7 +1620,7 @@ static int vhost_vdpa_get_vring_base(struct vhost_dev 
> *dev,
>  }
>
>  ret = vhost_vdpa_call(dev, VHOST_GET_VRING_BASE, ring);
> -trace_vhost_vdpa_get_vring_base(dev, ring->index, ring->num);
> +trace_vhost_vdpa_get_vring_base(dev, ring->index, ring->num, false);
>  return ret;
>  }
>
> --
> 1.8.3.1
>

Re: [PATCH 35/40] vdpa: add vhost_vdpa_set_address_space_id trace

[Eugenio Perez Martin]

On Thu, Dec 7, 2023 at 7:51 PM Si-Wei Liu  wrote:
>
> For better debuggability and observability.
>
> Signed-off-by: Si-Wei Liu 

Reviewed-by: Eugenio Pérez 

> ---
>  net/trace-events | 3 +++
>  net/vhost-vdpa.c | 3 +++
>  2 files changed, 6 insertions(+)
>
> diff --git a/net/trace-events b/net/trace-events
> index 823a071..aab666a 100644
> --- a/net/trace-events
> +++ b/net/trace-events
> @@ -23,3 +23,6 @@ colo_compare_tcp_info(const char *pkt, uint32_t seq, 
> uint32_t ack, int hdlen, in
>  # filter-rewriter.c
>  colo_filter_rewriter_pkt_info(const char *func, const char *src, const char 
> *dst, uint32_t seq, uint32_t ack, uint32_t flag) "%s: src/dst: %s/%s p: 
> seq/ack=%u/%u  flags=0x%x"
>  colo_filter_rewriter_conn_offset(uint32_t offset) ": offset=%u"
> +
> +# vhost-vdpa.c
> +vhost_vdpa_set_address_space_id(void *v, unsigned vq_group, unsigned 
> asid_num) "vhost_vdpa: %p vq_group: %u asid: %u"
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index 41714d1..84876b0 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -30,6 +30,7 @@
>  #include "migration/misc.h"
>  #include "hw/virtio/vhost.h"
>  #include "hw/virtio/vhost-vdpa.h"
> +#include "trace.h"
>
>  /* Todo:need to add the multiqueue support here */
>  typedef struct VhostVDPAState {
> @@ -365,6 +366,8 @@ static int vhost_vdpa_set_address_space_id(struct 
> vhost_vdpa *v,
>  };
>  int r;
>
> +trace_vhost_vdpa_set_address_space_id(v, vq_group, asid_num);
> +
>  r = ioctl(v->shared->device_fd, VHOST_VDPA_SET_GROUP_ASID, );
>  if (unlikely(r < 0)) {
>  error_report("Can't set vq group %u asid %u, errno=%d (%s)",
> --
> 1.8.3.1
>

[PATCH v7 10/11] hw/net: GMAC Tx Implementation

[Nabih Estefan]

From: Nabih Estefan Diaz 

- Implementation of Transmit function for packets
- Implementation for reading and writing from and to descriptors in
  memory for Tx

NOTE: This function implements the steps detailed in the datasheet for
transmitting messages from the GMAC.

Change-Id: I6b8ba0736fa5cc866025cd43cb2a90100e458446
Signed-off-by: Nabih Estefan 
Reviewed-by: Tyrone Ting 
---
 hw/net/npcm_gmac.c  | 151 
 hw/net/trace-events |   1 -
 2 files changed, 151 insertions(+), 1 deletion(-)

diff --git a/hw/net/npcm_gmac.c b/hw/net/npcm_gmac.c
index cd59ca5fd4..0098b00a49 100644
--- a/hw/net/npcm_gmac.c
+++ b/hw/net/npcm_gmac.c
@@ -267,6 +267,7 @@ static int gmac_write_tx_desc(dma_addr_t addr, struct 
NPCMGMACTxDesc *desc)
 }
 return 0;
 }
+
 static int gmac_rx_transfer_frame_to_buffer(uint32_t rx_buf_len,
 uint32_t *left_frame,
 uint32_t rx_buf_addr,
@@ -488,6 +489,156 @@ static ssize_t gmac_receive(NetClientState *nc, const 
uint8_t *buf, size_t len)
 gmac->regs[R_NPCM_DMA_HOST_RX_DESC] = desc_addr;
 return len;
 }
+
+static int gmac_tx_get_csum(uint32_t tdes1)
+{
+uint32_t mask = TX_DESC_TDES1_CHKSM_INS_CTRL_MASK(tdes1);
+int csum = 0;
+
+if (likely(mask > 0)) {
+csum |= CSUM_IP;
+}
+if (likely(mask > 1)) {
+csum |= CSUM_TCP | CSUM_UDP;
+}
+
+return csum;
+}
+
+static void gmac_try_send_next_packet(NPCMGMACState *gmac)
+{
+/*
+ * Comments about steps refer to steps for
+ * transmitting in page 384 of datasheet
+ */
+uint16_t tx_buffer_size = 2048;
+g_autofree uint8_t *tx_send_buffer = g_malloc(tx_buffer_size);
+uint32_t desc_addr;
+struct NPCMGMACTxDesc tx_desc;
+uint32_t tx_buf_addr, tx_buf_len;
+uint16_t length = 0;
+uint8_t *buf = tx_send_buffer;
+uint32_t prev_buf_size = 0;
+int csum = 0;
+
+/* steps 1&2 */
+if (!gmac->regs[R_NPCM_DMA_HOST_TX_DESC]) {
+gmac->regs[R_NPCM_DMA_HOST_TX_DESC] =
+NPCM_DMA_HOST_TX_DESC_MASK(gmac->regs[R_NPCM_DMA_TX_BASE_ADDR]);
+}
+desc_addr = gmac->regs[R_NPCM_DMA_HOST_TX_DESC];
+
+while (true) {
+gmac_dma_set_state(gmac, NPCM_DMA_STATUS_TX_PROCESS_STATE_SHIFT,
+NPCM_DMA_STATUS_TX_RUNNING_FETCHING_STATE);
+if (gmac_read_tx_desc(desc_addr, _desc)) {
+qemu_log_mask(LOG_GUEST_ERROR,
+  "TX Descriptor @ 0x%x can't be read\n",
+  desc_addr);
+return;
+}
+/* step 3 */
+
+trace_npcm_gmac_packet_desc_read(DEVICE(gmac)->canonical_path,
+desc_addr);
+trace_npcm_gmac_debug_desc_data(DEVICE(gmac)->canonical_path, _desc,
+tx_desc.tdes0, tx_desc.tdes1, tx_desc.tdes2, tx_desc.tdes3);
+
+/* 1 = DMA Owned, 0 = Software Owned */
+if (!(tx_desc.tdes0 & TX_DESC_TDES0_OWN)) {
+qemu_log_mask(LOG_GUEST_ERROR,
+  "TX Descriptor @ 0x%x is owned by software\n",
+  desc_addr);
+gmac->regs[R_NPCM_DMA_STATUS] |= NPCM_DMA_STATUS_TU;
+gmac_dma_set_state(gmac, NPCM_DMA_STATUS_TX_PROCESS_STATE_SHIFT,
+NPCM_DMA_STATUS_TX_SUSPENDED_STATE);
+gmac_update_irq(gmac);
+return;
+}
+
+gmac_dma_set_state(gmac, NPCM_DMA_STATUS_TX_PROCESS_STATE_SHIFT,
+NPCM_DMA_STATUS_TX_RUNNING_READ_STATE);
+/* Give the descriptor back regardless of what happens. */
+tx_desc.tdes0 &= ~TX_DESC_TDES0_OWN;
+
+if (tx_desc.tdes1 & TX_DESC_TDES1_FIRST_SEG_MASK) {
+csum = gmac_tx_get_csum(tx_desc.tdes1);
+}
+
+/* step 4 */
+tx_buf_addr = tx_desc.tdes2;
+gmac->regs[R_NPCM_DMA_CUR_TX_BUF_ADDR] = tx_buf_addr;
+tx_buf_len = TX_DESC_TDES1_BFFR1_SZ_MASK(tx_desc.tdes1);
+buf = _send_buffer[prev_buf_size];
+
+if ((prev_buf_size + tx_buf_len) > sizeof(buf)) {
+tx_buffer_size = prev_buf_size + tx_buf_len;
+tx_send_buffer = g_realloc(tx_send_buffer, tx_buffer_size);
+buf = _send_buffer[prev_buf_size];
+}
+
+/* step 5 */
+if (dma_memory_read(_space_memory, tx_buf_addr, buf,
+tx_buf_len, MEMTXATTRS_UNSPECIFIED)) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: Failed to read packet @ 
0x%x\n",
+__func__, tx_buf_addr);
+return;
+}
+length += tx_buf_len;
+prev_buf_size += tx_buf_len;
+
+/* If not chained we'll have a second buffer. */
+if (!(tx_desc.tdes1 & TX_DESC_TDES1_SEC_ADDR_CHND_MASK)) {
+tx_buf_addr = tx_desc.tdes3;
+gmac->regs[R_NPCM_DMA_CUR_TX_BUF_ADDR] = tx_buf_addr;
+tx_buf_len = TX_DESC_TDES1_BFFR2_SZ_MASK(tx_desc.tdes1);
+buf = 

[PATCH v7 04/11] hw/net: Add NPCMXXX GMAC device

[Nabih Estefan]

From: Hao Wu 

This patch implements the basic registers of GMAC device and sets
registers for networking functionalities.

Tested:
The following message shows up with the change:
Broadcom BCM54612E stmmac-0:00: attached PHY driver [Broadcom BCM54612E] 
(mii_bus:phy_addr=stmmac-0:00, irq=POLL)
stmmaceth f0802000.eth eth0: Link is Up - 1Gbps/Full - flow control rx/tx

Change-Id: Icbb37a03fc7bd5140f976e92cec3ee4c9010dfc1
Signed-off-by: Hao Wu 
Signed-off-by: Nabih Estefan Diaz 
Reviewed-by: Tyrone Ting 
---
 hw/net/meson.build |   2 +-
 hw/net/npcm_gmac.c | 395 +
 hw/net/trace-events|  11 ++
 include/hw/net/npcm_gmac.h | 170 
 4 files changed, 577 insertions(+), 1 deletion(-)
 create mode 100644 hw/net/npcm_gmac.c
 create mode 100644 include/hw/net/npcm_gmac.h

diff --git a/hw/net/meson.build b/hw/net/meson.build
index f64651c467..db6509f504 100644
--- a/hw/net/meson.build
+++ b/hw/net/meson.build
@@ -38,7 +38,7 @@ system_ss.add(when: 'CONFIG_I82596_COMMON', if_true: 
files('i82596.c'))
 system_ss.add(when: 'CONFIG_SUNHME', if_true: files('sunhme.c'))
 system_ss.add(when: 'CONFIG_FTGMAC100', if_true: files('ftgmac100.c'))
 system_ss.add(when: 'CONFIG_SUNGEM', if_true: files('sungem.c'))
-system_ss.add(when: 'CONFIG_NPCM7XX', if_true: files('npcm7xx_emc.c'))
+system_ss.add(when: 'CONFIG_NPCM7XX', if_true: files('npcm7xx_emc.c', 
'npcm_gmac.c'))
 
 system_ss.add(when: 'CONFIG_ETRAXFS', if_true: files('etraxfs_eth.c'))
 system_ss.add(when: 'CONFIG_COLDFIRE', if_true: files('mcf_fec.c'))
diff --git a/hw/net/npcm_gmac.c b/hw/net/npcm_gmac.c
new file mode 100644
index 00..5ce632858d
--- /dev/null
+++ b/hw/net/npcm_gmac.c
@@ -0,0 +1,395 @@
+/*
+ * Nuvoton NPCM7xx/8xx GMAC Module
+ *
+ * Copyright 2022 Google LLC
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ *
+ * Unsupported/unimplemented features:
+ * - MII is not implemented, MII_ADDR.BUSY and MII_DATA always return zero
+ * - Precision timestamp (PTP) is not implemented.
+ */
+
+#include "qemu/osdep.h"
+
+#include "hw/registerfields.h"
+#include "hw/net/mii.h"
+#include "hw/net/npcm_gmac.h"
+#include "migration/vmstate.h"
+#include "qemu/log.h"
+#include "qemu/units.h"
+#include "sysemu/dma.h"
+#include "trace.h"
+
+REG32(NPCM_DMA_BUS_MODE, 0x1000)
+REG32(NPCM_DMA_XMT_POLL_DEMAND, 0x1004)
+REG32(NPCM_DMA_RCV_POLL_DEMAND, 0x1008)
+REG32(NPCM_DMA_RCV_BASE_ADDR, 0x100c)
+REG32(NPCM_DMA_TX_BASE_ADDR, 0x1010)
+REG32(NPCM_DMA_STATUS, 0x1014)
+REG32(NPCM_DMA_CONTROL, 0x1018)
+REG32(NPCM_DMA_INTR_ENA, 0x101c)
+REG32(NPCM_DMA_MISSED_FRAME_CTR, 0x1020)
+REG32(NPCM_DMA_HOST_TX_DESC, 0x1048)
+REG32(NPCM_DMA_HOST_RX_DESC, 0x104c)
+REG32(NPCM_DMA_CUR_TX_BUF_ADDR, 0x1050)
+REG32(NPCM_DMA_CUR_RX_BUF_ADDR, 0x1054)
+REG32(NPCM_DMA_HW_FEATURE, 0x1058)
+
+REG32(NPCM_GMAC_MAC_CONFIG, 0x0)
+REG32(NPCM_GMAC_FRAME_FILTER, 0x4)
+REG32(NPCM_GMAC_HASH_HIGH, 0x8)
+REG32(NPCM_GMAC_HASH_LOW, 0xc)
+REG32(NPCM_GMAC_MII_ADDR, 0x10)
+REG32(NPCM_GMAC_MII_DATA, 0x14)
+REG32(NPCM_GMAC_FLOW_CTRL, 0x18)
+REG32(NPCM_GMAC_VLAN_FLAG, 0x1c)
+REG32(NPCM_GMAC_VERSION, 0x20)
+REG32(NPCM_GMAC_WAKEUP_FILTER, 0x28)
+REG32(NPCM_GMAC_PMT, 0x2c)
+REG32(NPCM_GMAC_LPI_CTRL, 0x30)
+REG32(NPCM_GMAC_TIMER_CTRL, 0x34)
+REG32(NPCM_GMAC_INT_STATUS, 0x38)
+REG32(NPCM_GMAC_INT_MASK, 0x3c)
+REG32(NPCM_GMAC_MAC0_ADDR_HI, 0x40)
+REG32(NPCM_GMAC_MAC0_ADDR_LO, 0x44)
+REG32(NPCM_GMAC_MAC1_ADDR_HI, 0x48)
+REG32(NPCM_GMAC_MAC1_ADDR_LO, 0x4c)
+REG32(NPCM_GMAC_MAC2_ADDR_HI, 0x50)
+REG32(NPCM_GMAC_MAC2_ADDR_LO, 0x54)
+REG32(NPCM_GMAC_MAC3_ADDR_HI, 0x58)
+REG32(NPCM_GMAC_MAC3_ADDR_LO, 0x5c)
+REG32(NPCM_GMAC_RGMII_STATUS, 0xd8)
+REG32(NPCM_GMAC_WATCHDOG, 0xdc)
+REG32(NPCM_GMAC_PTP_TCR, 0x700)
+REG32(NPCM_GMAC_PTP_SSIR, 0x704)
+REG32(NPCM_GMAC_PTP_STSR, 0x708)
+REG32(NPCM_GMAC_PTP_STNSR, 0x70c)
+REG32(NPCM_GMAC_PTP_STSUR, 0x710)
+REG32(NPCM_GMAC_PTP_STNSUR, 0x714)
+REG32(NPCM_GMAC_PTP_TAR, 0x718)
+REG32(NPCM_GMAC_PTP_TTSR, 0x71c)
+
+/* Register Fields */
+#define NPCM_GMAC_MII_ADDR_BUSY BIT(0)
+#define NPCM_GMAC_MII_ADDR_WRITEBIT(1)
+#define NPCM_GMAC_MII_ADDR_GR(rv)   extract16((rv), 6, 5)
+#define NPCM_GMAC_MII_ADDR_PA(rv)   extract16((rv), 11, 5)
+
+#define NPCM_GMAC_INT_MASK_LPIIMBIT(10)
+#define NPCM_GMAC_INT_MASK_PMTM BIT(3)
+#define NPCM_GMAC_INT_MASK_RGIM BIT(0)
+
+#define NPCM_DMA_BUS_MODE_SWR   BIT(0)
+
+static const uint32_t npcm_gmac_cold_reset_values[NPCM_GMAC_NR_REGS] = {
+

[PATCH v7 03/11] hw/misc: Add qtest for NPCM7xx PCI Mailbox

[Nabih Estefan]

From: Hao Wu 

This patches adds a qtest for NPCM7XX PCI Mailbox module.
It sends read and write requests to the module, and verifies that
the module contains the correct data after the requests.

Change-Id: I3948da277fc26352068420a26d8cc7cb2674cf40
Signed-off-by: Hao Wu 
Signed-off-by: Nabih Estefan 
Reviewed-by: Tyrone Ting 
---
 tests/qtest/meson.build |   1 +
 tests/qtest/npcm7xx_pci_mbox-test.c | 238 
 2 files changed, 239 insertions(+)
 create mode 100644 tests/qtest/npcm7xx_pci_mbox-test.c

diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 47dabf91d0..2ac79925f9 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -183,6 +183,7 @@ qtests_sparc64 = \
 qtests_npcm7xx = \
   ['npcm7xx_adc-test',
'npcm7xx_gpio-test',
+   'npcm7xx_pci_mbox-test',
'npcm7xx_pwm-test',
'npcm7xx_rng-test',
'npcm7xx_sdhci-test',
diff --git a/tests/qtest/npcm7xx_pci_mbox-test.c 
b/tests/qtest/npcm7xx_pci_mbox-test.c
new file mode 100644
index 00..24eec18e3c
--- /dev/null
+++ b/tests/qtest/npcm7xx_pci_mbox-test.c
@@ -0,0 +1,238 @@
+/*
+ * QTests for Nuvoton NPCM7xx PCI Mailbox Modules.
+ *
+ * Copyright 2021 Google LLC
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/bitops.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qnum.h"
+#include "libqtest-single.h"
+
+#define PCI_MBOX_BA 0xf0848000
+#define PCI_MBOX_IRQ8
+
+/* register offset */
+#define PCI_MBOX_STAT   0x00
+#define PCI_MBOX_CTL0x04
+#define PCI_MBOX_CMD0x08
+
+#define CODE_OK 0x00
+#define CODE_INVALID_OP 0xa0
+#define CODE_INVALID_SIZE   0xa1
+#define CODE_ERROR  0xff
+
+#define OP_READ 0x01
+#define OP_WRITE0x02
+#define OP_INVALID  0x41
+
+
+static int sock;
+static int fd;
+
+/*
+ * Create a local TCP socket with any port, then save off the port we got.
+ */
+static in_port_t open_socket(void)
+{
+struct sockaddr_in myaddr;
+socklen_t addrlen;
+
+myaddr.sin_family = AF_INET;
+myaddr.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
+myaddr.sin_port = 0;
+sock = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
+g_assert(sock != -1);
+g_assert(bind(sock, (struct sockaddr *) , sizeof(myaddr)) != -1);
+addrlen = sizeof(myaddr);
+g_assert(getsockname(sock, (struct sockaddr *)  , ) != -1);
+g_assert(listen(sock, 1) != -1);
+return ntohs(myaddr.sin_port);
+}
+
+static void setup_fd(void)
+{
+fd_set readfds;
+
+FD_ZERO();
+FD_SET(sock, );
+g_assert(select(sock + 1, , NULL, NULL, NULL) == 1);
+
+fd = accept(sock, NULL, 0);
+g_assert(fd >= 0);
+}
+
+static uint8_t read_response(uint8_t *buf, size_t len)
+{
+uint8_t code;
+ssize_t ret = read(fd, , 1);
+
+if (ret == -1) {
+return CODE_ERROR;
+}
+if (code != CODE_OK) {
+return code;
+}
+g_test_message("response code: %x", code);
+if (len > 0) {
+ret = read(fd, buf, len);
+if (ret < len) {
+return CODE_ERROR;
+}
+}
+return CODE_OK;
+}
+
+static void receive_data(uint64_t offset, uint8_t *buf, size_t len)
+{
+uint8_t op = OP_READ;
+uint8_t code;
+ssize_t rv;
+
+while (len > 0) {
+uint8_t size;
+
+if (len >= 8) {
+size = 8;
+} else if (len >= 4) {
+size = 4;
+} else if (len >= 2) {
+size = 2;
+} else {
+size = 1;
+}
+
+g_test_message("receiving %u bytes", size);
+/* Write op */
+rv = write(fd, , 1);
+g_assert_cmpint(rv, ==, 1);
+/* Write offset */
+rv = write(fd, (uint8_t *), sizeof(uint64_t));
+g_assert_cmpint(rv, ==, sizeof(uint64_t));
+/* Write size */
+g_assert_cmpint(write(fd, , 1), ==, 1);
+
+/* Read data and Expect response */
+code = read_response(buf, size);
+g_assert_cmphex(code, ==, CODE_OK);
+
+buf += size;
+offset += size;
+len -= size;
+}
+}
+
+static void send_data(uint64_t offset, const uint8_t *buf, size_t len)
+{
+uint8_t op = OP_WRITE;
+uint8_t code;
+ssize_t rv;
+
+while (len > 0) {
+uint8_t size;
+
+if (len >= 8) {
+size = 8;
+} else if (len >= 4) {
+size = 4;
+} else if (len >= 2) {
+size = 2;
+} else {
+size = 1;
+}
+
+   

[PATCH v7 09/11] hw/net: GMAC Rx Implementation

[Nabih Estefan]

From: Nabih Estefan Diaz 

- Implementation of Receive function for packets
- Implementation for reading and writing from and to descriptors in
  memory for Rx

When RX starts, we need to flush the queued packets so that they
can be received by the GMAC device. Without this it won't work
with TAP NIC device.

When RX descriptor list is full, it returns a DMA_STATUS for software to handle 
it. But there's no way to indicate the software ha handled all RX descriptors 
and the whole pipeline stalls.

We do something similar to NPCM7XX EMC to handle this case.

1. Return packet size when RX descriptor is full, effectively dropping these 
packets in such a case.
2. When software clears RX descriptor full bit, continue receiving further 
packets by flushing QEMU packet queue.

Change-Id: I53dd4169272d7c3e990fb17261014c2cd2a881d9
Signed-off-by: Hao Wu 
Signed-off-by: Nabih Estefan 
Reviewed-by: Tyrone Ting 
---
 hw/net/npcm_gmac.c | 331 -
 include/hw/net/npcm_gmac.h |  28 ++--
 2 files changed, 343 insertions(+), 16 deletions(-)

diff --git a/hw/net/npcm_gmac.c b/hw/net/npcm_gmac.c
index 220955346c..cd59ca5fd4 100644
--- a/hw/net/npcm_gmac.c
+++ b/hw/net/npcm_gmac.c
@@ -23,7 +23,11 @@
 #include "hw/registerfields.h"
 #include "hw/net/mii.h"
 #include "hw/net/npcm_gmac.h"
+#include "linux/if_ether.h"
 #include "migration/vmstate.h"
+#include "net/checksum.h"
+#include "net/net.h"
+#include "qemu/cutils.h"
 #include "qemu/log.h"
 #include "qemu/units.h"
 #include "sysemu/dma.h"
@@ -146,6 +150,17 @@ static void gmac_phy_set_link(NPCMGMACState *s, bool 
active)
 
 static bool gmac_can_receive(NetClientState *nc)
 {
+NPCMGMACState *gmac = NPCM_GMAC(qemu_get_nic_opaque(nc));
+
+/* If GMAC receive is disabled. */
+if (!(gmac->regs[R_NPCM_GMAC_MAC_CONFIG] & NPCM_GMAC_MAC_CONFIG_RX_EN)) {
+return false;
+}
+
+/* If GMAC DMA RX is stopped. */
+if (!(gmac->regs[R_NPCM_DMA_CONTROL] & NPCM_DMA_CONTROL_START_STOP_RX)) {
+return false;
+}
 return true;
 }
 
@@ -191,11 +206,288 @@ static void gmac_update_irq(NPCMGMACState *gmac)
 qemu_set_irq(gmac->irq, level);
 }
 
-static ssize_t gmac_receive(NetClientState *nc, const uint8_t *buf, size_t len)
+static int gmac_read_rx_desc(dma_addr_t addr, struct NPCMGMACRxDesc *desc)
+{
+if (dma_memory_read(_space_memory, addr, desc,
+sizeof(*desc), MEMTXATTRS_UNSPECIFIED)) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: Failed to read descriptor @ 0x%"
+  HWADDR_PRIx "\n", __func__, addr);
+return -1;
+}
+desc->rdes0 = le32_to_cpu(desc->rdes0);
+desc->rdes1 = le32_to_cpu(desc->rdes1);
+desc->rdes2 = le32_to_cpu(desc->rdes2);
+desc->rdes3 = le32_to_cpu(desc->rdes3);
+return 0;
+}
+
+static int gmac_write_rx_desc(dma_addr_t addr, struct NPCMGMACRxDesc *desc)
+{
+struct NPCMGMACRxDesc le_desc;
+le_desc.rdes0 = cpu_to_le32(desc->rdes0);
+le_desc.rdes1 = cpu_to_le32(desc->rdes1);
+le_desc.rdes2 = cpu_to_le32(desc->rdes2);
+le_desc.rdes3 = cpu_to_le32(desc->rdes3);
+if (dma_memory_write(_space_memory, addr, _desc,
+sizeof(le_desc), MEMTXATTRS_UNSPECIFIED)) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: Failed to write descriptor @ 0x%"
+  HWADDR_PRIx "\n", __func__, addr);
+return -1;
+}
+return 0;
+}
+
+static int gmac_read_tx_desc(dma_addr_t addr, struct NPCMGMACTxDesc *desc)
+{
+if (dma_memory_read(_space_memory, addr, desc,
+sizeof(*desc), MEMTXATTRS_UNSPECIFIED)) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: Failed to read descriptor @ 0x%"
+  HWADDR_PRIx "\n", __func__, addr);
+return -1;
+}
+desc->tdes0 = le32_to_cpu(desc->tdes0);
+desc->tdes1 = le32_to_cpu(desc->tdes1);
+desc->tdes2 = le32_to_cpu(desc->tdes2);
+desc->tdes3 = le32_to_cpu(desc->tdes3);
+return 0;
+}
+
+static int gmac_write_tx_desc(dma_addr_t addr, struct NPCMGMACTxDesc *desc)
 {
-/* Placeholder */
+struct NPCMGMACTxDesc le_desc;
+le_desc.tdes0 = cpu_to_le32(desc->tdes0);
+le_desc.tdes1 = cpu_to_le32(desc->tdes1);
+le_desc.tdes2 = cpu_to_le32(desc->tdes2);
+le_desc.tdes3 = cpu_to_le32(desc->tdes3);
+if (dma_memory_write(_space_memory, addr, _desc,
+sizeof(le_desc), MEMTXATTRS_UNSPECIFIED)) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: Failed to write descriptor @ 0x%"
+  HWADDR_PRIx "\n", __func__, addr);
+return -1;
+}
 return 0;
 }
+static int gmac_rx_transfer_frame_to_buffer(uint32_t rx_buf_len,
+uint32_t *left_frame,
+uint32_t rx_buf_addr,
+bool *eof_transferred,
+const uint8_t **frame_ptr,
+

[PATCH v7 11/11] tests/qtest: Adding PCS Module test to GMAC Qtest

[Nabih Estefan]

From: Nabih Estefan Diaz 

 - Add PCS Register check to npcm_gmac-test

Change-Id: Iddbd17df490004deadeb734a1dd7420a7e6963d5
Signed-off-by: Nabih Estefan 
Reviewed-by: Tyrone Ting 
---
 tests/qtest/npcm_gmac-test.c | 134 ++-
 1 file changed, 133 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/npcm_gmac-test.c b/tests/qtest/npcm_gmac-test.c
index 130a1599a8..0958b13814 100644
--- a/tests/qtest/npcm_gmac-test.c
+++ b/tests/qtest/npcm_gmac-test.c
@@ -20,6 +20,10 @@
 /* Name of the GMAC Device */
 #define TYPE_NPCM_GMAC "npcm-gmac"
 
+/* Address of the PCS Module */
+#define PCS_BASE_ADDRESS 0xf078
+#define NPCM_PCS_IND_AC_BA 0x1fe
+
 typedef struct GMACModule {
 int irq;
 uint64_t base_addr;
@@ -111,6 +115,62 @@ typedef enum NPCMRegister {
 NPCM_GMAC_PTP_STNSUR = 0x714,
 NPCM_GMAC_PTP_TAR = 0x718,
 NPCM_GMAC_PTP_TTSR = 0x71c,
+
+/* PCS Registers */
+NPCM_PCS_SR_CTL_ID1 = 0x3c0008,
+NPCM_PCS_SR_CTL_ID2 = 0x3c000a,
+NPCM_PCS_SR_CTL_STS = 0x3c0010,
+
+NPCM_PCS_SR_MII_CTRL = 0x3e,
+NPCM_PCS_SR_MII_STS = 0x3e0002,
+NPCM_PCS_SR_MII_DEV_ID1 = 0x3e0004,
+NPCM_PCS_SR_MII_DEV_ID2 = 0x3e0006,
+NPCM_PCS_SR_MII_AN_ADV = 0x3e0008,
+NPCM_PCS_SR_MII_LP_BABL = 0x3e000a,
+NPCM_PCS_SR_MII_AN_EXPN = 0x3e000c,
+NPCM_PCS_SR_MII_EXT_STS = 0x3e001e,
+
+NPCM_PCS_SR_TIM_SYNC_ABL = 0x3e0e10,
+NPCM_PCS_SR_TIM_SYNC_TX_MAX_DLY_LWR = 0x3e0e12,
+NPCM_PCS_SR_TIM_SYNC_TX_MAX_DLY_UPR = 0x3e0e14,
+NPCM_PCS_SR_TIM_SYNC_TX_MIN_DLY_LWR = 0x3e0e16,
+NPCM_PCS_SR_TIM_SYNC_TX_MIN_DLY_UPR = 0x3e0e18,
+NPCM_PCS_SR_TIM_SYNC_RX_MAX_DLY_LWR = 0x3e0e1a,
+NPCM_PCS_SR_TIM_SYNC_RX_MAX_DLY_UPR = 0x3e0e1c,
+NPCM_PCS_SR_TIM_SYNC_RX_MIN_DLY_LWR = 0x3e0e1e,
+NPCM_PCS_SR_TIM_SYNC_RX_MIN_DLY_UPR = 0x3e0e20,
+
+NPCM_PCS_VR_MII_MMD_DIG_CTRL1 = 0x3f,
+NPCM_PCS_VR_MII_AN_CTRL = 0x3f0002,
+NPCM_PCS_VR_MII_AN_INTR_STS = 0x3f0004,
+NPCM_PCS_VR_MII_TC = 0x3f0006,
+NPCM_PCS_VR_MII_DBG_CTRL = 0x3f000a,
+NPCM_PCS_VR_MII_EEE_MCTRL0 = 0x3f000c,
+NPCM_PCS_VR_MII_EEE_TXTIMER = 0x3f0010,
+NPCM_PCS_VR_MII_EEE_RXTIMER = 0x3f0012,
+NPCM_PCS_VR_MII_LINK_TIMER_CTRL = 0x3f0014,
+NPCM_PCS_VR_MII_EEE_MCTRL1 = 0x3f0016,
+NPCM_PCS_VR_MII_DIG_STS = 0x3f0020,
+NPCM_PCS_VR_MII_ICG_ERRCNT1 = 0x3f0022,
+NPCM_PCS_VR_MII_MISC_STS = 0x3f0030,
+NPCM_PCS_VR_MII_RX_LSTS = 0x3f0040,
+NPCM_PCS_VR_MII_MP_TX_BSTCTRL0 = 0x3f0070,
+NPCM_PCS_VR_MII_MP_TX_LVLCTRL0 = 0x3f0074,
+NPCM_PCS_VR_MII_MP_TX_GENCTRL0 = 0x3f007a,
+NPCM_PCS_VR_MII_MP_TX_GENCTRL1 = 0x3f007c,
+NPCM_PCS_VR_MII_MP_TX_STS = 0x3f0090,
+NPCM_PCS_VR_MII_MP_RX_GENCTRL0 = 0x3f00b0,
+NPCM_PCS_VR_MII_MP_RX_GENCTRL1 = 0x3f00b2,
+NPCM_PCS_VR_MII_MP_RX_LOS_CTRL0 = 0x3f00ba,
+NPCM_PCS_VR_MII_MP_MPLL_CTRL0 = 0x3f00f0,
+NPCM_PCS_VR_MII_MP_MPLL_CTRL1 = 0x3f00f2,
+NPCM_PCS_VR_MII_MP_MPLL_STS = 0x3f0110,
+NPCM_PCS_VR_MII_MP_MISC_CTRL2 = 0x3f0126,
+NPCM_PCS_VR_MII_MP_LVL_CTRL = 0x3f0130,
+NPCM_PCS_VR_MII_MP_MISC_CTRL0 = 0x3f0132,
+NPCM_PCS_VR_MII_MP_MISC_CTRL1 = 0x3f0134,
+NPCM_PCS_VR_MII_DIG_CTRL2 = 0x3f01c2,
+NPCM_PCS_VR_MII_DIG_ERRCNT_SEL = 0x3f01c4,
 } NPCMRegister;
 
 static uint32_t gmac_read(QTestState *qts, const GMACModule *mod,
@@ -119,6 +179,15 @@ static uint32_t gmac_read(QTestState *qts, const 
GMACModule *mod,
 return qtest_readl(qts, mod->base_addr + regno);
 }
 
+static uint16_t pcs_read(QTestState *qts, const GMACModule *mod,
+  NPCMRegister regno)
+{
+uint32_t write_value = (regno & 0x3ffe00) >> 9;
+qtest_writel(qts, PCS_BASE_ADDRESS + NPCM_PCS_IND_AC_BA, write_value);
+uint32_t read_offset = regno & 0x1ff;
+return qtest_readl(qts, PCS_BASE_ADDRESS + read_offset);
+}
+
 /* Check that GMAC registers are reset to default value */
 static void test_init(gconstpointer test_data)
 {
@@ -129,7 +198,12 @@ static void test_init(gconstpointer test_data)
 #define CHECK_REG32(regno, value) \
 do { \
 g_assert_cmphex(gmac_read(qts, mod, (regno)), ==, (value)); \
-} while (0)
+} while (0) ;
+
+#define CHECK_REG_PCS(regno, value) \
+do { \
+g_assert_cmphex(pcs_read(qts, mod, (regno)), ==, (value)); \
+} while (0) ;
 
 CHECK_REG32(NPCM_DMA_BUS_MODE, 0x00020100);
 CHECK_REG32(NPCM_DMA_XMT_POLL_DEMAND, 0);
@@ -180,6 +254,64 @@ static void test_init(gconstpointer test_data)
 CHECK_REG32(NPCM_GMAC_PTP_TAR, 0);
 CHECK_REG32(NPCM_GMAC_PTP_TTSR, 0);
 
+/* TODO Add registers PCS */
+if (mod->base_addr == 0xf0802000) {
+CHECK_REG_PCS(NPCM_PCS_SR_CTL_ID1, 0x699e)
+CHECK_REG_PCS(NPCM_PCS_SR_CTL_ID2, 0)
+CHECK_REG_PCS(NPCM_PCS_SR_CTL_STS, 0x8000)
+
+CHECK_REG_PCS(NPCM_PCS_SR_MII_CTRL, 0x1140)
+CHECK_REG_PCS(NPCM_PCS_SR_MII_STS, 0x0109)
+CHECK_REG_PCS(NPCM_PCS_SR_MII_DEV_ID1, 0x699e)
+CHECK_REG_PCS(NPCM_PCS_SR_MII_DEV_ID2, 0x0ced0)
+   

[PATCH v7 06/11] tests/qtest: Creating qtest for GMAC Module

[Nabih Estefan]

From: Nabih Estefan Diaz 

 - Created qtest to check initialization of registers in GMAC Module.
 - Implemented test into Build File.

Change-Id: I399e52af301492d0eb97b556c1f67be44122779d
Signed-off-by: Nabih Estefan 
Reviewed-by: Tyrone Ting 
---
 tests/qtest/meson.build  |   2 +
 tests/qtest/npcm_gmac-test.c | 209 +++
 2 files changed, 211 insertions(+)
 create mode 100644 tests/qtest/npcm_gmac-test.c

diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 2ac79925f9..e5c31b83bd 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -221,6 +221,8 @@ qtests_aarch64 = \
   (config_all_devices.has_key('CONFIG_RASPI') ? ['bcm2835-dma-test'] : []) +  \
   (config_all.has_key('CONFIG_TCG') and
\
config_all_devices.has_key('CONFIG_TPM_TIS_I2C') ? ['tpm-tis-i2c-test'] : 
[]) + \
+  (config_all_devices.has_key('CONFIG_ASPEED_SOC') ? qtests_aspeed : []) + \
+  (config_all_devices.has_key('CONFIG_NPCM7XX') ? qtests_npcm7xx : []) + \
   ['arm-cpu-features',
'numa-test',
'boot-serial-test',
diff --git a/tests/qtest/npcm_gmac-test.c b/tests/qtest/npcm_gmac-test.c
new file mode 100644
index 00..77a83c4c58
--- /dev/null
+++ b/tests/qtest/npcm_gmac-test.c
@@ -0,0 +1,209 @@
+/*
+ * QTests for Nuvoton NPCM7xx/8xx GMAC Modules.
+ *
+ * Copyright 2023 Google LLC
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ */
+
+#include "qemu/osdep.h"
+#include "libqos/libqos.h"
+
+/* Name of the GMAC Device */
+#define TYPE_NPCM_GMAC "npcm-gmac"
+
+typedef struct GMACModule {
+int irq;
+uint64_t base_addr;
+} GMACModule;
+
+typedef struct TestData {
+const GMACModule *module;
+} TestData;
+
+/* Values extracted from hw/arm/npcm8xx.c */
+static const GMACModule gmac_module_list[] = {
+{
+.irq= 14,
+.base_addr  = 0xf0802000
+},
+{
+.irq= 15,
+.base_addr  = 0xf0804000
+},
+{
+.irq= 16,
+.base_addr  = 0xf0806000
+},
+{
+.irq= 17,
+.base_addr  = 0xf0808000
+}
+};
+
+/* Returns the index of the GMAC module. */
+static int gmac_module_index(const GMACModule *mod)
+{
+ptrdiff_t diff = mod - gmac_module_list;
+
+g_assert_true(diff >= 0 && diff < ARRAY_SIZE(gmac_module_list));
+
+return diff;
+}
+
+/* 32-bit register indices. Taken from npcm_gmac.c */
+typedef enum NPCMRegister {
+/* DMA Registers */
+NPCM_DMA_BUS_MODE = 0x1000,
+NPCM_DMA_XMT_POLL_DEMAND = 0x1004,
+NPCM_DMA_RCV_POLL_DEMAND = 0x1008,
+NPCM_DMA_RCV_BASE_ADDR = 0x100c,
+NPCM_DMA_TX_BASE_ADDR = 0x1010,
+NPCM_DMA_STATUS = 0x1014,
+NPCM_DMA_CONTROL = 0x1018,
+NPCM_DMA_INTR_ENA = 0x101c,
+NPCM_DMA_MISSED_FRAME_CTR = 0x1020,
+NPCM_DMA_HOST_TX_DESC = 0x1048,
+NPCM_DMA_HOST_RX_DESC = 0x104c,
+NPCM_DMA_CUR_TX_BUF_ADDR = 0x1050,
+NPCM_DMA_CUR_RX_BUF_ADDR = 0x1054,
+NPCM_DMA_HW_FEATURE = 0x1058,
+
+/* GMAC Registers */
+NPCM_GMAC_MAC_CONFIG = 0x0,
+NPCM_GMAC_FRAME_FILTER = 0x4,
+NPCM_GMAC_HASH_HIGH = 0x8,
+NPCM_GMAC_HASH_LOW = 0xc,
+NPCM_GMAC_MII_ADDR = 0x10,
+NPCM_GMAC_MII_DATA = 0x14,
+NPCM_GMAC_FLOW_CTRL = 0x18,
+NPCM_GMAC_VLAN_FLAG = 0x1c,
+NPCM_GMAC_VERSION = 0x20,
+NPCM_GMAC_WAKEUP_FILTER = 0x28,
+NPCM_GMAC_PMT = 0x2c,
+NPCM_GMAC_LPI_CTRL = 0x30,
+NPCM_GMAC_TIMER_CTRL = 0x34,
+NPCM_GMAC_INT_STATUS = 0x38,
+NPCM_GMAC_INT_MASK = 0x3c,
+NPCM_GMAC_MAC0_ADDR_HI = 0x40,
+NPCM_GMAC_MAC0_ADDR_LO = 0x44,
+NPCM_GMAC_MAC1_ADDR_HI = 0x48,
+NPCM_GMAC_MAC1_ADDR_LO = 0x4c,
+NPCM_GMAC_MAC2_ADDR_HI = 0x50,
+NPCM_GMAC_MAC2_ADDR_LO = 0x54,
+NPCM_GMAC_MAC3_ADDR_HI = 0x58,
+NPCM_GMAC_MAC3_ADDR_LO = 0x5c,
+NPCM_GMAC_RGMII_STATUS = 0xd8,
+NPCM_GMAC_WATCHDOG = 0xdc,
+NPCM_GMAC_PTP_TCR = 0x700,
+NPCM_GMAC_PTP_SSIR = 0x704,
+NPCM_GMAC_PTP_STSR = 0x708,
+NPCM_GMAC_PTP_STNSR = 0x70c,
+NPCM_GMAC_PTP_STSUR = 0x710,
+NPCM_GMAC_PTP_STNSUR = 0x714,
+NPCM_GMAC_PTP_TAR = 0x718,
+NPCM_GMAC_PTP_TTSR = 0x71c,
+} NPCMRegister;
+
+static uint32_t gmac_read(QTestState *qts, const GMACModule *mod,
+  NPCMRegister regno)
+{
+return qtest_readl(qts, mod->base_addr + regno);
+}
+
+/* Check that GMAC registers are reset to default value */
+static void test_init(gconstpointer test_data)
+{
+const TestData *td = test_data;
+const GMACModule *mod = td->module;
+

[PATCH v7 08/11] hw/net: General GMAC Implementation

[Nabih Estefan]

From: Nabih Estefan Diaz 

- General GMAC Register handling
- GMAC IRQ Handling
- Added traces in some methods for debugging
- Lots of declarations for accessing information on GMAC Descriptors 
(npcm_gmac.h file)

NOTE: With code on this state, the GMAC can boot-up properly and will show up 
in the ifconfig command on the BMC

Change-Id: I01d161843c3a3f0bd8b51e11237cff1497f334d1
Signed-off-by: Nabih Estefan 
Reviewed-by: Tyrone Ting 
---
 hw/net/npcm_gmac.c |  26 -
 include/hw/net/npcm_gmac.h | 198 ++---
 2 files changed, 184 insertions(+), 40 deletions(-)

diff --git a/hw/net/npcm_gmac.c b/hw/net/npcm_gmac.c
index 6f8109e0ee..220955346c 100644
--- a/hw/net/npcm_gmac.c
+++ b/hw/net/npcm_gmac.c
@@ -305,22 +305,6 @@ static void npcm_gmac_write(void *opaque, hwaddr offset,
 break;
 
 case A_NPCM_GMAC_MAC_CONFIG:
-prev = gmac->regs[offset / sizeof(uint32_t)];
-gmac->regs[offset / sizeof(uint32_t)] = v;
-
-/* If transmit is being enabled for first time, update desc addr */
-if (~(prev & NPCM_GMAC_MAC_CONFIG_TX_EN) &
- (v & NPCM_GMAC_MAC_CONFIG_TX_EN)) {
-gmac->regs[R_NPCM_DMA_HOST_TX_DESC] =
-gmac->regs[R_NPCM_DMA_TX_BASE_ADDR];
-}
-
-/* If receive is being enabled for first time, update desc addr */
-if (~(prev & NPCM_GMAC_MAC_CONFIG_RX_EN) &
- (v & NPCM_GMAC_MAC_CONFIG_RX_EN)) {
-gmac->regs[R_NPCM_DMA_HOST_RX_DESC] =
-gmac->regs[R_NPCM_DMA_RX_BASE_ADDR];
-}
 break;
 
 case A_NPCM_GMAC_MII_ADDR:
@@ -371,16 +355,6 @@ static void npcm_gmac_write(void *opaque, hwaddr offset,
   "%s: Write of read-only bits of reg: offset: 0x%04"
HWADDR_PRIx ", value: 0x%04" PRIx64 "\n",
DEVICE(gmac)->canonical_path, offset, v);
-} else {
-/* for W1c bits, implement W1C */
-gmac->regs[offset / sizeof(uint32_t)] &=
-~NPCM_DMA_STATUS_W1C_MASK(v);
-if (v & NPCM_DMA_STATUS_NIS_BITS) {
-gmac->regs[offset / sizeof(uint32_t)] &= ~NPCM_DMA_STATUS_NIS;
-}
-if (v & NPCM_DMA_STATUS_AIS_BITS) {
-gmac->regs[offset / sizeof(uint32_t)] &= ~NPCM_DMA_STATUS_AIS;
-}
 }
 break;
 
diff --git a/include/hw/net/npcm_gmac.h b/include/hw/net/npcm_gmac.h
index e5729e83ea..c97eb6fe6e 100644
--- a/include/hw/net/npcm_gmac.h
+++ b/include/hw/net/npcm_gmac.h
@@ -34,13 +34,15 @@ struct NPCMGMACRxDesc {
 };
 
 /* NPCMGMACRxDesc.flags values */
-/* RDES2 and RDES3 are buffer address pointers */
-/* Owner: 0 = software, 1 = gmac */
-#define RX_DESC_RDES0_OWNER_MASK BIT(31)
+/* RDES2 and RDES3 are buffer addresses */
+/* Owner: 0 = software, 1 = dma */
+#define RX_DESC_RDES0_OWN BIT(31)
 /* Destination Address Filter Fail */
-#define RX_DESC_RDES0_DEST_ADDR_FILT_FAIL_MASK BIT(30)
-/* Frame length*/
-#define RX_DESC_RDES0_FRAME_LEN_MASK(word) extract32(word, 16, 29)
+#define RX_DESC_RDES0_DEST_ADDR_FILT_FAIL BIT(30)
+/* Frame length */
+#define RX_DESC_RDES0_FRAME_LEN_MASK(word) extract32(word, 16, 14)
+/* Frame length Shift*/
+#define RX_DESC_RDES0_FRAME_LEN_SHIFT 16
 /* Error Summary */
 #define RX_DESC_RDES0_ERR_SUMM_MASK BIT(15)
 /* Descriptor Error */
@@ -83,9 +85,9 @@ struct NPCMGMACRxDesc {
 /* Receive Buffer 2 Size */
 #define RX_DESC_RDES1_BFFR2_SZ_SHIFT 11
 #define RX_DESC_RDES1_BFFR2_SZ_MASK(word) extract32(word, \
-RX_DESC_RDES1_BFFR2_SZ_SHIFT, 10 + RX_DESC_RDES1_BFFR2_SZ_SHIFT)
+RX_DESC_RDES1_BFFR2_SZ_SHIFT, 11)
 /* Receive Buffer 1 Size */
-#define RX_DESC_RDES1_BFFR1_SZ_MASK(word) extract32(word, 0, 10)
+#define RX_DESC_RDES1_BFFR1_SZ_MASK(word) extract32(word, 0, 11)
 
 
 struct NPCMGMACTxDesc {
@@ -96,9 +98,9 @@ struct NPCMGMACTxDesc {
 };
 
 /* NPCMGMACTxDesc.flags values */
-/* TDES2 and TDES3 are buffer address pointers */
+/* TDES2 and TDES3 are buffer addresses */
 /* Owner: 0 = software, 1 = gmac */
-#define TX_DESC_TDES0_OWNER_MASK BIT(31)
+#define TX_DESC_TDES0_OWN BIT(31)
 /* Tx Time Stamp Status */
 #define TX_DESC_TDES0_TTSS_MASK BIT(17)
 /* IP Header Error */
@@ -122,7 +124,7 @@ struct NPCMGMACTxDesc {
 /* VLAN Frame */
 #define TX_DESC_TDES0_VLAN_FRM_MASK BIT(7)
 /* Collision Count */
-#define TX_DESC_TDES0_COLL_CNT_MASK(word) extract32(word, 3, 6)
+#define TX_DESC_TDES0_COLL_CNT_MASK(word) extract32(word, 3, 4)
 /* Excessive Deferral */
 #define TX_DESC_TDES0_EXCS_DEF_MASK BIT(2)
 /* Underflow Error */
@@ -137,7 +139,7 @@ struct NPCMGMACTxDesc {
 /* Last Segment */
 #define TX_DESC_TDES1_FIRST_SEG_MASK BIT(29)
 /* Checksum Insertion Control */
-#define TX_DESC_TDES1_CHKSM_INS_CTRL_MASK(word) extract32(word, 27, 28)
+#define TX_DESC_TDES1_CHKSM_INS_CTRL_MASK(word) extract32(word, 27, 2)
 /* Disable Cyclic Redundancy Check */
 #define TX_DESC_TDES1_DIS_CDC_MASK BIT(26)
 /* Transmit End of Ring */
@@ 

[PATCH v7 01/11] hw/misc: Add Nuvoton's PCI Mailbox Module

[Nabih Estefan]

From: Hao Wu 

The PCI Mailbox Module is a high-bandwidth communcation module
between a Nuvoton BMC and CPU. It features 16KB RAM that are both
accessible by the BMC and core CPU. and supports interrupt for
both sides.

This patch implements the BMC side of the PCI mailbox module.
Communication with the core CPU is emulated via a chardev and
will be in a follow-up patch.

Change-Id: If73254b333c5aa2b8dea08e853111ab87f3a9326
Signed-off-by: Hao Wu 
Signed-off-by: Nabih Estefan 
Reviewed-by: Tyrone Ting 
---
 hw/arm/npcm7xx.c   |  16 +-
 hw/misc/meson.build|   1 +
 hw/misc/npcm7xx_pci_mbox.c | 324 +
 hw/misc/trace-events   |   5 +
 include/hw/arm/npcm7xx.h   |   1 +
 include/hw/misc/npcm7xx_pci_mbox.h |  81 
 6 files changed, 427 insertions(+), 1 deletion(-)
 create mode 100644 hw/misc/npcm7xx_pci_mbox.c
 create mode 100644 include/hw/misc/npcm7xx_pci_mbox.h

diff --git a/hw/arm/npcm7xx.c b/hw/arm/npcm7xx.c
index 15ff21d047..c69e936669 100644
--- a/hw/arm/npcm7xx.c
+++ b/hw/arm/npcm7xx.c
@@ -53,6 +53,9 @@
 /* ADC Module */
 #define NPCM7XX_ADC_BA  (0xf000c000)
 
+/* PCI Mailbox Module */
+#define NPCM7XX_PCI_MBOX_BA (0xf0848000)
+
 /* Internal AHB SRAM */
 #define NPCM7XX_RAM3_BA (0xc0008000)
 #define NPCM7XX_RAM3_SZ (4 * KiB)
@@ -83,6 +86,10 @@ enum NPCM7xxInterrupt {
 NPCM7XX_UART1_IRQ,
 NPCM7XX_UART2_IRQ,
 NPCM7XX_UART3_IRQ,
+NPCM7XX_PECI_IRQ= 6,
+NPCM7XX_PCI_MBOX_IRQ= 8,
+NPCM7XX_KCS_HIB_IRQ = 9,
+NPCM7XX_GMAC1_IRQ   = 14,
 NPCM7XX_EMC1RX_IRQ  = 15,
 NPCM7XX_EMC1TX_IRQ,
 NPCM7XX_MMC_IRQ = 26,
@@ -706,6 +713,14 @@ static void npcm7xx_realize(DeviceState *dev, Error **errp)
 }
 }
 
+/* PCI Mailbox. Cannot fail */
+sysbus_realize(SYS_BUS_DEVICE(>pci_mbox), _abort);
+sysbus_mmio_map(SYS_BUS_DEVICE(>pci_mbox), 0, NPCM7XX_PCI_MBOX_BA);
+sysbus_mmio_map(SYS_BUS_DEVICE(>pci_mbox), 1,
+NPCM7XX_PCI_MBOX_BA + NPCM7XX_PCI_MBOX_RAM_SIZE);
+sysbus_connect_irq(SYS_BUS_DEVICE(>pci_mbox), 0,
+   npcm7xx_irq(s, NPCM7XX_PCI_MBOX_IRQ));
+
 /* RAM2 (SRAM) */
 memory_region_init_ram(>sram, OBJECT(dev), "ram2",
NPCM7XX_RAM2_SZ, _abort);
@@ -765,7 +780,6 @@ static void npcm7xx_realize(DeviceState *dev, Error **errp)
 create_unimplemented_device("npcm7xx.usbd[8]",  0xf0838000,   4 * KiB);
 create_unimplemented_device("npcm7xx.usbd[9]",  0xf0839000,   4 * KiB);
 create_unimplemented_device("npcm7xx.sd",   0xf084,   8 * KiB);
-create_unimplemented_device("npcm7xx.pcimbx",   0xf0848000, 512 * KiB);
 create_unimplemented_device("npcm7xx.aes",  0xf0858000,   4 * KiB);
 create_unimplemented_device("npcm7xx.des",  0xf0859000,   4 * KiB);
 create_unimplemented_device("npcm7xx.sha",  0xf085a000,   4 * KiB);
diff --git a/hw/misc/meson.build b/hw/misc/meson.build
index 36c20d5637..0ead2e9ede 100644
--- a/hw/misc/meson.build
+++ b/hw/misc/meson.build
@@ -73,6 +73,7 @@ system_ss.add(when: 'CONFIG_NPCM7XX', if_true: files(
   'npcm7xx_clk.c',
   'npcm7xx_gcr.c',
   'npcm7xx_mft.c',
+  'npcm7xx_pci_mbox.c',
   'npcm7xx_pwm.c',
   'npcm7xx_rng.c',
 ))
diff --git a/hw/misc/npcm7xx_pci_mbox.c b/hw/misc/npcm7xx_pci_mbox.c
new file mode 100644
index 00..c770ad6fcf
--- /dev/null
+++ b/hw/misc/npcm7xx_pci_mbox.c
@@ -0,0 +1,324 @@
+/*
+ * Nuvoton NPCM7xx PCI Mailbox Module
+ *
+ * Copyright 2021 Google LLC
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ */
+
+#include "qemu/osdep.h"
+#include "chardev/char-fe.h"
+#include "hw/irq.h"
+#include "hw/qdev-clock.h"
+#include "hw/qdev-properties-system.h"
+#include "hw/misc/npcm7xx_pci_mbox.h"
+#include "hw/registerfields.h"
+#include "migration/vmstate.h"
+#include "qapi/error.h"
+#include "qapi/visitor.h"
+#include "qemu/bitops.h"
+#include "qemu/error-report.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+#include "qemu/timer.h"
+#include "qemu/units.h"
+#include "trace.h"
+
+REG32(NPCM7XX_PCI_MBOX_BMBXSTAT, 0x00);
+REG32(NPCM7XX_PCI_MBOX_BMBXCTL, 0x04);
+REG32(NPCM7XX_PCI_MBOX_BMBXCMD, 0x08);
+
+enum NPCM7xxPCIMBoxOperation {
+NPCM7XX_PCI_MBOX_OP_READ = 1,
+NPCM7XX_PCI_MBOX_OP_WRITE,
+};
+
+#define NPCM7XX_PCI_MBOX_OFFSET_BYTES 8
+
+/* Response code */
+#define NPCM7XX_PCI_MBOX_OK 0
+#define NPCM7XX_PCI_MBOX_INVALID_OP 0xa0
+#define 

[PATCH v7 05/11] hw/arm: Add GMAC devices to NPCM7XX SoC

[Nabih Estefan]

From: Hao Wu 

Change-Id: I0e323acbe9d1ac5138764e7fe70a8423af414454
Signed-off-by: Hao Wu 
Signed-off-by: Nabih Estefan 
Reviewed-by: Tyrone Ting 
---
 hw/arm/npcm7xx.c | 36 ++--
 include/hw/arm/npcm7xx.h |  2 ++
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/hw/arm/npcm7xx.c b/hw/arm/npcm7xx.c
index c9e87162cb..12e11250e1 100644
--- a/hw/arm/npcm7xx.c
+++ b/hw/arm/npcm7xx.c
@@ -91,6 +91,7 @@ enum NPCM7xxInterrupt {
 NPCM7XX_GMAC1_IRQ   = 14,
 NPCM7XX_EMC1RX_IRQ  = 15,
 NPCM7XX_EMC1TX_IRQ,
+NPCM7XX_GMAC2_IRQ,
 NPCM7XX_MMC_IRQ = 26,
 NPCM7XX_PSPI2_IRQ   = 28,
 NPCM7XX_PSPI1_IRQ   = 31,
@@ -234,6 +235,12 @@ static const hwaddr npcm7xx_pspi_addr[] = {
 0xf0201000,
 };
 
+/* Register base address for each GMAC Module */
+static const hwaddr npcm7xx_gmac_addr[] = {
+0xf0802000,
+0xf0804000,
+};
+
 static const struct {
 hwaddr regs_addr;
 uint32_t unconnected_pins;
@@ -462,6 +469,10 @@ static void npcm7xx_init(Object *obj)
 object_initialize_child(obj, "pspi[*]", >pspi[i], TYPE_NPCM_PSPI);
 }
 
+for (i = 0; i < ARRAY_SIZE(s->gmac); i++) {
+object_initialize_child(obj, "gmac[*]", >gmac[i], TYPE_NPCM_GMAC);
+}
+
 object_initialize_child(obj, "pci-mbox", >pci_mbox,
 TYPE_NPCM7XX_PCI_MBOX);
 object_initialize_child(obj, "mmc", >mmc, TYPE_NPCM7XX_SDHCI);
@@ -695,6 +706,29 @@ static void npcm7xx_realize(DeviceState *dev, Error **errp)
 sysbus_connect_irq(sbd, 1, npcm7xx_irq(s, rx_irq));
 }
 
+/*
+ * GMAC Modules. Cannot fail.
+ */
+QEMU_BUILD_BUG_ON(ARRAY_SIZE(npcm7xx_gmac_addr) != ARRAY_SIZE(s->gmac));
+QEMU_BUILD_BUG_ON(ARRAY_SIZE(s->gmac) != 2);
+for (i = 0; i < ARRAY_SIZE(s->gmac); i++) {
+SysBusDevice *sbd = SYS_BUS_DEVICE(>gmac[i]);
+
+/*
+ * The device exists regardless of whether it's connected to a QEMU
+ * netdev backend. So always instantiate it even if there is no
+ * backend.
+ */
+sysbus_realize(sbd, _abort);
+sysbus_mmio_map(sbd, 0, npcm7xx_gmac_addr[i]);
+int irq = i == 0 ? NPCM7XX_GMAC1_IRQ : NPCM7XX_GMAC2_IRQ;
+/*
+ * N.B. The values for the second argument sysbus_connect_irq are
+ * chosen to match the registration order in npcm7xx_emc_realize.
+ */
+sysbus_connect_irq(sbd, 0, npcm7xx_irq(s, irq));
+}
+
 /*
  * Flash Interface Unit (FIU). Can fail if incorrect number of chip selects
  * specified, but this is a programming error.
@@ -765,8 +799,6 @@ static void npcm7xx_realize(DeviceState *dev, Error **errp)
 create_unimplemented_device("npcm7xx.siox[2]",  0xf0102000,   4 * KiB);
 create_unimplemented_device("npcm7xx.ahbpci",   0xf040,   1 * MiB);
 create_unimplemented_device("npcm7xx.mcphy",0xf05f,  64 * KiB);
-create_unimplemented_device("npcm7xx.gmac1",0xf0802000,   8 * KiB);
-create_unimplemented_device("npcm7xx.gmac2",0xf0804000,   8 * KiB);
 create_unimplemented_device("npcm7xx.vcd",  0xf081,  64 * KiB);
 create_unimplemented_device("npcm7xx.ece",  0xf082,   8 * KiB);
 create_unimplemented_device("npcm7xx.vdma", 0xf0822000,   8 * KiB);
diff --git a/include/hw/arm/npcm7xx.h b/include/hw/arm/npcm7xx.h
index cec3792a2e..9e5cf639a2 100644
--- a/include/hw/arm/npcm7xx.h
+++ b/include/hw/arm/npcm7xx.h
@@ -30,6 +30,7 @@
 #include "hw/misc/npcm7xx_pwm.h"
 #include "hw/misc/npcm7xx_rng.h"
 #include "hw/net/npcm7xx_emc.h"
+#include "hw/net/npcm_gmac.h"
 #include "hw/nvram/npcm7xx_otp.h"
 #include "hw/timer/npcm7xx_timer.h"
 #include "hw/ssi/npcm7xx_fiu.h"
@@ -105,6 +106,7 @@ struct NPCM7xxState {
 OHCISysBusState ohci;
 NPCM7xxFIUState fiu[2];
 NPCM7xxEMCState emc[2];
+NPCMGMACState   gmac[2];
 NPCM7xxPCIMBoxState pci_mbox;
 NPCM7xxSDHCIState   mmc;
 NPCMPSPIState   pspi[2];
-- 
2.43.0.472.g3155946c3a-goog

[PATCH v7 07/11] include/hw/net: Implemented Classes and Masks for GMAC Descriptors

[Nabih Estefan]

From: Nabih Estefan Diaz 

 - Implemeted classes for GMAC Receive and Transmit Descriptors
 - Implemented Masks for said descriptors

Change-Id: I1d3c7fafcade53ebf5917f09fd2d78620d14cfd5
Signed-off-by: Nabih Estefan 
Reviewed-by: Tyrone Ting 
---
 hw/net/npcm_gmac.c   | 183 +++
 hw/net/trace-events  |   9 ++
 include/hw/net/npcm_gmac.h   |   2 -
 tests/qtest/npcm_gmac-test.c |   2 +-
 4 files changed, 150 insertions(+), 46 deletions(-)

diff --git a/hw/net/npcm_gmac.c b/hw/net/npcm_gmac.c
index 5ce632858d..6f8109e0ee 100644
--- a/hw/net/npcm_gmac.c
+++ b/hw/net/npcm_gmac.c
@@ -32,7 +32,7 @@
 REG32(NPCM_DMA_BUS_MODE, 0x1000)
 REG32(NPCM_DMA_XMT_POLL_DEMAND, 0x1004)
 REG32(NPCM_DMA_RCV_POLL_DEMAND, 0x1008)
-REG32(NPCM_DMA_RCV_BASE_ADDR, 0x100c)
+REG32(NPCM_DMA_RX_BASE_ADDR, 0x100c)
 REG32(NPCM_DMA_TX_BASE_ADDR, 0x1010)
 REG32(NPCM_DMA_STATUS, 0x1014)
 REG32(NPCM_DMA_CONTROL, 0x1018)
@@ -91,7 +91,8 @@ REG32(NPCM_GMAC_PTP_TTSR, 0x71c)
 #define NPCM_DMA_BUS_MODE_SWR   BIT(0)
 
 static const uint32_t npcm_gmac_cold_reset_values[NPCM_GMAC_NR_REGS] = {
-[R_NPCM_GMAC_VERSION] = 0x1037,
+/* Reduce version to 3.2 so that the kernel can enable interrupt. */
+[R_NPCM_GMAC_VERSION] = 0x1032,
 [R_NPCM_GMAC_TIMER_CTRL]  = 0x03e8,
 [R_NPCM_GMAC_MAC0_ADDR_HI]= 0x8000,
 [R_NPCM_GMAC_MAC0_ADDR_LO]= 0x,
@@ -125,12 +126,12 @@ static const uint16_t phy_reg_init[] = {
 [MII_EXTSTAT]   = 0x3000, /* 1000BASTE_T full-duplex capable */
 };
 
-static void npcm_gmac_soft_reset(NPCMGMACState *s)
+static void npcm_gmac_soft_reset(NPCMGMACState *gmac)
 {
-memcpy(s->regs, npcm_gmac_cold_reset_values,
+memcpy(gmac->regs, npcm_gmac_cold_reset_values,
NPCM_GMAC_NR_REGS * sizeof(uint32_t));
 /* Clear reset bits */
-s->regs[R_NPCM_DMA_BUS_MODE] &= ~NPCM_DMA_BUS_MODE_SWR;
+gmac->regs[R_NPCM_DMA_BUS_MODE] &= ~NPCM_DMA_BUS_MODE_SWR;
 }
 
 static void gmac_phy_set_link(NPCMGMACState *s, bool active)
@@ -148,11 +149,53 @@ static bool gmac_can_receive(NetClientState *nc)
 return true;
 }
 
-static ssize_t gmac_receive(NetClientState *nc, const uint8_t *buf, size_t 
len1)
+/*
+ * Function that updates the GMAC IRQ
+ * It find the logical OR of the enabled bits for NIS (if enabled)
+ * It find the logical OR of the enabled bits for AIS (if enabled)
+ */
+static void gmac_update_irq(NPCMGMACState *gmac)
 {
-return 0;
+/*
+ * Check if the normal interrupts summery is enabled
+ * if so, add the bits for the summary that are enabled
+ */
+if (gmac->regs[R_NPCM_DMA_INTR_ENA] & gmac->regs[R_NPCM_DMA_STATUS] &
+(NPCM_DMA_INTR_ENAB_NIE_BITS))
+{
+gmac->regs[R_NPCM_DMA_STATUS] |=  NPCM_DMA_STATUS_NIS;
+}
+/*
+ * Check if the abnormal interrupts summery is enabled
+ * if so, add the bits for the summary that are enabled
+ */
+if (gmac->regs[R_NPCM_DMA_INTR_ENA] & gmac->regs[R_NPCM_DMA_STATUS] &
+(NPCM_DMA_INTR_ENAB_AIE_BITS))
+{
+gmac->regs[R_NPCM_DMA_STATUS] |=  NPCM_DMA_STATUS_AIS;
+}
+
+/* Get the logical OR of both normal and abnormal interrupts */
+int level = !!((gmac->regs[R_NPCM_DMA_STATUS] &
+gmac->regs[R_NPCM_DMA_INTR_ENA] &
+NPCM_DMA_STATUS_NIS) |
+   (gmac->regs[R_NPCM_DMA_STATUS] &
+   gmac->regs[R_NPCM_DMA_INTR_ENA] &
+   NPCM_DMA_STATUS_AIS));
+
+/* Set the IRQ */
+trace_npcm_gmac_update_irq(DEVICE(gmac)->canonical_path,
+   gmac->regs[R_NPCM_DMA_STATUS],
+   gmac->regs[R_NPCM_DMA_INTR_ENA],
+   level);
+qemu_set_irq(gmac->irq, level);
 }
 
+static ssize_t gmac_receive(NetClientState *nc, const uint8_t *buf, size_t len)
+{
+/* Placeholder */
+return 0;
+}
 static void gmac_cleanup(NetClientState *nc)
 {
 /* Nothing to do yet. */
@@ -166,7 +209,7 @@ static void gmac_set_link(NetClientState *nc)
 gmac_phy_set_link(s, !nc->link_down);
 }
 
-static void npcm_gmac_mdio_access(NPCMGMACState *s, uint16_t v)
+static void npcm_gmac_mdio_access(NPCMGMACState *gmac, uint16_t v)
 {
 bool busy = v & NPCM_GMAC_MII_ADDR_BUSY;
 uint8_t is_write;
@@ -183,33 +226,38 @@ static void npcm_gmac_mdio_access(NPCMGMACState *s, 
uint16_t v)
 
 
 if (v & NPCM_GMAC_MII_ADDR_WRITE) {
-data = s->regs[R_NPCM_GMAC_MII_DATA];
+data = gmac->regs[R_NPCM_GMAC_MII_DATA];
 /* Clear reset bit for BMCR register */
 switch (gr) {
 case MII_BMCR:
 data &= ~MII_BMCR_RESET;
-/* Complete auto-negotiation immediately and set as complete */
-if (data & MII_BMCR_AUTOEN) {
+/* Autonegotiation is a W1C bit*/
+if (data & MII_BMCR_ANRESTART) {
 /* Tells 

[PATCH v7 02/11] hw/arm: Add PCI mailbox module to Nuvoton SoC

[Nabih Estefan]

From: Hao Wu 

This patch wires the PCI mailbox module to Nuvoton SoC.

Change-Id: I64ab900bc8bd7c379a11d86f2b8dcfccb49e66ab
Signed-off-by: Hao Wu 
Signed-off-by: Nabih Estefan 
Reviewed-by: Tyrone Ting 
---
 docs/system/arm/nuvoton.rst | 2 ++
 hw/arm/npcm7xx.c| 3 ++-
 include/hw/arm/npcm7xx.h| 1 +
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/docs/system/arm/nuvoton.rst b/docs/system/arm/nuvoton.rst
index 0424cae4b0..e611099545 100644
--- a/docs/system/arm/nuvoton.rst
+++ b/docs/system/arm/nuvoton.rst
@@ -50,6 +50,8 @@ Supported devices
  * Ethernet controller (EMC)
  * Tachometer
  * Peripheral SPI controller (PSPI)
+ * BIOS POST code FIFO
+ * PCI Mailbox
 
 Missing devices
 ---
diff --git a/hw/arm/npcm7xx.c b/hw/arm/npcm7xx.c
index c69e936669..c9e87162cb 100644
--- a/hw/arm/npcm7xx.c
+++ b/hw/arm/npcm7xx.c
@@ -86,7 +86,6 @@ enum NPCM7xxInterrupt {
 NPCM7XX_UART1_IRQ,
 NPCM7XX_UART2_IRQ,
 NPCM7XX_UART3_IRQ,
-NPCM7XX_PECI_IRQ= 6,
 NPCM7XX_PCI_MBOX_IRQ= 8,
 NPCM7XX_KCS_HIB_IRQ = 9,
 NPCM7XX_GMAC1_IRQ   = 14,
@@ -463,6 +462,8 @@ static void npcm7xx_init(Object *obj)
 object_initialize_child(obj, "pspi[*]", >pspi[i], TYPE_NPCM_PSPI);
 }
 
+object_initialize_child(obj, "pci-mbox", >pci_mbox,
+TYPE_NPCM7XX_PCI_MBOX);
 object_initialize_child(obj, "mmc", >mmc, TYPE_NPCM7XX_SDHCI);
 }
 
diff --git a/include/hw/arm/npcm7xx.h b/include/hw/arm/npcm7xx.h
index 273090ac60..cec3792a2e 100644
--- a/include/hw/arm/npcm7xx.h
+++ b/include/hw/arm/npcm7xx.h
@@ -105,6 +105,7 @@ struct NPCM7xxState {
 OHCISysBusState ohci;
 NPCM7xxFIUState fiu[2];
 NPCM7xxEMCState emc[2];
+NPCM7xxPCIMBoxState pci_mbox;
 NPCM7xxSDHCIState   mmc;
 NPCMPSPIState   pspi[2];
 };
-- 
2.43.0.472.g3155946c3a-goog