PING: [for-8.0 v2 00/11] Refactor cryptodev

[zhenwei pi]

Hi, Markus & Michael

Could you please take a look at the changes of QAPI part?

On 12/22/22 10:04, zhenwei pi wrote:



On 12/20/22 23:36, Michael S. Tsirkin wrote:

On Tue, Nov 22, 2022 at 10:07:45PM +0800, zhenwei pi wrote:

v1 -> v2:
- fix coding style and use 'g_strjoin()' instead of 'char services[128]'
   (suggested by Dr. David Alan Gilbert)
- wrapper function 'cryptodev_backend_account' to record statistics, and
   allocate sym_stat/asym_stat in cryptodev base class. see patch:
   'cryptodev: Support statistics'.
- add more arguments into struct CryptoDevBackendOpInfo, then
   cryptodev_backend_crypto_operation() uses *op_info only.
- support cryptodev QoS settings(BPS), both QEMU command line and 
QMP

   command works fine.
- add myself as the maintainer for cryptodev.

v1:
- introduce cryptodev.json to describe the attributes of crypto 
device, then

   drop duplicated type declare, remove some virtio related dependence.
- add statistics: OPS and bandwidth.
- add QMP command: query-cryptodev
- add HMP info command: cryptodev
- misc fix: detect akcipher capability instead of exposing akcipher 
service

   unconditionally.



Can we get ACK on QAPI things please?
Thanks!



Hi, Markus & Michael

Could you please review the changes of QAPI part?


Zhenwei Pi (11):
   cryptodev: Introduce cryptodev.json
   cryptodev: Remove 'name' & 'model' fields
   cryptodev: Introduce cryptodev alg type in QAPI
   cryptodev: Introduce server type in QAPI
   cryptodev: Introduce 'query-cryptodev' QMP command
   cryptodev: Support statistics
   cryptodev-builtin: Detect akcipher capability
   hmp: add cryptodev info command
   cryptodev: Use CryptoDevBackendOpInfo for operation
   cryptodev: support QoS
   MAINTAINERS: add myself as the maintainer for cryptodev

  MAINTAINERS |   2 +
  backends/cryptodev-builtin.c    |  42 +++--
  backends/cryptodev-lkcf.c   |  19 +-
  backends/cryptodev-vhost-user.c |  13 +-
  backends/cryptodev-vhost.c  |   4 +-
  backends/cryptodev.c    | 295 +---
  hmp-commands-info.hx    |  14 ++
  hw/virtio/virtio-crypto.c   |  48 --
  include/monitor/hmp.h   |   1 +
  include/sysemu/cryptodev.h  |  94 +-
  monitor/hmp-cmds.c  |  36 
  qapi/cryptodev.json | 144 
  qapi/meson.build    |   1 +
  qapi/qapi-schema.json   |   1 +
  qapi/qom.json   |   8 +-
  15 files changed, 604 insertions(+), 118 deletions(-)
  create mode 100644 qapi/cryptodev.json

--
2.20.1






--
zhenwei pi

RE: [PATCH v10 2/9] KVM: Introduce per-page memory attributes

[Wang, Wei W]

On Tuesday, January 3, 2023 9:40 AM, Chao Peng wrote:
> > Because guest memory defaults to private, and now this patch stores
> > the attributes with KVM_MEMORY_ATTRIBUTE_PRIVATE instead of
> _SHARED,
> > it would bring more KVM_EXIT_MEMORY_FAULT exits at the beginning of
> > boot time. Maybe it can be optimized somehow in other places? e.g. set
> > mem attr in advance.
> 
> KVM defaults to 'shared' because this ioctl can also be potentially used by
> normal VMs and 'shared' sounds a value meaningful for both normal VMs and
> confidential VMs. 

Do you mean a normal VM could have pages marked private? What's the usage?
(If all the pages are just marked shared for normal VMs, then why do we need it)

> As for more KVM_EXIT_MEMORY_FAULT exits during the
> booting time, yes, setting all memory to 'private' for confidential VMs 
> through
> this ioctl in userspace before guest launch is an approach for KVM userspace 
> to
> 'override' the KVM default and reduce the number of implicit conversions.

Most pages of a confidential VM are likely to be private pages. It seems more 
efficient
(and not difficult to check vm_type) to have KVM defaults to "private" for 
confidential VMs
and defaults to "shared" for normal VMs.

Re: [PATCH 0/6] Resolve TYPE_PIIX3_XEN_DEVICE

[Chuck Zmudzinski]

On 1/2/23 4:34 PM, Bernhard Beschow wrote:
> This series first renders TYPE_PIIX3_XEN_DEVICE redundant and finally removes
> it. The motivation is to 1/ decouple PIIX from Xen and 2/ to make Xen in the 
> PC
> machine agnostic to the precise southbridge being used. 2/ will become
> particularily interesting once PIIX4 becomes usable in the PC machine, 
> avoiding
> the "Frankenstein" use of PIIX4_ACPI in PIIX3.
>
> Testing done:
> None, because I don't know how to conduct this properly :(
>
> Based-on: <20221221170003.2929-1-shen...@gmail.com>
>   "[PATCH v4 00/30] Consolidate PIIX south bridges"
>
> Bernhard Beschow (6):
>   include/hw/xen/xen: Make xen_piix3_set_irq() generic and rename it
>   hw/isa/piix: Reuse piix3_realize() in piix3_xen_realize()
>   hw/isa/piix: Wire up Xen PCI IRQ handling outside of PIIX3
>   hw/isa/piix: Avoid Xen-specific variant of piix_write_config()
>   hw/isa/piix: Resolve redundant k->config_write assignments
>   hw/isa/piix: Resolve redundant TYPE_PIIX3_XEN_DEVICE
>
>  hw/i386/pc_piix.c | 34 --
>  hw/i386/xen/xen-hvm.c |  9 +++--
>  hw/isa/piix.c | 66 +--

This file does not exist on the Qemu master branch.
But hw/isa/piix3.c and hw/isa/piix4.c do exist.

I tried renaming it from piix.c to piix3.c in the patch, but
the patch set still does not apply cleanly on my tree.

Is this patch set re-based against something other than
the current master Qemu branch?

I have a system that is suitable for testing this patch set, but
I need guidance on how to apply it to the Qemu source tree.

Thanks,

Chuck Zmudzinski

>  include/hw/southbridge/piix.h |  1 -
>  include/hw/xen/xen.h  |  2 +-
>  stubs/xen-hw-stub.c   |  2 +-
>  6 files changed, 40 insertions(+), 74 deletions(-)
>

Re: [PATCH 0/6] target/i386: Support new Intel platform Instructions in CPUID enumeration

[Jiaxi Chen]

Kindly ping for any comments.

BR,
Jiaxi

On 12/8/2022 3:19 PM, Jiaxi Chen wrote:
> Latest Intel platform Granite Rapids/Sierra Forest has introduced below
> new instructions and CPUIDs:
> 
>  - CMPccXADD CPUID.(EAX=7,ECX=1):EAX[bit 7]
>  - AMX-FP16 CPUID.(EAX=7,ECX=1):EAX[bit 21]
>  - AVX-IFMA CPUID.(EAX=7,ECX=1):EAX[bit 23]
>  - AVX-VNNI-INT8 CPUID.(EAX=7,ECX=1):EDX[bit 4]
>  - AVX-NE-CONVERT CPUID.(EAX=7,ECX=1):EDX[bit 5]
>  - PREFETCHITI CPUID.(EAX=7,ECX=1):EDX[bit 14]
> 
> Details can be found in recent Intel ISE (Instruction Set Extensions)[1].
> 
> Linux 6.2 will support for advertising these features to userspace. KVM
> patches have been merged into kvm/next[2]. This patch series adds CPUID
> definitions of the corresponding features in QEMU. 
> 
> [1] Intel ISE: https://cdrdv2.intel.com/v1/dl/getContent/671368
> [2] kvm/next: https://git.kernel.org/pub/scm/virt/kvm/kvm.git
> 
> Jiaxi Chen (6):
>   target/i386: Add support for CMPCCXADD in CPUID enumeration
>   target/i386: Add support for AMX-FP16 in CPUID enumeration
>   target/i386: Add support for AVX-IFMA in CPUID enumeration
>   target/i386: Add support for AVX-VNNI-INT8 in CPUID enumeration
>   target/i386: Add support for AVX-NE-CONVERT in CPUID enumeration
>   target/i386: Add support for PREFETCHIT0/1 in CPUID enumeration
> 
>  target/i386/cpu.c | 26 +++---
>  target/i386/cpu.h | 15 +++
>  2 files changed, 38 insertions(+), 3 deletions(-)
> 
> 
> base-commit: ea3a008d2d9ced9c4f93871c823baee237047f93

-- 
Regards,
Jiaxi

Re: [PATCH v10 2/9] KVM: Introduce per-page memory attributes

[Chao Peng]

On Wed, Dec 28, 2022 at 04:28:01PM +0800, Chenyi Qiang wrote:
...
> > +static int kvm_vm_ioctl_set_mem_attributes(struct kvm *kvm,
> > +  struct kvm_memory_attributes *attrs)
> > +{
> > +   gfn_t start, end;
> > +   unsigned long i;
> > +   void *entry;
> > +   u64 supported_attrs = kvm_supported_mem_attributes(kvm);
> > +
> > +   /* flags is currently not used. */
> > +   if (attrs->flags)
> > +   return -EINVAL;
> > +   if (attrs->attributes & ~supported_attrs)
> > +   return -EINVAL;
> > +   if (attrs->size == 0 || attrs->address + attrs->size < attrs->address)
> > +   return -EINVAL;
> > +   if (!PAGE_ALIGNED(attrs->address) || !PAGE_ALIGNED(attrs->size))
> > +   return -EINVAL;
> > +
> > +   start = attrs->address >> PAGE_SHIFT;
> > +   end = (attrs->address + attrs->size - 1 + PAGE_SIZE) >> PAGE_SHIFT;
> > +
> > +   entry = attrs->attributes ? xa_mk_value(attrs->attributes) : NULL;
> > +
> 
> Because guest memory defaults to private, and now this patch stores the
> attributes with KVM_MEMORY_ATTRIBUTE_PRIVATE instead of _SHARED, it
> would bring more KVM_EXIT_MEMORY_FAULT exits at the beginning of boot
> time. Maybe it can be optimized somehow in other places? e.g. set mem
> attr in advance.

KVM defaults to 'shared' because this ioctl can also be potentially used
by normal VMs and 'shared' sounds a value meaningful for both normal VMs
and confidential VMs. As for more KVM_EXIT_MEMORY_FAULT exits during the
booting time, yes, setting all memory to 'private' for confidential VMs
through this ioctl in userspace before guest launch is an approach for
KVM userspace to 'override' the KVM default and reduce the number of
implicit conversions.

Thanks,
Chao
> 
> > +   mutex_lock(>lock);
> > +   for (i = start; i < end; i++)
> > +   if (xa_err(xa_store(>mem_attr_array, i, entry,
> > +   GFP_KERNEL_ACCOUNT)))
> > +   break;
> > +   mutex_unlock(>lock);
> > +
> > +   attrs->address = i << PAGE_SHIFT;
> > +   attrs->size = (end - i) << PAGE_SHIFT;
> > +
> > +   return 0;
> > +}
> > +#endif /* CONFIG_HAVE_KVM_MEMORY_ATTRIBUTES */
> > +
> >  struct kvm_memory_slot *gfn_to_memslot(struct kvm *kvm, gfn_t gfn)
> >  {
> > return __gfn_to_memslot(kvm_memslots(kvm), gfn);

Re: [PATCH v4 1/2] tpm: convert tpmdev options processing to new visitor format

[Stefan Berger]

On 12/30/22 22:40, James Bottomley wrote:

On Fri, 2022-12-30 at 12:01 -0500, Stefan Berger wrote:

On 12/30/22 10:24, James Bottomley wrote:

[...]

@@ -2906,9 +2893,7 @@ void qemu_init(int argc, char **argv)
   break;
   #ifdef CONFIG_TPM
   case QEMU_OPTION_tpmdev:
-    if (tpm_config_parse(qemu_find_opts("tpmdev"),
optarg) < 0) {
-    exit(1);
-    }
+    tpm_config_parse(optarg);


The patches don't apply to upstream's master.


I think it depends how you apply them.  If you use git, they do except
a minor merge conflict in tpm_passthrough.c


It has changed quite a bit there, so I let you rebase the series.

   Stefan

Re: [PATCH v6] xen/pt: reserve PCI slot 2 for Intel igd-passthru

[Chuck Zmudzinski]

On 1/2/23 12:46 PM, Michael S. Tsirkin wrote:
> On Sun, Jan 01, 2023 at 06:52:03PM -0500, Chuck Zmudzinski wrote:
> > Intel specifies that the Intel IGD must occupy slot 2 on the PCI bus,
> > as noted in docs/igd-assign.txt in the Qemu source code.
> > 
> > Currently, when the xl toolstack is used to configure a Xen HVM guest with
> > Intel IGD passthrough to the guest with the Qemu upstream device model,
> > a Qemu emulated PCI device will occupy slot 2 and the Intel IGD will occupy
> > a different slot. This problem often prevents the guest from booting.
> > 
> > The only available workaround is not good: Configure Xen HVM guests to use
> > the old and no longer maintained Qemu traditional device model available
> > from xenbits.xen.org which does reserve slot 2 for the Intel IGD.
> > 
> > To implement this feature in the Qemu upstream device model for Xen HVM
> > guests, introduce the following new functions, types, and macros:
> > 
> > * XEN_PT_DEVICE_CLASS declaration, based on the existing TYPE_XEN_PT_DEVICE
> > * XEN_PT_DEVICE_GET_CLASS macro helper function for XEN_PT_DEVICE_CLASS
> > * typedef XenPTQdevRealize function pointer
> > * XEN_PCI_IGD_SLOT_MASK, the value of slot_reserved_mask to reserve slot 2
> > * xen_igd_reserve_slot and xen_igd_clear_slot functions
> > 
> > The new xen_igd_reserve_slot function uses the existing slot_reserved_mask
> > member of PCIBus to reserve PCI slot 2 for Xen HVM guests configured using
> > the xl toolstack with the gfx_passthru option enabled, which sets the
> > igd-passthru=on option to Qemu for the Xen HVM machine type.
> > 
> > The new xen_igd_reserve_slot function also needs to be implemented in
> > hw/xen/xen_pt_stub.c to prevent FTBFS during the link stage for the case
> > when Qemu is configured with --enable-xen and --disable-xen-pci-passthrough,
> > in which case it does nothing.
> > 
> > The new xen_igd_clear_slot function overrides qdev->realize of the parent
> > PCI device class to enable the Intel IGD to occupy slot 2 on the PCI bus
> > since slot 2 was reserved by xen_igd_reserve_slot when the PCI bus was
> > created in hw/i386/pc_piix.c for the case when igd-passthru=on.
> > 
> > Move the call to xen_host_pci_device_get, and the associated error
> > handling, from xen_pt_realize to the new xen_igd_clear_slot function to
> > initialize the device class and vendor values which enables the checks for
> > the Intel IGD to succeed. The verification that the host device is an
> > Intel IGD to be passed through is done by checking the domain, bus, slot,
> > and function values as well as by checking that gfx_passthru is enabled,
> > the device class is VGA, and the device vendor in Intel.
> > 
> > Signed-off-by: Chuck Zmudzinski 
>
> I'm not sure why is the issue xen specific. Can you explain?
> Doesn't it affect kvm too?

Recall from docs/igd-assign.txt that there are two modes for
igd passthrough: legacy and upt, and the igd needs to be
at slot 2 only when using legacy mode which gives one
single guest exclusive access to the Intel igd.

It's only xen specific insofar as xen does not have support
for the upt mode so xen must use legacy mode which
requires the igd to be at slot 2. I am not an expert with
kvm, but if I understand correctly, with kvm one can use
the upt mode with the Intel i915 kvmgt kernel module
and in that case the guest will see a virtual Intel gpu
that can be at any arbitrary slot when using kvmgt, and
also, in that case, more than one guest can access the
igd through the kvmgt kernel module.

Again, I am not an expert and do not have as much
experience with kvm, but if I understand correctly it is
possible to use the legacy mode with kvm and I think you
are correct that if one uses kvm in legacy mode and without
using the Intel i915 kvmgt kernel module, then it would be
necessary to reserve slot 2 for the igd on kvm.

Your question makes me curious, and I have not been able
to determine if anyone has tried igd passthrough using
legacy mode on kvm with recent versions of linux and qemu.
I will try reproducing the problem on kvm in legacy mode with
current versions of linux and qemu and report my findings.
With kvm, there might be enough flexibility to specify the
slot number for every pci device in the guest. Such a
capability is not available using the xenlight toolstack
for managing xen guests, so I have been using this patch
to ensure that the Intel igd is at slot 2 with xen guests
created by the xenlight toolstack.

The patch as is will only fix the problem on xen, so if the
problem exists on kvm also, I agree that the patch should
be modified to also fix it on kvm.

Chuck

Re: [PATCH v3] tests/qtest: netdev: test stream and dgram backends

[Laurent Vivier]

On 12/12/22 14:20, Thomas Huth wrote:

On 09/11/2022 14.03, Laurent Vivier wrote:
[...]

diff --git a/tests/qtest/netdev-socket.c b/tests/qtest/netdev-socket.c
new file mode 100644
index ..b6b59244a282
--- /dev/null
+++ b/tests/qtest/netdev-socket.c
@@ -0,0 +1,435 @@
+/*
+ * QTest testcase for netdev stream and dgram
+ *
+ * Copyright (c) 2022 Red Hat, Inc.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include 
+#include "../unit/socket-helpers.h"
+#include "libqtest.h"
+
+#define CONNECTION_TIMEOUT    5
+
+#define EXPECT_STATE(q, e, t) \
+do {  \
+    char *resp = qtest_hmp(q, "info network");    \
+    if (t) {  \
+    strrchr(resp, t)[0] = 0;  \
+    } \
+    g_test_timer_start(); \
+    while (g_test_timer_elapsed() < CONNECTION_TIMEOUT) { \
+    if (strcmp(resp, e) == 0) {   \
+    break;    \
+    } \
+    g_free(resp); \
+    resp = qtest_hmp(q, "info network");  \
+    if (t) {  \
+    strrchr(resp, t)[0] = 0;  \
+    } \
+    } \
+    g_assert_cmpstr(resp, ==, e); \
+    g_free(resp); \
+} while (0)


Wouldn't it be possible to write this without the duplicated qtest_hmp() ? Something like 
this:


#define EXPECT_STATE(q, e, t) \
do {  \
     char *resp = NULL;    \
     g_test_timer_start(); \
     do {  \
     g_free(resp); \
     resp = qtest_hmp(q, "info network");  \
     if (t) {  \
     strrchr(resp, t)[0] = 0;  \
     } \
     if (g_strequal(resp, e)) {    \
     break;    \
     } \
     } while (g_test_timer_elapsed() < CONNECTION_TIMEOUT); \
     g_assert_cmpstr(resp, ==, e); \
     g_free(resp); \
} while (0)

?


OK



Also matching strings against the output of a HMP command sound very fragile - isn't there 
a way to do this with QMP instead?


No, there is no similar command with QMP. There was a tentative in the past but it has 
never been merged.


https://wiki.qemu.org/Google_Summer_of_Code_2010/QMP#query-netdev



[...]

+int main(int argc, char **argv)
+{
+    int ret;
+    bool has_ipv4, has_ipv6, has_afunix;
+    gchar dir[] = "/tmp/netdev-socket.XX";


No more hard-coded /tmp/ paths, please. We're currently in progress of enabling the qtests 
on Windows, too. Please use g_dir_make_tmp() or something similar instead.


ok




+    g_test_init(, , NULL);
+
+    if (socket_check_protocol_support(_ipv4, _ipv6) < 0) {
+    g_printerr("socket_check_protocol_support() failed\n");
+    goto end;
+    }
+
+    if (g_mkdtemp(dir) == NULL) {
+    g_error("g_mkdtemp: %s", g_strerror(errno));
+    }
+    tmpdir = dir;
+
+    if (has_ipv4) {
+    qtest_add_func("/netdev/stream/inet/ipv4", test_stream_inet_ipv4);
+    qtest_add_func("/netdev/dgram/inet", test_dgram_inet);
+    qtest_add_func("/netdev/dgram/mcast", test_dgram_mcast);
+    }
+    if (has_ipv6) {
+    qtest_add_func("/netdev/stream/inet/ipv6", test_stream_inet_ipv6);
+    }
+
+    socket_check_afunix_support(_afunix);
+    if (has_afunix) {
+    qtest_add_func("/netdev/dgram/unix", test_dgram_unix);
+    qtest_add_func("/netdev/stream/unix", test_stream_unix);
+    qtest_add_func("/netdev/stream/unix/abstract",
+   test_stream_unix_abstract);
+    qtest_add_func("/netdev/stream/fd", test_stream_fd);
+    qtest_add_func("/netdev/dgram/fd", test_dgram_fd);
+    }
+
+end:
+    ret = g_test_run();
+
+    g_rmdir(dir);


Maybe check the return code of g_rmdir(), to make sure that all temporary files in the 
directory have indeed been cleaned up successfully?


If it fails there is nothing we can do, I think it's better to ignore the 
result.




+    return ret;
+}


  Thomas



Thanks,
Laurent

[PATCH 3/6] hw/isa/piix: Wire up Xen PCI IRQ handling outside of PIIX3

[Bernhard Beschow]

xen_intx_set_irq() doesn't depend on PIIX state. In order to resolve
TYPE_PIIX3_XEN_DEVICE and in order to make Xen agnostic about the
precise south bridge being used, set up Xen's PCI IRQ handling of PIIX3
in the board.

Signed-off-by: Bernhard Beschow 
---
 hw/i386/pc_piix.c | 12 
 hw/isa/piix.c | 24 +---
 2 files changed, 13 insertions(+), 23 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index aacdb72b7c..c02f68010d 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -67,6 +67,7 @@
 #include "kvm/kvm-cpu.h"
 
 #define MAX_IDE_BUS 2
+#define XEN_PIIX_NUM_PIRQS 128ULL
 
 #ifdef CONFIG_IDE_ISA
 static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 };
@@ -246,6 +247,17 @@ static void pc_init1(MachineState *machine,
  _abort);
 pci_realize_and_unref(pci_dev, pci_bus, _fatal);
 
+if (xen_enabled()) {
+/*
+ * Xen supports additional interrupt routes from the PCI devices to
+ * the IOAPIC: the four pins of each PCI device on the bus are also
+ * connected to the IOAPIC directly.
+ * These additional routes can be discovered through ACPI.
+ */
+pci_bus_irqs(pci_bus, xen_intx_set_irq, pci_dev,
+ XEN_PIIX_NUM_PIRQS);
+}
+
 dev = DEVICE(object_resolve_path_component(OBJECT(pci_dev), "pic"));
 for (i = 0; i < ISA_NUM_IRQS; i++) {
 qdev_connect_gpio_out(dev, i, x86ms->gsi[i]);
diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index a1281c2d77..ac04781f46 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -38,8 +38,6 @@
 #include "migration/vmstate.h"
 #include "hw/acpi/acpi_aml_interface.h"
 
-#define XEN_PIIX_NUM_PIRQS  128ULL
-
 static void piix_set_irq_pic(PIIXState *piix, int pic_irq)
 {
 qemu_set_irq(piix->pic.in_irqs[pic_irq],
@@ -487,33 +485,13 @@ static const TypeInfo piix3_info = {
 .class_init= piix3_class_init,
 };
 
-static void piix3_xen_realize(PCIDevice *dev, Error **errp)
-{
-ERRP_GUARD();
-PIIXState *piix3 = PIIX_PCI_DEVICE(dev);
-PCIBus *pci_bus = pci_get_bus(dev);
-
-piix3_realize(dev, errp);
-if (*errp) {
-return;
-}
-
-/*
- * Xen supports additional interrupt routes from the PCI devices to
- * the IOAPIC: the four pins of each PCI device on the bus are also
- * connected to the IOAPIC directly.
- * These additional routes can be discovered through ACPI.
- */
-pci_bus_irqs(pci_bus, xen_intx_set_irq, piix3, XEN_PIIX_NUM_PIRQS);
-}
-
 static void piix3_xen_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
 PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
 k->config_write = piix3_write_config_xen;
-k->realize = piix3_xen_realize;
+k->realize = piix3_realize;
 /* 82371SB PIIX3 PCI-to-ISA bridge (Step A1) */
 k->device_id = PCI_DEVICE_ID_INTEL_82371SB_0;
 dc->vmsd = _piix3;
-- 
2.39.0

[PATCH 4/6] hw/isa/piix: Avoid Xen-specific variant of piix_write_config()

[Bernhard Beschow]

Subscribe to pci_bus_fire_intx_routing_notifier() instead which allows for
having a common piix_write_config() for all PIIX device models.

While at it, move the subscription into machine code in order to resolve
TYPE_PIIX3_XEN_DEVICE.

In a possible future followup, pci_bus_fire_intx_routing_notifier() could
be adjusted in such a way that subscribing to it doesn't require
knowledge of the device firing it.

Signed-off-by: Bernhard Beschow 
---
 hw/i386/pc_piix.c | 18 ++
 hw/isa/piix.c | 22 +-
 2 files changed, 19 insertions(+), 21 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index c02f68010d..7ef0054b3a 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -86,6 +86,21 @@ static int pci_slot_get_pirq(PCIDevice *pci_dev, int 
pci_intx)
 return (pci_intx + slot_addend) & 3;
 }
 
+static void piix_intx_routing_notifier_xen(PCIDevice *dev)
+{
+int i;
+
+/* Scan for updates to PCI link routes (0x60-0x63). */
+for (i = 0; i < PIIX_NUM_PIRQS; i++) {
+uint8_t v = dev->config_read(dev, PIIX_PIRQCA + i, 1);
+if (v & 0x80) {
+v = 0;
+}
+v &= 0xf;
+xen_set_pci_link_route(i, v);
+}
+}
+
 /* PC hardware initialisation */
 static void pc_init1(MachineState *machine,
  const char *host_type, const char *pci_type)
@@ -248,6 +263,9 @@ static void pc_init1(MachineState *machine,
 pci_realize_and_unref(pci_dev, pci_bus, _fatal);
 
 if (xen_enabled()) {
+pci_device_set_intx_routing_notifier(
+pci_dev, piix_intx_routing_notifier_xen);
+
 /*
  * Xen supports additional interrupt routes from the PCI devices to
  * the IOAPIC: the four pins of each PCI device on the bus are also
diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index ac04781f46..d4cdb3dadb 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -125,26 +125,6 @@ static void piix_write_config(PCIDevice *dev, uint32_t 
address, uint32_t val,
 }
 }
 
-static void piix3_write_config_xen(PCIDevice *dev,
-   uint32_t address, uint32_t val, int len)
-{
-int i;
-
-/* Scan for updates to PCI link routes (0x60-0x63). */
-for (i = 0; i < len; i++) {
-uint8_t v = (val >> (8 * i)) & 0xff;
-if (v & 0x80) {
-v = 0;
-}
-v &= 0xf;
-if (((address + i) >= PIIX_PIRQCA) && ((address + i) <= PIIX_PIRQCD)) {
-xen_set_pci_link_route(address + i - PIIX_PIRQCA, v);
-}
-}
-
-piix_write_config(dev, address, val, len);
-}
-
 static void piix_reset(DeviceState *dev)
 {
 PIIXState *d = PIIX_PCI_DEVICE(dev);
@@ -490,7 +470,7 @@ static void piix3_xen_class_init(ObjectClass *klass, void 
*data)
 DeviceClass *dc = DEVICE_CLASS(klass);
 PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-k->config_write = piix3_write_config_xen;
+k->config_write = piix_write_config;
 k->realize = piix3_realize;
 /* 82371SB PIIX3 PCI-to-ISA bridge (Step A1) */
 k->device_id = PCI_DEVICE_ID_INTEL_82371SB_0;
-- 
2.39.0

[PATCH 1/6] include/hw/xen/xen: Make xen_piix3_set_irq() generic and rename it

[Bernhard Beschow]

xen_piix3_set_irq() hardcoded the number of PCI IRQ lines. Get it from
the PCI bus instead.

Signed-off-by: Bernhard Beschow 
---
 hw/i386/xen/xen-hvm.c | 9 ++---
 hw/isa/piix.c | 2 +-
 include/hw/xen/xen.h  | 2 +-
 stubs/xen-hw-stub.c   | 2 +-
 4 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index e4293d6d66..59e8246a48 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -142,10 +142,13 @@ int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
 return irq_num + (PCI_SLOT(pci_dev->devfn) << 2);
 }
 
-void xen_piix3_set_irq(void *opaque, int irq_num, int level)
+void xen_intx_set_irq(void *opaque, int irq_num, int level)
 {
-xen_set_pci_intx_level(xen_domid, 0, 0, irq_num >> 2,
-   irq_num & 3, level);
+PCIDevice *pci_dev = opaque;
+PCIBus *pci_bus = pci_get_bus(pci_dev);
+
+xen_set_pci_intx_level(xen_domid, 0, 0, irq_num / pci_bus->nirq,
+   irq_num % pci_bus->nirq, level);
 }
 
 int xen_set_pci_link_route(uint8_t link, uint8_t irq)
diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index ae8a27c53c..dc6014a4e4 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -504,7 +504,7 @@ static void piix3_xen_realize(PCIDevice *dev, Error **errp)
  * connected to the IOAPIC directly.
  * These additional routes can be discovered through ACPI.
  */
-pci_bus_irqs(pci_bus, xen_piix3_set_irq, piix3, XEN_PIIX_NUM_PIRQS);
+pci_bus_irqs(pci_bus, xen_intx_set_irq, piix3, XEN_PIIX_NUM_PIRQS);
 }
 
 static void piix3_xen_class_init(ObjectClass *klass, void *data)
diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
index afdf9c436a..7c83ecf6b9 100644
--- a/include/hw/xen/xen.h
+++ b/include/hw/xen/xen.h
@@ -22,7 +22,7 @@ extern bool xen_domid_restrict;
 
 int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num);
 int xen_set_pci_link_route(uint8_t link, uint8_t irq);
-void xen_piix3_set_irq(void *opaque, int irq_num, int level);
+void xen_intx_set_irq(void *opaque, int irq_num, int level);
 void xen_hvm_inject_msi(uint64_t addr, uint32_t data);
 int xen_is_pirq_msi(uint32_t msi_data);
 
diff --git a/stubs/xen-hw-stub.c b/stubs/xen-hw-stub.c
index 34a22f2ad7..7d7ffe83a9 100644
--- a/stubs/xen-hw-stub.c
+++ b/stubs/xen-hw-stub.c
@@ -15,7 +15,7 @@ int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
 return -1;
 }
 
-void xen_piix3_set_irq(void *opaque, int irq_num, int level)
+void xen_intx_set_irq(void *opaque, int irq_num, int level)
 {
 }
 
-- 
2.39.0

[PATCH 6/6] hw/isa/piix: Resolve redundant TYPE_PIIX3_XEN_DEVICE

[Bernhard Beschow]

During the last patches, TYPE_PIIX3_XEN_DEVICE turned into a clone of
TYPE_PIIX3_DEVICE. Remove this redundancy.

Signed-off-by: Bernhard Beschow 
---
 hw/i386/pc_piix.c |  4 +---
 hw/isa/piix.c | 20 
 include/hw/southbridge/piix.h |  1 -
 3 files changed, 1 insertion(+), 24 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 7ef0054b3a..76d98183ac 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -235,8 +235,6 @@ static void pc_init1(MachineState *machine,
 if (pcmc->pci_enabled) {
 DeviceState *dev;
 PCIDevice *pci_dev;
-const char *type = xen_enabled() ? TYPE_PIIX3_XEN_DEVICE
- : TYPE_PIIX3_DEVICE;
 int i;
 
 pci_bus = i440fx_init(pci_type,
@@ -250,7 +248,7 @@ static void pc_init1(MachineState *machine,
: pci_slot_get_pirq);
 pcms->bus = pci_bus;
 
-pci_dev = pci_new_multifunction(-1, true, type);
+pci_dev = pci_new_multifunction(-1, true, TYPE_PIIX3_DEVICE);
 object_property_set_bool(OBJECT(pci_dev), "has-usb",
  machine_usb(machine), _abort);
 object_property_set_bool(OBJECT(pci_dev), "has-acpi",
diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index 98e9b12661..e4587352c9 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -33,7 +33,6 @@
 #include "hw/qdev-properties.h"
 #include "hw/ide/piix.h"
 #include "hw/isa/isa.h"
-#include "hw/xen/xen.h"
 #include "sysemu/runstate.h"
 #include "migration/vmstate.h"
 #include "hw/acpi/acpi_aml_interface.h"
@@ -465,24 +464,6 @@ static const TypeInfo piix3_info = {
 .class_init= piix3_class_init,
 };
 
-static void piix3_xen_class_init(ObjectClass *klass, void *data)
-{
-DeviceClass *dc = DEVICE_CLASS(klass);
-PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
-
-k->realize = piix3_realize;
-/* 82371SB PIIX3 PCI-to-ISA bridge (Step A1) */
-k->device_id = PCI_DEVICE_ID_INTEL_82371SB_0;
-dc->vmsd = _piix3;
-}
-
-static const TypeInfo piix3_xen_info = {
-.name  = TYPE_PIIX3_XEN_DEVICE,
-.parent= TYPE_PIIX_PCI_DEVICE,
-.instance_init = piix3_init,
-.class_init= piix3_xen_class_init,
-};
-
 static void piix4_realize(PCIDevice *dev, Error **errp)
 {
 ERRP_GUARD();
@@ -534,7 +515,6 @@ static void piix3_register_types(void)
 {
 type_register_static(_pci_type_info);
 type_register_static(_info);
-type_register_static(_xen_info);
 type_register_static(_info);
 }
 
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 65ad8569da..b1fc94a742 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -77,7 +77,6 @@ struct PIIXState {
 OBJECT_DECLARE_SIMPLE_TYPE(PIIXState, PIIX_PCI_DEVICE)
 
 #define TYPE_PIIX3_DEVICE "PIIX3"
-#define TYPE_PIIX3_XEN_DEVICE "PIIX3-xen"
 #define TYPE_PIIX4_PCI_DEVICE "piix4-isa"
 
 #endif
-- 
2.39.0

[PATCH 2/6] hw/isa/piix: Reuse piix3_realize() in piix3_xen_realize()

[Bernhard Beschow]

This is a preparational patch for the next one to make the following
more obvious:

First, pci_bus_irqs() is now called twice in case of Xen where the
second call overrides the pci_set_irq_fn with the Xen variant.

Second, pci_bus_set_route_irq_fn() is now also called in Xen mode.

Signed-off-by: Bernhard Beschow 
---
 hw/isa/piix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index dc6014a4e4..a1281c2d77 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -493,7 +493,7 @@ static void piix3_xen_realize(PCIDevice *dev, Error **errp)
 PIIXState *piix3 = PIIX_PCI_DEVICE(dev);
 PCIBus *pci_bus = pci_get_bus(dev);
 
-pci_piix_realize(dev, TYPE_PIIX3_USB_UHCI, errp);
+piix3_realize(dev, errp);
 if (*errp) {
 return;
 }
-- 
2.39.0

[PATCH 5/6] hw/isa/piix: Resolve redundant k->config_write assignments

[Bernhard Beschow]

The previous patch unified handling of piix_write_config() accross all
PIIX device models which allows for assigning k->config_write once in the
base class.

Signed-off-by: Bernhard Beschow 
---
 hw/isa/piix.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index d4cdb3dadb..98e9b12661 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -396,6 +396,7 @@ static void pci_piix_class_init(ObjectClass *klass, void 
*data)
 PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 AcpiDevAmlIfClass *adevc = ACPI_DEV_AML_IF_CLASS(klass);
 
+k->config_write = piix_write_config;
 dc->reset   = piix_reset;
 dc->desc= "ISA bridge";
 dc->hotpluggable   = false;
@@ -451,7 +452,6 @@ static void piix3_class_init(ObjectClass *klass, void *data)
 DeviceClass *dc = DEVICE_CLASS(klass);
 PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-k->config_write = piix_write_config;
 k->realize = piix3_realize;
 /* 82371SB PIIX3 PCI-to-ISA bridge (Step A1) */
 k->device_id = PCI_DEVICE_ID_INTEL_82371SB_0;
@@ -470,7 +470,6 @@ static void piix3_xen_class_init(ObjectClass *klass, void 
*data)
 DeviceClass *dc = DEVICE_CLASS(klass);
 PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-k->config_write = piix_write_config;
 k->realize = piix3_realize;
 /* 82371SB PIIX3 PCI-to-ISA bridge (Step A1) */
 k->device_id = PCI_DEVICE_ID_INTEL_82371SB_0;
@@ -519,7 +518,6 @@ static void piix4_class_init(ObjectClass *klass, void *data)
 DeviceClass *dc = DEVICE_CLASS(klass);
 PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-k->config_write = piix_write_config;
 k->realize = piix4_realize;
 k->device_id = PCI_DEVICE_ID_INTEL_82371AB_0;
 dc->vmsd = _piix4;
-- 
2.39.0

[PATCH 0/6] Resolve TYPE_PIIX3_XEN_DEVICE

[Bernhard Beschow]

This series first renders TYPE_PIIX3_XEN_DEVICE redundant and finally removes
it. The motivation is to 1/ decouple PIIX from Xen and 2/ to make Xen in the PC
machine agnostic to the precise southbridge being used. 2/ will become
particularily interesting once PIIX4 becomes usable in the PC machine, avoiding
the "Frankenstein" use of PIIX4_ACPI in PIIX3.

Testing done:
None, because I don't know how to conduct this properly :(

Based-on: <20221221170003.2929-1-shen...@gmail.com>
  "[PATCH v4 00/30] Consolidate PIIX south bridges"

Bernhard Beschow (6):
  include/hw/xen/xen: Make xen_piix3_set_irq() generic and rename it
  hw/isa/piix: Reuse piix3_realize() in piix3_xen_realize()
  hw/isa/piix: Wire up Xen PCI IRQ handling outside of PIIX3
  hw/isa/piix: Avoid Xen-specific variant of piix_write_config()
  hw/isa/piix: Resolve redundant k->config_write assignments
  hw/isa/piix: Resolve redundant TYPE_PIIX3_XEN_DEVICE

 hw/i386/pc_piix.c | 34 --
 hw/i386/xen/xen-hvm.c |  9 +++--
 hw/isa/piix.c | 66 +--
 include/hw/southbridge/piix.h |  1 -
 include/hw/xen/xen.h  |  2 +-
 stubs/xen-hw-stub.c   |  2 +-
 6 files changed, 40 insertions(+), 74 deletions(-)

-- 
2.39.0

Re: [PATCH v3 04/26] configure: don't enable cross compilers unless in target_list

[Alex Bennée]

Stefan Weil  writes:

> Am 21.10.22 um 00:10 schrieb Richard Henderson:
>> On 10/20/22 21:51, Alex Bennée wrote:
>>> This avoids the unfortunate effect of always builds the pc-bios blobs
>>> for targets the user isn't interested in.
>>>
>>> Suggested-by: Paolo Bonzini 
>>> Signed-off-by: Alex Bennée 
>>> ---
>>>   configure | 9 +
>>>   1 file changed, 9 insertions(+)
>>>
>>> diff --git a/configure b/configure
>>> index 81561be7c1..dd6f58dcde 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -1877,6 +1877,15 @@ probe_target_compiler() {
>>>     container_cross_ranlib=
>>>     container_cross_strip=
>>> +  # We shall skip configuring the target compiler if the user didn't
>>> +  # bother enabling an appropriate guest. This avoids building
>>> +  # extraneous firmware images and tests.
>>> +  if test "${target_list#*$1}" != "$1"; then
>>> +  break;
>
>
> Isn't break limited for exiting from for, while, or until loop? (*)
> If yes, it's wrongly used here. sh does not complain, but other
> shells do.

There is already a fix waiting in the PR:

  20221223172135.3450109-1-alex.ben...@linaro.org
  
>
> Stefan
>
> *) https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v3 4/7] hw/misc: AXP209 PMU Emulation

[Strahinja Jankovic]

Ping

This is the only remaining patch from the series waiting for review. Thanks!

Best regards,
Strahinja

On Mon, Dec 26, 2022 at 11:03 PM Strahinja Jankovic
 wrote:
>
> This patch adds minimal support for AXP-209 PMU.
> Most important is chip ID since U-Boot SPL expects version 0x1. Besides
> the chip ID register, reset values for two more registers used by A10
> U-Boot SPL are covered.
>
> Signed-off-by: Strahinja Jankovic 
> ---
>  MAINTAINERS  |   2 +
>  hw/misc/Kconfig  |   4 +
>  hw/misc/axp209.c | 238 +++
>  hw/misc/meson.build  |   1 +
>  hw/misc/trace-events |   5 +
>  5 files changed, 250 insertions(+)
>  create mode 100644 hw/misc/axp209.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b270eb8e5b..354da68249 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -576,12 +576,14 @@ ARM Machines
>  Allwinner-a10
>  M: Beniamino Galvani 
>  M: Peter Maydell 
> +R: Strahinja Jankovic 
>  L: qemu-...@nongnu.org
>  S: Odd Fixes
>  F: hw/*/allwinner*
>  F: include/hw/*/allwinner*
>  F: hw/arm/cubieboard.c
>  F: docs/system/arm/cubieboard.rst
> +F: hw/misc/axp209.c
>
>  Allwinner-h3
>  M: Niek Linnenbank 
> diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
> index 052fb54310..eaeddca277 100644
> --- a/hw/misc/Kconfig
> +++ b/hw/misc/Kconfig
> @@ -180,4 +180,8 @@ config ALLWINNER_A10_CCM
>  config ALLWINNER_A10_DRAMC
>  bool
>
> +config AXP209_PMU
> +bool
> +depends on I2C
> +
>  source macio/Kconfig
> diff --git a/hw/misc/axp209.c b/hw/misc/axp209.c
> new file mode 100644
> index 00..2908ed99a6
> --- /dev/null
> +++ b/hw/misc/axp209.c
> @@ -0,0 +1,238 @@
> +/*
> + * AXP-209 PMU Emulation
> + *
> + * Copyright (C) 2022 Strahinja Jankovic 
> + *
> + * 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.
> + *
> + * SPDX-License-Identifier: MIT
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/log.h"
> +#include "trace.h"
> +#include "hw/i2c/i2c.h"
> +#include "migration/vmstate.h"
> +
> +#define TYPE_AXP209_PMU "axp209_pmu"
> +
> +#define AXP209(obj) \
> +OBJECT_CHECK(AXP209I2CState, (obj), TYPE_AXP209_PMU)
> +
> +/* registers */
> +enum {
> +REG_POWER_STATUS = 0x0u,
> +REG_OPERATING_MODE,
> +REG_OTG_VBUS_STATUS,
> +REG_CHIP_VERSION,
> +REG_DATA_CACHE_0,
> +REG_DATA_CACHE_1,
> +REG_DATA_CACHE_2,
> +REG_DATA_CACHE_3,
> +REG_DATA_CACHE_4,
> +REG_DATA_CACHE_5,
> +REG_DATA_CACHE_6,
> +REG_DATA_CACHE_7,
> +REG_DATA_CACHE_8,
> +REG_DATA_CACHE_9,
> +REG_DATA_CACHE_A,
> +REG_DATA_CACHE_B,
> +REG_POWER_OUTPUT_CTRL = 0x12u,
> +REG_DC_DC2_OUT_V_CTRL = 0x23u,
> +REG_DC_DC2_DVS_CTRL = 0x25u,
> +REG_DC_DC3_OUT_V_CTRL = 0x27u,
> +REG_LDO2_4_OUT_V_CTRL,
> +REG_LDO3_OUT_V_CTRL,
> +REG_VBUS_CH_MGMT = 0x30u,
> +REG_SHUTDOWN_V_CTRL,
> +REG_SHUTDOWN_CTRL,
> +REG_CHARGE_CTRL_1,
> +REG_CHARGE_CTRL_2,
> +REG_SPARE_CHARGE_CTRL,
> +REG_PEK_KEY_CTRL,
> +REG_DC_DC_FREQ_SET,
> +REG_CHR_TEMP_TH_SET,
> +REG_CHR_HIGH_TEMP_TH_CTRL,
> +REG_IPSOUT_WARN_L1,
> +REG_IPSOUT_WARN_L2,
> +REG_DISCHR_TEMP_TH_SET,
> +REG_DISCHR_HIGH_TEMP_TH_CTRL,
> +REG_IRQ_BANK_1_CTRL = 0x40u,
> +REG_IRQ_BANK_2_CTRL,
> +REG_IRQ_BANK_3_CTRL,
> +REG_IRQ_BANK_4_CTRL,
> +REG_IRQ_BANK_5_CTRL,
> +REG_IRQ_BANK_1_STAT = 0x48u,
> +REG_IRQ_BANK_2_STAT,
> +REG_IRQ_BANK_3_STAT,
> +REG_IRQ_BANK_4_STAT,
> +REG_IRQ_BANK_5_STAT,
> +REG_ADC_ACIN_V_H = 0x56u,
> +REG_ADC_ACIN_V_L,
> +REG_ADC_ACIN_CURR_H,
> +REG_ADC_ACIN_CURR_L,
> +REG_ADC_VBUS_V_H,
> +REG_ADC_VBUS_V_L,
> +REG_ADC_VBUS_CURR_H,
> +REG_ADC_VBUS_CURR_L,
> +REG_ADC_INT_TEMP_H,
> +REG_ADC_INT_TEMP_L,
> +REG_ADC_TEMP_SENS_V_H = 0x62u,
> +REG_ADC_TEMP_SENS_V_L,
> +REG_ADC_BAT_V_H = 0x78u,
> +REG_ADC_BAT_V_L,
> +REG_ADC_BAT_DISCHR_CURR_H,
> +

Re: [PATCH] m25p80: Add the is25wp256 SFPD table

[Michael Walle]

Am 2023-01-02 19:49, schrieb Guenter Roeck:

On Mon, Jan 02, 2023 at 06:42:03PM +0100, Michael Walle wrote:

Am 2023-01-02 17:23, schrieb Guenter Roeck:
> On Mon, Jan 02, 2023 at 04:43:49PM +0100, Michael Walle wrote:
> > Am 2023-01-02 14:53, schrieb Cédric Le Goater:
> > > On 12/27/22 07:31, Tudor Ambarus wrote:
> > > >
> > > >
> > > > On 25.12.2022 14:18, Ben Dooks wrote:
> > > > > On Wed, Dec 21, 2022 at 06:36:02PM +0100, Cédric Le Goater wrote:
> > > > > > On 12/21/22 13:22, Guenter Roeck wrote:
> > > > > > > Generated from hardware using the following command and
> > > > > > > then padding
> > > > > > > with 0xff to fill out a power-of-2:
> > > > > > > xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
> > > > > > >
> > > > > > > Cc: Michael Walle 
> > > > > > > Cc: Tudor Ambarus 
> > > > > > > Signed-off-by: Guenter Roeck 
> > > > > >
> > > > > > Reviewed-by: Cédric Le Goater 
> > > > >
> > > > > If SFDP is a standard, couldn't we have an function to generate
> > > > > it from
> > > > > the flash parameters?
> > > > >
> > > >
> > > > No, it's not practical as you have to specify all the flash parameters
> > > > at flash declaration.
> > >
> > > Indeed and the definition of flash models in QEMU is far to cover all
> > > the SFDP
> > > features. The known_devices array of m25p80 would be huge ! However, we
> > > could
> > > generate some of the SFDP tables if no raw data is provided. It could be
> > > useful
> > > for testing drivers.
> >
> > I don't think adding (incomplete) SFDP tables makes sense for any real
> > devices. E.g. sometimes our linux driver will look at specific bits in
> > SFDP to figure out what particular flash device is attached. For
> > example
> > when there are different flashes with the same jedec id.
> >
> > But since the last released kernel, we support a generic SFDP
> > driver, which
> > is used when there is no matching driver for the flash's jedec id.
> > Theoretically, you can build your own flash device (with a unique
> > id) and
> > generate the sfdp tables for that one.
> >
> How about older kernels versions ? Would those still support such
> (virtual ?)
> flash devices ?

No with older kernels you'd be out of luck. They will just print 
"unknown

flash
id" and skip the device.


That would mean that qemu versions including this change could no 
longer

be used to test flash support on older kernel versions. That would be
extremely undesirable. I'd rather live with the current code and still 
be

able to test older kernels.


Your board wouldn't need to use it. I don't think that emulating a real
spi flash device would go away. But it might still make sense to have
such a flash device, one example already mentioned is for testing
drivers.

-michael

[PATCH v5 0/2] hw/nvme: Support for Namespaces Management from guest OS

[Jonathan Derrick]

From: Michael Kropaczek 

Description:

Currently namespaces could be configured as follows:
1. Legacy Namespace - just one namespace within Nvme controller's
   where the back-end was specified for nvme device by -drive parameter
   pointing directly to the image file.
2. Additional Namespaces - specified by nvme-ns devices each having its
   own back-end. To have multiple namespaces each needed to be specified
   at Qemu's command line being associated with the most recently defined
   nvme-bus from nvme device.
   If a such additional namespace should be attached and/or detached by the
   guest OS, nvme controller has to be linked with another device nvme-subsys.

All that have a static nature, all need to be specified at the Qemu's 
command line, all specified virtual nvme entities will be processed during
Qemu's start-up then created and provided to the guest OS.

To have a support for nvme create-ns and delete-ns commands with specified
parameters a different approach is needed.
Virtual devices representing namespaces need to be created and/or deleted 
during Qemu's running session, at anytime. The back-end image sizes for a
namespace must accommodate the payload size and size of metadata resulted
from specified parameters. The total capacity of the nvme controller
altogether with un-allocated capacity needs to be taken into account and
updated according to nvme create-ns and delete-ns commands respectively.

Here is the approach:
The nvme device will get new parameter:
 - auto-ns-path, which specifies the path to the storage area where back-end
   image and necessary config files located stored.

The virtual devices representing namespaces will be created dynamically during
the Qemu running session following issuance of nvme create-ns and delete-ns
commands from the guest OS. QOM classes and instances will be created utilizing
existing configuration scheme used during Qemu's start-up. Back-end image files
will be neither created nor deleted during Qemu's startup or running session.
Instead a set of back-end image files and relevant config will be created by
qemu-img tool with createns sub-command prior to Qemu's session.
Required parameters are: -S serial number which must match serial parameter of
qemu-system-xx -device nvme command line specification, -C total capacity, and
optional -N that will set a maximal limit on number of allowed
namespaces (default 256) which will be followed by path name pointing to
storage location corresponding to auto-ns-path of qemu-system-xx -device nvme
parameter.

Those created back-end image files will be pre-loaded during Qemu's start-up
and then during running Qemu's session will be associated or disassociated with
QOM namespaces virtual instances, as a result of issuing nvme create-ns or
delete-ns commands. The associated back-end image file for relevant namespace
will be re-sized as follows: delete-ns command will truncate image file to the
size of 0, whereas create-ns command will re-size the image file to the size
provided by nvme create-ns command parameters. Truncating/re-sizing is a result
of blk_truncate() API which utilizes co-routines and should not block Qemu main
thread while scheduling AIO operations. It is assured that all settings will
retain over Qemu's start-ups and shutdowns. The implementation makes it
possible to combine the existing "Additional Namespace" implementation with the
new "Managed Namespaces". Those will coexist with obvious restrictions, like
both will share the same NsIds space, "static" namespaces cannot be deleted or
if its NsId specified at Qemu's command line will conflicts with previously
created one by nvme create-ns (and retained), this will lead to an abort of
Qemu at its start up.

More than one of NVMe controllers associated with NVMe subsystem are supported.
This feature requires that parameters serial= and subsys= of additional
controllers must match those of the primary controller and auto-ns-path=
must not be specified.

Michael Kropaczek (2):
  hw/nvme: Support for Namespaces Management from guest OS - create-ns
  hw/nvme: Support for Namespaces Management from guest OS - delete-ns

 docs/system/devices/nvme.rst |  60 +-
 hw/nvme/cfg_key_checker.c|  51 +
 hw/nvme/ctrl-cfg.c   | 219 +
 hw/nvme/ctrl.c   | 311 -
 hw/nvme/meson.build  |   2 +-
 hw/nvme/ns-backend.c | 288 +++
 hw/nvme/ns.c | 366 +++
 hw/nvme/nvme.h   |  32 ++-
 hw/nvme/subsys.c |  11 +-
 hw/nvme/trace-events |   3 +
 include/block/nvme.h |  31 +++
 include/hw/nvme/ctrl-cfg.h   |  24 +++
 include/hw/nvme/ns-cfg.h |  28 +++
 include/hw/nvme/nvme-cfg.h   | 168 
 qemu-img-cmds.hx |   6 +
 qemu-img.c   | 132 +
 16 files changed, 1678 insertions(+), 54 deletions(-)
 create mode 100644 

[PATCH v5 1/2] hw/nvme: Support for Namespaces Management from guest OS - create-ns

[Jonathan Derrick]

From: Michael Kropaczek 

Added support for NVMEe NameSpaces Mangement allowing the guest OS to
create namespaces by issuing nvme create-ns command.
It is an extension to currently implemented Qemu nvme virtual device.
Virtual devices representing namespaces will be created and/or deleted
during Qemu's running session, at anytime.

Signed-off-by: Michael Kropaczek 
---
 docs/system/devices/nvme.rst |  59 ++-
 hw/nvme/cfg_key_checker.c|  51 ++
 hw/nvme/ctrl-cfg.c   | 219 ++
 hw/nvme/ctrl.c   | 245 -
 hw/nvme/meson.build  |   2 +-
 hw/nvme/ns-backend.c | 283 +
 hw/nvme/ns.c | 295 ++-
 hw/nvme/nvme.h   |  30 +++-
 hw/nvme/subsys.c |  11 +-
 hw/nvme/trace-events |   2 +
 include/block/nvme.h |  30 
 include/hw/nvme/ctrl-cfg.h   |  24 +++
 include/hw/nvme/ns-cfg.h |  28 
 include/hw/nvme/nvme-cfg.h   | 168 
 qemu-img-cmds.hx |   6 +
 qemu-img.c   | 132 
 16 files changed, 1531 insertions(+), 54 deletions(-)
 create mode 100644 hw/nvme/cfg_key_checker.c
 create mode 100644 hw/nvme/ctrl-cfg.c
 create mode 100644 hw/nvme/ns-backend.c
 create mode 100644 include/hw/nvme/ctrl-cfg.h
 create mode 100644 include/hw/nvme/ns-cfg.h
 create mode 100644 include/hw/nvme/nvme-cfg.h

diff --git a/docs/system/devices/nvme.rst b/docs/system/devices/nvme.rst
index 30f841ef62..6b3bee5e5d 100644
--- a/docs/system/devices/nvme.rst
+++ b/docs/system/devices/nvme.rst
@@ -92,6 +92,63 @@ There are a number of parameters available:
   attach the namespace to a specific ``nvme`` device (identified by an ``id``
   parameter on the controller device).
 
+Additional Namespaces managed by guest OS Namespaces Management
+-
+
+.. code-block:: console
+
+   -device nvme,id=nvme-ctrl,serial=1234,subsys=nvme-subsys,auto-ns-path=path
+
+Parameters:
+
+``auto-ns-path=``
+  If specified indicates a support for dynamic management of nvme namespaces
+  by means of nvme create-ns command. This path points
+  to the storage area for backend images must exist. Additionally it requires
+  that parameter `ns-subsys` must be specified whereas parameter `drive`
+  must not. The legacy namespace backend is disabled, instead, a pair of
+  files 'nvme__ns_.cfg' and 'nvme__ns_.img'
+  will refer to respective namespaces. The create-ns, attach-ns
+  and detach-ns commands, issued at the guest side, will make changes to
+  those files accordingly.
+  For each namespace exists an image file in raw format and a config file
+  containing namespace parameters and state of the attachment allowing QEMU
+  to configure namespaces accordingly during start up. If for instance an
+  image file has a size of 0 bytes this will be interpreted as non existent
+  namespace. Issuing create-ns command will change the status in the config
+  files and and will re-size the image file accordingly so the image file
+  will be associated with the respective namespace. The main config file
+  nvme__ctrl.cfg keeps the track of allocated capacity to the
+  namespaces within the nvme controller.
+  As it is the case of a typical hard drive, backend images together with
+  config files need to be created. For this reason the qemu-img tool has
+  been extended by adding createns command.
+
+   qemu-img createns {-S  -C }
+ [-N ] {}
+
+  Parameters:
+  -S and -C and  are mandatory, `-S` must match `serial` parameter
+  and  must match `auto-ns-path` parameter of "-device nvme,..."
+  specification.
+  -N is optional, if specified it will set a limit to the number of potential
+  namespaces and will reduce the number of backend images and config files
+  accordingly. As a default, a set of images of 0 bytes size and default
+  config files for 256 namespaces will be created, a total of 513 files.
+
+Please note that ``nvme-ns`` device is not required to support of dynamic
+namespaces management feature. It is not prohibited to assign a such device to
+``nvme`` device specified to support dynamic namespace management if one has
+an use case to do so, however, it will only coexist and be out of the scope of
+Namespaces Management. NsIds will be consistently managed, creation (create-ns)
+of a namespace will not allocate the NsId already being taken. If ``nvme-ns``
+device conflicts with previously created one by create-ns (the same NsId),
+it will break QEMU's start up.
+More than one of NVMe controllers associated with NVMe subsystem are supported.
+This feature requires that parameters ``serial=`` and ``subsys=`` of additional
+controllers must match those of the primary controller and ``auto-ns-path=``
+must not be specified.
+
 NVM Subsystems
 --
 
@@ -320,4 +377,4 @@ controller are:
 
 .. code-block:: 

[PATCH v5 2/2] hw/nvme: Support for Namespaces Management from guest OS - delete-ns

[Jonathan Derrick]

From: Michael Kropaczek 

Added support for NVMEe NameSpaces Mangement allowing the guest OS to
delete namespaces by issuing nvme delete-ns command.
It is an extension to currently implemented Qemu nvme virtual device.
Virtual devices representing namespaces will be created and/or deleted
during Qemu's running session, at anytime.

Signed-off-by: Michael Kropaczek 
---
 docs/system/devices/nvme.rst |  9 ++--
 hw/nvme/ctrl.c   | 82 
 hw/nvme/ns-backend.c |  5 +++
 hw/nvme/ns.c | 71 +++
 hw/nvme/nvme.h   |  2 +
 hw/nvme/trace-events |  1 +
 include/block/nvme.h |  1 +
 7 files changed, 159 insertions(+), 12 deletions(-)

diff --git a/docs/system/devices/nvme.rst b/docs/system/devices/nvme.rst
index 6b3bee5e5d..f19072f1bc 100644
--- a/docs/system/devices/nvme.rst
+++ b/docs/system/devices/nvme.rst
@@ -103,12 +103,12 @@ Parameters:
 
 ``auto-ns-path=``
   If specified indicates a support for dynamic management of nvme namespaces
-  by means of nvme create-ns command. This path points
+  by means of nvme create-ns and nvme delete-ns commands. This path points
   to the storage area for backend images must exist. Additionally it requires
   that parameter `ns-subsys` must be specified whereas parameter `drive`
   must not. The legacy namespace backend is disabled, instead, a pair of
   files 'nvme__ns_.cfg' and 'nvme__ns_.img'
-  will refer to respective namespaces. The create-ns, attach-ns
+  will refer to respective namespaces. The create-ns, delete-ns, attach-ns
   and detach-ns commands, issued at the guest side, will make changes to
   those files accordingly.
   For each namespace exists an image file in raw format and a config file
@@ -140,8 +140,9 @@ Please note that ``nvme-ns`` device is not required to 
support of dynamic
 namespaces management feature. It is not prohibited to assign a such device to
 ``nvme`` device specified to support dynamic namespace management if one has
 an use case to do so, however, it will only coexist and be out of the scope of
-Namespaces Management. NsIds will be consistently managed, creation (create-ns)
-of a namespace will not allocate the NsId already being taken. If ``nvme-ns``
+Namespaces Management. Deletion (delete-ns) will render an error for this
+namespace. NsIds will be consistently managed, creation (create-ns) of
+a namespace will not allocate the NsId already being taken. If ``nvme-ns``
 device conflicts with previously created one by create-ns (the same NsId),
 it will break QEMU's start up.
 More than one of NVMe controllers associated with NVMe subsystem are supported.
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 5ed35d7cf4..e0fac3c151 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -144,12 +144,12 @@
  *
  * - `auto-ns-path`
  *   If specified indicates a support for dynamic management of nvme namespaces
- *   by means of nvme create-ns command. This path pointing
+ *   by means of nvme create-ns and nvme delete-ns commands. This path pointing
  *   to a storage area for backend images must exist. Additionally it requires
  *   that parameter `ns-subsys` must be specified whereas parameter `drive`
  *   must not. The legacy namespace backend is disabled, instead, a pair of
  *   files 'nvme__ns_.cfg' and 'nvme__ns_.img'
- *   will refer to respective namespaces. The create-ns, attach-ns
+ *   will refer to respective namespaces. The create-ns, delete-ns, attach-ns
  *   and detach-ns commands, issued at the guest side, will make changes to
  *   those files accordingly.
  *   For each namespace exists an image file in raw format and a config file
@@ -5702,17 +5702,13 @@ static uint16_t nvme_ns_mgmt_create(NvmeCtrl *n, 
NvmeRequest *req, uint32_t nsid
 }
 
 if (nvme_cfg_update(n, ns->size, NVME_NS_ALLOC_CHK)) {
-/* place for delete-ns */
-error_setg(_err, "Insufficient capacity, an orphaned 
ns[%"PRIu32"] will be left behind", nsid);
-error_report_err(local_err);
+nvme_ns_delete(n, nsid, NULL);
 return NVME_NS_INSUFFICIENT_CAPAC | NVME_DNR;
 }
 (void)nvme_cfg_update(n, ns->size, NVME_NS_ALLOC);
 if (nvme_cfg_save(n)) {
 (void)nvme_cfg_update(n, ns->size, NVME_NS_DEALLOC);
-/* place for delete-ns */
-error_setg(_err, "Cannot save conf file, an orphaned 
ns[%"PRIu32"] will be left behind", nsid);
-error_report_err(local_err);
+nvme_ns_delete(n, nsid, NULL);
 return NVME_INVALID_FIELD | NVME_DNR;
 }
 
@@ -5726,6 +5722,66 @@ fail:
 return NVME_SUCCESS;
 }
 
+static uint16_t nvme_ns_mgmt_delete(NvmeCtrl *n, uint32_t nsid)
+{
+NvmeNamespace *ns = NULL;
+uint16_t first = nsid;
+uint16_t last = nsid;
+uint16_t i;
+uint64_t image_size;
+Error *local_err = NULL;
+
+if (!nsid) {
+return NVME_INVALID_FIELD | NVME_DNR;
+}
+
+if (!n->params.ns_directory) {
+

Re: [PATCH] m25p80: Add the is25wp256 SFPD table

[Guenter Roeck]

On Mon, Jan 02, 2023 at 06:42:03PM +0100, Michael Walle wrote:
> Am 2023-01-02 17:23, schrieb Guenter Roeck:
> > On Mon, Jan 02, 2023 at 04:43:49PM +0100, Michael Walle wrote:
> > > Am 2023-01-02 14:53, schrieb Cédric Le Goater:
> > > > On 12/27/22 07:31, Tudor Ambarus wrote:
> > > > >
> > > > >
> > > > > On 25.12.2022 14:18, Ben Dooks wrote:
> > > > > > On Wed, Dec 21, 2022 at 06:36:02PM +0100, Cédric Le Goater wrote:
> > > > > > > On 12/21/22 13:22, Guenter Roeck wrote:
> > > > > > > > Generated from hardware using the following command and
> > > > > > > > then padding
> > > > > > > > with 0xff to fill out a power-of-2:
> > > > > > > > xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
> > > > > > > >
> > > > > > > > Cc: Michael Walle 
> > > > > > > > Cc: Tudor Ambarus 
> > > > > > > > Signed-off-by: Guenter Roeck 
> > > > > > >
> > > > > > > Reviewed-by: Cédric Le Goater 
> > > > > >
> > > > > > If SFDP is a standard, couldn't we have an function to generate
> > > > > > it from
> > > > > > the flash parameters?
> > > > > >
> > > > >
> > > > > No, it's not practical as you have to specify all the flash parameters
> > > > > at flash declaration.
> > > >
> > > > Indeed and the definition of flash models in QEMU is far to cover all
> > > > the SFDP
> > > > features. The known_devices array of m25p80 would be huge ! However, we
> > > > could
> > > > generate some of the SFDP tables if no raw data is provided. It could be
> > > > useful
> > > > for testing drivers.
> > > 
> > > I don't think adding (incomplete) SFDP tables makes sense for any real
> > > devices. E.g. sometimes our linux driver will look at specific bits in
> > > SFDP to figure out what particular flash device is attached. For
> > > example
> > > when there are different flashes with the same jedec id.
> > > 
> > > But since the last released kernel, we support a generic SFDP
> > > driver, which
> > > is used when there is no matching driver for the flash's jedec id.
> > > Theoretically, you can build your own flash device (with a unique
> > > id) and
> > > generate the sfdp tables for that one.
> > > 
> > How about older kernels versions ? Would those still support such
> > (virtual ?)
> > flash devices ?
> 
> No with older kernels you'd be out of luck. They will just print "unknown
> flash
> id" and skip the device.

That would mean that qemu versions including this change could no longer
be used to test flash support on older kernel versions. That would be
extremely undesirable. I'd rather live with the current code and still be
able to test older kernels.

Thanks,
Guenter

Re: [PATCH v4 08/30] hw/i386/pc: Create RTC controllers in south bridges

[Bernhard Beschow]

Am 2. Januar 2023 17:03:29 UTC schrieb Thomas Huth :
>On 21/12/2022 17.59, Bernhard Beschow wrote:
>> Just like in the real hardware (and in PIIX4), create the RTC
>> controllers in the south bridges.
>> 
>> Signed-off-by: Bernhard Beschow 
>> Reviewed-by: Michael S. Tsirkin 
>> Message-Id: <20221022150508.26830-11-shen...@gmail.com>
>> ---
>>   hw/i386/pc.c  | 12 +++-
>>   hw/i386/pc_piix.c |  8 
>>   hw/i386/pc_q35.c  |  1 +
>>   hw/isa/Kconfig|  2 ++
>>   hw/isa/lpc_ich9.c |  8 
>>   hw/isa/piix3.c| 15 +++
>>   include/hw/i386/ich9.h|  2 ++
>>   include/hw/southbridge/piix.h |  3 +++
>>   8 files changed, 50 insertions(+), 1 deletion(-)
>> 
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index fa69b6f43e..d154eedcb3 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -1299,7 +1299,17 @@ void pc_basic_device_init(struct PCMachineState *pcms,
>>   pit_alt_irq = qdev_get_gpio_in(hpet, HPET_LEGACY_PIT_INT);
>>   rtc_irq = qdev_get_gpio_in(hpet, HPET_LEGACY_RTC_INT);
>>   }
>> -*rtc_state = mc146818_rtc_init(isa_bus, 2000, rtc_irq);
>> +
>> +if (rtc_irq) {
>> +qdev_connect_gpio_out(DEVICE(*rtc_state), 0, rtc_irq);
>> +} else {
>> +uint32_t irq = object_property_get_uint(OBJECT(*rtc_state),
>> +"irq",
>> +_fatal);
>> +isa_connect_gpio_out(*rtc_state, 0, irq);
>> +}
>> +object_property_add_alias(OBJECT(pcms), "rtc-time", OBJECT(*rtc_state),
>> +  "date");
>I think you could turn now the "ISADevice **rtc_state" parameter of this 
>function into a normal "ISADevice *rtc_state" since the pointer is not a 
>return value anymore.

This is done in patch 9/30: 
https://lists.gnu.org/archive/html/qemu-devel/2022-12/msg03799.html

Best regards,
Bernhard

>
> Thomas
>

[PATCH] target/arm: Allow users to set the number of VFP registers

[Cédric Le Goater]

Cortex A7 CPUs with an FPU implementing VFPv4 without NEON support
have 16 64-bit FPU registers and not 32 registers. Let users set the
number of VFP registers with a CPU property.

The primary use case of this property is for the Cortex A7 of the
Aspeed AST2600 SoC.

Signed-off-by: Cédric Le Goater 
---
 target/arm/cpu.h|  2 ++
 hw/arm/aspeed_ast2600.c |  2 ++
 target/arm/cpu.c| 14 ++
 3 files changed, 18 insertions(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 2b4bd20f9d..55e2e9a584 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -897,6 +897,8 @@ struct ArchCPU {
 bool has_pmu;
 /* CPU has VFP */
 bool has_vfp;
+/* CPU has 32 VFP registers */
+bool has_vfp_d32;
 /* CPU has Neon */
 bool has_neon;
 /* CPU has M-profile DSP extension */
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index cd75465c2b..37f43b4165 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -309,6 +309,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, 
Error **errp)
 _abort);
 object_property_set_bool(OBJECT(>cpu[i]), "neon", false,
 _abort);
+object_property_set_bool(OBJECT(>cpu[i]), "vfp-d32", false,
+_abort);
 object_property_set_link(OBJECT(>cpu[i]), "memory",
  OBJECT(s->memory), _abort);
 
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 2fa022f62b..27af57ea9a 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1258,6 +1258,9 @@ static Property arm_cpu_cfgend_property =
 static Property arm_cpu_has_vfp_property =
 DEFINE_PROP_BOOL("vfp", ARMCPU, has_vfp, true);
 
+static Property arm_cpu_has_vfp_d32_property =
+DEFINE_PROP_BOOL("vfp-d32", ARMCPU, has_vfp_d32, true);
+
 static Property arm_cpu_has_neon_property =
 DEFINE_PROP_BOOL("neon", ARMCPU, has_neon, true);
 
@@ -1384,8 +1387,11 @@ void arm_cpu_post_init(Object *obj)
 ? cpu_isar_feature(aa64_fp_simd, cpu)
 : cpu_isar_feature(aa32_vfp, cpu)) {
 cpu->has_vfp = true;
+cpu->has_vfp_d32 = true;
 if (!kvm_enabled()) {
 qdev_property_add_static(DEVICE(obj), _cpu_has_vfp_property);
+qdev_property_add_static(DEVICE(obj),
+ _cpu_has_vfp_d32_property);
 }
 }
 
@@ -1650,6 +1656,14 @@ static void arm_cpu_realizefn(DeviceState *dev, Error 
**errp)
 return;
 }
 
+if (!cpu->has_vfp_d32) {
+uint32_t u;
+
+u = cpu->isar.mvfr0;
+u = FIELD_DP32(u, MVFR0, SIMDREG, 1); /* 16 registers */
+cpu->isar.mvfr0 = u;
+}
+
 if (!cpu->has_vfp) {
 uint64_t t;
 uint32_t u;
-- 
2.38.1

Re: [PATCH v6] xen/pt: reserve PCI slot 2 for Intel igd-passthru

[Michael S. Tsirkin]

On Sun, Jan 01, 2023 at 06:52:03PM -0500, Chuck Zmudzinski wrote:
> Intel specifies that the Intel IGD must occupy slot 2 on the PCI bus,
> as noted in docs/igd-assign.txt in the Qemu source code.
> 
> Currently, when the xl toolstack is used to configure a Xen HVM guest with
> Intel IGD passthrough to the guest with the Qemu upstream device model,
> a Qemu emulated PCI device will occupy slot 2 and the Intel IGD will occupy
> a different slot. This problem often prevents the guest from booting.
> 
> The only available workaround is not good: Configure Xen HVM guests to use
> the old and no longer maintained Qemu traditional device model available
> from xenbits.xen.org which does reserve slot 2 for the Intel IGD.
> 
> To implement this feature in the Qemu upstream device model for Xen HVM
> guests, introduce the following new functions, types, and macros:
> 
> * XEN_PT_DEVICE_CLASS declaration, based on the existing TYPE_XEN_PT_DEVICE
> * XEN_PT_DEVICE_GET_CLASS macro helper function for XEN_PT_DEVICE_CLASS
> * typedef XenPTQdevRealize function pointer
> * XEN_PCI_IGD_SLOT_MASK, the value of slot_reserved_mask to reserve slot 2
> * xen_igd_reserve_slot and xen_igd_clear_slot functions
> 
> The new xen_igd_reserve_slot function uses the existing slot_reserved_mask
> member of PCIBus to reserve PCI slot 2 for Xen HVM guests configured using
> the xl toolstack with the gfx_passthru option enabled, which sets the
> igd-passthru=on option to Qemu for the Xen HVM machine type.
> 
> The new xen_igd_reserve_slot function also needs to be implemented in
> hw/xen/xen_pt_stub.c to prevent FTBFS during the link stage for the case
> when Qemu is configured with --enable-xen and --disable-xen-pci-passthrough,
> in which case it does nothing.
> 
> The new xen_igd_clear_slot function overrides qdev->realize of the parent
> PCI device class to enable the Intel IGD to occupy slot 2 on the PCI bus
> since slot 2 was reserved by xen_igd_reserve_slot when the PCI bus was
> created in hw/i386/pc_piix.c for the case when igd-passthru=on.
> 
> Move the call to xen_host_pci_device_get, and the associated error
> handling, from xen_pt_realize to the new xen_igd_clear_slot function to
> initialize the device class and vendor values which enables the checks for
> the Intel IGD to succeed. The verification that the host device is an
> Intel IGD to be passed through is done by checking the domain, bus, slot,
> and function values as well as by checking that gfx_passthru is enabled,
> the device class is VGA, and the device vendor in Intel.
> 
> Signed-off-by: Chuck Zmudzinski 

I'm not sure why is the issue xen specific. Can you explain?
Doesn't it affect kvm too?

> ---
> Notes that might be helpful to reviewers of patched code in hw/xen:
> 
> The new functions and types are based on recommendations from Qemu docs:
> https://qemu.readthedocs.io/en/latest/devel/qom.html
> 
> Notes that might be helpful to reviewers of patched code in hw/i386:
> 
> The small patch to hw/i386/pc_piix.c is protected by CONFIG_XEN so it does
> not affect builds that do not have CONFIG_XEN defined.
> 
> xen_igd_gfx_pt_enabled() in the patched hw/i386/pc_piix.c file is an
> existing function that is only true when Qemu is built with
> xen-pci-passthrough enabled and the administrator has configured the Xen
> HVM guest with Qemu's igd-passthru=on option.
> 
> v2: Remove From:  tag at top of commit message
> 
> v3: Changed the test for the Intel IGD in xen_igd_clear_slot:
> 
> if (is_igd_vga_passthrough(>real_device) &&
> (s->real_device.vendor_id == PCI_VENDOR_ID_INTEL)) {
> 
> is changed to
> 
> if (xen_igd_gfx_pt_enabled() && (s->hostaddr.slot == 2)
> && (s->hostaddr.function == 0)) {
> 
> I hoped that I could use the test in v2, since it matches the
> other tests for the Intel IGD in Qemu and Xen, but those tests
> do not work because the necessary data structures are not set with
> their values yet. So instead use the test that the administrator
> has enabled gfx_passthru and the device address on the host is
> 02.0. This test does detect the Intel IGD correctly.
> 
> v4: Use brchu...@aol.com instead of brchu...@netscape.net for the author's
> email address to match the address used by the same author in commits
> be9c61da and c0e86b76
> 
> Change variable for XEN_PT_DEVICE_CLASS: xptc changed to xpdc
> 
> v5: The patch of xen_pt.c was re-worked to allow a more consistent test
> for the Intel IGD that uses the same criteria as in other places.
> This involved moving the call to xen_host_pci_device_get from
> xen_pt_realize to xen_igd_clear_slot and updating the checks for the
> Intel IGD in xen_igd_clear_slot:
> 
> if (xen_igd_gfx_pt_enabled() && (s->hostaddr.slot == 2)
> && (s->hostaddr.function == 0)) {
> 
> is changed to
> 
> if (is_igd_vga_passthrough(>real_device) &&
> s->real_device.domain == 0 && s->real_device.bus == 0 &&
> 

Re: [PATCH] m25p80: Add the is25wp256 SFPD table

[Michael Walle]

Am 2023-01-02 17:23, schrieb Guenter Roeck:

On Mon, Jan 02, 2023 at 04:43:49PM +0100, Michael Walle wrote:

Am 2023-01-02 14:53, schrieb Cédric Le Goater:
> On 12/27/22 07:31, Tudor Ambarus wrote:
> >
> >
> > On 25.12.2022 14:18, Ben Dooks wrote:
> > > On Wed, Dec 21, 2022 at 06:36:02PM +0100, Cédric Le Goater wrote:
> > > > On 12/21/22 13:22, Guenter Roeck wrote:
> > > > > Generated from hardware using the following command and
> > > > > then padding
> > > > > with 0xff to fill out a power-of-2:
> > > > > xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
> > > > >
> > > > > Cc: Michael Walle 
> > > > > Cc: Tudor Ambarus 
> > > > > Signed-off-by: Guenter Roeck 
> > > >
> > > > Reviewed-by: Cédric Le Goater 
> > >
> > > If SFDP is a standard, couldn't we have an function to generate
> > > it from
> > > the flash parameters?
> > >
> >
> > No, it's not practical as you have to specify all the flash parameters
> > at flash declaration.
>
> Indeed and the definition of flash models in QEMU is far to cover all
> the SFDP
> features. The known_devices array of m25p80 would be huge ! However, we
> could
> generate some of the SFDP tables if no raw data is provided. It could be
> useful
> for testing drivers.

I don't think adding (incomplete) SFDP tables makes sense for any real
devices. E.g. sometimes our linux driver will look at specific bits in
SFDP to figure out what particular flash device is attached. For 
example

when there are different flashes with the same jedec id.

But since the last released kernel, we support a generic SFDP driver, 
which

is used when there is no matching driver for the flash's jedec id.
Theoretically, you can build your own flash device (with a unique id) 
and

generate the sfdp tables for that one.

How about older kernels versions ? Would those still support such 
(virtual ?)

flash devices ?


No with older kernels you'd be out of luck. They will just print 
"unknown flash

id" and skip the device.

-michael

Re: [PATCH 2/4] target/m68k: pass sign directly into make_quotient()

[Richard Henderson]

On 1/2/23 02:10, Mark Cave-Ayland wrote:

I think you need an "abs(floatx80_to_int32())" in both cases as you do in PATCH 
4


Or in fact

 sign = extractFloatx80Sign(res);
 quot = floatx80_to_int32(floatx80_abs(res->d), status);
 make_quotient(env, sign, quot);


Thanks for the suggestion. Just out of curiosity, how does moving the abs to before the 
integer conversion make a difference here? Is it because floatx80_to_int32() can fail in 
some circumstances because of the sign of the result?


It's a simple and operation on floats, instead of cmp+cmov on integers.

I think it's a touch clearer as well, having just saved the fp sign, you discard it before 
moving on to the next thing.



r~

Re: [PATCH v4 08/30] hw/i386/pc: Create RTC controllers in south bridges

[Thomas Huth]

On 21/12/2022 17.59, Bernhard Beschow wrote:

Just like in the real hardware (and in PIIX4), create the RTC
controllers in the south bridges.

Signed-off-by: Bernhard Beschow 
Reviewed-by: Michael S. Tsirkin 
Message-Id: <20221022150508.26830-11-shen...@gmail.com>
---
  hw/i386/pc.c  | 12 +++-
  hw/i386/pc_piix.c |  8 
  hw/i386/pc_q35.c  |  1 +
  hw/isa/Kconfig|  2 ++
  hw/isa/lpc_ich9.c |  8 
  hw/isa/piix3.c| 15 +++
  include/hw/i386/ich9.h|  2 ++
  include/hw/southbridge/piix.h |  3 +++
  8 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index fa69b6f43e..d154eedcb3 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1299,7 +1299,17 @@ void pc_basic_device_init(struct PCMachineState *pcms,
  pit_alt_irq = qdev_get_gpio_in(hpet, HPET_LEGACY_PIT_INT);
  rtc_irq = qdev_get_gpio_in(hpet, HPET_LEGACY_RTC_INT);
  }
-*rtc_state = mc146818_rtc_init(isa_bus, 2000, rtc_irq);
+
+if (rtc_irq) {
+qdev_connect_gpio_out(DEVICE(*rtc_state), 0, rtc_irq);
+} else {
+uint32_t irq = object_property_get_uint(OBJECT(*rtc_state),
+"irq",
+_fatal);
+isa_connect_gpio_out(*rtc_state, 0, irq);
+}
+object_property_add_alias(OBJECT(pcms), "rtc-time", OBJECT(*rtc_state),
+  "date");
I think you could turn now the "ISADevice **rtc_state" parameter of this 
function into a normal "ISADevice *rtc_state" since the pointer is not a 
return value anymore.


 Thomas

Re: [PATCH v4] hw/rtc/mc146818rtc: Make this rtc device target independent

[Thomas Huth]

On 02/01/2023 17.47, Bernhard Beschow wrote:



Am 2. Januar 2023 16:09:08 UTC schrieb Thomas Huth :

On 02/01/2023 14.36, Thomas Huth wrote:

On 31/12/2022 00.45, Bernhard Beschow wrote:


Am 29. Dezember 2022 10:58:48 UTC schrieb Thomas Huth :

[...]

static uint32_t rtc_periodic_clock_ticks(RTCState *s)
{
@@ -922,14 +911,15 @@ static void rtc_realizefn(DeviceState *dev, Error **errp)
  rtc_set_date_from_host(isadev);

  switch (s->lost_tick_policy) {
-#ifdef TARGET_I386
-    case LOST_TICK_POLICY_SLEW:
-    s->coalesced_timer =
-    timer_new_ns(rtc_clock, rtc_coalesced_timer, s);
-    break;
-#endif
  case LOST_TICK_POLICY_DISCARD:
  break;
+    case LOST_TICK_POLICY_SLEW:
+    /* Slew tick policy is only available on x86 */
+    if (arch_type == QEMU_ARCH_I386) {


This reflects the intention much better than before, which is nice.

How does `arch_type` play together with qemu-system-all? IIUC it should be 
possible to load all arch backends simultaneously while `arch_type` is an 
external symbol defined by each arch backend differently. So this seems to 
conflict.


I assume that there still will be a main arch_type for the current selected 
machine? ... not sure how this will exactly work, though ...


Can we just add a property such as "slew-tick-policy-available" instead? It 
should default to false and all x86 machines would need to opt in explicitly.


Sounds like a good idea, it's certainly better than checking arch_type here ... 
I'll give it a try, thanks!


I've now had a look at this, and it's also getting ugly: Since the property has 
to be set before realize() is done, the setting of the property has to be added 
to the mc146818_rtc_init() function. Thus this function would need a new 
parameter - and it then needs to be changed all over the place, i.e. also for 
all the non-x86 machines, defeating the idea of a default value...

Maybe it makes more sense to check for a TYPE_X86_MACHINE machine type instead?


Maybe you could base your patch on 
https://lists.gnu.org/archive/html/qemu-devel/2022-12/msg03795.html ?


That would help, indeed. ... OK, then let's postpone my clean-up until your 
series has landed.


 Thomas

Re: [PATCH v4] hw/rtc/mc146818rtc: Make this rtc device target independent

[Bernhard Beschow]

Am 2. Januar 2023 16:09:08 UTC schrieb Thomas Huth :
>On 02/01/2023 14.36, Thomas Huth wrote:
>> On 31/12/2022 00.45, Bernhard Beschow wrote:
>>> 
>>> Am 29. Dezember 2022 10:58:48 UTC schrieb Thomas Huth :
>[...]
 static uint32_t rtc_periodic_clock_ticks(RTCState *s)
 {
 @@ -922,14 +911,15 @@ static void rtc_realizefn(DeviceState *dev, Error 
 **errp)
  rtc_set_date_from_host(isadev);
 
  switch (s->lost_tick_policy) {
 -#ifdef TARGET_I386
 -    case LOST_TICK_POLICY_SLEW:
 -    s->coalesced_timer =
 -    timer_new_ns(rtc_clock, rtc_coalesced_timer, s);
 -    break;
 -#endif
  case LOST_TICK_POLICY_DISCARD:
  break;
 +    case LOST_TICK_POLICY_SLEW:
 +    /* Slew tick policy is only available on x86 */
 +    if (arch_type == QEMU_ARCH_I386) {
>>> 
>>> This reflects the intention much better than before, which is nice.
>>> 
>>> How does `arch_type` play together with qemu-system-all? IIUC it should be 
>>> possible to load all arch backends simultaneously while `arch_type` is an 
>>> external symbol defined by each arch backend differently. So this seems to 
>>> conflict.
>> 
>> I assume that there still will be a main arch_type for the current selected 
>> machine? ... not sure how this will exactly work, though ...
>> 
>>> Can we just add a property such as "slew-tick-policy-available" instead? It 
>>> should default to false and all x86 machines would need to opt in 
>>> explicitly.
>> 
>> Sounds like a good idea, it's certainly better than checking arch_type here 
>> ... I'll give it a try, thanks!
>
>I've now had a look at this, and it's also getting ugly: Since the property 
>has to be set before realize() is done, the setting of the property has to be 
>added to the mc146818_rtc_init() function. Thus this function would need a new 
>parameter - and it then needs to be changed all over the place, i.e. also for 
>all the non-x86 machines, defeating the idea of a default value...
>
>Maybe it makes more sense to check for a TYPE_X86_MACHINE machine type instead?

Maybe you could base your patch on 
https://lists.gnu.org/archive/html/qemu-devel/2022-12/msg03795.html ? This 
patch looks like it should get you covered and is part of my PIIX consolidation 
series [1] which seems to be ready to be queued into Phil's mips-next tree. My 
series is currently just blocked by a MIPS-related  regression.

Best regards,
Bernhard

[1] https://lists.gnu.org/archive/html/qemu-devel/2022-12/msg03788.html

>
> Thomas
>

Re: [PATCH] m25p80: Add the is25wp256 SFPD table

[Guenter Roeck]

On Mon, Jan 02, 2023 at 04:43:49PM +0100, Michael Walle wrote:
> Am 2023-01-02 14:53, schrieb Cédric Le Goater:
> > On 12/27/22 07:31, Tudor Ambarus wrote:
> > > 
> > > 
> > > On 25.12.2022 14:18, Ben Dooks wrote:
> > > > On Wed, Dec 21, 2022 at 06:36:02PM +0100, Cédric Le Goater wrote:
> > > > > On 12/21/22 13:22, Guenter Roeck wrote:
> > > > > > Generated from hardware using the following command and
> > > > > > then padding
> > > > > > with 0xff to fill out a power-of-2:
> > > > > > xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
> > > > > > 
> > > > > > Cc: Michael Walle 
> > > > > > Cc: Tudor Ambarus 
> > > > > > Signed-off-by: Guenter Roeck 
> > > > > 
> > > > > Reviewed-by: Cédric Le Goater 
> > > > 
> > > > If SFDP is a standard, couldn't we have an function to generate
> > > > it from
> > > > the flash parameters?
> > > > 
> > > 
> > > No, it's not practical as you have to specify all the flash parameters
> > > at flash declaration.
> > 
> > Indeed and the definition of flash models in QEMU is far to cover all
> > the SFDP
> > features. The known_devices array of m25p80 would be huge ! However, we
> > could
> > generate some of the SFDP tables if no raw data is provided. It could be
> > useful
> > for testing drivers.
> 
> I don't think adding (incomplete) SFDP tables makes sense for any real
> devices. E.g. sometimes our linux driver will look at specific bits in
> SFDP to figure out what particular flash device is attached. For example
> when there are different flashes with the same jedec id.
> 
> But since the last released kernel, we support a generic SFDP driver, which
> is used when there is no matching driver for the flash's jedec id.
> Theoretically, you can build your own flash device (with a unique id) and
> generate the sfdp tables for that one.
> 
How about older kernels versions ? Would those still support such (virtual ?)
flash devices ?

Thanks,
Guenter

[PATCH] meson: allow disablind the installation of keymaps

[casantos]

From: Carlos Santos 

There are situatuions in which the keyboard maps are not necessary (e.g.
when building only tools or linux-user emulator). Add an option to avoid
installing them, as already possible to do with firmware blobs.

Signed-off-by: Carlos Santos 
---
 configure | 2 ++
 meson_options.txt | 2 ++
 pc-bios/keymaps/meson.build   | 6 --
 scripts/meson-buildoptions.sh | 4 
 4 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 789a4f6cc9..c6ed6a23d0 100755
--- a/configure
+++ b/configure
@@ -889,6 +889,8 @@ for opt do
   ;;
   --disable-blobs) meson_option_parse --disable-install-blobs ""
   ;;
+  --disable-keymaps) meson_option_parse --disable-install-keymaps ""
+  ;;
   --enable-vfio-user-server) vfio_user_server="enabled"
   ;;
   --disable-vfio-user-server) vfio_user_server="disabled"
diff --git a/meson_options.txt b/meson_options.txt
index 559a571b6b..be27137e98 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -48,6 +48,8 @@ option('module_upgrades', type : 'boolean', value : false,
description: 'try to load modules from alternate paths for upgrades')
 option('install_blobs', type : 'boolean', value : true,
description: 'install provided firmware blobs')
+option('install_keymaps', type : 'boolean', value : true,
+   description: 'install provided keyboard maps')
 option('sparse', type : 'feature', value : 'auto',
description: 'sparse checker')
 option('guest_agent', type : 'feature', value : 'auto',
diff --git a/pc-bios/keymaps/meson.build b/pc-bios/keymaps/meson.build
index 06c75e646b..7d80c23005 100644
--- a/pc-bios/keymaps/meson.build
+++ b/pc-bios/keymaps/meson.build
@@ -47,7 +47,7 @@ if native_qemu_keymap.found()
build_by_default: true,
output: km,
command: [native_qemu_keymap, '-f', '@OUTPUT@', 
args.split()],
-   install: true,
+   install: get_option('install_keymaps'),
install_dir: qemu_datadir / 'keymaps')
   endforeach
 
@@ -56,4 +56,6 @@ else
   install_data(keymaps.keys(), install_dir: qemu_datadir / 'keymaps')
 endif
 
-install_data(['sl', 'sv'], install_dir: qemu_datadir / 'keymaps')
+if get_option('install_keymaps')
+  install_data(['sl', 'sv'], install_dir: qemu_datadir / 'keymaps')
+endif
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index aa6e30ea91..f17d9c196e 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -11,6 +11,8 @@ meson_options_help() {
   printf "%s\n" '  --datadir=VALUE  Data file directory [share]'
   printf "%s\n" '  --disable-coroutine-pool coroutine freelist (better 
performance)'
   printf "%s\n" '  --disable-install-blobs  install provided firmware blobs'
+  printf "%s\n" '  --disable-install-keymaps'
+  printf "%s\n" '   install provided keyboard maps'
   printf "%s\n" '  --docdir=VALUE   Base directory for documentation 
installation'
   printf "%s\n" '   (can be empty) [share/doc]'
   printf "%s\n" '  --enable-block-drv-whitelist-in-tools'
@@ -291,6 +293,8 @@ _meson_option_parse() {
 --includedir=*) quote_sh "-Dincludedir=$2" ;;
 --enable-install-blobs) printf "%s" -Dinstall_blobs=true ;;
 --disable-install-blobs) printf "%s" -Dinstall_blobs=false ;;
+--enable-install-keymaps) printf "%s" -Dinstall_keymaps=true ;;
+--disable-install-keymaps) printf "%s" -Dinstall_keymaps=false ;;
 --interp-prefix=*) quote_sh "-Dinterp_prefix=$2" ;;
 --enable-jack) printf "%s" -Djack=enabled ;;
 --disable-jack) printf "%s" -Djack=disabled ;;
-- 
2.31.1

Re: [PATCH v4] hw/rtc/mc146818rtc: Make this rtc device target independent

[Thomas Huth]

On 02/01/2023 14.36, Thomas Huth wrote:

On 31/12/2022 00.45, Bernhard Beschow wrote:


Am 29. Dezember 2022 10:58:48 UTC schrieb Thomas Huth :

[...]

static uint32_t rtc_periodic_clock_ticks(RTCState *s)
{
@@ -922,14 +911,15 @@ static void rtc_realizefn(DeviceState *dev, Error 
**errp)

 rtc_set_date_from_host(isadev);

 switch (s->lost_tick_policy) {
-#ifdef TARGET_I386
-    case LOST_TICK_POLICY_SLEW:
-    s->coalesced_timer =
-    timer_new_ns(rtc_clock, rtc_coalesced_timer, s);
-    break;
-#endif
 case LOST_TICK_POLICY_DISCARD:
 break;
+    case LOST_TICK_POLICY_SLEW:
+    /* Slew tick policy is only available on x86 */
+    if (arch_type == QEMU_ARCH_I386) {


This reflects the intention much better than before, which is nice.

How does `arch_type` play together with qemu-system-all? IIUC it should be 
possible to load all arch backends simultaneously while `arch_type` is an 
external symbol defined by each arch backend differently. So this seems to 
conflict.


I assume that there still will be a main arch_type for the current selected 
machine? ... not sure how this will exactly work, though ...


Can we just add a property such as "slew-tick-policy-available" instead? 
It should default to false and all x86 machines would need to opt in 
explicitly.


Sounds like a good idea, it's certainly better than checking arch_type here 
... I'll give it a try, thanks!


I've now had a look at this, and it's also getting ugly: Since the property 
has to be set before realize() is done, the setting of the property has to 
be added to the mc146818_rtc_init() function. Thus this function would need 
a new parameter - and it then needs to be changed all over the place, i.e. 
also for all the non-x86 machines, defeating the idea of a default value...


Maybe it makes more sense to check for a TYPE_X86_MACHINE machine type instead?

 Thomas

[PATCH 1/1] hw/arm/sbsa-ref.c: Start APs powered off

[Rebecca Cran]

For the SBSA-REF machine start all APs in the powered-off state.
This reduces host CPU usage until PSCI_CPU_ON is called when the APs
are needed.

Signed-off-by: Rebecca Cran 
---
 hw/arm/sbsa-ref.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index 4bb444684f40..cf0af35c7807 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -753,6 +753,12 @@ static void sbsa_ref_init(MachineState *machine)
 _abort);
 }
 
+/* Configure all APs to be powered off at start */
+if (n != 0 && object_property_find(cpuobj, "start-powered-off")) {
+object_property_set_bool(cpuobj, "start-powered-off",
+ true, _abort);
+}
+
 object_property_set_link(cpuobj, "memory", OBJECT(sysmem),
  _abort);
 
-- 
2.30.2

Re: [PATCH] m25p80: Add the is25wp256 SFPD table

[Michael Walle]

Am 2023-01-02 14:53, schrieb Cédric Le Goater:

On 12/27/22 07:31, Tudor Ambarus wrote:



On 25.12.2022 14:18, Ben Dooks wrote:

On Wed, Dec 21, 2022 at 06:36:02PM +0100, Cédric Le Goater wrote:

On 12/21/22 13:22, Guenter Roeck wrote:
Generated from hardware using the following command and then 
padding

with 0xff to fill out a power-of-2:
xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp

Cc: Michael Walle 
Cc: Tudor Ambarus 
Signed-off-by: Guenter Roeck 


Reviewed-by: Cédric Le Goater 


If SFDP is a standard, couldn't we have an function to generate it 
from

the flash parameters?



No, it's not practical as you have to specify all the flash parameters
at flash declaration.


Indeed and the definition of flash models in QEMU is far to cover all 
the SFDP
features. The known_devices array of m25p80 would be huge ! However, we 
could
generate some of the SFDP tables if no raw data is provided. It could 
be useful

for testing drivers.


I don't think adding (incomplete) SFDP tables makes sense for any real
devices. E.g. sometimes our linux driver will look at specific bits in
SFDP to figure out what particular flash device is attached. For example
when there are different flashes with the same jedec id.

But since the last released kernel, we support a generic SFDP driver, 
which

is used when there is no matching driver for the flash's jedec id.
Theoretically, you can build your own flash device (with a unique id) 
and

generate the sfdp tables for that one.

-michael

Re: [PATCH qemu] x86: don't let decompressed kernel image clobber setup_data

[Ard Biesheuvel]

On Mon, 2 Jan 2023 at 14:37, Borislav Petkov  wrote:
>
> On Mon, Jan 02, 2023 at 10:32:03AM +0100, Ard Biesheuvel wrote:
> > So instead of appending data to the compressed image and assuming that
> > it will stay in place, create or extend a memory reservation
> > elsewhere, and refer to its absolute address in setup_data.
>
> From my limited experience with all those boot protocols, I'd say hardcoding
> stuff is always a bad idea. But, we already more or less hardcode, or rather
> codify through the setup header contract how stuff needs to get accessed.
>
> And yeah, maybe specifying an absolute address and size for a blob of data and
> putting that address and size in the setup header so that all the parties
> involved are where what is, is probably better.
>

Exactly. In the EFI case, this was problematic because we would need
to introduce a new way to pass memory reservations between QEMU and
the firmware. But I don't think that issue should affect legacy BIOS
boot, and we could just reserve the memory in the E820 table AFAIK.

Re: [PATCH] ccid-card-emulated: fix cast warning

[Marc-André Lureau]

On Mon, Nov 14, 2022 at 5:22 PM  wrote:

> From: Marc-André Lureau 
>
> ../hw/usb/ccid-card-emulated.c: In function 'handle_apdu_thread':
> ../hw/usb/ccid-card-emulated.c:251:24: error: cast from pointer to integer
> of different size [-Werror=pointer-to-int-cast]
>   251 | assert((unsigned long)event > 1000);
>
> Signed-off-by: Marc-André Lureau 
>

ping


> ---
>  hw/usb/ccid-card-emulated.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c
> index ee41a81801..c328660075 100644
> --- a/hw/usb/ccid-card-emulated.c
> +++ b/hw/usb/ccid-card-emulated.c
> @@ -248,7 +248,7 @@ static void *handle_apdu_thread(void* arg)
>  WITH_QEMU_LOCK_GUARD(>vreader_mutex) {
>  while (!QSIMPLEQ_EMPTY(>guest_apdu_list)) {
>  event = QSIMPLEQ_FIRST(>guest_apdu_list);
> -assert((unsigned long)event > 1000);
> +assert(event != NULL);
>  QSIMPLEQ_REMOVE_HEAD(>guest_apdu_list, entry);
>  if (event->p.data.type != EMUL_GUEST_APDU) {
>  DPRINTF(card, 1, "unexpected message in
> handle_apdu_thread\n");
> --
> 2.38.1
>
>

Re: [PATCH v2 09/11] hw/arm/aspeed_ast10x0: Map HACE peripheral

[Cédric Le Goater]

On 12/30/22 12:35, Philippe Mathieu-Daudé wrote:

Since I don't have access to the datasheet, the relevant
values were found in:
https://github.com/AspeedTech-BMC/zephyr/blob/v00.01.08/dts/arm/aspeed/ast10x0.dtsi

Before on Zephyr:

   uart:~$ hash test
   sha256_test
   tv[0]:hash_final error
   sha384_test
   tv[0]:hash_final error
   sha512_test
   tv[0]:hash_final error
   [00:00:06.278,000]  hace_global: HACE poll timeout
   [00:00:09.324,000]  hace_global: HACE poll timeout
   [00:00:12.261,000]  hace_global: HACE poll timeout

   uart:~$ crypto aes256_cbc_vault
   aes256_cbc vault key 1
   [00:00:06.699,000]  hace_global: aspeed_crypto_session_setup
   [00:00:06.699,000]  hace_global: data->cmd: 1c2098
   [00:00:06.699,000]  hace_global: crypto_data_src: 93340
   [00:00:06.699,000]  hace_global: crypto_data_dst: 93348
   [00:00:06.699,000]  hace_global: crypto_ctx_base: 93300
   [00:00:06.699,000]  hace_global: crypto_data_len: 8040
   [00:00:06.699,000]  hace_global: crypto_cmd_reg:  11c2098
   [00:00:09.743,000]  hace_global: HACE_STS: 0
   [00:00:09.743,000]  hace_global: HACE poll timeout
   [00:00:09.743,000]  crypto: CBC mode ENCRYPT - Failed
   [00:00:09.743,000]  hace_global: aspeed_crypto_session_free
   uart:~$

After:

   uart:~$ hash test
   sha256_test
   tv[0]:PASS
   tv[1]:PASS
   tv[2]:PASS
   tv[3]:PASS
   tv[4]:PASS
   sha384_test
   tv[0]:PASS
   tv[1]:PASS
   tv[2]:PASS
   tv[3]:PASS
   tv[4]:PASS
   tv[5]:PASS
   sha512_test
   tv[0]:PASS
   tv[1]:PASS
   tv[2]:PASS
   tv[3]:PASS
   tv[4]:PASS
   tv[5]:PASS

   uart:~$ crypto aes256_cbc_vault
   aes256_cbc vault key 1
   Was waiting for:
   6b c1 be e2 2e 40 9f 96 e9 3d 7e 11 73 93 17 2a
   ae 2d 8a 57 1e 03 ac 9c 9e b7 6f ac 45 af 8e 51
   30 c8 1c 46 a3 5c e4 11 e5 fb c1 19 1a 0a 52 ef
   f6 9f 24 45 df 4f 9b 17 ad 2b 41 7b e6 6c 37 10

But got:
   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00


I think the HACE model only supports hash for now.




   [00:00:05.771,000]  hace_global: aspeed_crypto_session_setup
   [00:00:05.772,000]  hace_global: data->cmd: 1c2098
   [00:00:05.772,000]  hace_global: crypto_data_src: 93340
   [00:00:05.772,000]  hace_global: crypto_data_dst: 93348
   [00:00:05.772,000]  hace_global: crypto_ctx_base: 93300
   [00:00:05.772,000]  hace_global: crypto_data_len: 8040
   [00:00:05.772,000]  hace_global: crypto_cmd_reg:  11c2098
   [00:00:05.772,000]  hace_global: HACE_STS: 1000
   [00:00:05.772,000]  crypto: Output length (encryption): 80
   [00:00:05.772,000]  hace_global: aspeed_crypto_session_free
   [00:00:05.772,000]  hace_global: aspeed_crypto_session_setup
   [00:00:05.772,000]  hace_global: data->cmd: 1c2018
   [00:00:05.772,000]  hace_global: crypto_data_src: 93340
   [00:00:05.772,000]  hace_global: crypto_data_dst: 93348
   [00:00:05.772,000]  hace_global: crypto_ctx_base: 93300
   [00:00:05.772,000]  hace_global: crypto_data_len: 8040
   [00:00:05.772,000]  hace_global: crypto_cmd_reg:  11c2018
   [00:00:05.772,000]  hace_global: HACE_STS: 1000
   [00:00:05.772,000]  crypto: Output length (decryption): 64
   [00:00:05.772,000]  crypto: CBC mode DECRYPT - Mismatch between 
plaintext and decrypted cipher text
   [00:00:05.774,000]  hace_global: aspeed_crypto_session_free
   uart:~$

Reviewed-by: Peter Delevoryas 
Signed-off-by: Philippe Mathieu-Daudé 


Reviewed-by: Cédric Le Goater 

Thanks,

C.


---
  hw/arm/aspeed_ast10x0.c | 15 +++
  1 file changed, 15 insertions(+)

diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
index e74e2660ce..7a7443a95b 100644
--- a/hw/arm/aspeed_ast10x0.c
+++ b/hw/arm/aspeed_ast10x0.c
@@ -29,6 +29,7 @@ static const hwaddr aspeed_soc_ast1030_memmap[] = {
  [ASPEED_DEV_SPI1]  = 0x7E63,
  [ASPEED_DEV_SPI2]  = 0x7E64,
  [ASPEED_DEV_UDC]   = 0x7E6A2000,
+[ASPEED_DEV_HACE]  = 0x7E6D,
  [ASPEED_DEV_SCU]   = 0x7E6E2000,
  [ASPEED_DEV_JTAG0] = 0x7E6E4000,
  [ASPEED_DEV_JTAG1] = 0x7E6E4100,
@@ -166,6 +167,9 @@ static void aspeed_soc_ast1030_init(Object *obj)
  snprintf(typename, sizeof(typename), "aspeed.gpio-%s", socname);
  object_initialize_child(obj, "gpio", >gpio, typename);
  
+snprintf(typename, sizeof(typename), "aspeed.hace-%s", socname);

+object_initialize_child(obj, "hace", >hace, typename);
+
  object_initialize_child(obj, "iomem", >iomem, 
TYPE_UNIMPLEMENTED_DEVICE);
  object_initialize_child(obj, "sbc-unimplemented", >sbc_unimplemented,
  TYPE_UNIMPLEMENTED_DEVICE);
@@ -359,6 +363,17 @@ static void aspeed_soc_ast1030_realize(DeviceState 
*dev_soc, Error **errp)
  }
  aspeed_mmio_map(s, SYS_BUS_DEVICE(>sbc), 0, 
sc->memmap[ASPEED_DEV_SBC]);
  
+/* HACE */

+object_property_set_link(OBJECT(>hace), "dram", OBJECT(>sram),
+

Re: [PATCH] m25p80: Add the is25wp256 SFPD table

[Cédric Le Goater]

On 12/27/22 07:31, Tudor Ambarus wrote:



On 25.12.2022 14:18, Ben Dooks wrote:

On Wed, Dec 21, 2022 at 06:36:02PM +0100, Cédric Le Goater wrote:

On 12/21/22 13:22, Guenter Roeck wrote:

Generated from hardware using the following command and then padding
with 0xff to fill out a power-of-2:
xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp

Cc: Michael Walle 
Cc: Tudor Ambarus 
Signed-off-by: Guenter Roeck 


Reviewed-by: Cédric Le Goater 


If SFDP is a standard, couldn't we have an function to generate it from
the flash parameters?



No, it's not practical as you have to specify all the flash parameters
at flash declaration.


Indeed and the definition of flash models in QEMU is far to cover all the SFDP
features. The known_devices array of m25p80 would be huge ! However, we could
generate some of the SFDP tables if no raw data is provided. It could be useful
for testing drivers.

Thanks,

C.

Re: [PATCH v2 11/11] tests/avocado: Test Aspeed Zephyr SDK v00.01.08 on AST1030 board

[Cédric Le Goater]

On 12/30/22 12:35, Philippe Mathieu-Daudé wrote:

Add a very quick test that runs some commands in a Zephyr shell:

   $ tests/venv/bin/avocado --show=app,console run -t os:zephyr tests/avocado
   (2/2) 
tests/avocado/machine_aspeed.py:AST1030Machine.test_ast1030_zephyros_1_07:
   console: *** Booting Zephyr OS build v00.01.07  ***
   console: ast1030_evb demo
   console: SOC: AST1030-A1
   console: uart:~$ kernel stacks
   console: 0x36910 wdt_background (real size 1024):unused 988  usage 
36 / 1024 (3 %)
   console: 0x36ad8 shell_uart (real size 4096):unused 3084 usage 
1012 / 4096 (24 %)
   console: 0x2edb8 ADC0   (real size 400): unused 260  usage 140 / 400 
(35 %)
   console: 0x2f0f0 ADC1   (real size 400): unused 260  usage 140 / 400 
(35 %)
   console: 0x3b098 sysworkq   (real size 1024):unused 860  usage 
164 / 1024 (16 %)
   console: 0x36cc0 usbdworkq  (real size 1024):unused 860  usage 
164 / 1024 (16 %)
   console: 0x36bd8 usbworkq   (real size 1024):unused 860  usage 
164 / 1024 (16 %)
   console: 0x36a10 logging(real size 768): unused 548  usage 220 / 768 
(28 %)
   console: 0x36ef8 idle 00(real size 320): unused 268  usage 52 / 320 
(16 %)
   console: 0x47800 IRQ 00 (real size 2048):unused 1504 usage 
544 / 2048 (26 %)
   console: uart:~$ otp info scu
   console: SCU BIT   reg_protect Description
   console: 
   console: 0x500   0x0   0x0 Disable ARM CM4 CPU boot (TXD5)
   console: 0x500   0x1   0x0/Reserved
   console: 0x500   0x2   0x0\ "
   console: 0x500   0x3   0x0 Address offset of single chip ABR mode
   console: 0x500   0x4   0x0/Reserved
   console: 0x500   0x5   0x0| "
   console: 0x500   0x6   0x0| "
   console: 0x500   0x7   0x0| "
   console: 0x500   0x8   0x0| "
   console: 0x500   0x9   0x0| "
   console: 0x500   0xA   0x0| "
   console: 0x500   0xB   0x0| "
   console: 0x500   0xC   0x0| "
   console: 0x500   0xD   0x0| "
   console: 0x500   0xE   0x0| "
   console: 0x500   0xF   0x0| "
   console: 0x500   0x10  0x0\ "
   console: 0x500   0x11  0x0 Disabl3 ARM JTAG debug
   console: 0x500   0x12  0x0/Reserved
   console: 0x500   0x13  0x0| "
   console: 0x500   0x14  0x0| "
   console: 0x500   0x15  0x0| "
   console: 0x500   0x16  0x0| "
   console: 0x500   0x17  0x0| "
   console: 0x500   0x18  0x0| "
   console: 0x500   0x19  0x0| "
   console: 0x500   0x1A  0x0| "
   console: 0x500   0x1B  0x0| "
   console: 0x500   0x1C  0x0| "
   console: 0x500   0x1D  0x0| "
   console: 0x500   0x1E  0x0| "
   console: 0x500   0x1F  0x0\ "
   console: 0x510   0x0   0x0/Reserved
   console: 0x510   0x1   0x0| "
   console: 0x510   0x2   0x0| "
   console: 0x510   0x3   0x0\ "
   console: 0x510   0x4   0x0 Disable debug interfaces
   console: 0x510   0x5   0x0/Reserved
   console: 0x510   0x6   0x0| "
   console: 0x510   0x7   0x0\ "
   console: 0x510   0x8   0x0 Enable boot from Uart5 by Pin Strap
   console: 0x510   0x9   0x0/Reserved
   console: 0x510   0xA   0x0\ "
   console: 0x510   0xB   0x0 Enable boot SPI ABR
   console: 0x510   0xC   0x0 Boot SPI ABR Mode
   console: 0x510   0xD   0x0/Boot SPI flash size
   console: 0x510   0xE   0x0| "
   console: 0x510   0xF   0x0\ "
   console: 0x510   0x10  0x0/Reserved
   console: 0x510   0x11  0x0| "
   console: 0x510   0x12  0x0| "
   console: 0x510   0x13  0x0| "
   console: 0x510   0x14  0x0| "
   console: 0x510   0x15  0x0\ "
   console: 0x510   0x16  0x0 Enable boot SPI auxiliary control pins
   console: 0x510   0x19  0x0/Reserved
   console: 0x510   0x1A  0x0| "
   console: 0x510   0x1B  0x0| "
   console: 0x510   0x1C  0x0| "
   console: 0x510   0x1D  0x0| "
   console: 0x510   0x1E  0x0| "
   console: 0x510   0x1F  0x0\ "
   console: 0x510   0x1E  0x0 Enable dedicate GPIO strap pins
   console: 0x510   0x1F  0x0 Enable Secure Boot by Pin Strap
   console: uart:~$ hwinfo devid
   console: Length: 8
   console: ID: 0x01800180
   console: uart:~$ crypto aes256_cbc_vault
   console: aes256_cbc vault key 1
   console: Was waiting for:
   console: 6b c1 be e2 2e 40 9f 96 e9 3d 7e 11 73 93 17 2a
   

Re: [PATCH v2 10/11] hw/arm/aspeed_ast10x0: Add TODO comment to use Cortex-M4F

[Cédric Le Goater]

On 12/30/22 12:35, Philippe Mathieu-Daudé wrote:

This SoC uses a Cortex-M4F. QEMU only implements a M4,
which is good enough. Add a TODO note in case the M4F
is added.


How complex would it be to add the FPU version of the M4 ? I suppose we have
all the instructions already implemented ?
 

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Peter Delevoryas 


Reviewed-by: Cédric Le Goater 

Thanks,

C.




---
  hw/arm/aspeed_ast10x0.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
index 7a7443a95b..a3bcbef24a 100644
--- a/hw/arm/aspeed_ast10x0.c
+++ b/hw/arm/aspeed_ast10x0.c
@@ -421,7 +421,7 @@ static void aspeed_soc_ast1030_class_init(ObjectClass 
*klass, void *data)
  dc->realize = aspeed_soc_ast1030_realize;
  
  sc->name = "ast1030-a1";

-sc->cpu_type = ARM_CPU_TYPE_NAME("cortex-m4");
+sc->cpu_type = ARM_CPU_TYPE_NAME("cortex-m4"); /* TODO cortex-m4f */
  sc->silicon_rev = AST1030_A1_SILICON_REV;
  sc->sram_size = 768 * KiB;
  sc->secsram_size = 9 * KiB;

Re: [PATCH v2 07/11] hw/arm/aspeed_ast10x0: Map I3C peripheral

[Cédric Le Goater]

On 12/30/22 12:35, Philippe Mathieu-Daudé wrote:

Since I don't have access to the datasheet, the relevant
values were found in:
https://github.com/AspeedTech-BMC/zephyr/blob/v00.01.08/dts/arm/aspeed/ast10x0.dtsi

Reviewed-by: Peter Delevoryas 
Signed-off-by: Philippe Mathieu-Daudé 

I3C is really a dummy model. I hope we can grow the modeling with time and add 
some
device models.

Reviewed-by: Cédric Le Goater 

Thanks,

C.


---
  hw/arm/aspeed_ast10x0.c | 16 
  1 file changed, 16 insertions(+)

diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
index d7dbc1a801..6c6d33d4a0 100644
--- a/hw/arm/aspeed_ast10x0.c
+++ b/hw/arm/aspeed_ast10x0.c
@@ -54,6 +54,7 @@ static const hwaddr aspeed_soc_ast1030_memmap[] = {
  [ASPEED_DEV_WDT]   = 0x7E785000,
  [ASPEED_DEV_LPC]   = 0x7E789000,
  [ASPEED_DEV_PECI]  = 0x7E78B000,
+[ASPEED_DEV_I3C]   = 0x7E7A,
  [ASPEED_DEV_I2C]   = 0x7E7B,
  };
  
@@ -89,6 +90,7 @@ static const int aspeed_soc_ast1030_irqmap[] = {

  [ASPEED_DEV_ADC]   = 46,
  [ASPEED_DEV_SPI1]  = 65,
  [ASPEED_DEV_SPI2]  = 66,
+[ASPEED_DEV_I3C]   = 102, /* 102 -> 105 */
  [ASPEED_DEV_I2C]   = 110, /* 110 ~ 123 */
  [ASPEED_DEV_KCS]   = 138, /* 138 -> 142 */
  [ASPEED_DEV_UDC]   = 9,
@@ -130,6 +132,8 @@ static void aspeed_soc_ast1030_init(Object *obj)
  snprintf(typename, sizeof(typename), "aspeed.i2c-%s", socname);
  object_initialize_child(obj, "i2c", >i2c, typename);
  
+object_initialize_child(obj, "i3c", >i3c, TYPE_ASPEED_I3C);

+
  snprintf(typename, sizeof(typename), "aspeed.timer-%s", socname);
  object_initialize_child(obj, "timerctrl", >timerctrl, typename);
  
@@ -240,6 +244,18 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)

  sysbus_connect_irq(SYS_BUS_DEVICE(>i2c.busses[i]), 0, irq);
  }
  
+/* I3C */

+if (!sysbus_realize(SYS_BUS_DEVICE(>i3c), errp)) {
+return;
+}
+aspeed_mmio_map(s, SYS_BUS_DEVICE(>i3c), 0, sc->memmap[ASPEED_DEV_I3C]);
+for (i = 0; i < ASPEED_I3C_NR_DEVICES; i++) {
+qemu_irq irq = qdev_get_gpio_in(DEVICE(>armv7m),
+sc->irqmap[ASPEED_DEV_I3C] + i);
+/* The AST1030 I3C controller has one IRQ per bus. */
+sysbus_connect_irq(SYS_BUS_DEVICE(>i3c.devices[i]), 0, irq);
+}
+
  /* PECI */
  if (!sysbus_realize(SYS_BUS_DEVICE(>peci), errp)) {
  return;

Re: [PATCH v2 06/11] hw/arm/aspeed_ast10x0: Add various unimplemented peripherals

[Cédric Le Goater]

On 12/30/22 12:34, Philippe Mathieu-Daudé wrote:

Based on booting Zephyr demo from [1] running QEMU with
'-d unimp' and checking missing devices in [2].

[1] https://github.com/AspeedTech-BMC/zephyr/releases/tag/v00.01.07
[2] 
https://github.com/AspeedTech-BMC/zephyr/blob/v00.01.08/dts/arm/aspeed/ast10x0.dtsi

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Peter Delevoryas 


Reviewed-by: Cédric Le Goater 

Thanks,

C.



---
  hw/arm/aspeed_ast10x0.c | 35 +++
  include/hw/arm/aspeed_soc.h | 11 +++
  2 files changed, 46 insertions(+)

diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
index 3500294df7..d7dbc1a801 100644
--- a/hw/arm/aspeed_ast10x0.c
+++ b/hw/arm/aspeed_ast10x0.c
@@ -28,10 +28,15 @@ static const hwaddr aspeed_soc_ast1030_memmap[] = {
  [ASPEED_DEV_FMC]   = 0x7E62,
  [ASPEED_DEV_SPI1]  = 0x7E63,
  [ASPEED_DEV_SPI2]  = 0x7E64,
+[ASPEED_DEV_UDC]   = 0x7E6A2000,
  [ASPEED_DEV_SCU]   = 0x7E6E2000,
+[ASPEED_DEV_JTAG0] = 0x7E6E4000,
+[ASPEED_DEV_JTAG1] = 0x7E6E4100,
  [ASPEED_DEV_ADC]   = 0x7E6E9000,
+[ASPEED_DEV_ESPI]  = 0x7E6EE000,
  [ASPEED_DEV_SBC]   = 0x7E6F2000,
  [ASPEED_DEV_GPIO]  = 0x7E78,
+[ASPEED_DEV_SGPIOM]= 0x7E780500,
  [ASPEED_DEV_TIMER1]= 0x7E782000,
  [ASPEED_DEV_UART1] = 0x7E783000,
  [ASPEED_DEV_UART2] = 0x7E78D000,
@@ -79,12 +84,17 @@ static const int aspeed_soc_ast1030_irqmap[] = {
  [ASPEED_DEV_LPC]   = 35,
  [ASPEED_DEV_PECI]  = 38,
  [ASPEED_DEV_FMC]   = 39,
+[ASPEED_DEV_ESPI]  = 42,
  [ASPEED_DEV_PWM]   = 44,
  [ASPEED_DEV_ADC]   = 46,
  [ASPEED_DEV_SPI1]  = 65,
  [ASPEED_DEV_SPI2]  = 66,
  [ASPEED_DEV_I2C]   = 110, /* 110 ~ 123 */
  [ASPEED_DEV_KCS]   = 138, /* 138 -> 142 */
+[ASPEED_DEV_UDC]   = 9,
+[ASPEED_DEV_SGPIOM]= 51,
+[ASPEED_DEV_JTAG0] = 27,
+[ASPEED_DEV_JTAG1] = 53,
  };
  
  static qemu_irq aspeed_soc_ast1030_get_irq(AspeedSoCState *s, int dev)

@@ -155,6 +165,15 @@ static void aspeed_soc_ast1030_init(Object *obj)
  object_initialize_child(obj, "iomem", >iomem, 
TYPE_UNIMPLEMENTED_DEVICE);
  object_initialize_child(obj, "sbc-unimplemented", >sbc_unimplemented,
  TYPE_UNIMPLEMENTED_DEVICE);
+object_initialize_child(obj, "pwm", >pwm, TYPE_UNIMPLEMENTED_DEVICE);
+object_initialize_child(obj, "espi", >espi, TYPE_UNIMPLEMENTED_DEVICE);
+object_initialize_child(obj, "udc", >udc, TYPE_UNIMPLEMENTED_DEVICE);
+object_initialize_child(obj, "sgpiom", >sgpiom,
+TYPE_UNIMPLEMENTED_DEVICE);
+object_initialize_child(obj, "jtag[0]", >jtag[0],
+TYPE_UNIMPLEMENTED_DEVICE);
+object_initialize_child(obj, "jtag[1]", >jtag[1],
+TYPE_UNIMPLEMENTED_DEVICE);
  }
  
  static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)

@@ -337,6 +356,22 @@ static void aspeed_soc_ast1030_realize(DeviceState 
*dev_soc, Error **errp)
  sc->memmap[ASPEED_DEV_GPIO]);
  sysbus_connect_irq(SYS_BUS_DEVICE(>gpio), 0,
 aspeed_soc_get_irq(s, ASPEED_DEV_GPIO));
+
+aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(>pwm), "aspeed.pwm",
+  sc->memmap[ASPEED_DEV_PWM], 0x100);
+
+aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(>espi), "aspeed.espi",
+  sc->memmap[ASPEED_DEV_ESPI], 0x800);
+
+aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(>udc), "aspeed.udc",
+  sc->memmap[ASPEED_DEV_UDC], 0x1000);
+aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(>sgpiom), 
"aspeed.sgpiom",
+  sc->memmap[ASPEED_DEV_SGPIOM], 0x100);
+
+aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(>jtag[0]), 
"aspeed.jtag",
+  sc->memmap[ASPEED_DEV_JTAG0], 0x20);
+aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(>jtag[1]), 
"aspeed.jtag",
+  sc->memmap[ASPEED_DEV_JTAG1], 0x20);
  }
  
  static void aspeed_soc_ast1030_class_init(ObjectClass *klass, void *data)

diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 8389200b2d..9a5e3c0bac 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -44,6 +44,7 @@
  #define ASPEED_CPUS_NUM  2
  #define ASPEED_MACS_NUM  4
  #define ASPEED_UARTS_NUM 13
+#define ASPEED_JTAG_NUM  2
  
  struct AspeedSoCState {

  /*< private >*/
@@ -87,6 +88,11 @@ struct AspeedSoCState {
  UnimplementedDeviceState video;
  UnimplementedDeviceState emmc_boot_controller;
  UnimplementedDeviceState dpmcu;
+UnimplementedDeviceState pwm;
+UnimplementedDeviceState espi;
+UnimplementedDeviceState udc;
+UnimplementedDeviceState sgpiom;
+ 

Re: [PATCH v2 05/11] hw/misc/aspeed_hace: Do not crash if address_space_map() failed

[Cédric Le Goater]

On 12/30/22 12:34, Philippe Mathieu-Daudé wrote:

address_space_map() can fail:

   uart:~$ hash test
   sha256_test
   tv[0]:
   Segmentation fault: 11
   Thread 3 "qemu-system-arm" received signal SIGSEGV, Segmentation fault.
   gen_acc_mode_iov (req_len=0x718b7778, id=, 
iov=0x718b7780, s=0x56ce0bd0)
   at ../hw/misc/aspeed_hace.c:171
   171 if (has_padding(s, [id], *req_len, _msg_len, 
_offset)) {
   (gdb) bt
   #0  gen_acc_mode_iov (req_len=0x718b7778, id=, 
iov=0x718b7780, s=0x56ce0bd0)
   at ../hw/misc/aspeed_hace.c:171
   #1  do_hash_operation (s=s@entry=0x56ce0bd0, algo=3, 
sg_mode=sg_mode@entry=true, acc_mode=acc_mode@entry=true)
   at ../hw/misc/aspeed_hace.c:224
   #2  0x559bdbb8 in aspeed_hace_write (opaque=, addr=12, 
data=262488, size=)
   at ../hw/misc/aspeed_hace.c:358

This change doesn't fix much, but at least the guest
can't crash QEMU anymore. Instead it is still usable:

   uart:~$ hash test
   sha256_test
   tv[0]:hash_final error
   sha384_test
   tv[0]:hash_final error
   sha512_test
   tv[0]:hash_final error
   [00:00:06.278,000]  hace_global: HACE poll timeout
   [00:00:09.324,000]  hace_global: HACE poll timeout
   [00:00:12.261,000]  hace_global: HACE poll timeout
   uart:~$

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Peter Delevoryas 


This is a tough model. I am glad you are taking a look at it.

Reviewed-by: Cédric Le Goater 

Thanks,

C.


---
  hw/misc/aspeed_hace.c | 21 +++--
  1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index ac21be306c..12a761f1f5 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -193,6 +193,7 @@ static void do_hash_operation(AspeedHACEState *s, int algo, 
bool sg_mode,
  size_t digest_len = 0;
  int niov = 0;
  int i;
+void *haddr;
  
  if (sg_mode) {

  uint32_t len = 0;
@@ -217,9 +218,13 @@ static void do_hash_operation(AspeedHACEState *s, int 
algo, bool sg_mode,
  addr &= SG_LIST_ADDR_MASK;
  
  plen = len & SG_LIST_LEN_MASK;

-iov[i].iov_base = address_space_map(>dram_as, addr, , 
false,
-MEMTXATTRS_UNSPECIFIED);
-
+haddr = address_space_map(>dram_as, addr, , false,
+  MEMTXATTRS_UNSPECIFIED);
+if (haddr == NULL) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: qcrypto failed\n", 
__func__);
+return;
+}
+iov[i].iov_base = haddr;
  if (acc_mode) {
  niov = gen_acc_mode_iov(s, iov, i, );
  
@@ -230,10 +235,14 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,

  } else {
  hwaddr len = s->regs[R_HASH_SRC_LEN];
  
+haddr = address_space_map(>dram_as, s->regs[R_HASH_SRC],

+  , false, MEMTXATTRS_UNSPECIFIED);
+if (haddr == NULL) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: qcrypto failed\n", __func__);
+return;
+}
+iov[0].iov_base = haddr;
  iov[0].iov_len = len;
-iov[0].iov_base = address_space_map(>dram_as, s->regs[R_HASH_SRC],
-, false,
-MEMTXATTRS_UNSPECIFIED);
  i = 1;
  
  if (s->iov_count) {

Re: [PATCH qemu] x86: don't let decompressed kernel image clobber setup_data

[Borislav Petkov]

On Mon, Jan 02, 2023 at 10:32:03AM +0100, Ard Biesheuvel wrote:
> So instead of appending data to the compressed image and assuming that
> it will stay in place, create or extend a memory reservation
> elsewhere, and refer to its absolute address in setup_data.

>From my limited experience with all those boot protocols, I'd say hardcoding
stuff is always a bad idea. But, we already more or less hardcode, or rather
codify through the setup header contract how stuff needs to get accessed.

And yeah, maybe specifying an absolute address and size for a blob of data and
putting that address and size in the setup header so that all the parties
involved are where what is, is probably better.

But WTH do I know...

-- 
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v2 04/11] hw/arm/aspeed: Use the IEC binary prefix definitions

[Cédric Le Goater]

On 12/30/22 12:34, Philippe Mathieu-Daudé wrote:

IEC binary prefixes ease code review: the unit is explicit.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Peter Delevoryas 


Reviewed-by: Cédric Le Goater 

Thanks,

C.


---
  hw/arm/aspeed_ast10x0.c | 3 ++-
  hw/arm/aspeed_ast2600.c | 3 ++-
  hw/arm/aspeed_soc.c | 4 ++--
  3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
index 122b3fd3f3..3500294df7 100644
--- a/hw/arm/aspeed_ast10x0.c
+++ b/hw/arm/aspeed_ast10x0.c
@@ -10,6 +10,7 @@
   */
  
  #include "qemu/osdep.h"

+#include "qemu/units.h"
  #include "qapi/error.h"
  #include "exec/address-spaces.h"
  #include "sysemu/sysemu.h"
@@ -348,7 +349,7 @@ static void aspeed_soc_ast1030_class_init(ObjectClass 
*klass, void *data)
  sc->name = "ast1030-a1";
  sc->cpu_type = ARM_CPU_TYPE_NAME("cortex-m4");
  sc->silicon_rev = AST1030_A1_SILICON_REV;
-sc->sram_size = 0xc;
+sc->sram_size = 768 * KiB;
  sc->spis_num = 2;
  sc->ehcis_num = 0;
  sc->wdts_num = 4;
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index a79e05ddbd..72df72a540 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -8,6 +8,7 @@
   */
  
  #include "qemu/osdep.h"

+#include "qemu/units.h"
  #include "qapi/error.h"
  #include "hw/misc/unimp.h"
  #include "hw/arm/aspeed_soc.h"
@@ -619,7 +620,7 @@ static void aspeed_soc_ast2600_class_init(ObjectClass *oc, 
void *data)
  sc->name = "ast2600-a3";
  sc->cpu_type = ARM_CPU_TYPE_NAME("cortex-a7");
  sc->silicon_rev  = AST2600_A3_SILICON_REV;
-sc->sram_size= 0x16400;
+sc->sram_size= 89 * KiB;
  sc->spis_num = 2;
  sc->ehcis_num= 2;
  sc->wdts_num = 4;
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index 2c0924d311..677342c9ed 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -517,7 +517,7 @@ static void aspeed_soc_ast2400_class_init(ObjectClass *oc, 
void *data)
  sc->name = "ast2400-a1";
  sc->cpu_type = ARM_CPU_TYPE_NAME("arm926");
  sc->silicon_rev  = AST2400_A1_SILICON_REV;
-sc->sram_size= 0x8000;
+sc->sram_size= 32 * KiB;
  sc->spis_num = 1;
  sc->ehcis_num= 1;
  sc->wdts_num = 2;
@@ -544,7 +544,7 @@ static void aspeed_soc_ast2500_class_init(ObjectClass *oc, 
void *data)
  sc->name = "ast2500-a1";
  sc->cpu_type = ARM_CPU_TYPE_NAME("arm1176");
  sc->silicon_rev  = AST2500_A1_SILICON_REV;
-sc->sram_size= 0x9000;
+sc->sram_size= 36 * KiB;
  sc->spis_num = 2;
  sc->ehcis_num= 2;
  sc->wdts_num = 3;

Re: [PATCH v4] hw/rtc/mc146818rtc: Make this rtc device target independent

[Thomas Huth]

On 31/12/2022 00.45, Bernhard Beschow wrote:



Am 29. Dezember 2022 10:58:48 UTC schrieb Thomas Huth :

The only reason for this code being target dependent is the apic-related
code in rtc_policy_slew_deliver_irq(). Since these apic functions are rather
simple, we can easily move them into a new, separate file (apic_irqcount.c)
which will always be compiled and linked if either APIC or the mc146818 device
are required. This way we can get rid of the #ifdef TARGET_I386 switches in
mc146818rtc.c and declare it in the softmmu_ss instead of specific_ss, so
that the code only gets compiled once for all targets.

Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Mark Cave-Ayland 
Signed-off-by: Thomas Huth 
---
v4: Check for QEMU_ARCH_I386 instead of looking for an APIC


Can we find a more appropriate name for the helpers than "apic" then? If the 
slew tick policy is a workaround for (x86-) KVM I propose to do s/apic/kvm/ while still 
compiling for every target.


Yes, since the IRQ-counting is also used by the old i8259 PIC, it likely 
makes sense to rename the functions.



static uint32_t rtc_periodic_clock_ticks(RTCState *s)
{
@@ -922,14 +911,15 @@ static void rtc_realizefn(DeviceState *dev, Error **errp)
 rtc_set_date_from_host(isadev);

 switch (s->lost_tick_policy) {
-#ifdef TARGET_I386
-case LOST_TICK_POLICY_SLEW:
-s->coalesced_timer =
-timer_new_ns(rtc_clock, rtc_coalesced_timer, s);
-break;
-#endif
 case LOST_TICK_POLICY_DISCARD:
 break;
+case LOST_TICK_POLICY_SLEW:
+/* Slew tick policy is only available on x86 */
+if (arch_type == QEMU_ARCH_I386) {


This reflects the intention much better than before, which is nice.

How does `arch_type` play together with qemu-system-all? IIUC it should be 
possible to load all arch backends simultaneously while `arch_type` is an 
external symbol defined by each arch backend differently. So this seems to 
conflict.


I assume that there still will be a main arch_type for the current selected 
machine? ... not sure how this will exactly work, though ...



Can we just add a property such as "slew-tick-policy-available" instead? It 
should default to false and all x86 machines would need to opt in explicitly.


Sounds like a good idea, it's certainly better than checking arch_type here 
... I'll give it a try, thanks!


 Thomas

Re: [PATCH v2 03/11] hw/watchdog/wdt_aspeed: Log unimplemented registers as UNIMP level

[Cédric Le Goater]

On 12/30/22 12:34, Philippe Mathieu-Daudé wrote:

Add more Aspeed watchdog registers from [*].

Since guests can righteously access them, log the access at
'unimplemented' level instead of 'guest-errors'.

[*] 
https://github.com/AspeedTech-BMC/zephyr/blob/v00.01.08/drivers/watchdog/wdt_aspeed.c#L31

Signed-off-by: Philippe Mathieu-Daudé 


LGTM.

We need to decide how to address the #regs per soc. I would introduce a class
attribute and define ASPEED_WDT_REGS_MAX as the maximum of all, or possibly
allocate the regs array in the realize routine. This is a little more work.

Thanks,

C.




---
  hw/watchdog/wdt_aspeed.c | 13 +
  include/hw/watchdog/wdt_aspeed.h |  2 +-
  2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c
index eefca31ae4..d267aa185c 100644
--- a/hw/watchdog/wdt_aspeed.c
+++ b/hw/watchdog/wdt_aspeed.c
@@ -42,6 +42,11 @@
  #define WDT_PUSH_PULL_MAGIC (0xA8 << 24)
  #define WDT_OPEN_DRAIN_MAGIC(0x8A << 24)
  #define WDT_RESET_MASK1 (0x1c / 4)
+#define WDT_RESET_MASK2 (0x20 / 4)
+
+#define WDT_SW_RESET_CTRL   (0x24 / 4)
+#define WDT_SW_RESET_MASK1  (0x28 / 4)
+#define WDT_SW_RESET_MASK2  (0x2c / 4)
  
  #define WDT_TIMEOUT_STATUS  (0x10 / 4)

  #define WDT_TIMEOUT_CLEAR   (0x14 / 4)
@@ -83,6 +88,10 @@ static uint64_t aspeed_wdt_read(void *opaque, hwaddr offset, 
unsigned size)
  return s->regs[WDT_RESET_MASK1];
  case WDT_TIMEOUT_STATUS:
  case WDT_TIMEOUT_CLEAR:
+case WDT_RESET_MASK2:
+case WDT_SW_RESET_CTRL:
+case WDT_SW_RESET_MASK1:
+case WDT_SW_RESET_MASK2:
  qemu_log_mask(LOG_UNIMP,
"%s: uninmplemented read at offset 0x%" HWADDR_PRIx 
"\n",
__func__, offset);
@@ -190,6 +199,10 @@ static void aspeed_wdt_write(void *opaque, hwaddr offset, 
uint64_t data,
  
  case WDT_TIMEOUT_STATUS:

  case WDT_TIMEOUT_CLEAR:
+case WDT_RESET_MASK2:
+case WDT_SW_RESET_CTRL:
+case WDT_SW_RESET_MASK1:
+case WDT_SW_RESET_MASK2:
  qemu_log_mask(LOG_UNIMP,
"%s: uninmplemented write at offset 0x%" HWADDR_PRIx 
"\n",
__func__, offset);
diff --git a/include/hw/watchdog/wdt_aspeed.h b/include/hw/watchdog/wdt_aspeed.h
index db91ee6b51..e90ef86651 100644
--- a/include/hw/watchdog/wdt_aspeed.h
+++ b/include/hw/watchdog/wdt_aspeed.h
@@ -21,7 +21,7 @@ OBJECT_DECLARE_TYPE(AspeedWDTState, AspeedWDTClass, 
ASPEED_WDT)
  #define TYPE_ASPEED_2600_WDT TYPE_ASPEED_WDT "-ast2600"
  #define TYPE_ASPEED_1030_WDT TYPE_ASPEED_WDT "-ast1030"
  
-#define ASPEED_WDT_REGS_MAX(0x20 / 4)

+#define ASPEED_WDT_REGS_MAX(0x30 / 4)
  
  struct AspeedWDTState {

  /*< private >*/

Re: [PATCH v2 02/11] hw/watchdog/wdt_aspeed: Extend MMIO range to cover more registers

[Cédric Le Goater]

On 12/31/22 23:52, Dong, Eddie wrote:

When booting the Zephyr demo in [1] we get:

   aspeed.io: unimplemented device write (size 4, offset 0x185128, value
0x030f1ff1) <--
   aspeed.io: unimplemented device write (size 4, offset 0x18512c, value
0x03f1)

This corresponds to this Zephyr code [2]:

   static int aspeed_wdt_init(const struct device *dev)
   {
 const struct aspeed_wdt_config *config = dev->config;
 struct aspeed_wdt_data *const data = dev->data;
 uint32_t reg_val;

 /* disable WDT by default */
 reg_val = sys_read32(config->ctrl_base + WDT_CTRL_REG);
 reg_val &= ~WDT_CTRL_ENABLE;
 sys_write32(reg_val, config->ctrl_base + WDT_CTRL_REG);

 sys_write32(data->rst_mask1,
 config->ctrl_base + WDT_SW_RESET_MASK1_REG);   <--
 sys_write32(data->rst_mask2,
 config->ctrl_base + WDT_SW_RESET_MASK2_REG);

 return 0;
   }

The register definitions are [3]:

   #define WDT_RELOAD_VAL_REG  0x0004
   #define WDT_RESTART_REG 0x0008
   #define WDT_CTRL_REG0x000C
   #define WDT_TIMEOUT_STATUS_REG  0x0010
   #define WDT_TIMEOUT_STATUS_CLR_REG  0x0014
   #define WDT_RESET_MASK1_REG 0x001C
   #define WDT_RESET_MASK2_REG 0x0020
   #define WDT_SW_RESET_MASK1_REG  0x0028   <--
   #define WDT_SW_RESET_MASK2_REG  0x002C
   #define WDT_SW_RESET_CTRL_REG   0x0024

Currently QEMU only cover a MMIO region of size 0x20:

   #define ASPEED_WDT_REGS_MAX(0x20 / 4)

Change to map the whole 'iosize' which might be bigger, covering the other


The root cause is that ASPEED_WDT_REGS_MAX is too small, right?
Probably the Qemu is emulating an old version of the hardware.

Given the meaning of ASPEED_WDT_REGS_MAX, it should be larger than iosize, not?
Probably ASPEED_WDT_REGS_MAX should be per device type (aspeed_2400/2500),


yes. We would need a new class attribute for it. Please use these values, they
should be correct.

   #regsiosize

AST2400   0x18/4  0x20
AST2500   0x20/4  0x20
AST2600   0x30/4  0x40
AST1030   0x4C/4  0x80


AFAICT, the WDT logic was changed in a compatible way with the previous 
generation.

Thanks

C.


while iosize is for all devices, and its initial value comes from the per 
device type REGS_MAX.


registers. The MemoryRegionOps read/write handlers will report the accesses
as out-of-bounds guest-errors, but the next commit will report them as
unimplemented.

[1] https://github.com/AspeedTech-BMC/zephyr/releases/tag/v00.01.07
[2] https://github.com/AspeedTech-BMC/zephyr/commit/2e99f10ac27b
[3] https://github.com/AspeedTech-
BMC/zephyr/blob/v00.01.08/drivers/watchdog/wdt_aspeed.c#L31

Reviewed-by: Peter Delevoryas 
Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/watchdog/wdt_aspeed.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c index
958725a1b5..eefca31ae4 100644
--- a/hw/watchdog/wdt_aspeed.c
+++ b/hw/watchdog/wdt_aspeed.c
@@ -260,6 +260,7 @@ static void aspeed_wdt_realize(DeviceState *dev,
Error **errp)  {
  SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
  AspeedWDTState *s = ASPEED_WDT(dev);
+AspeedWDTClass *awc = ASPEED_WDT_GET_CLASS(dev);

  assert(s->scu);

@@ -271,7 +272,7 @@ static void aspeed_wdt_realize(DeviceState *dev,
Error **errp)
  s->pclk_freq = PCLK_HZ;

  memory_region_init_io(>iomem, OBJECT(s), _wdt_ops, s,
-  TYPE_ASPEED_WDT, ASPEED_WDT_REGS_MAX * 4);
+  TYPE_ASPEED_WDT, awc->iosize);
  sysbus_init_mmio(sbd, >iomem);
  }

--
2.38.1

Re: [PATCH v3 04/26] configure: don't enable cross compilers unless in target_list

[Stefan Weil via]

Am 21.10.22 um 00:10 schrieb Richard Henderson:

On 10/20/22 21:51, Alex Bennée wrote:

This avoids the unfortunate effect of always builds the pc-bios blobs
for targets the user isn't interested in.

Suggested-by: Paolo Bonzini 
Signed-off-by: Alex Bennée 
---
  configure | 9 +
  1 file changed, 9 insertions(+)

diff --git a/configure b/configure
index 81561be7c1..dd6f58dcde 100755
--- a/configure
+++ b/configure
@@ -1877,6 +1877,15 @@ probe_target_compiler() {
    container_cross_ranlib=
    container_cross_strip=
+  # We shall skip configuring the target compiler if the user didn't
+  # bother enabling an appropriate guest. This avoids building
+  # extraneous firmware images and tests.
+  if test "${target_list#*$1}" != "$1"; then
+  break;



Isn't break limited for exiting from for, while, or until loop? (*)
If yes, it's wrongly used here. sh does not complain, but other
shells do.

Stefan

*) https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html

Re: [PATCH v2 08/11] hw/arm/aspeed_ast10x0: Map the secure SRAM

[Cédric Le Goater]

On 12/30/22 12:35, Philippe Mathieu-Daudé wrote:

Some SRAM appears to be used by the Secure Boot unit and
crypto accelerators. Name it 'secure sram'.

Note, the SRAM base address was already present but unused
(the 'SBC' index is used for the MMIO peripheral).

Interestingly using CFLAGS=-Winitializer-overrides reports:

   ../hw/arm/aspeed_ast10x0.c:32:30: warning: initializer overrides prior 
initialization of this subobject [-Winitializer-overrides]
 [ASPEED_DEV_SBC]   = 0x7E6F2000,
  ^~
   ../hw/arm/aspeed_ast10x0.c:24:30: note: previous initialization is here
 [ASPEED_DEV_SBC]   = 0x7900,
  ^~
This fixes with Zephyr:

   uart:~$ rsa test
   rsa test vector[0]:
   [00:00:26.156,000]  os: * BUS FAULT *
   [00:00:26.157,000]  os:   Precise data bus error
   [00:00:26.157,000]  os:   BFAR Address: 0x7900
   [00:00:26.158,000]  os: r0/a1:  0x7900  r1/a2:  0x  r2/a3:  
0x1800
   [00:00:26.158,000]  os: r3/a4:  0x79001800 r12/ip:  0x0800 r14/lr:  
0x0001098d
   [00:00:26.158,000]  os:  xpsr:  0x8100
   [00:00:26.158,000]  os: Faulting instruction address (r15/pc): 
0x0001e1bc
   [00:00:26.158,000]  os: >>> ZEPHYR FATAL ERROR 0: CPU exception on CPU 0
   [00:00:26.158,000]  os: Current thread: 0x38248 (shell_uart)
   [00:00:26.165,000]  os: Halting system

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Peter Delevoryas 
---
  hw/arm/aspeed_ast10x0.c | 11 ++-
  include/hw/arm/aspeed_soc.h |  3 +++
  2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
index 6c6d33d4a0..e74e2660ce 100644
--- a/hw/arm/aspeed_ast10x0.c
+++ b/hw/arm/aspeed_ast10x0.c
@@ -22,7 +22,7 @@
  
  static const hwaddr aspeed_soc_ast1030_memmap[] = {

  [ASPEED_DEV_SRAM]  = 0x,
-[ASPEED_DEV_SBC]   = 0x7900,
+[ASPEED_DEV_SECSRAM]   = 0x7900,
  [ASPEED_DEV_IOMEM] = 0x7E60,
  [ASPEED_DEV_PWM]   = 0x7E61,
  [ASPEED_DEV_FMC]   = 0x7E62,
@@ -222,6 +222,14 @@ static void aspeed_soc_ast1030_realize(DeviceState 
*dev_soc, Error **errp)
  memory_region_add_subregion(s->memory,
  sc->memmap[ASPEED_DEV_SRAM],
  >sram);
+memory_region_init_ram(>secsram, OBJECT(s), "sec.sram",
+   sc->secsram_size, );
+if (err != NULL) {
+error_propagate(errp, err);
+return;
+}
+memory_region_add_subregion(s->memory, sc->memmap[ASPEED_DEV_SECSRAM],
+>secsram);
  
  /* SCU */

  if (!sysbus_realize(SYS_BUS_DEVICE(>scu), errp)) {
@@ -401,6 +409,7 @@ static void aspeed_soc_ast1030_class_init(ObjectClass 
*klass, void *data)
  sc->cpu_type = ARM_CPU_TYPE_NAME("cortex-m4");
  sc->silicon_rev = AST1030_A1_SILICON_REV;
  sc->sram_size = 768 * KiB;
+sc->secsram_size = 9 * KiB;


256  * KiB


  sc->spis_num = 2;
  sc->ehcis_num = 0;
  sc->wdts_num = 4;
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 9a5e3c0bac..bd1e03e78a 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -71,6 +71,7 @@ struct AspeedSoCState {
  AspeedSMCState spi[ASPEED_SPIS_NUM];
  EHCISysBusState ehci[ASPEED_EHCIS_NUM];
  AspeedSBCState sbc;
+MemoryRegion secsram;
  UnimplementedDeviceState sbc_unimplemented;
  AspeedSDMCState sdmc;
  AspeedWDTState wdt[ASPEED_WDTS_NUM];
@@ -105,6 +106,7 @@ struct AspeedSoCClass {
  const char *cpu_type;
  uint32_t silicon_rev;
  uint64_t sram_size;
+uint64_t secsram_size;
  int spis_num;
  int ehcis_num;
  int wdts_num;
@@ -143,6 +145,7 @@ enum {
  ASPEED_DEV_SCU,
  ASPEED_DEV_ADC,
  ASPEED_DEV_SBC,
+ASPEED_DEV_SECSRAM,


May be keep the ASPEED_DEV_SBC_ prefix ?

Thanks,

C.


  ASPEED_DEV_EMMC_BC,
  ASPEED_DEV_VIDEO,
  ASPEED_DEV_SRAM,

Re: [PATCH v2 01/11] hw/watchdog/wdt_aspeed: Rename MMIO region size as 'iosize'

[Cédric Le Goater]

On 12/31/22 23:43, Dong, Eddie wrote:


Avoid confusing two different things:
- the WDT I/O region size ('iosize')
- at which offset the SoC map the WDT ('offset') While it is often the same, we
can map smaller region sizes at larger offsets.

Here we are interested in the I/O region size, so rename as 'iosize'.

Reviewed-by: Peter Delevoryas 
Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/arm/aspeed_ast10x0.c  | 2 +-
  hw/arm/aspeed_ast2600.c  | 2 +-
  hw/arm/aspeed_soc.c  | 2 +-
  hw/watchdog/wdt_aspeed.c | 8 
  include/hw/watchdog/wdt_aspeed.h | 2 +-
  5 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c index
4d0b9b115f..122b3fd3f3 100644
--- a/hw/arm/aspeed_ast10x0.c
+++ b/hw/arm/aspeed_ast10x0.c
@@ -325,7 +325,7 @@ static void aspeed_soc_ast1030_realize(DeviceState
*dev_soc, Error **errp)
  return;
  }
  aspeed_mmio_map(s, SYS_BUS_DEVICE(>wdt[i]), 0,
-sc->memmap[ASPEED_DEV_WDT] + i * awc->offset);
+sc->memmap[ASPEED_DEV_WDT] + i * awc->iosize);



How does the specification say here (I didn't find it)?
I read this is for a case where multiple WDTs are implemented.
And offset means the distance io registers of WDT[1] are located from WDT[0].


The specs say 'offset'


Or does the spec explicitly says the io registers of WDT[1] is located right 
after
WDT[0] without any gaps in this case, iosize is better)?


The IO regions are contiguous but size/width is larger than the register set.

That said, the model takes some shortcuts and Phil's proposal is a sign that
we need to clarify and distinguish the number of regs, the region width and
the offset where it is mapped in the overall MMIO region of the SoC.





  }

  /* GPIO */
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c index
cd75465c2b..a79e05ddbd 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -472,7 +472,7 @@ static void aspeed_soc_ast2600_realize(DeviceState
*dev, Error **errp)
  return;
  }
  aspeed_mmio_map(s, SYS_BUS_DEVICE(>wdt[i]), 0,
-sc->memmap[ASPEED_DEV_WDT] + i * awc->offset);
+sc->memmap[ASPEED_DEV_WDT] + i * awc->iosize);


May be, as a clarification, introduce :

  hwaddr wdt_offset = sc->memmap[ASPEED_DEV_WDT] + i * awc->iosize




  }

  /* RAM */
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c index
b05b9dd416..2c0924d311 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -393,7 +393,7 @@ static void aspeed_soc_realize(DeviceState *dev,
Error **errp)
  return;
  }
  aspeed_mmio_map(s, SYS_BUS_DEVICE(>wdt[i]), 0,
-sc->memmap[ASPEED_DEV_WDT] + i * awc->offset);
+sc->memmap[ASPEED_DEV_WDT] + i * awc->iosize);
  }

  /* RAM  */
diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c index
d753693a2e..958725a1b5 100644
--- a/hw/watchdog/wdt_aspeed.c
+++ b/hw/watchdog/wdt_aspeed.c
@@ -309,7 +309,7 @@ static void aspeed_2400_wdt_class_init(ObjectClass
*klass, void *data)
  AspeedWDTClass *awc = ASPEED_WDT_CLASS(klass);

  dc->desc = "ASPEED 2400 Watchdog Controller";
-awc->offset = 0x20;
+awc->iosize = 0x20;
  awc->ext_pulse_width_mask = 0xff;
  awc->reset_ctrl_reg = SCU_RESET_CONTROL1;
  awc->wdt_reload = aspeed_wdt_reload; @@ -346,7 +346,7 @@ static void
aspeed_2500_wdt_class_init(ObjectClass *klass, void *data)
  AspeedWDTClass *awc = ASPEED_WDT_CLASS(klass);

  dc->desc = "ASPEED 2500 Watchdog Controller";
-awc->offset = 0x20;
+awc->iosize = 0x20;
  awc->ext_pulse_width_mask = 0xf;
  awc->reset_ctrl_reg = SCU_RESET_CONTROL1;
  awc->reset_pulse = aspeed_2500_wdt_reset_pulse; @@ -369,7 +369,7
@@ static void aspeed_2600_wdt_class_init(ObjectClass *klass, void *data)
  AspeedWDTClass *awc = ASPEED_WDT_CLASS(klass);

  dc->desc = "ASPEED 2600 Watchdog Controller";
-awc->offset = 0x40;
+awc->iosize = 0x40;
  awc->ext_pulse_width_mask = 0xf; /* TODO */
  awc->reset_ctrl_reg = AST2600_SCU_RESET_CONTROL1;
  awc->reset_pulse = aspeed_2500_wdt_reset_pulse; @@ -392,7 +392,7
@@ static void aspeed_1030_wdt_class_init(ObjectClass *klass, void *data)
  AspeedWDTClass *awc = ASPEED_WDT_CLASS(klass);

  dc->desc = "ASPEED 1030 Watchdog Controller";
-awc->offset = 0x80;
+awc->iosize = 0x80;
  awc->ext_pulse_width_mask = 0xf; /* TODO */
  awc->reset_ctrl_reg = AST2600_SCU_RESET_CONTROL1;
  awc->reset_pulse = aspeed_2500_wdt_reset_pulse; diff --git
a/include/hw/watchdog/wdt_aspeed.h
b/include/hw/watchdog/wdt_aspeed.h
index dfa5dfa424..db91ee6b51 100644
--- a/include/hw/watchdog/wdt_aspeed.h
+++ b/include/hw/watchdog/wdt_aspeed.h
@@ -40,7 +40,7 @@ struct AspeedWDTState {  struct AspeedWDTClass {
  

Re: [PATCH] chardev: clean up chardev-parallel.c

[Marc-André Lureau]

On Thu, Dec 22, 2022 at 11:42 AM Paolo Bonzini  wrote:
>
> Replace HAVE_CHARDEV_PARPORT with a Meson conditional, remove unnecessary
> defines, and close the file descriptor on FreeBSD/DragonFly.
>
> Signed-off-by: Paolo Bonzini 
> ---
>  chardev/char-parallel.c | 15 ++-
>  chardev/meson.build |  5 -
>  include/qemu/osdep.h|  5 -
>  3 files changed, 6 insertions(+), 19 deletions(-)
>
> diff --git a/chardev/char-parallel.c b/chardev/char-parallel.c
> index 05e7efbd6ca9..a5164f975af3 100644
> --- a/chardev/char-parallel.c
> +++ b/chardev/char-parallel.c
> @@ -238,7 +238,6 @@ static void qemu_chr_open_pp_fd(Chardev *chr,
>  }
>  #endif
>
> -#ifdef HAVE_CHARDEV_PARPORT
>  static void qmp_chardev_open_parallel(Chardev *chr,
>ChardevBackend *backend,
>bool *be_opened,
> @@ -276,29 +275,21 @@ static void char_parallel_class_init(ObjectClass *oc, 
> void *data)
>
>  cc->parse = qemu_chr_parse_parallel;
>  cc->open = qmp_chardev_open_parallel;
> -#if defined(__linux__)
>  cc->chr_ioctl = pp_ioctl;
> -#elif defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || \
> -defined(__DragonFly__)
> -cc->chr_ioctl = pp_ioctl;
> -#endif
>  }
>
>  static void char_parallel_finalize(Object *obj)
>  {
> -#if defined(__linux__)
>  Chardev *chr = CHARDEV(obj);
>  ParallelChardev *drv = PARALLEL_CHARDEV(chr);
>  int fd = drv->fd;
>
> +#if defined(__linux__)
>  pp_hw_mode(drv, IEEE1284_MODE_COMPAT);
>  ioctl(fd, PPRELEASE);
> +#endif
>  close(fd);
>  qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
> -#elif defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || \
> -defined(__DragonFly__)
> -/* FIXME: close fd? */
> -#endif
>  }
>
>  static const TypeInfo char_parallel_type_info = {
> @@ -315,5 +306,3 @@ static void register_types(void)
>  }
>
>  type_init(register_types);
> -
> -#endif
> diff --git a/chardev/meson.build b/chardev/meson.build
> index 664f77b8879a..ceedb68d4f95 100644
> --- a/chardev/meson.build
> +++ b/chardev/meson.build
> @@ -14,9 +14,12 @@ chardev_ss.add(files(
>  ))
>  chardev_ss.add(when: 'CONFIG_POSIX', if_true: [files(
>'char-fd.c',
> -  'char-parallel.c',
>'char-pty.c',
>  ), util])
> +if targetos in ['linux', 'gnu/kfreebsd', 'freebsd', 'dragonfly']
> +  'char-parallel.c',

diff --git a/chardev/meson.build b/chardev/meson.build
index ceedb68d4f..789b50056a 100644
--- a/chardev/meson.build
+++ b/chardev/meson.build
@@ -17,7 +17,7 @@ chardev_ss.add(when: 'CONFIG_POSIX', if_true: [files(
   'char-pty.c',
 ), util])
 if targetos in ['linux', 'gnu/kfreebsd', 'freebsd', 'dragonfly']
-  'char-parallel.c',
+  chardev_ss.add(files('char-parallel.c'))
 endif


With that:
Reviewed-by: Marc-André Lureau 

Feel free to queue in the next build-sys/meson PR.

thanks

> +endif
> +
>  chardev_ss.add(when: 'CONFIG_WIN32', if_true: files(
>'char-console.c',
>'char-win-stdio.c',
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index b9c4307779c5..4886361be6a7 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -421,11 +421,6 @@ void qemu_anon_ram_free(void *ptr, size_t size);
>  #define HAVE_CHARDEV_SERIAL 1
>  #endif
>
> -#if defined(__linux__) || defined(__FreeBSD__) ||   \
> -defined(__FreeBSD_kernel__) || defined(__DragonFly__)
> -#define HAVE_CHARDEV_PARPORT 1
> -#endif
> -
>  #if defined(__HAIKU__)
>  #define SIGIO SIGPOLL
>  #endif
> --
> 2.38.1
>
>


-- 
Marc-André Lureau

[PATCH v5 11/11] hw/riscv/boot.c: make riscv_load_initrd() static

[Daniel Henrique Barboza]

The only remaining caller is riscv_load_kernel_and_initrd() which
belongs to the same file.

Signed-off-by: Daniel Henrique Barboza 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Bin Meng 
---
 hw/riscv/boot.c | 80 -
 include/hw/riscv/boot.h |  1 -
 2 files changed, 40 insertions(+), 41 deletions(-)

diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index 4888d5c1e0..e868fb6ade 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -173,6 +173,46 @@ target_ulong riscv_load_firmware(const char 
*firmware_filename,
 exit(1);
 }
 
+static void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry)
+{
+const char *filename = machine->initrd_filename;
+uint64_t mem_size = machine->ram_size;
+void *fdt = machine->fdt;
+hwaddr start, end;
+ssize_t size;
+
+g_assert(filename != NULL);
+
+/*
+ * We want to put the initrd far enough into RAM that when the
+ * kernel is uncompressed it will not clobber the initrd. However
+ * on boards without much RAM we must ensure that we still leave
+ * enough room for a decent sized initrd, and on boards with large
+ * amounts of RAM we must avoid the initrd being so far up in RAM
+ * that it is outside lowmem and inaccessible to the kernel.
+ * So for boards with less  than 256MB of RAM we put the initrd
+ * halfway into RAM, and for boards with 256MB of RAM or more we put
+ * the initrd at 128MB.
+ */
+start = kernel_entry + MIN(mem_size / 2, 128 * MiB);
+
+size = load_ramdisk(filename, start, mem_size - start);
+if (size == -1) {
+size = load_image_targphys(filename, start, mem_size - start);
+if (size == -1) {
+error_report("could not load ramdisk '%s'", filename);
+exit(1);
+}
+}
+
+/* Some RISC-V machines (e.g. opentitan) don't have a fdt. */
+if (fdt) {
+end = start + size;
+qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-start", start);
+qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-end", end);
+}
+}
+
 target_ulong riscv_load_kernel(MachineState *machine,
target_ulong kernel_start_addr,
bool load_initrd,
@@ -225,46 +265,6 @@ out:
 return kernel_entry;
 }
 
-void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry)
-{
-const char *filename = machine->initrd_filename;
-uint64_t mem_size = machine->ram_size;
-void *fdt = machine->fdt;
-hwaddr start, end;
-ssize_t size;
-
-g_assert(filename != NULL);
-
-/*
- * We want to put the initrd far enough into RAM that when the
- * kernel is uncompressed it will not clobber the initrd. However
- * on boards without much RAM we must ensure that we still leave
- * enough room for a decent sized initrd, and on boards with large
- * amounts of RAM we must avoid the initrd being so far up in RAM
- * that it is outside lowmem and inaccessible to the kernel.
- * So for boards with less  than 256MB of RAM we put the initrd
- * halfway into RAM, and for boards with 256MB of RAM or more we put
- * the initrd at 128MB.
- */
-start = kernel_entry + MIN(mem_size / 2, 128 * MiB);
-
-size = load_ramdisk(filename, start, mem_size - start);
-if (size == -1) {
-size = load_image_targphys(filename, start, mem_size - start);
-if (size == -1) {
-error_report("could not load ramdisk '%s'", filename);
-exit(1);
-}
-}
-
-/* Some RISC-V machines (e.g. opentitan) don't have a fdt. */
-if (fdt) {
-end = start + size;
-qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-start", start);
-qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-end", end);
-}
-}
-
 uint64_t riscv_load_fdt(hwaddr dram_base, uint64_t mem_size, void *fdt)
 {
 uint64_t temp, fdt_addr;
diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h
index c3de897371..cbd131bad7 100644
--- a/include/hw/riscv/boot.h
+++ b/include/hw/riscv/boot.h
@@ -47,7 +47,6 @@ target_ulong riscv_load_kernel(MachineState *machine,
target_ulong firmware_end_addr,
bool load_initrd,
symbol_fn_t sym_cb);
-void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry);
 uint64_t riscv_load_fdt(hwaddr dram_start, uint64_t dram_size, void *fdt);
 void riscv_setup_rom_reset_vec(MachineState *machine, RISCVHartArrayState 
*harts,
hwaddr saddr,
-- 
2.39.0

[PATCH v5 10/11] hw/riscv/boot.c: consolidate all kernel init in riscv_load_kernel()

[Daniel Henrique Barboza]

The microchip_icicle_kit, sifive_u, spike and virt boards are now doing
the same steps when '-kernel' is used:

- execute load_kernel()
- load init_rd()
- write kernel_cmdline

Let's fold everything inside riscv_load_kernel() to avoid code
repetition. To not change the behavior of boards that aren't calling
riscv_load_init(), add an 'load_initrd' flag to riscv_load_kernel() and
allow these boards to opt out from initrd loading.

Cc: Palmer Dabbelt 
Signed-off-by: Daniel Henrique Barboza 
---
 hw/riscv/boot.c| 22 +++---
 hw/riscv/microchip_pfsoc.c | 12 ++--
 hw/riscv/opentitan.c   |  2 +-
 hw/riscv/sifive_e.c|  3 ++-
 hw/riscv/sifive_u.c| 12 ++--
 hw/riscv/spike.c   | 11 +--
 hw/riscv/virt.c| 12 ++--
 include/hw/riscv/boot.h|  1 +
 8 files changed, 30 insertions(+), 45 deletions(-)

diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index 2594276223..4888d5c1e0 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -175,10 +175,12 @@ target_ulong riscv_load_firmware(const char 
*firmware_filename,
 
 target_ulong riscv_load_kernel(MachineState *machine,
target_ulong kernel_start_addr,
+   bool load_initrd,
symbol_fn_t sym_cb)
 {
 const char *kernel_filename = machine->kernel_filename;
 uint64_t kernel_load_base, kernel_entry;
+void *fdt = machine->fdt;
 
 g_assert(kernel_filename != NULL);
 
@@ -192,21 +194,35 @@ target_ulong riscv_load_kernel(MachineState *machine,
 if (load_elf_ram_sym(kernel_filename, NULL, NULL, NULL,
  NULL, _load_base, NULL, NULL, 0,
  EM_RISCV, 1, 0, NULL, true, sym_cb) > 0) {
-return kernel_load_base;
+kernel_entry = kernel_load_base;
+goto out;
 }
 
 if (load_uimage_as(kernel_filename, _entry, NULL, NULL,
NULL, NULL, NULL) > 0) {
-return kernel_entry;
+goto out;
 }
 
 if (load_image_targphys_as(kernel_filename, kernel_start_addr,
current_machine->ram_size, NULL) > 0) {
-return kernel_start_addr;
+kernel_entry = kernel_start_addr;
+goto out;
 }
 
 error_report("could not load kernel '%s'", kernel_filename);
 exit(1);
+
+out:
+if (load_initrd && machine->initrd_filename) {
+riscv_load_initrd(machine, kernel_entry);
+}
+
+if (fdt && machine->kernel_cmdline && *machine->kernel_cmdline) {
+qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
+machine->kernel_cmdline);
+}
+
+return kernel_entry;
 }
 
 void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry)
diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
index 82ae5e7023..c45023a2b1 100644
--- a/hw/riscv/microchip_pfsoc.c
+++ b/hw/riscv/microchip_pfsoc.c
@@ -629,16 +629,8 @@ static void microchip_icicle_kit_machine_init(MachineState 
*machine)
 kernel_start_addr = riscv_calc_kernel_start_addr(>soc.u_cpus,
  firmware_end_addr);
 
-kernel_entry = riscv_load_kernel(machine, kernel_start_addr, NULL);
-
-if (machine->initrd_filename) {
-riscv_load_initrd(machine, kernel_entry);
-}
-
-if (machine->kernel_cmdline && *machine->kernel_cmdline) {
-qemu_fdt_setprop_string(machine->fdt, "/chosen",
-"bootargs", machine->kernel_cmdline);
-}
+kernel_entry = riscv_load_kernel(machine, kernel_start_addr,
+ true, NULL);
 
 /* Compute the fdt load address in dram */
 fdt_load_addr = riscv_load_fdt(memmap[MICROCHIP_PFSOC_DRAM_LO].base,
diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
index 64d5d435b9..f6fd9725a5 100644
--- a/hw/riscv/opentitan.c
+++ b/hw/riscv/opentitan.c
@@ -101,7 +101,7 @@ static void opentitan_board_init(MachineState *machine)
 }
 
 if (machine->kernel_filename) {
-riscv_load_kernel(machine, memmap[IBEX_DEV_RAM].base, NULL);
+riscv_load_kernel(machine, memmap[IBEX_DEV_RAM].base, false, NULL);
 }
 }
 
diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
index 3e3f4b0088..6835d1c807 100644
--- a/hw/riscv/sifive_e.c
+++ b/hw/riscv/sifive_e.c
@@ -114,7 +114,8 @@ static void sifive_e_machine_init(MachineState *machine)
   memmap[SIFIVE_E_DEV_MROM].base, 
_space_memory);
 
 if (machine->kernel_filename) {
-riscv_load_kernel(machine, memmap[SIFIVE_E_DEV_DTIM].base, NULL);
+riscv_load_kernel(machine, memmap[SIFIVE_E_DEV_DTIM].base,
+  false, NULL);
 }
 }
 
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index bac394c959..9a75d4aa62 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -598,16 +598,8 

[PATCH v5 09/11] hw/riscv/boot.c: use MachineState in riscv_load_kernel()

[Daniel Henrique Barboza]

All callers are using kernel_filename as machine->kernel_filename.

This will also simplify the changes in riscv_load_kernel() that we're
going to do next.

Cc: Palmer Dabbelt 
Signed-off-by: Daniel Henrique Barboza 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Bin Meng 
---
 hw/riscv/boot.c| 3 ++-
 hw/riscv/microchip_pfsoc.c | 3 +--
 hw/riscv/opentitan.c   | 3 +--
 hw/riscv/sifive_e.c| 3 +--
 hw/riscv/sifive_u.c| 3 +--
 hw/riscv/spike.c   | 3 +--
 hw/riscv/virt.c| 3 +--
 include/hw/riscv/boot.h| 2 +-
 8 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index d3e780c3b6..2594276223 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -173,10 +173,11 @@ target_ulong riscv_load_firmware(const char 
*firmware_filename,
 exit(1);
 }
 
-target_ulong riscv_load_kernel(const char *kernel_filename,
+target_ulong riscv_load_kernel(MachineState *machine,
target_ulong kernel_start_addr,
symbol_fn_t sym_cb)
 {
+const char *kernel_filename = machine->kernel_filename;
 uint64_t kernel_load_base, kernel_entry;
 
 g_assert(kernel_filename != NULL);
diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
index 1e9b0a420e..82ae5e7023 100644
--- a/hw/riscv/microchip_pfsoc.c
+++ b/hw/riscv/microchip_pfsoc.c
@@ -629,8 +629,7 @@ static void microchip_icicle_kit_machine_init(MachineState 
*machine)
 kernel_start_addr = riscv_calc_kernel_start_addr(>soc.u_cpus,
  firmware_end_addr);
 
-kernel_entry = riscv_load_kernel(machine->kernel_filename,
- kernel_start_addr, NULL);
+kernel_entry = riscv_load_kernel(machine, kernel_start_addr, NULL);
 
 if (machine->initrd_filename) {
 riscv_load_initrd(machine, kernel_entry);
diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
index 85ffdac5be..64d5d435b9 100644
--- a/hw/riscv/opentitan.c
+++ b/hw/riscv/opentitan.c
@@ -101,8 +101,7 @@ static void opentitan_board_init(MachineState *machine)
 }
 
 if (machine->kernel_filename) {
-riscv_load_kernel(machine->kernel_filename,
-  memmap[IBEX_DEV_RAM].base, NULL);
+riscv_load_kernel(machine, memmap[IBEX_DEV_RAM].base, NULL);
 }
 }
 
diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
index d65d2fd869..3e3f4b0088 100644
--- a/hw/riscv/sifive_e.c
+++ b/hw/riscv/sifive_e.c
@@ -114,8 +114,7 @@ static void sifive_e_machine_init(MachineState *machine)
   memmap[SIFIVE_E_DEV_MROM].base, 
_space_memory);
 
 if (machine->kernel_filename) {
-riscv_load_kernel(machine->kernel_filename,
-  memmap[SIFIVE_E_DEV_DTIM].base, NULL);
+riscv_load_kernel(machine, memmap[SIFIVE_E_DEV_DTIM].base, NULL);
 }
 }
 
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index c40885ed5c..bac394c959 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -598,8 +598,7 @@ static void sifive_u_machine_init(MachineState *machine)
 kernel_start_addr = riscv_calc_kernel_start_addr(>soc.u_cpus,
  firmware_end_addr);
 
-kernel_entry = riscv_load_kernel(machine->kernel_filename,
- kernel_start_addr, NULL);
+kernel_entry = riscv_load_kernel(machine, kernel_start_addr, NULL);
 
 if (machine->initrd_filename) {
 riscv_load_initrd(machine, kernel_entry);
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 99dec74fe8..bff9475686 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -307,8 +307,7 @@ static void spike_board_init(MachineState *machine)
 kernel_start_addr = riscv_calc_kernel_start_addr(>soc[0],
  firmware_end_addr);
 
-kernel_entry = riscv_load_kernel(machine->kernel_filename,
- kernel_start_addr,
+kernel_entry = riscv_load_kernel(machine, kernel_start_addr,
  htif_symbol_callback);
 
 if (machine->initrd_filename) {
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 02f1369843..c8e35f861e 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -1281,8 +1281,7 @@ static void virt_machine_done(Notifier *notifier, void 
*data)
 kernel_start_addr = riscv_calc_kernel_start_addr(>soc[0],
  firmware_end_addr);
 
-kernel_entry = riscv_load_kernel(machine->kernel_filename,
- kernel_start_addr, NULL);
+kernel_entry = riscv_load_kernel(machine, kernel_start_addr, NULL);
 
 if (machine->initrd_filename) {
 riscv_load_initrd(machine, kernel_entry);
diff --git 

[PATCH v5 07/11] hw/riscv: write bootargs 'chosen' FDT after riscv_load_kernel()

[Daniel Henrique Barboza]

The sifive_u, spike and virt machines are writing the 'bootargs' FDT
node during their respective create_fdt().

Given that bootargs is written only when '-append' is used, and this
option is only allowed with the '-kernel' option, which in turn is
already being check before executing riscv_load_kernel(), write
'bootargs' in the same code path as riscv_load_kernel().

Cc: Palmer Dabbelt 
Signed-off-by: Daniel Henrique Barboza 
Reviewed-by: Bin Meng 
Reviewed-by: Philippe Mathieu-Daudé 
---
 hw/riscv/sifive_u.c | 11 +--
 hw/riscv/spike.c|  9 +
 hw/riscv/virt.c | 11 +--
 3 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 37f5087172..3e6df87b5b 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -117,7 +117,6 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry 
*memmap,
 error_report("load_device_tree() failed");
 exit(1);
 }
-goto update_bootargs;
 } else {
 fdt = ms->fdt = create_device_tree(_size);
 if (!fdt) {
@@ -510,11 +509,6 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry 
*memmap,
 qemu_fdt_setprop_string(fdt, "/aliases", "serial0", nodename);
 
 g_free(nodename);
-
-update_bootargs:
-if (cmdline && *cmdline) {
-qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline);
-}
 }
 
 static void sifive_u_machine_reset(void *opaque, int n, int level)
@@ -611,6 +605,11 @@ static void sifive_u_machine_init(MachineState *machine)
 riscv_load_initrd(machine->initrd_filename, machine->ram_size,
   kernel_entry, machine->fdt);
 }
+
+if (machine->kernel_cmdline && *machine->kernel_cmdline) {
+qemu_fdt_setprop_string(machine->fdt, "/chosen", "bootargs",
+machine->kernel_cmdline);
+}
 } else {
/*
 * If dynamic firmware is used, it doesn't know where is the next mode
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 5668fe0694..60e2912be5 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -179,10 +179,6 @@ static void create_fdt(SpikeState *s, const MemMapEntry 
*memmap,
 
 qemu_fdt_add_subnode(fdt, "/chosen");
 qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", "/htif");
-
-if (cmdline && *cmdline) {
-qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline);
-}
 }
 
 static bool spike_test_elf_image(char *filename)
@@ -319,6 +315,11 @@ static void spike_board_init(MachineState *machine)
 riscv_load_initrd(machine->initrd_filename, machine->ram_size,
   kernel_entry, machine->fdt);
 }
+
+if (machine->kernel_cmdline && *machine->kernel_cmdline) {
+qemu_fdt_setprop_string(machine->fdt, "/chosen", "bootargs",
+machine->kernel_cmdline);
+}
 } else {
/*
 * If dynamic firmware is used, it doesn't know where is the next mode
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 5967b136b4..6c946b6def 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -1012,7 +1012,6 @@ static void create_fdt(RISCVVirtState *s, const 
MemMapEntry *memmap,
 error_report("load_device_tree() failed");
 exit(1);
 }
-goto update_bootargs;
 } else {
 mc->fdt = create_device_tree(>fdt_size);
 if (!mc->fdt) {
@@ -1050,11 +1049,6 @@ static void create_fdt(RISCVVirtState *s, const 
MemMapEntry *memmap,
 create_fdt_fw_cfg(s, memmap);
 create_fdt_pmu(s);
 
-update_bootargs:
-if (cmdline && *cmdline) {
-qemu_fdt_setprop_string(mc->fdt, "/chosen", "bootargs", cmdline);
-}
-
 /* Pass seed to RNG */
 qemu_guest_getrandom_nofail(rng_seed, sizeof(rng_seed));
 qemu_fdt_setprop(mc->fdt, "/chosen", "rng-seed", rng_seed, 
sizeof(rng_seed));
@@ -1294,6 +1288,11 @@ static void virt_machine_done(Notifier *notifier, void 
*data)
 riscv_load_initrd(machine->initrd_filename, machine->ram_size,
   kernel_entry, machine->fdt);
 }
+
+if (machine->kernel_cmdline && *machine->kernel_cmdline) {
+qemu_fdt_setprop_string(machine->fdt, "/chosen", "bootargs",
+machine->kernel_cmdline);
+}
 } else {
/*
 * If dynamic firmware is used, it doesn't know where is the next mode
-- 
2.39.0

[PATCH v5 03/11] hw/riscv/sifive_u: use 'fdt' from MachineState

[Daniel Henrique Barboza]

The MachineState object provides a 'fdt' pointer that is already being
used by other RISC-V machines, and it's also used by the 'dumpdtb' QMP
command.

Remove the 'fdt' pointer from SiFiveUState and use MachineState::fdt
instead.

Cc: Palmer Dabbelt 
Signed-off-by: Daniel Henrique Barboza 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Alistair Francis 
Reviewed-by: Bin Meng 
---
 hw/riscv/sifive_u.c | 15 ++-
 include/hw/riscv/sifive_u.h |  3 ---
 2 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index a58ddb36ac..ddceb750ea 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -98,7 +98,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry 
*memmap,
 {
 MachineState *ms = MACHINE(qdev_get_machine());
 void *fdt;
-int cpu;
+int cpu, fdt_size;
 uint32_t *cells;
 char *nodename;
 uint32_t plic_phandle, prci_phandle, gpio_phandle, phandle = 1;
@@ -112,14 +112,14 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry 
*memmap,
 };
 
 if (ms->dtb) {
-fdt = s->fdt = load_device_tree(ms->dtb, >fdt_size);
+fdt = ms->fdt = load_device_tree(ms->dtb, _size);
 if (!fdt) {
 error_report("load_device_tree() failed");
 exit(1);
 }
 goto update_bootargs;
 } else {
-fdt = s->fdt = create_device_tree(>fdt_size);
+fdt = ms->fdt = create_device_tree(_size);
 if (!fdt) {
 error_report("create_device_tree() failed");
 exit(1);
@@ -612,9 +612,9 @@ static void sifive_u_machine_init(MachineState *machine)
 hwaddr end = riscv_load_initrd(machine->initrd_filename,
machine->ram_size, kernel_entry,
);
-qemu_fdt_setprop_cell(s->fdt, "/chosen",
+qemu_fdt_setprop_cell(machine->fdt, "/chosen",
   "linux,initrd-start", start);
-qemu_fdt_setprop_cell(s->fdt, "/chosen", "linux,initrd-end",
+qemu_fdt_setprop_cell(machine->fdt, "/chosen", "linux,initrd-end",
   end);
 }
 } else {
@@ -627,14 +627,11 @@ static void sifive_u_machine_init(MachineState *machine)
 
 /* Compute the fdt load address in dram */
 fdt_load_addr = riscv_load_fdt(memmap[SIFIVE_U_DEV_DRAM].base,
-   machine->ram_size, s->fdt);
+   machine->ram_size, machine->fdt);
 if (!riscv_is_32bit(>soc.u_cpus)) {
 start_addr_hi32 = (uint64_t)start_addr >> 32;
 }
 
-/* Set machine->fdt for 'dumpdtb' QMP/HMP command */
-machine->fdt = s->fdt;
-
 /* reset vector */
 uint32_t reset_vec[12] = {
 s->msel,   /* MSEL pin state */
diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
index e680d61ece..4a8828a30e 100644
--- a/include/hw/riscv/sifive_u.h
+++ b/include/hw/riscv/sifive_u.h
@@ -67,9 +67,6 @@ typedef struct SiFiveUState {
 /*< public >*/
 SiFiveUSoCState soc;
 
-void *fdt;
-int fdt_size;
-
 bool start_in_flash;
 uint32_t msel;
 uint32_t serial;
-- 
2.39.0

[PATCH v5 05/11] hw/riscv/spike.c: load initrd right after riscv_load_kernel()

[Daniel Henrique Barboza]

This will make the code more in line with what the other boards are
doing. We'll also avoid an extra check to machine->kernel_filename since
we already checked that before executing riscv_load_kernel().

Signed-off-by: Daniel Henrique Barboza 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Alistair Francis 
Reviewed-by: Bin Meng 
---
 hw/riscv/spike.c | 31 +++
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 25c5420ee6..004dfb2d5b 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -302,6 +302,10 @@ static void spike_board_init(MachineState *machine)
 g_free(firmware_name);
 }
 
+/* Create device tree */
+create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline,
+   riscv_is_32bit(>soc[0]), htif_custom_base);
+
 /* Load kernel */
 if (machine->kernel_filename) {
 kernel_start_addr = riscv_calc_kernel_start_addr(>soc[0],
@@ -310,6 +314,17 @@ static void spike_board_init(MachineState *machine)
 kernel_entry = riscv_load_kernel(machine->kernel_filename,
  kernel_start_addr,
  htif_symbol_callback);
+
+if (machine->initrd_filename) {
+hwaddr start;
+hwaddr end = riscv_load_initrd(machine->initrd_filename,
+   machine->ram_size, kernel_entry,
+   );
+qemu_fdt_setprop_cell(machine->fdt, "/chosen",
+  "linux,initrd-start", start);
+qemu_fdt_setprop_cell(machine->fdt, "/chosen", "linux,initrd-end",
+  end);
+}
 } else {
/*
 * If dynamic firmware is used, it doesn't know where is the next mode
@@ -318,22 +333,6 @@ static void spike_board_init(MachineState *machine)
 kernel_entry = 0;
 }
 
-/* Create device tree */
-create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline,
-   riscv_is_32bit(>soc[0]), htif_custom_base);
-
-/* Load initrd */
-if (machine->kernel_filename && machine->initrd_filename) {
-hwaddr start;
-hwaddr end = riscv_load_initrd(machine->initrd_filename,
-   machine->ram_size, kernel_entry,
-   );
-qemu_fdt_setprop_cell(machine->fdt, "/chosen",
-  "linux,initrd-start", start);
-qemu_fdt_setprop_cell(machine->fdt, "/chosen", "linux,initrd-end",
-  end);
-}
-
 /* Compute the fdt load address in dram */
 fdt_load_addr = riscv_load_fdt(memmap[SPIKE_DRAM].base,
machine->ram_size, machine->fdt);
-- 
2.39.0

[PATCH v5 08/11] hw/riscv/boot.c: use MachineState in riscv_load_initrd()

[Daniel Henrique Barboza]

'filename', 'mem_size' and 'fdt' from riscv_load_initrd() can all be
retrieved by the MachineState object for all callers.

Cc: Palmer Dabbelt 
Signed-off-by: Daniel Henrique Barboza 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Bin Meng 
---
 hw/riscv/boot.c| 6 --
 hw/riscv/microchip_pfsoc.c | 3 +--
 hw/riscv/sifive_u.c| 3 +--
 hw/riscv/spike.c   | 3 +--
 hw/riscv/virt.c| 3 +--
 include/hw/riscv/boot.h| 3 +--
 6 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index 6b948d1c9e..d3e780c3b6 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -208,9 +208,11 @@ target_ulong riscv_load_kernel(const char *kernel_filename,
 exit(1);
 }
 
-void riscv_load_initrd(const char *filename, uint64_t mem_size,
-   uint64_t kernel_entry, void *fdt)
+void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry)
 {
+const char *filename = machine->initrd_filename;
+uint64_t mem_size = machine->ram_size;
+void *fdt = machine->fdt;
 hwaddr start, end;
 ssize_t size;
 
diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
index 593a799549..1e9b0a420e 100644
--- a/hw/riscv/microchip_pfsoc.c
+++ b/hw/riscv/microchip_pfsoc.c
@@ -633,8 +633,7 @@ static void microchip_icicle_kit_machine_init(MachineState 
*machine)
  kernel_start_addr, NULL);
 
 if (machine->initrd_filename) {
-riscv_load_initrd(machine->initrd_filename, machine->ram_size,
-  kernel_entry, machine->fdt);
+riscv_load_initrd(machine, kernel_entry);
 }
 
 if (machine->kernel_cmdline && *machine->kernel_cmdline) {
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 3e6df87b5b..c40885ed5c 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -602,8 +602,7 @@ static void sifive_u_machine_init(MachineState *machine)
  kernel_start_addr, NULL);
 
 if (machine->initrd_filename) {
-riscv_load_initrd(machine->initrd_filename, machine->ram_size,
-  kernel_entry, machine->fdt);
+riscv_load_initrd(machine, kernel_entry);
 }
 
 if (machine->kernel_cmdline && *machine->kernel_cmdline) {
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 60e2912be5..99dec74fe8 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -312,8 +312,7 @@ static void spike_board_init(MachineState *machine)
  htif_symbol_callback);
 
 if (machine->initrd_filename) {
-riscv_load_initrd(machine->initrd_filename, machine->ram_size,
-  kernel_entry, machine->fdt);
+riscv_load_initrd(machine, kernel_entry);
 }
 
 if (machine->kernel_cmdline && *machine->kernel_cmdline) {
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 6c946b6def..02f1369843 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -1285,8 +1285,7 @@ static void virt_machine_done(Notifier *notifier, void 
*data)
  kernel_start_addr, NULL);
 
 if (machine->initrd_filename) {
-riscv_load_initrd(machine->initrd_filename, machine->ram_size,
-  kernel_entry, machine->fdt);
+riscv_load_initrd(machine, kernel_entry);
 }
 
 if (machine->kernel_cmdline && *machine->kernel_cmdline) {
diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h
index e37e1d1238..cfd72ecabf 100644
--- a/include/hw/riscv/boot.h
+++ b/include/hw/riscv/boot.h
@@ -46,8 +46,7 @@ target_ulong riscv_load_firmware(const char 
*firmware_filename,
 target_ulong riscv_load_kernel(const char *kernel_filename,
target_ulong firmware_end_addr,
symbol_fn_t sym_cb);
-void riscv_load_initrd(const char *filename, uint64_t mem_size,
-   uint64_t kernel_entry, void *fdt);
+void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry);
 uint64_t riscv_load_fdt(hwaddr dram_start, uint64_t dram_size, void *fdt);
 void riscv_setup_rom_reset_vec(MachineState *machine, RISCVHartArrayState 
*harts,
hwaddr saddr,
-- 
2.39.0

[PATCH v5 02/11] hw/riscv/spike: use 'fdt' from MachineState

[Daniel Henrique Barboza]

The MachineState object provides a 'fdt' pointer that is already being
used by other RISC-V machines, and it's also used by the 'dumpdtb' QMP
command.

Remove the 'fdt' pointer from SpikeState and use MachineState::fdt
instead.

Signed-off-by: Daniel Henrique Barboza 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Alistair Francis 
Reviewed-by: Bin Meng 
---
 hw/riscv/spike.c | 12 +---
 include/hw/riscv/spike.h |  2 --
 2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 1679c325d5..25c5420ee6 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -53,6 +53,7 @@ static void create_fdt(SpikeState *s, const MemMapEntry 
*memmap,
bool is_32_bit, bool htif_custom_base)
 {
 void *fdt;
+int fdt_size;
 uint64_t addr, size;
 unsigned long clint_addr;
 int cpu, socket;
@@ -65,7 +66,7 @@ static void create_fdt(SpikeState *s, const MemMapEntry 
*memmap,
 "sifive,clint0", "riscv,clint0"
 };
 
-fdt = s->fdt = create_device_tree(>fdt_size);
+fdt = mc->fdt = create_device_tree(_size);
 if (!fdt) {
 error_report("create_device_tree() failed");
 exit(1);
@@ -327,18 +328,15 @@ static void spike_board_init(MachineState *machine)
 hwaddr end = riscv_load_initrd(machine->initrd_filename,
machine->ram_size, kernel_entry,
);
-qemu_fdt_setprop_cell(s->fdt, "/chosen",
+qemu_fdt_setprop_cell(machine->fdt, "/chosen",
   "linux,initrd-start", start);
-qemu_fdt_setprop_cell(s->fdt, "/chosen", "linux,initrd-end",
+qemu_fdt_setprop_cell(machine->fdt, "/chosen", "linux,initrd-end",
   end);
 }
 
 /* Compute the fdt load address in dram */
 fdt_load_addr = riscv_load_fdt(memmap[SPIKE_DRAM].base,
-   machine->ram_size, s->fdt);
-
-/* Set machine->fdt for 'dumpdtb' QMP/HMP command */
-machine->fdt = s->fdt;
+   machine->ram_size, machine->fdt);
 
 /* load the reset vector */
 riscv_setup_rom_reset_vec(machine, >soc[0], memmap[SPIKE_DRAM].base,
diff --git a/include/hw/riscv/spike.h b/include/hw/riscv/spike.h
index 73d69234de..d13a147942 100644
--- a/include/hw/riscv/spike.h
+++ b/include/hw/riscv/spike.h
@@ -37,8 +37,6 @@ struct SpikeState {
 
 /*< public >*/
 RISCVHartArrayState soc[SPIKE_SOCKETS_MAX];
-void *fdt;
-int fdt_size;
 };
 
 enum {
-- 
2.39.0

[PATCH v5 04/11] hw/riscv/boot.c: exit early if filename is NULL in load functions

[Daniel Henrique Barboza]

riscv_load_firmware(), riscv_load_initrd() and riscv_load_kernel() works
under the assumption that a 'filename' parameter is always not NULL.

This is currently the case since all callers of these functions are
checking for NULL before calling them. Add an g_assert() to make sure
that a NULL value in these cases are to be considered a bug.

Suggested-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Daniel Henrique Barboza 
---
 hw/riscv/boot.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index 98b80af51b..31aa3385a0 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -153,6 +153,8 @@ target_ulong riscv_load_firmware(const char 
*firmware_filename,
 uint64_t firmware_entry, firmware_end;
 ssize_t firmware_size;
 
+g_assert(firmware_filename != NULL);
+
 if (load_elf_ram_sym(firmware_filename, NULL, NULL, NULL,
  _entry, NULL, _end, NULL,
  0, EM_RISCV, 1, 0, NULL, true, sym_cb) > 0) {
@@ -177,6 +179,8 @@ target_ulong riscv_load_kernel(const char *kernel_filename,
 {
 uint64_t kernel_load_base, kernel_entry;
 
+g_assert(kernel_filename != NULL);
+
 /*
  * NB: Use low address not ELF entry point to ensure that the fw_dynamic
  * behaviour when loading an ELF matches the fw_payload, fw_jump and BBL
@@ -209,6 +213,8 @@ hwaddr riscv_load_initrd(const char *filename, uint64_t 
mem_size,
 {
 ssize_t size;
 
+g_assert(filename != NULL);
+
 /*
  * We want to put the initrd far enough into RAM that when the
  * kernel is uncompressed it will not clobber the initrd. However
-- 
2.39.0

[PATCH v5 00/11] riscv: OpenSBI boot test and cleanups

[Daniel Henrique Barboza]

Hi,

This new version is still rebased on top of [1]:

"[PATCH v2 00/12] hw/riscv: Improve Spike HTIF emulation fidelity"

from Bin Meng.

The change from v4 is on patch 9 where we added an extra flag in
riscv_load_kernel() to allow for boards that don't load initrd
(e.g. opentitan and sifive_e) to opt out from loading it altogether.

* Patch without reviews: 9

Changes from v4:
- patch 9:
  - added a 'load_init' flag in riscv_load_kernel() to control whether
the function should execute riscv_load_initrd() or not
v4 link: https://lists.gnu.org/archive/html/qemu-devel/2022-12/msg04652.html

Changes from v3:
- patch 1:
  - fixed more instances of 'opensbi' and 'Opensbi' to 'OpenSBI'
  - changed tests order
- patch 4 (new):
  - added a g_assert(filename) guard in riscv_load_initrd() and
riscv_load_kernel()
v3 link: https://mail.gnu.org/archive/html/qemu-devel/2022-12/msg04491.html 

Changes from v2:
- patch 1:
  - reduced code repetition with a boot_opensbi() helper
  - renamed 'opensbi' to 'OpenSBI' in the file header
- patch 9:
  - renamed riscv_load_kernel() to riscv_load_kernel_and_initrd()
v2 link: https://mail.gnu.org/archive/html/qemu-devel/2022-12/msg04466.html


Changes from v1:
- patches were rebased with [1]
- patches 13-15: removed
  * will be re-sent in a follow-up series
- patches 4-5: removed since they're picked by Bin in [1]
- patch 1:
  - added a 'skip' riscv32 spike test
v1 link: https://mail.gnu.org/archive/html/qemu-devel/2022-12/msg03860.html


Based-on: <20221227064812.1903326-1-bm...@tinylab.org>

Cc: Alistair Francis 
Cc: Bin Meng 

[1] https://patchwork.ozlabs.org/project/qemu-devel/list/?series=334352

Daniel Henrique Barboza (11):
  tests/avocado: add RISC-V OpenSBI boot test
  hw/riscv/spike: use 'fdt' from MachineState
  hw/riscv/sifive_u: use 'fdt' from MachineState
  hw/riscv/boot.c: exit early if filename is NULL in load functions
  hw/riscv/spike.c: load initrd right after riscv_load_kernel()
  hw/riscv: write initrd 'chosen' FDT inside riscv_load_initrd()
  hw/riscv: write bootargs 'chosen' FDT after riscv_load_kernel()
  hw/riscv/boot.c: use MachineState in riscv_load_initrd()
  hw/riscv/boot.c: use MachineState in riscv_load_kernel()
  hw/riscv/boot.c: consolidate all kernel init in riscv_load_kernel()
  hw/riscv/boot.c: make riscv_load_initrd() static

 hw/riscv/boot.c| 91 +++---
 hw/riscv/microchip_pfsoc.c | 20 +---
 hw/riscv/opentitan.c   |  3 +-
 hw/riscv/sifive_e.c|  4 +-
 hw/riscv/sifive_u.c| 32 +++-
 hw/riscv/spike.c   | 37 --
 hw/riscv/virt.c| 21 +---
 include/hw/riscv/boot.h|  5 +-
 include/hw/riscv/sifive_u.h|  3 --
 include/hw/riscv/spike.h   |  2 -
 tests/avocado/riscv_opensbi.py | 65 
 11 files changed, 150 insertions(+), 133 deletions(-)
 create mode 100644 tests/avocado/riscv_opensbi.py

-- 
2.39.0

[PATCH v5 06/11] hw/riscv: write initrd 'chosen' FDT inside riscv_load_initrd()

[Daniel Henrique Barboza]

riscv_load_initrd() returns the initrd end addr while also writing a
'start' var to mark the addr start. These informations are being used
just to write the initrd FDT node. Every existing caller of
riscv_load_initrd() is writing the FDT in the same manner.

We can simplify things by writing the FDT inside riscv_load_initrd(),
sparing callers from having to manage start/end addrs to write the FDT
themselves.

An 'if (fdt)' check is already inserted at the end of the function
because we'll end up using it later on with other boards that doesn´t
have a FDT.

Cc: Palmer Dabbelt 
Signed-off-by: Daniel Henrique Barboza 
Reviewed-by: Bin Meng 
Reviewed-by: Philippe Mathieu-Daudé 
---
 hw/riscv/boot.c| 18 --
 hw/riscv/microchip_pfsoc.c | 10 ++
 hw/riscv/sifive_u.c| 10 ++
 hw/riscv/spike.c   | 10 ++
 hw/riscv/virt.c| 10 ++
 include/hw/riscv/boot.h|  4 ++--
 6 files changed, 22 insertions(+), 40 deletions(-)

diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index 31aa3385a0..6b948d1c9e 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -208,9 +208,10 @@ target_ulong riscv_load_kernel(const char *kernel_filename,
 exit(1);
 }
 
-hwaddr riscv_load_initrd(const char *filename, uint64_t mem_size,
- uint64_t kernel_entry, hwaddr *start)
+void riscv_load_initrd(const char *filename, uint64_t mem_size,
+   uint64_t kernel_entry, void *fdt)
 {
+hwaddr start, end;
 ssize_t size;
 
 g_assert(filename != NULL);
@@ -226,18 +227,23 @@ hwaddr riscv_load_initrd(const char *filename, uint64_t 
mem_size,
  * halfway into RAM, and for boards with 256MB of RAM or more we put
  * the initrd at 128MB.
  */
-*start = kernel_entry + MIN(mem_size / 2, 128 * MiB);
+start = kernel_entry + MIN(mem_size / 2, 128 * MiB);
 
-size = load_ramdisk(filename, *start, mem_size - *start);
+size = load_ramdisk(filename, start, mem_size - start);
 if (size == -1) {
-size = load_image_targphys(filename, *start, mem_size - *start);
+size = load_image_targphys(filename, start, mem_size - start);
 if (size == -1) {
 error_report("could not load ramdisk '%s'", filename);
 exit(1);
 }
 }
 
-return *start + size;
+/* Some RISC-V machines (e.g. opentitan) don't have a fdt. */
+if (fdt) {
+end = start + size;
+qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-start", start);
+qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-end", end);
+}
 }
 
 uint64_t riscv_load_fdt(hwaddr dram_base, uint64_t mem_size, void *fdt)
diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
index b10321b564..593a799549 100644
--- a/hw/riscv/microchip_pfsoc.c
+++ b/hw/riscv/microchip_pfsoc.c
@@ -633,14 +633,8 @@ static void microchip_icicle_kit_machine_init(MachineState 
*machine)
  kernel_start_addr, NULL);
 
 if (machine->initrd_filename) {
-hwaddr start;
-hwaddr end = riscv_load_initrd(machine->initrd_filename,
-   machine->ram_size, kernel_entry,
-   );
-qemu_fdt_setprop_cell(machine->fdt, "/chosen",
-  "linux,initrd-start", start);
-qemu_fdt_setprop_cell(machine->fdt, "/chosen",
-  "linux,initrd-end", end);
+riscv_load_initrd(machine->initrd_filename, machine->ram_size,
+  kernel_entry, machine->fdt);
 }
 
 if (machine->kernel_cmdline && *machine->kernel_cmdline) {
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index ddceb750ea..37f5087172 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -608,14 +608,8 @@ static void sifive_u_machine_init(MachineState *machine)
  kernel_start_addr, NULL);
 
 if (machine->initrd_filename) {
-hwaddr start;
-hwaddr end = riscv_load_initrd(machine->initrd_filename,
-   machine->ram_size, kernel_entry,
-   );
-qemu_fdt_setprop_cell(machine->fdt, "/chosen",
-  "linux,initrd-start", start);
-qemu_fdt_setprop_cell(machine->fdt, "/chosen", "linux,initrd-end",
-  end);
+riscv_load_initrd(machine->initrd_filename, machine->ram_size,
+  kernel_entry, machine->fdt);
 }
 } else {
/*
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 004dfb2d5b..5668fe0694 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -316,14 +316,8 @@ static void spike_board_init(MachineState *machine)
  htif_symbol_callback);
 

[PATCH v5 01/11] tests/avocado: add RISC-V OpenSBI boot test

[Daniel Henrique Barboza]

This test is used to do a quick sanity check to ensure that we're able
to run the existing QEMU FW image.

'sifive_u', 'spike' and 'virt' riscv64 machines, and 'sifive_u' and
'virt' 32 bit machines are able to run the default RISCV64_BIOS_BIN |
RISCV32_BIOS_BIN firmware with minimal options.

The riscv32 'spike' machine isn't bootable at this moment, requiring an
OpenSBI fix [1] and QEMU side changes [2]. We could just leave at that
or add a 'skip' test to remind us about it. To work as a reminder that
we have a riscv32 'spike' test that should be enabled as soon as OpenSBI
QEMU rom receives the fix, we're adding a 'skip' test:

(06/18) tests/avocado/riscv_opensbi.py:RiscvOpenSBI.test_riscv32_spike:
SKIP: requires OpenSBI fix to work

[1] 
https://patchwork.ozlabs.org/project/opensbi/patch/20221226033603.1860569-1-bm...@tinylab.org/
[2] https://patchwork.ozlabs.org/project/qemu-devel/list/?series=334159

Cc: Cleber Rosa 
Cc: Philippe Mathieu-Daudé 
Reviewed-by: Bin Meng 
Tested-by: Bin Meng 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Daniel Henrique Barboza 
---
 tests/avocado/riscv_opensbi.py | 65 ++
 1 file changed, 65 insertions(+)
 create mode 100644 tests/avocado/riscv_opensbi.py

diff --git a/tests/avocado/riscv_opensbi.py b/tests/avocado/riscv_opensbi.py
new file mode 100644
index 00..e02f0d404a
--- /dev/null
+++ b/tests/avocado/riscv_opensbi.py
@@ -0,0 +1,65 @@
+# OpenSBI boot test for RISC-V machines
+#
+# Copyright (c) 2022, Ventana Micro
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later.  See the COPYING file in the top-level directory.
+
+from avocado_qemu import QemuSystemTest
+from avocado import skip
+from avocado_qemu import wait_for_console_pattern
+
+class RiscvOpenSBI(QemuSystemTest):
+"""
+:avocado: tags=accel:tcg
+"""
+timeout = 5
+
+def boot_opensbi(self):
+self.vm.set_console()
+self.vm.launch()
+wait_for_console_pattern(self, 'Platform Name')
+wait_for_console_pattern(self, 'Boot HART MEDELEG')
+
+@skip("requires OpenSBI fix to work")
+def test_riscv32_spike(self):
+"""
+:avocado: tags=arch:riscv32
+:avocado: tags=machine:spike
+"""
+self.boot_opensbi()
+
+def test_riscv64_spike(self):
+"""
+:avocado: tags=arch:riscv64
+:avocado: tags=machine:spike
+"""
+self.boot_opensbi()
+
+def test_riscv32_sifive_u(self):
+"""
+:avocado: tags=arch:riscv32
+:avocado: tags=machine:sifive_u
+"""
+self.boot_opensbi()
+
+def test_riscv64_sifive_u(self):
+"""
+:avocado: tags=arch:riscv64
+:avocado: tags=machine:sifive_u
+"""
+self.boot_opensbi()
+
+def test_riscv32_virt(self):
+"""
+:avocado: tags=arch:riscv32
+:avocado: tags=machine:virt
+"""
+self.boot_opensbi()
+
+def test_riscv64_virt(self):
+"""
+:avocado: tags=arch:riscv64
+:avocado: tags=machine:virt
+"""
+self.boot_opensbi()
-- 
2.39.0

Re: [PATCH RFC 3/4] vdagent: add live migration support

[Marc-André Lureau]

Hi


On Fri, Dec 30, 2022 at 6:49 PM  wrote:
>
> From: "dengp...@chinatelecom.cn" 
>
> To support live migration, we made the following 2 modifications:
> 1. save the caps field of VDAgentChardev.
> 2. register vdagent to qemu-clipboard after
>vm device state being reloaded during live migration.
>
> Signed-off-by: dengp...@chinatelecom.cn 
> Signed-off-by: liuy...@chinatelecom.cn 
> ---
>  ui/trace-events |  1 +
>  ui/vdagent.c| 28 
>  2 files changed, 29 insertions(+)
>
> diff --git a/ui/trace-events b/ui/trace-events
> index 5e50b60da5..ccacd867d1 100644
> --- a/ui/trace-events
> +++ b/ui/trace-events
> @@ -144,6 +144,7 @@ vdagent_cb_grab_discard(const char *name, int cur, int 
> recv) "selection %s, cur:
>  vdagent_cb_grab_type(const char *name) "type %s"
>  vdagent_cb_serial_discard(uint32_t current, uint32_t received) "current=%u, 
> received=%u"
>  vdagent_recv_caps(uint32_t caps) "received caps %u"
> +vdagent_migration_caps(uint32_t caps) "migrated caps %u"
>
>  # dbus.c
>  dbus_registered_listener(const char *bus_name) "peer %s"
> diff --git a/ui/vdagent.c b/ui/vdagent.c
> index 38061d5b38..1193abe348 100644
> --- a/ui/vdagent.c
> +++ b/ui/vdagent.c
> @@ -6,6 +6,7 @@
>  #include "qemu/units.h"
>  #include "hw/qdev-core.h"
>  #include "migration/blocker.h"
> +#include "migration/vmstate.h"
>  #include "ui/clipboard.h"
>  #include "ui/console.h"
>  #include "ui/input.h"
> @@ -906,6 +907,31 @@ static void vdagent_chr_parse(QemuOpts *opts, 
> ChardevBackend *backend,
>
>  /* -- */
>
> +static int vdagent_post_load(void *opaque, int version_id)
> +{
> +VDAgentChardev *vd = QEMU_VDAGENT_CHARDEV(opaque);
> +
> +trace_vdagent_migration_caps(vd->caps);
> +
> +if (vd->caps) {
> +vdagent_register_to_qemu_clipboard(vd);
> +qemu_input_handler_activate(vd->mouse_hs);
> +}
> +
> +return 0;
> +}
> +
> +static const VMStateDescription vmstate_vdagent = {
> +.name = "vdagent",
> +.version_id = 1,
> +.minimum_version_id = 1,
> +.post_load = vdagent_post_load,
> +.fields = (VMStateField[]) {
> +VMSTATE_UINT32(caps, VDAgentChardev),

You are missing a lot of states from VDAgentChardev. Most of the
fields must be saved/restored.

> +VMSTATE_END_OF_LIST()
> +},
> +};
> +
>  static void vdagent_chr_class_init(ObjectClass *oc, void *data)
>  {
>  ChardevClass *cc = CHARDEV_CLASS(oc);
> @@ -922,6 +948,8 @@ static void vdagent_chr_init(Object *obj)
>  VDAgentChardev *vd = QEMU_VDAGENT_CHARDEV(obj);
>
>  buffer_init(>outbuf, "vdagent-outbuf");
> +
> +vmstate_register(NULL, 0, _vdagent, vd);
>  error_setg(>migration_blocker,
> "The vdagent chardev doesn't yet support migration");
>  }
> --
> 2.27.0
>
>


--
Marc-André Lureau

Re: [PATCH RFC 2/4] vdagent: refactor vdagent_chr_recv_caps function

[Marc-André Lureau]

On Fri, Dec 30, 2022 at 6:49 PM  wrote:
>
> From: "dengp...@chinatelecom.cn" 
>
> Abstract vdagent registry logic into
> vdagent_register_to_qemu_clipboard.
>
> Note that trace log of vdagent_recv_caps also be added.
>
> Signed-off-by: dengp...@chinatelecom.cn 
> Signed-off-by: liuy...@chinatelecom.cn 

Reviewed-by: Marc-André Lureau 

> ---
>  ui/trace-events |  1 +
>  ui/vdagent.c| 20 +---
>  2 files changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/ui/trace-events b/ui/trace-events
> index 977577fbba..5e50b60da5 100644
> --- a/ui/trace-events
> +++ b/ui/trace-events
> @@ -143,6 +143,7 @@ vdagent_cb_grab_selection(const char *name) "selection %s"
>  vdagent_cb_grab_discard(const char *name, int cur, int recv) "selection %s, 
> cur:%d recv:%d"
>  vdagent_cb_grab_type(const char *name) "type %s"
>  vdagent_cb_serial_discard(uint32_t current, uint32_t received) "current=%u, 
> received=%u"
> +vdagent_recv_caps(uint32_t caps) "received caps %u"
>
>  # dbus.c
>  dbus_registered_listener(const char *bus_name) "peer %s"
> diff --git a/ui/vdagent.c b/ui/vdagent.c
> index 645383b4ec..38061d5b38 100644
> --- a/ui/vdagent.c
> +++ b/ui/vdagent.c
> @@ -696,6 +696,16 @@ static void vdagent_chr_open(Chardev *chr,
>  *be_opened = true;
>  }
>
> +static void vdagent_register_to_qemu_clipboard(VDAgentChardev *vd)
> +{
> +if (have_clipboard(vd) && vd->cbpeer.notifier.notify == NULL) {
> +vd->cbpeer.name = "vdagent";
> +vd->cbpeer.notifier.notify = vdagent_clipboard_notify;
> +vd->cbpeer.request = vdagent_clipboard_request;
> +qemu_clipboard_peer_register(>cbpeer);
> +}
> +}
> +
>  static void vdagent_chr_recv_caps(VDAgentChardev *vd, VDAgentMessage *msg)
>  {
>  VDAgentAnnounceCapabilities *caps = (void *)msg->data;
> @@ -720,14 +730,10 @@ static void vdagent_chr_recv_caps(VDAgentChardev *vd, 
> VDAgentMessage *msg)
>  qemu_input_handler_activate(vd->mouse_hs);
>  }
>
> -memset(vd->last_serial, 0, sizeof(vd->last_serial));
> +trace_vdagent_recv_caps(vd->caps);
>
> -if (have_clipboard(vd) && vd->cbpeer.notifier.notify == NULL) {
> -vd->cbpeer.name = "vdagent";
> -vd->cbpeer.notifier.notify = vdagent_clipboard_notify;
> -vd->cbpeer.request = vdagent_clipboard_request;
> -qemu_clipboard_peer_register(>cbpeer);
> -}
> +memset(vd->last_serial, 0, sizeof(vd->last_serial));
> +vdagent_register_to_qemu_clipboard(vd);
>  }
>
>  static void vdagent_chr_recv_msg(VDAgentChardev *vd, VDAgentMessage *msg)
> --
> 2.27.0
>
>


-- 
Marc-André Lureau

[GIT PULL 0/4] Host Memory Backends and Memory devices queue 2023-01-02

[David Hildenbrand]

The following changes since commit 222059a0fccf4af3be776fe35a5ea2d6a68f9a0b:

  Merge tag 'pull-ppc-20221221' of https://gitlab.com/danielhb/qemu into 
staging (2022-12-21 18:08:09 +)

are available in the Git repository at:

  https://github.com/davidhildenbrand/qemu.git tags/mem-2023-01-02

for you to fetch changes up to 6bb613f0812d1364fc8fcf0846647446884d5148:

  hostmem: Honor multiple preferred nodes if possible (2022-12-28 14:59:55 
+0100)


Hi,

"Host Memory Backends" and "Memory devices" queue ("mem"):
- virtio-mem fixes
- Use new MPOL_PREFERRED_MANY mbind() policy for memory backends if
  possible


Chenyi Qiang (2):
  virtio-mem: Fix the bitmap index of the section offset
  virtio-mem: Fix the iterator variable in a vmem->rdl_list loop

Michal Privoznik (1):
  hostmem: Honor multiple preferred nodes if possible

Philippe Mathieu-Daudé (1):
  virtio-mem: Fix typo in function name

 backends/hostmem.c | 19 +--
 hw/virtio/virtio-mem.c | 18 +-
 meson.build|  5 +
 3 files changed, 31 insertions(+), 11 deletions(-)

-- 
2.39.0

[GIT PULL 2/4] virtio-mem: Fix the iterator variable in a vmem->rdl_list loop

[David Hildenbrand]

From: Chenyi Qiang 

It should be the variable rdl2 to revert the already-notified listeners.

Fixes: 2044969f0b ("virtio-mem: Implement RamDiscardManager interface")
Signed-off-by: Chenyi Qiang 
Message-Id: <20221228090312.17276-1-chenyi.qi...@intel.com>
Cc: qemu-sta...@nongnu.org
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: David Hildenbrand 
---
 hw/virtio/virtio-mem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
index 5c22c4b876..2b0271442b 100644
--- a/hw/virtio/virtio-mem.c
+++ b/hw/virtio/virtio-mem.c
@@ -341,7 +341,7 @@ static int virtio_mem_notify_plug(VirtIOMEM *vmem, uint64_t 
offset,
 if (ret) {
 /* Notify all already-notified listeners. */
 QLIST_FOREACH(rdl2, >rdl_list, next) {
-MemoryRegionSection tmp = *rdl->section;
+MemoryRegionSection tmp = *rdl2->section;
 
 if (rdl2 == rdl) {
 break;
-- 
2.39.0

[GIT PULL 3/4] virtio-mem: Fix typo in function name

[David Hildenbrand]

From: Philippe Mathieu-Daudé 

Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20221228130956.80515-1-phi...@linaro.org>
Signed-off-by: David Hildenbrand 
---
 hw/virtio/virtio-mem.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
index 2b0271442b..1ed1f5a4af 100644
--- a/hw/virtio/virtio-mem.c
+++ b/hw/virtio/virtio-mem.c
@@ -207,7 +207,7 @@ static int virtio_mem_for_each_unplugged_range(const 
VirtIOMEM *vmem, void *arg,
  *
  * Returns false if the intersection is empty, otherwise returns true.
  */
-static bool virito_mem_intersect_memory_section(MemoryRegionSection *s,
+static bool virtio_mem_intersect_memory_section(MemoryRegionSection *s,
 uint64_t offset, uint64_t size)
 {
 uint64_t start = MAX(s->offset_within_region, offset);
@@ -245,7 +245,7 @@ static int virtio_mem_for_each_plugged_section(const 
VirtIOMEM *vmem,
   first_bit + 1) - 1;
 size = (last_bit - first_bit + 1) * vmem->block_size;
 
-if (!virito_mem_intersect_memory_section(, offset, size)) {
+if (!virtio_mem_intersect_memory_section(, offset, size)) {
 break;
 }
 ret = cb(, arg);
@@ -277,7 +277,7 @@ static int virtio_mem_for_each_unplugged_section(const 
VirtIOMEM *vmem,
  first_bit + 1) - 1;
 size = (last_bit - first_bit + 1) * vmem->block_size;
 
-if (!virito_mem_intersect_memory_section(, offset, size)) {
+if (!virtio_mem_intersect_memory_section(, offset, size)) {
 break;
 }
 ret = cb(, arg);
@@ -313,7 +313,7 @@ static void virtio_mem_notify_unplug(VirtIOMEM *vmem, 
uint64_t offset,
 QLIST_FOREACH(rdl, >rdl_list, next) {
 MemoryRegionSection tmp = *rdl->section;
 
-if (!virito_mem_intersect_memory_section(, offset, size)) {
+if (!virtio_mem_intersect_memory_section(, offset, size)) {
 continue;
 }
 rdl->notify_discard(rdl, );
@@ -329,7 +329,7 @@ static int virtio_mem_notify_plug(VirtIOMEM *vmem, uint64_t 
offset,
 QLIST_FOREACH(rdl, >rdl_list, next) {
 MemoryRegionSection tmp = *rdl->section;
 
-if (!virito_mem_intersect_memory_section(, offset, size)) {
+if (!virtio_mem_intersect_memory_section(, offset, size)) {
 continue;
 }
 ret = rdl->notify_populate(rdl, );
@@ -346,7 +346,7 @@ static int virtio_mem_notify_plug(VirtIOMEM *vmem, uint64_t 
offset,
 if (rdl2 == rdl) {
 break;
 }
-if (!virito_mem_intersect_memory_section(, offset, size)) {
+if (!virtio_mem_intersect_memory_section(, offset, size)) {
 continue;
 }
 rdl2->notify_discard(rdl2, );
-- 
2.39.0

[GIT PULL 4/4] hostmem: Honor multiple preferred nodes if possible

[David Hildenbrand]

From: Michal Privoznik 

If a memory-backend is configured with mode
HOST_MEM_POLICY_PREFERRED then
host_memory_backend_memory_complete() calls mbind() as:

  mbind(..., MPOL_PREFERRED, nodemask, ...);

Here, 'nodemask' is a bitmap of host NUMA nodes and corresponds
to the .host-nodes attribute. Therefore, there can be multiple
nodes specified. However, the documentation to MPOL_PREFERRED
says:

  MPOL_PREFERRED
This mode sets the preferred node for allocation. ...
If nodemask specifies more than one node ID, the first node
in the mask will be selected as the preferred node.

Therefore, only the first node is honored and the rest is
silently ignored. Well, with recent changes to the kernel and
numactl we can do better.

The Linux kernel added in v5.15 via commit cfcaa66f8032
("mm/hugetlb: add support for mempolicy MPOL_PREFERRED_MANY")
support for MPOL_PREFERRED_MANY, which accepts multiple preferred
NUMA nodes instead.

Then, numa_has_preferred_many() API was introduced to numactl
(v2.0.15~26) allowing applications to query kernel support.

Wiring this all together, we can pass MPOL_PREFERRED_MANY to the
mbind() call instead and stop ignoring multiple nodes, silently.

Signed-off-by: Michal Privoznik 
Message-Id: 

Reviewed-by: David Hildenbrand 
Signed-off-by: David Hildenbrand 
---
 backends/hostmem.c | 19 +--
 meson.build|  5 +
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/backends/hostmem.c b/backends/hostmem.c
index 8640294c10..747e7838c0 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -23,7 +23,12 @@
 
 #ifdef CONFIG_NUMA
 #include 
+#include 
 QEMU_BUILD_BUG_ON(HOST_MEM_POLICY_DEFAULT != MPOL_DEFAULT);
+/*
+ * HOST_MEM_POLICY_PREFERRED may either translate to MPOL_PREFERRED or
+ * MPOL_PREFERRED_MANY, see comments further below.
+ */
 QEMU_BUILD_BUG_ON(HOST_MEM_POLICY_PREFERRED != MPOL_PREFERRED);
 QEMU_BUILD_BUG_ON(HOST_MEM_POLICY_BIND != MPOL_BIND);
 QEMU_BUILD_BUG_ON(HOST_MEM_POLICY_INTERLEAVE != MPOL_INTERLEAVE);
@@ -346,6 +351,7 @@ host_memory_backend_memory_complete(UserCreatable *uc, 
Error **errp)
  * before mbind(). note: MPOL_MF_STRICT is ignored on hugepages so
  * this doesn't catch hugepage case. */
 unsigned flags = MPOL_MF_STRICT | MPOL_MF_MOVE;
+int mode = backend->policy;
 
 /* check for invalid host-nodes and policies and give more verbose
  * error messages than mbind(). */
@@ -369,9 +375,18 @@ host_memory_backend_memory_complete(UserCreatable *uc, 
Error **errp)
BITS_TO_LONGS(MAX_NODES + 1) * sizeof(unsigned long));
 assert(maxnode <= MAX_NODES);
 
+#ifdef HAVE_NUMA_HAS_PREFERRED_MANY
+if (mode == MPOL_PREFERRED && numa_has_preferred_many() > 0) {
+/*
+ * Replace with MPOL_PREFERRED_MANY otherwise the mbind() below
+ * silently picks the first node.
+ */
+mode = MPOL_PREFERRED_MANY;
+}
+#endif
+
 if (maxnode &&
-mbind(ptr, sz, backend->policy, backend->host_nodes, maxnode + 1,
-  flags)) {
+mbind(ptr, sz, mode, backend->host_nodes, maxnode + 1, flags)) {
 if (backend->policy != MPOL_DEFAULT || errno != ENOSYS) {
 error_setg_errno(errp, errno,
  "cannot bind memory to host NUMA nodes");
diff --git a/meson.build b/meson.build
index 4c6f8a674a..3f31db5963 100644
--- a/meson.build
+++ b/meson.build
@@ -1858,6 +1858,11 @@ config_host_data.set('CONFIG_LINUX_AIO', libaio.found())
 config_host_data.set('CONFIG_LINUX_IO_URING', linux_io_uring.found())
 config_host_data.set('CONFIG_LIBPMEM', libpmem.found())
 config_host_data.set('CONFIG_NUMA', numa.found())
+if numa.found()
+  config_host_data.set('HAVE_NUMA_HAS_PREFERRED_MANY',
+   cc.has_function('numa_has_preferred_many',
+   dependencies: numa))
+endif
 config_host_data.set('CONFIG_OPENGL', opengl.found())
 config_host_data.set('CONFIG_PROFILER', get_option('profiler'))
 config_host_data.set('CONFIG_RBD', rbd.found())
-- 
2.39.0

Re: [PATCH RFC 1/4] vdagent: fix memory leak when vdagent_disconnect is called

[Marc-André Lureau]

Hi

On Fri, Dec 30, 2022 at 6:48 PM  wrote:
>
> From: "dengp...@chinatelecom.cn" 
>
> Memory free should be done in vdagent_disconnect using
> qemu_input_handler_unregister, replace qemu_input_handler_deactivate
> with that.
>
> Signed-off-by: dengp...@chinatelecom.cn 
> Signed-off-by: liuy...@chinatelecom.cn 
> ---
>  ui/vdagent.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ui/vdagent.c b/ui/vdagent.c
> index 4bf50f0c4d..645383b4ec 100644
> --- a/ui/vdagent.c
> +++ b/ui/vdagent.c
> @@ -863,7 +863,7 @@ static void vdagent_disconnect(VDAgentChardev *vd)
>  vdagent_reset_bufs(vd);
>  vd->caps = 0;
>  if (vd->mouse_hs) {
> -qemu_input_handler_deactivate(vd->mouse_hs);
> +qemu_input_handler_unregister(vd->mouse_hs);

It looks like vdagent_disconnect() was meant to be called when the
agent connection is lost, vdagent_chr_set_fe_open(fe_open=false), so
it can later be initialized again during vdagent_chr_recv_caps(). Not
sure why this isn't done.

Imho, you should instead add qemu_input_handler_unregister() call to
vdagent_chr_fini() for now


-- 
Marc-André Lureau

[GIT PULL 1/4] virtio-mem: Fix the bitmap index of the section offset

[David Hildenbrand]

From: Chenyi Qiang 

vmem->bitmap indexes the memory region of the virtio-mem backend at a
granularity of block_size. To calculate the index of target section offset,
the block_size should be divided instead of the bitmap_size.

Fixes: 2044969f0b ("virtio-mem: Implement RamDiscardManager interface")
Signed-off-by: Chenyi Qiang 
Message-Id: <20221216062231.11181-1-chenyi.qi...@intel.com>
Reviewed-by: David Hildenbrand 
Reviewed-by: Michael S. Tsirkin 
Cc: qemu-sta...@nongnu.org
Signed-off-by: David Hildenbrand 
---
 hw/virtio/virtio-mem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
index d96bde1fab..5c22c4b876 100644
--- a/hw/virtio/virtio-mem.c
+++ b/hw/virtio/virtio-mem.c
@@ -235,7 +235,7 @@ static int virtio_mem_for_each_plugged_section(const 
VirtIOMEM *vmem,
 uint64_t offset, size;
 int ret = 0;
 
-first_bit = s->offset_within_region / vmem->bitmap_size;
+first_bit = s->offset_within_region / vmem->block_size;
 first_bit = find_next_bit(vmem->bitmap, vmem->bitmap_size, first_bit);
 while (first_bit < vmem->bitmap_size) {
 MemoryRegionSection tmp = *s;
@@ -267,7 +267,7 @@ static int virtio_mem_for_each_unplugged_section(const 
VirtIOMEM *vmem,
 uint64_t offset, size;
 int ret = 0;
 
-first_bit = s->offset_within_region / vmem->bitmap_size;
+first_bit = s->offset_within_region / vmem->block_size;
 first_bit = find_next_zero_bit(vmem->bitmap, vmem->bitmap_size, first_bit);
 while (first_bit < vmem->bitmap_size) {
 MemoryRegionSection tmp = *s;
-- 
2.39.0

Re: [PATCH v2] pflash: Only read non-zero parts of backend image

[Gerd Hoffmann]

  Hi,

> > Moving away from pflash for efi variable storage would cause alot of
> > churn through the whole stack.  firmware, qemu, libvirt, upper
> > management, all affected.  Is that worth the trouble?  Using pflash
> > isn't that much of a problem IMHO.
> 
> Agreed. pflash is a bit clunky but not a huge problem atm (although
> setting up and tearing down the r/o memslot for every read resp. write
> results in some performance issues under kvm/arm64)
> 
> *If* we decide to replace it, I would suggest an emulated ROM for the
> executable image (without any emulated programming facility
> whatsoever)

Sure.

> and a paravirtualized get/setvariable interface which can
> be used in a sane way to virtualize secure boot without having to
> emulate SMM or other secure world firmware interfaces.

Suggestions how to do that best?  The only option I can see is moving
the variable policy processing to the host, so any variable update
requests are checked even in case the guest OS bypasses the firmware
(which it can easily do when we don't have SMM mode to restrict access
to the paravirtual efi variable service device).

take care,
  Gerd

Re: [PATCH 0/9] hw/arm/aspeed_ast10x0: Map more peripherals & few more fixes

[Cédric Le Goater]

On 12/29/22 16:23, Philippe Mathieu-Daudé wrote:

Trying to fix some bugs triggered running Zephyr.

Still 2 bugs:

1/
uart:~$ sensor get SYSCLK
[00:00:23.592,000]  os: * USAGE FAULT *
[00:00:23.593,000]  os:   Illegal use of the EPSR
[00:00:23.593,000]  os: r0/a1:  0x00033448  r1/a2:  0x  r2/a3:  
0x00047f50
[00:00:23.593,000]  os: r3/a4:  0x r12/ip:  0x r14/lr:  
0x0fbd
[00:00:23.593,000]  os:  xpsr:  0x6000
[00:00:23.593,000]  os: Faulting instruction address (r15/pc): 0x
[00:00:23.593,000]  os: >>> ZEPHYR FATAL ERROR 0: CPU exception on CPU 0
[00:00:23.594,000]  os: Current thread: 0x38248 (shell_uart)
[00:00:23.601,000]  os: Halting system

2/
uart:~$ mcuboot
[00:01:04.990,000]  os: * BUS FAULT *
[00:01:04.990,000]  os:   Instruction bus error
[00:01:04.991,000]  os: r0/a1:  0x  r1/a2:  0x0000  r2/a3:  
0x00047ef0
[00:01:04.991,000]  os: r3/a4:  0x0010 r12/ip:  0x6df7ecb5 r14/lr:  
0x000188ed
[00:01:04.991,000]  os:  xpsr:  0x6100
[00:01:04.991,000]  os: Faulting instruction address (r15/pc): 0x6df7ecb4
[00:01:04.991,000]  os: >>> ZEPHYR FATAL ERROR 0: CPU exception on CPU 0
[00:01:04.991,000]  os: Current thread: 0x38248 (shell_uart)
[00:01:04.994,000]  os: Halting system


IN:
PMSA MPU lookup for reading at 0x0001d400 mmu_idx 65 -> Hit (prot rwx)
0x0001d5a2:  6869   ldr  r1, [r5, #4]
0x0001d5a4:  4421   add  r1, r4
0x0001d5a6:  6883   ldr  r3, [r0, #8]
0x0001d5a8:  681c   ldr  r4, [r3]
0x0001d5aa:  463a   mov  r2, r7
0x0001d5ac:  4633   mov  r3, r6
0x0001d5ae:  46a4   mov  ip, r4
0x0001d5b0:  e8bd 41f0  pop.w{r4, r5, r6, r7, r8, lr}
0x0001d5b4:  4760   bx   ip

PMSA MPU lookup for reading at 0x0008 mmu_idx 65 -> Hit (prot rwx)
PMSA MPU lookup for execute at 0x6df7ecb4 mmu_idx 65 -> Hit (prot rwx)
Taking exception 3 [Prefetch Abort] on CPU 0
...at fault address 0x6df7ecb4
...with CFSR.IBUSERR
PMSA MPU lookup for writing at 0x00047ec8 mmu_idx 65 -> Hit (prot rwx)
PMSA MPU lookup for writing at 0x00047ecc mmu_idx 65 -> Hit (prot rwx)
PMSA MPU lookup for writing at 0x00047ed0 mmu_idx 65 -> Hit (prot rwx)
PMSA MPU lookup for writing at 0x00047ed4 mmu_idx 65 -> Hit (prot rwx)
PMSA MPU lookup for writing at 0x00047ed8 mmu_idx 65 -> Hit (prot rwx)
PMSA MPU lookup for writing at 0x00047edc mmu_idx 65 -> Hit (prot rwx)
PMSA MPU lookup for writing at 0x00047ee0 mmu_idx 65 -> Hit (prot rwx)
PMSA MPU lookup for writing at 0x00047ee4 mmu_idx 65 -> Hit (prot rwx)
...taking pending nonsecure exception 5
...loading from element 5 of non-secure vector table at 0x14
...loaded new PC 0xa0cd


HACE isn't really functional there. I probably screwed smth while wiring
the peripheral. Not obvious without access to the datasheet.


The HACE logic is quite complex and the model might be a bit fragile for some
modes. I think accumulation still has some problems.

Here are drivers for it :

  
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/crypto/aspeed
  
https://github.com/openbmc/u-boot/blob/v2019.04-aspeed-openbmc/drivers/crypto/aspeed_hace_v1.c
  
https://github.com/openbmc/u-boot/blob/v2019.04-aspeed-openbmc/drivers/crypto/aspeed_hace.c

Thanks,

C.



Philippe Mathieu-Daudé (9):
   hw/watchdog/wdt_aspeed: Map the whole MMIO range
   hw/arm/aspeed: Use the IEC binary prefix definitions
   hw/arm/aspeed_ast10x0: Add various unimplemented peripherals
   hw/arm/aspeed_ast10x0: Map I3C peripheral
   hw/arm/aspeed_ast10x0: Map the secure SRAM
   hw/arm/aspeed_ast10x0: Map HACE peripheral
   hw/misc/aspeed_hace: Do not crash if address_space_map() failed
   hw/arm/aspeed_ast10x0: Add TODO comment to use Cortex-M4F
   tests/avocado: Test Aspeed Zephyr SDK v00.01.08 on AST1030 board

  hw/arm/aspeed_ast10x0.c  | 84 ++--
  hw/arm/aspeed_ast2600.c  |  5 +-
  hw/arm/aspeed_soc.c  |  6 +--
  hw/misc/aspeed_hace.c| 21 +---
  hw/watchdog/wdt_aspeed.c | 12 +++--
  include/hw/arm/aspeed_soc.h  | 14 ++
  include/hw/watchdog/wdt_aspeed.h |  2 +-
  tests/avocado/machine_aspeed.py  | 41 +++-
  8 files changed, 163 insertions(+), 22 deletions(-)

Re: [PATCH] usbredir: Do not detach usb if backend chardev disconnect

[Gerd Hoffmann]

On Thu, Dec 22, 2022 at 09:21:25PM +0800, Minglei Liu wrote:
> ping !
> please review this patch : [PATCH] usbredir: Do not detach usb if backend
> chardev disconnect - minglei.liu (kernel.org)
> 
> 
> minglei.liu  于2022年11月9日周三 19:56写道：
> 
> > If the network between qemu and usbredirserver is temporarily disconnected,
> > the USB device in the VM will be unplugged. If the reconnect parameter is
> > configured for the backend chardev, the device will be reconnected later.
> > But from the inside of the VM, this USB device has experienced unplug and
> > re-plug, if the USB storage device has been mounted in the VM before,
> > the drive letter will change after the device is re-plugged.
> >
> > So in this case, we no longer unplug the device, and operations to the USB
> > is returned immediately at this point.

If you are lucky, and it also depends on the kind of device.

I don't think this is a safe thing to do in general.  You just don't
know what state the usb device is in when you are re-connecting, and
you also don't know whenever a re-connect will ever happen.

take care,
  Gerd

[PATCH] Update scripts/meson-buildoptions.sh

[Alessandro Di Federico via]

Note: `Makefile` relies on modification dates in the source tree to
detect changes to `meson_options.txt`. However, git does not track
those. Therefore, the following was necessary to regenerate
`meson-buildoptions.sh`:

touch meson_options.txt
cd "$BUILD_DIR"
make update-buildoptions

Signed-off-by: Alessandro Di Federico 
---
 scripts/meson-buildoptions.sh | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index aa6e30ea911..0f71e92dcba 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -10,6 +10,9 @@ meson_options_help() {
   printf "%s\n" '   affects only QEMU, not tools like 
qemu-img)'
   printf "%s\n" '  --datadir=VALUE  Data file directory [share]'
   printf "%s\n" '  --disable-coroutine-pool coroutine freelist (better 
performance)'
+  printf "%s\n" '  --disable-hexagon-idef-parser'
+  printf "%s\n" '   use idef-parser to automatically 
generate TCG'
+  printf "%s\n" '   code for the Hexagon frontend'
   printf "%s\n" '  --disable-install-blobs  install provided firmware blobs'
   printf "%s\n" '  --docdir=VALUE   Base directory for documentation 
installation'
   printf "%s\n" '   (can be empty) [share/doc]'
@@ -40,7 +43,8 @@ meson_options_help() {
   printf "%s\n" '  --enable-trace-backends=CHOICES'
   printf "%s\n" '   Set available tracing backends 
[log] (choices:'
   printf "%s\n" '   
dtrace/ftrace/log/nop/simple/syslog/ust)'
-  printf "%s\n" '  --firmwarepath=VALUESsearch PATH for firmware files 
[share/qemu-firmware]'
+  printf "%s\n" '  --firmwarepath=VALUESsearch PATH for firmware files 
[share/qemu-'
+  printf "%s\n" '   firmware]'
   printf "%s\n" '  --iasl=VALUE Path to ACPI disassembler'
   printf "%s\n" '  --includedir=VALUE   Header file directory [include]'
   printf "%s\n" '  --interp-prefix=VALUEwhere to find shared libraries 
etc., use %M for'
@@ -93,7 +97,7 @@ meson_options_help() {
   printf "%s\n" '  glusterfs   Glusterfs block device driver'
   printf "%s\n" '  gnutls  GNUTLS cryptography support'
   printf "%s\n" '  gtk GTK+ user interface'
-  printf "%s\n" '  gtk-clipboard   clipboard support for GTK (EXPERIMENTAL, 
MAY HANG)'
+  printf "%s\n" '  gtk-clipboard   clipboard support for the gtk UI 
(EXPERIMENTAL, MAY HANG)'
   printf "%s\n" '  guest-agent Build QEMU Guest Agent'
   printf "%s\n" '  guest-agent-msi Build MSI package for the QEMU Guest Agent'
   printf "%s\n" '  hax HAX acceleration support'
@@ -156,6 +160,8 @@ meson_options_help() {
   printf "%s\n" '  usb-redir   libusbredir support'
   printf "%s\n" '  vde vde network backend support'
   printf "%s\n" '  vdi vdi image format support'
+  printf "%s\n" '  vduse-blk-export'
+  printf "%s\n" '  VDUSE block export support'
   printf "%s\n" '  vfio-user-server'
   printf "%s\n" '  vfio-user server support'
   printf "%s\n" '  vhost-cryptovhost-user crypto backend support'
@@ -164,8 +170,6 @@ meson_options_help() {
   printf "%s\n" '  vhost-user  vhost-user backend support'
   printf "%s\n" '  vhost-user-blk-server'
   printf "%s\n" '  build vhost-user-blk server'
-  printf "%s\n" '  vduse-blk-export'
-  printf "%s\n" '  VDUSE block export support'
   printf "%s\n" '  vhost-vdpa  vhost-vdpa kernel backend support'
   printf "%s\n" '  virglrenderer   virgl rendering support'
   printf "%s\n" '  virtfs  virtio-9p support'
@@ -283,6 +287,8 @@ _meson_option_parse() {
 --disable-guest-agent-msi) printf "%s" -Dguest_agent_msi=disabled ;;
 --enable-hax) printf "%s" -Dhax=enabled ;;
 --disable-hax) printf "%s" -Dhax=disabled ;;
+--enable-hexagon-idef-parser) printf "%s" -Dhexagon_idef_parser=true ;;
+--disable-hexagon-idef-parser) printf "%s" -Dhexagon_idef_parser=false ;;
 --enable-hvf) printf "%s" -Dhvf=enabled ;;
 --disable-hvf) printf "%s" -Dhvf=disabled ;;
 --iasl=*) quote_sh "-Diasl=$2" ;;
@@ -429,6 +435,8 @@ _meson_option_parse() {
 --disable-vde) printf "%s" -Dvde=disabled ;;
 --enable-vdi) printf "%s" -Dvdi=enabled ;;
 --disable-vdi) printf "%s" -Dvdi=disabled ;;
+--enable-vduse-blk-export) printf "%s" -Dvduse_blk_export=enabled ;;
+--disable-vduse-blk-export) printf "%s" -Dvduse_blk_export=disabled ;;
 --enable-vfio-user-server) printf "%s" -Dvfio_user_server=enabled ;;
 --disable-vfio-user-server) printf "%s" -Dvfio_user_server=disabled ;;
 --enable-vhost-crypto) printf "%s" -Dvhost_crypto=enabled ;;
@@ -441,8 +449,6 @@ _meson_option_parse() {
 --disable-vhost-user) printf "%s" -Dvhost_user=disabled ;;
 --enable-vhost-user-blk-server) printf 

Re: [PATCH 1/9] hw/watchdog/wdt_aspeed: Map the whole MMIO range

[Cédric Le Goater]

On 12/30/22 08:32, Philippe Mathieu-Daudé wrote:

On 29/12/22 21:42, Peter Delevoryas wrote:

On Thu, Dec 29, 2022 at 04:23:17PM +0100, Philippe Mathieu-Daudé wrote:

Avoid confusing two different things:
- the WDT I/O region size ('iosize')
- at which offset the SoC map the WDT ('offset')
While it is often the same, we can map smaller region sizes at
larger offsets.

Here we are interested in the I/O region size. Rename as 'iosize'
and map the whole range, not only the first implemented registers.
Unimplemented registers accesses are already logged as guest-errors.

Otherwise when booting the demo in [*] we get:

   aspeed.io: unimplemented device write (size 4, offset 0x1851a8, value 
0x030f1ff1)
   aspeed.io: unimplemented device write (size 4, offset 0x1851ac, value 
0x03f1)
   aspeed.io: unimplemented device write (size 4, offset 0x185128, value 
0x030f1ff1)
   aspeed.io: unimplemented device write (size 4, offset 0x18512c, value 
0x03f1)

[*] https://github.com/AspeedTech-BMC/zephyr/releases/tag/v00.01.07

Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/arm/aspeed_ast10x0.c  |  2 +-
  hw/arm/aspeed_ast2600.c  |  2 +-
  hw/arm/aspeed_soc.c  |  2 +-
  hw/watchdog/wdt_aspeed.c | 12 +++-
  include/hw/watchdog/wdt_aspeed.h |  2 +-
  5 files changed, 11 insertions(+), 9 deletions(-)




diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c
index d753693a2e..c1311ce74c 100644
--- a/hw/watchdog/wdt_aspeed.c
+++ b/hw/watchdog/wdt_aspeed.c
@@ -260,6 +260,7 @@ static void aspeed_wdt_realize(DeviceState *dev, Error 
**errp)
  {
  SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
  AspeedWDTState *s = ASPEED_WDT(dev);
+    AspeedWDTClass *awc = ASPEED_WDT_GET_CLASS(dev);
  assert(s->scu);
@@ -270,8 +271,9 @@ static void aspeed_wdt_realize(DeviceState *dev, Error 
**errp)
   */
  s->pclk_freq = PCLK_HZ;
+    assert(awc->iosize >= ASPEED_WDT_REGS_MAX * 4);
  memory_region_init_io(>iomem, OBJECT(s), _wdt_ops, s,
-  TYPE_ASPEED_WDT, ASPEED_WDT_REGS_MAX * 4);
+  TYPE_ASPEED_WDT, awc->iosize);


Does this fix the unimplemented thing you referred to in the commit message?


Yes, I'll reword the description to be clearer.


I'm not sure which part of this diff actually removes the unimplemented traces.


Having:

   #define ASPEED_WDT_REGS_MAX    (0x20 / 4)

Before this patch, we map regions of 0x20 ...


@@ -392,7 +394,7 @@ static void aspeed_1030_wdt_class_init(ObjectClass *klass, 
void *data)
  AspeedWDTClass *awc = ASPEED_WDT_CLASS(klass);
  dc->desc = "ASPEED 1030 Watchdog Controller";
-    awc->offset = 0x80;
+    awc->iosize = 0x80;


... every span of 0x80, so there is a gap of 0x60, apparently accessed
by the Zephyr WDT driver (address 0x28 - register #10 - is accessed in
the traces).

 From the driver source we can see [1] added in [2]:

     #define WDT_RELOAD_VAL_REG  0x0004
     #define WDT_RESTART_REG 0x0008
     #define WDT_CTRL_REG    0x000C
     #define WDT_TIMEOUT_STATUS_REG  0x0010
     #define WDT_TIMEOUT_STATUS_CLR_REG  0x0014
     #define WDT_RESET_MASK1_REG 0x001C
     #define WDT_RESET_MASK2_REG 0x0020
     #define WDT_SW_RESET_MASK1_REG  0x0028   <--
     #define WDT_SW_RESET_MASK2_REG  0x002C
     #define WDT_SW_RESET_CTRL_REG   0x0024

So the traces likely correspond to this code:

   static int aspeed_wdt_init(const struct device *dev)
   {
     const struct aspeed_wdt_config *config = dev->config;
     struct aspeed_wdt_data *const data = dev->data;
     uint32_t reg_val;

     /* disable WDT by default */
     reg_val = sys_read32(config->ctrl_base + WDT_CTRL_REG);
     reg_val &= ~WDT_CTRL_ENABLE;
     sys_write32(reg_val, config->ctrl_base + WDT_CTRL_REG);

     sys_write32(data->rst_mask1,
     config->ctrl_base + WDT_SW_RESET_MASK1_REG);
     sys_write32(data->rst_mask2,
     config->ctrl_base + WDT_SW_RESET_MASK2_REG);

     return 0;
   }


Yes. These registers have been on the TODO list for 3/4 years :/


After this patch, we map (in this case) a MMIO region of 0x80.
Accesses to address 0x28 hits this device model but is handled
as 'WDT register not covered'.


If we are to change things, adding the definitions of the software mode
reset regs in the WDT model would be cleaner. All accesses to these regs
could generate an UNIMP log saying that "support for software mode reset"
is not implemented.

Thanks,

C.
 


Better would be to extend the Aspeed WDT model in QEMU, or at
least report the valid-but-unimplemented registers as UNIMP instead
of GUEST_ERRORS.

[1] 
https://github.com/AspeedTech-BMC/zephyr/blob/v00.01.08/drivers/watchdog/wdt_aspeed.c#L31
[2] https://github.com/AspeedTech-BMC/zephyr/commit/2e99f10ac27b


diff --git a/include/hw/watchdog/wdt_aspeed.h b/include/hw/watchdog/wdt_aspeed.h
index dfa5dfa424..db91ee6b51 100644
--- 

Re: [PATCH 1/9] hw/watchdog/wdt_aspeed: Map the whole MMIO range

[Cédric Le Goater]

On 12/29/22 16:23, Philippe Mathieu-Daudé wrote:

Avoid confusing two different things:
- the WDT I/O region size ('iosize')
- at which offset the SoC map the WDT ('offset')
While it is often the same, we can map smaller region sizes at
larger offsets.

Here we are interested in the I/O region size. Rename as 'iosize'
and map the whole range, not only the first implemented registers.
Unimplemented registers accesses are already logged as guest-errors.

Otherwise when booting the demo in [*] we get:

   aspeed.io: unimplemented device write (size 4, offset 0x1851a8, value 
0x030f1ff1)
   aspeed.io: unimplemented device write (size 4, offset 0x1851ac, value 
0x03f1)
   aspeed.io: unimplemented device write (size 4, offset 0x185128, value 
0x030f1ff1)
   aspeed.io: unimplemented device write (size 4, offset 0x18512c, value 
0x03f1)


These are unimplemented registers related to software mode reset, which is a new
feature of the AST2600 SoC. The AST10x0 SoCs add a few more regs, hence the 
larger
MMIO width for the WDT registers. Until now, they have been harmless.



[*] https://github.com/AspeedTech-BMC/zephyr/releases/tag/v00.01.07

Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/arm/aspeed_ast10x0.c  |  2 +-
  hw/arm/aspeed_ast2600.c  |  2 +-
  hw/arm/aspeed_soc.c  |  2 +-
  hw/watchdog/wdt_aspeed.c | 12 +++-
  include/hw/watchdog/wdt_aspeed.h |  2 +-
  5 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
index 4d0b9b115f..122b3fd3f3 100644
--- a/hw/arm/aspeed_ast10x0.c
+++ b/hw/arm/aspeed_ast10x0.c
@@ -325,7 +325,7 @@ static void aspeed_soc_ast1030_realize(DeviceState 
*dev_soc, Error **errp)
  return;
  }
  aspeed_mmio_map(s, SYS_BUS_DEVICE(>wdt[i]), 0,
-sc->memmap[ASPEED_DEV_WDT] + i * awc->offset);
+sc->memmap[ASPEED_DEV_WDT] + i * awc->iosize);
  }
  
  /* GPIO */

diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index cd75465c2b..a79e05ddbd 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -472,7 +472,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, 
Error **errp)
  return;
  }
  aspeed_mmio_map(s, SYS_BUS_DEVICE(>wdt[i]), 0,
-sc->memmap[ASPEED_DEV_WDT] + i * awc->offset);
+sc->memmap[ASPEED_DEV_WDT] + i * awc->iosize);
  }
  
  /* RAM */

diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index b05b9dd416..2c0924d311 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -393,7 +393,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error 
**errp)
  return;
  }
  aspeed_mmio_map(s, SYS_BUS_DEVICE(>wdt[i]), 0,
-sc->memmap[ASPEED_DEV_WDT] + i * awc->offset);
+sc->memmap[ASPEED_DEV_WDT] + i * awc->iosize);
  }
  
  /* RAM  */

diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c
index d753693a2e..c1311ce74c 100644
--- a/hw/watchdog/wdt_aspeed.c
+++ b/hw/watchdog/wdt_aspeed.c
@@ -260,6 +260,7 @@ static void aspeed_wdt_realize(DeviceState *dev, Error 
**errp)
  {
  SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
  AspeedWDTState *s = ASPEED_WDT(dev);
+AspeedWDTClass *awc = ASPEED_WDT_GET_CLASS(dev);
  
  assert(s->scu);
  
@@ -270,8 +271,9 @@ static void aspeed_wdt_realize(DeviceState *dev, Error **errp)

   */
  s->pclk_freq = PCLK_HZ;
  
+assert(awc->iosize >= ASPEED_WDT_REGS_MAX * 4);

  memory_region_init_io(>iomem, OBJECT(s), _wdt_ops, s,
-  TYPE_ASPEED_WDT, ASPEED_WDT_REGS_MAX * 4);
+  TYPE_ASPEED_WDT, awc->iosize);
  sysbus_init_mmio(sbd, >iomem);
  }
  
@@ -309,7 +311,7 @@ static void aspeed_2400_wdt_class_init(ObjectClass *klass, void *data)

  AspeedWDTClass *awc = ASPEED_WDT_CLASS(klass);
  
  dc->desc = "ASPEED 2400 Watchdog Controller";

-awc->offset = 0x20;
+awc->iosize = 0x20;
  awc->ext_pulse_width_mask = 0xff;
  awc->reset_ctrl_reg = SCU_RESET_CONTROL1;
  awc->wdt_reload = aspeed_wdt_reload;
@@ -346,7 +348,7 @@ static void aspeed_2500_wdt_class_init(ObjectClass *klass, 
void *data)
  AspeedWDTClass *awc = ASPEED_WDT_CLASS(klass);
  
  dc->desc = "ASPEED 2500 Watchdog Controller";

-awc->offset = 0x20;
+awc->iosize = 0x20;
  awc->ext_pulse_width_mask = 0xf;
  awc->reset_ctrl_reg = SCU_RESET_CONTROL1;
  awc->reset_pulse = aspeed_2500_wdt_reset_pulse;
@@ -369,7 +371,7 @@ static void aspeed_2600_wdt_class_init(ObjectClass *klass, 
void *data)
  AspeedWDTClass *awc = ASPEED_WDT_CLASS(klass);
  
  dc->desc = "ASPEED 2600 Watchdog Controller";

-awc->offset = 0x40;
+awc->iosize = 0x40;
  awc->ext_pulse_width_mask = 0xf; /* TODO */
  awc->reset_ctrl_reg = AST2600_SCU_RESET_CONTROL1;
  

Re: [PATCH 2/4] target/m68k: pass sign directly into make_quotient()

[Mark Cave-Ayland]

On 01/01/2023 18:53, Richard Henderson wrote:


On 1/1/23 09:26, Laurent Vivier wrote:

Le 01/01/2023 à 15:43, Mark Cave-Ayland a écrit :

This enables the quotient parameter to be changed from int32_t to uint32_t and
also allows the extra sign logic in make_quotient() to be removed.

Signed-off-by: Mark Cave-Ayland 
---
  target/m68k/fpu_helper.c | 15 +--
  1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/target/m68k/fpu_helper.c b/target/m68k/fpu_helper.c
index 0932c464fd..ae839785fa 100644
--- a/target/m68k/fpu_helper.c
+++ b/target/m68k/fpu_helper.c
@@ -515,15 +515,8 @@ uint32_t HELPER(fmovemd_ld_postinc)(CPUM68KState *env, 
uint32_t addr,

  return fmovem_postinc(env, addr, mask, cpu_ld_float64_ra);
  }
-static void make_quotient(CPUM68KState *env, int32_t quotient)
+static void make_quotient(CPUM68KState *env, int sign, uint32_t quotient)
  {
-    int sign;
-
-    sign = quotient < 0;
-    if (sign) {
-    quotient = -quotient;
-    }
-
  quotient = (sign << 7) | (quotient & 0x7f);
  env->fpsr = (env->fpsr & ~FPSR_QT_MASK) | (quotient << FPSR_QT_SHIFT);
  }
@@ -536,7 +529,8 @@ void HELPER(fmod)(CPUM68KState *env, FPReg *res, FPReg *val0, 
FPReg *val1)

  return;
  }
-    make_quotient(env, floatx80_to_int32(res->d, >fp_status));
+    make_quotient(env, extractFloatx80Sign(res->d),
+  floatx80_to_int32(res->d, >fp_status));
  }
  void HELPER(frem)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
@@ -547,7 +541,8 @@ void HELPER(frem)(CPUM68KState *env, FPReg *res, FPReg *val0, 
FPReg *val1)

  return;
  }
-    make_quotient(env, floatx80_to_int32(res->d, >fp_status));
+    make_quotient(env, extractFloatx80Sign(res->d),
+  floatx80_to_int32(res->d, >fp_status));
  }
  void HELPER(fgetexp)(CPUM68KState *env, FPReg *res, FPReg *val)


I think you need an "abs(floatx80_to_int32())" in both cases as you do in PATCH 
4


Or in fact

     sign = extractFloatx80Sign(res);
     quot = floatx80_to_int32(floatx80_abs(res->d), status);
     make_quotient(env, sign, quot);


Thanks for the suggestion. Just out of curiosity, how does moving the abs to before 
the integer conversion make a difference here? Is it because floatx80_to_int32() can 
fail in some circumstances because of the sign of the result?



ATB,

Mark.

Re: [PATCH v2 02/11] hw/watchdog/wdt_aspeed: Extend MMIO range to cover more registers

[Philippe Mathieu-Daudé]

On 31/12/22 23:52, Dong, Eddie wrote:


   #define WDT_RELOAD_VAL_REG  0x0004
   #define WDT_RESTART_REG 0x0008
   #define WDT_CTRL_REG0x000C
   #define WDT_TIMEOUT_STATUS_REG  0x0010
   #define WDT_TIMEOUT_STATUS_CLR_REG  0x0014
   #define WDT_RESET_MASK1_REG 0x001C
   #define WDT_RESET_MASK2_REG 0x0020
   #define WDT_SW_RESET_MASK1_REG  0x0028   <--
   #define WDT_SW_RESET_MASK2_REG  0x002C
   #define WDT_SW_RESET_CTRL_REG   0x0024

Currently QEMU only cover a MMIO region of size 0x20:

   #define ASPEED_WDT_REGS_MAX(0x20 / 4)

Change to map the whole 'iosize' which might be bigger, covering the other


The root cause is that ASPEED_WDT_REGS_MAX is too small, right?
Probably the Qemu is emulating an old version of the hardware.

Given the meaning of ASPEED_WDT_REGS_MAX, it should be larger than iosize, not?
Probably ASPEED_WDT_REGS_MAX should be per device type (aspeed_2400/2500),
while iosize is for all devices, and its initial value comes from the per 
device type REGS_MAX.


You are correct. I don't have access to the specifications neither,
but I suspect you are right, the current watchdog is modelling an
older version that what the AST1030 has.

I started these WDT patches to understand the unimplemented accesses
done by Zephyr, but eventually they resulted irrelevant to the fixes
(see following patches) so I'll simply drop them.

Thanks,

Phil.

Re: [PATCH 2/4] target/m68k: pass sign directly into make_quotient()

[Mark Cave-Ayland]

On 01/01/2023 17:26, Laurent Vivier wrote:


Le 01/01/2023 à 15:43, Mark Cave-Ayland a écrit :

This enables the quotient parameter to be changed from int32_t to uint32_t and
also allows the extra sign logic in make_quotient() to be removed.

Signed-off-by: Mark Cave-Ayland 
---
  target/m68k/fpu_helper.c | 15 +--
  1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/target/m68k/fpu_helper.c b/target/m68k/fpu_helper.c
index 0932c464fd..ae839785fa 100644
--- a/target/m68k/fpu_helper.c
+++ b/target/m68k/fpu_helper.c
@@ -515,15 +515,8 @@ uint32_t HELPER(fmovemd_ld_postinc)(CPUM68KState *env, 
uint32_t addr,

  return fmovem_postinc(env, addr, mask, cpu_ld_float64_ra);
  }
-static void make_quotient(CPUM68KState *env, int32_t quotient)
+static void make_quotient(CPUM68KState *env, int sign, uint32_t quotient)
  {
-    int sign;
-
-    sign = quotient < 0;
-    if (sign) {
-    quotient = -quotient;
-    }
-
  quotient = (sign << 7) | (quotient & 0x7f);
  env->fpsr = (env->fpsr & ~FPSR_QT_MASK) | (quotient << FPSR_QT_SHIFT);
  }
@@ -536,7 +529,8 @@ void HELPER(fmod)(CPUM68KState *env, FPReg *res, FPReg *val0, 
FPReg *val1)

  return;
  }
-    make_quotient(env, floatx80_to_int32(res->d, >fp_status));
+    make_quotient(env, extractFloatx80Sign(res->d),
+  floatx80_to_int32(res->d, >fp_status));
  }
  void HELPER(frem)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
@@ -547,7 +541,8 @@ void HELPER(frem)(CPUM68KState *env, FPReg *res, FPReg *val0, 
FPReg *val1)

  return;
  }
-    make_quotient(env, floatx80_to_int32(res->d, >fp_status));
+    make_quotient(env, extractFloatx80Sign(res->d),
+  floatx80_to_int32(res->d, >fp_status));
  }
  void HELPER(fgetexp)(CPUM68KState *env, FPReg *res, FPReg *val)


I think you need an "abs(floatx80_to_int32())" in both cases as you do in PATCH 
4


Ah yes that's probably true - I suspect I didn't notice because the static tests fail 
immediately until patches 3 and 4.



ATB,

Mark.

[PATCH] linux-user: fix bug about incorrect base addresss of idt and gdt on i386 and x86_64

[fanwenjie]

On linux user mode, CPUX86State::idt::base and CPUX86State::gdt::base from 
Different CPUX86State Objects have same value, It is incorrect! Every 
CPUX86State::idt::base and Every CPUX86State::gdt::base Must points to 
independent memory space. 

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1405
Signed-off-by: fanwenjie 

---
 linux-user/i386/cpu_loop.c | 10 ++
 linux-user/main.c  | 11 +++
 2 files changed, 21 insertions(+)

diff --git a/linux-user/i386/cpu_loop.c b/linux-user/i386/cpu_loop.c
index 865413c08f..1f23bc5e3a 100644
--- a/linux-user/i386/cpu_loop.c
+++ b/linux-user/i386/cpu_loop.c
@@ -314,8 +314,18 @@ void cpu_loop(CPUX86State *env)
 }
 }
 
+static void target_cpu_free(void *obj)
+{
+CPUArchState* env = ((CPUState*)obj)->env_ptr;
+target_munmap(env->idt.base, sizeof(uint64_t) * (env->idt.limit + 1));
+target_munmap(env->gdt.base, sizeof(uint64_t) * TARGET_GDT_ENTRIES);
+g_free(obj);
+}
+
 void target_cpu_copy_regs(CPUArchState *env, struct target_pt_regs *regs)
 {
+CPUState* cpu = env_cpu(env);
+OBJECT(cpu)->free = target_cpu_free;
 env->cr[0] = CR0_PG_MASK | CR0_WP_MASK | CR0_PE_MASK;
 env->hflags |= HF_PE_MASK | HF_CPL_MASK;
 if (env->features[FEAT_1_EDX] & CPUID_SSE) {
diff --git a/linux-user/main.c b/linux-user/main.c
index a17fed045b..2276040548 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -234,6 +234,17 @@ CPUArchState *cpu_copy(CPUArchState *env)
 
 new_cpu->tcg_cflags = cpu->tcg_cflags;
 memcpy(new_env, env, sizeof(CPUArchState));
+#if defined(TARGET_I386) || defined(TARGET_X86_64)
+new_env->gdt.base = target_mmap(0, sizeof(uint64_t) * TARGET_GDT_ENTRIES,
+PROT_READ|PROT_WRITE,
+MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
+new_env->idt.base = target_mmap(0, sizeof(uint64_t) * (env->idt.limit + 1),
+PROT_READ|PROT_WRITE,
+MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
+memcpy((void*)new_env->gdt.base, (void*)env->gdt.base, sizeof(uint64_t) * 
TARGET_GDT_ENTRIES);
+memcpy((void*)new_env->idt.base, (void*)env->idt.base, sizeof(uint64_t) * 
(env->idt.limit + 1));
+OBJECT(new_cpu)->free = OBJECT(cpu)->free;
+#endif
 
 /* Clone all break/watchpoints.
Note: Once we support ptrace with hw-debug register access, make sure
-- 
2.34.1

[PATCH] linux-user: fix bug about incorrect base addresss of idt and gdt on i386 and x86_64

[fanwenjie]

---
 linux-user/i386/cpu_loop.c | 10 ++
 linux-user/main.c  | 11 +++
 2 files changed, 21 insertions(+)

diff --git a/linux-user/i386/cpu_loop.c b/linux-user/i386/cpu_loop.c
index 865413c08f..1f23bc5e3a 100644
--- a/linux-user/i386/cpu_loop.c
+++ b/linux-user/i386/cpu_loop.c
@@ -314,8 +314,18 @@ void cpu_loop(CPUX86State *env)
 }
 }
 
+static void target_cpu_free(void *obj)
+{
+CPUArchState* env = ((CPUState*)obj)->env_ptr;
+target_munmap(env->idt.base, sizeof(uint64_t) * (env->idt.limit + 1));
+target_munmap(env->gdt.base, sizeof(uint64_t) * TARGET_GDT_ENTRIES);
+g_free(obj);
+}
+
 void target_cpu_copy_regs(CPUArchState *env, struct target_pt_regs *regs)
 {
+CPUState* cpu = env_cpu(env);
+OBJECT(cpu)->free = target_cpu_free;
 env->cr[0] = CR0_PG_MASK | CR0_WP_MASK | CR0_PE_MASK;
 env->hflags |= HF_PE_MASK | HF_CPL_MASK;
 if (env->features[FEAT_1_EDX] & CPUID_SSE) {
diff --git a/linux-user/main.c b/linux-user/main.c
index a17fed045b..2276040548 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -234,6 +234,17 @@ CPUArchState *cpu_copy(CPUArchState *env)
 
 new_cpu->tcg_cflags = cpu->tcg_cflags;
 memcpy(new_env, env, sizeof(CPUArchState));
+#if defined(TARGET_I386) || defined(TARGET_X86_64)
+new_env->gdt.base = target_mmap(0, sizeof(uint64_t) * TARGET_GDT_ENTRIES,
+PROT_READ|PROT_WRITE,
+MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
+new_env->idt.base = target_mmap(0, sizeof(uint64_t) * (env->idt.limit + 1),
+PROT_READ|PROT_WRITE,
+MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
+memcpy((void*)new_env->gdt.base, (void*)env->gdt.base, sizeof(uint64_t) * 
TARGET_GDT_ENTRIES);
+memcpy((void*)new_env->idt.base, (void*)env->idt.base, sizeof(uint64_t) * 
(env->idt.limit + 1));
+OBJECT(new_cpu)->free = OBJECT(cpu)->free;
+#endif
 
 /* Clone all break/watchpoints.
Note: Once we support ptrace with hw-debug register access, make sure
-- 
2.34.1

[PATCH] linux-user: fix bug about incorrect base addresss of idt and gdt on i386 and x86_64

[fanwj]

On linux user mode, CPUX86State::idt::base and CPUX86State::gdt::base from 
Different CPUX86State Objects have same value, It is incorrect! Every 
CPUX86State::idt::base and Every CPUX86State::gdt::base Must points to 
independent memory space. Resolves: 
https://gitlab.com/qemu-project/qemu/-/issues/1405 Signed-off-by: fanwenjie --- 
linux-user/i386/cpu_loop.c | 10 ++ linux-user/main.c | 11 +++ 2 
files changed, 21 insertions(+) diff --git a/linux-user/i386/cpu_loop.c 
b/linux-user/i386/cpu_loop.c index 865413c08f..1f23bc5e3a 100644 --- 
a/linux-user/i386/cpu_loop.c +++ b/linux-user/i386/cpu_loop.c @@ -314,8 +314,18 
@@ void cpu_loop(CPUX86State *env) } } +static void target_cpu_free(void *obj) 
+{ + CPUArchState* env = ((CPUState*)obj)->env_ptr; + 
target_munmap(env->idt.base, sizeof(uint64_t) * (env->idt.limit + 1)); + 
target_munmap(env->gdt.base, sizeof(uint64_t) * TARGET_GDT_ENTRIES); + 
g_free(obj); +} + void target_cpu_copy_regs(CPUArchState *env, struct 
target_pt_regs *regs) { + CPUState* cpu = env_cpu(env); + OBJECT(cpu)->free = 
target_cpu_free; env->cr[0] = CR0_PG_MASK | CR0_WP_MASK | CR0_PE_MASK; 
env->hflags |= HF_PE_MASK | HF_CPL_MASK; if (env->features[FEAT_1_EDX] & 
CPUID_SSE) { diff --git a/linux-user/main.c b/linux-user/main.c index 
a17fed045b..2276040548 100644 --- a/linux-user/main.c +++ b/linux-user/main.c 
@@ -234,6 +234,17 @@ CPUArchState *cpu_copy(CPUArchState *env) 
new_cpu->tcg_cflags = cpu->tcg_cflags; memcpy(new_env, env, 
sizeof(CPUArchState)); +#if defined(TARGET_I386) || defined(TARGET_X86_64) + 
new_env->gdt.base = target_mmap(0, sizeof(uint64_t) * TARGET_GDT_ENTRIES, + 
PROT_READ|PROT_WRITE, + MAP_ANONYMOUS|MAP_PRIVATE, -1, 0); + new_env->idt.base 
= target_mmap(0, sizeof(uint64_t) * (env->idt.limit + 1), + 
PROT_READ|PROT_WRITE, + MAP_ANONYMOUS|MAP_PRIVATE, -1, 0); + 
memcpy((void*)new_env->gdt.base, (void*)env->gdt.base, sizeof(uint64_t) * 
TARGET_GDT_ENTRIES); + memcpy((void*)new_env->idt.base, (void*)env->idt.base, 
sizeof(uint64_t) * (env->idt.limit + 1)); + OBJECT(new_cpu)->free = 
OBJECT(cpu)->free; +#endif /* Clone all break/watchpoints. Note: Once we 
support ptrace with hw-debug register access, make sure -- 2.34.1

Re: [PATCH qemu] x86: don't let decompressed kernel image clobber setup_data

[Ard Biesheuvel]

On Mon, 2 Jan 2023 at 07:17, Borislav Petkov  wrote:
>
> On Mon, Jan 02, 2023 at 07:01:50AM +0100, Borislav Petkov wrote:
> > On Sat, Dec 31, 2022 at 07:31:21PM -0800, H. Peter Anvin wrote:
> > > It would probably be a good idea to add a "maximum physical address for
> > > initrd/setup_data/cmdline" field to struct kernel_info, though. It appears
> > > right now that those fields are being identity-mapped in the decompressor,
> > > and that means that if 48-bit addressing is used, physical memory may 
> > > extend
> > > past the addressable range.
> >
> > Yeah, we will probably need that too.
> >
> > Btw, looka here - it can't get any more obvious than that after dumping
> > setup_data too:
> >
> > early console in setup code
> > early console in extract_kernel
> > input_data: 0x040f92bf
> > input_len: 0x00f1c325
> > output: 0x0100
> > output_len: 0x03c5e7d8
> > kernel_total_size: 0x04428000
> > needed_size: 0x0460
> > boot_params->hdr.setup_data: 0x010203b0
> > trampoline_32bit: 0x0009d000
> >
> > Decompressing Linux... Parsing ELF... done.
> > Booting the kernel.
> > 
> >
> > Aligning them vertically:
> >
> > output:   0x0100
> > output_len:   0x03c5e7d8
> > kernel_total_size:0x04428000
> > needed_size:  0x0460
> > boot_params->hdr.setup_data:  0x010203b0
>
> Ok, waait a minute:
>
> 
> Field name: pref_address
> Type:   read (reloc)
> Offset/size:0x258/8
> Protocol:   2.10+
> 
>
>   This field, if nonzero, represents a preferred load address for the
>   kernel.  A relocating bootloader should attempt to load at this
>   address if possible.
>
>   A non-relocatable kernel will unconditionally move itself and to run
>   at this address.
>
> so a kernel loader (qemu in this case) already knows where the kernel goes:
>
> boot_params->hdr.setup_data: 0x01020450
> boot_params->hdr.pref_address: 0x0100
> ^
>
> now, considering that same kernel loader (qemu) knows how big that kernel is:
>
> kernel_total_size: 0x04428000
>
> should that loader *not* put anything that the kernel will use in the range
>
> pref_addr + kernel_total_size
>

This seems to be related to another issue that was discussed in the
context of this change, but affecting EFI boot not legacy BIOS boot
[0].

So, in a nutshell, we have the following pieces:
- QEMU, which manages a directory of files and other data blobs, and
exposes them via its fw_cfg interface.
- SeaBIOS, which invokes the fw_cfg interface to load the 'kernel'
blob at its preferred address
- The boot code in the kernel, which interprets the various fields in
the setup header to figure out where the compressed image lives etc

So the problem here, which applies to SETUP_DTB as well as
SETUP_RNG_SEED, is that the internal file representation of the kernel
blob (which does not have an absolute address at this point, it's just
a file in the fw_cfg filesystem) is augmented with:
1) setup_data linked-list entries carrying absolute addresses that are
assumed to be valid once SeaBIOS loads the file to memory
2) DTB and/or RNG seed blobs appended to the compressed 'kernel' blob,
but without updating that file's internal metadata

Issue 1) is what broke EFI boot, given that EFI interprets the kernel
blob as a PE/COFF image and hands it to the Loadimage() boot service,
which has no awareness of boot_params or setup_data and so just
ignores it and loads the image at an arbitrary address, resulting in
setup_data absolute address values pointing to bogus places.

It seems that now, we have another issue 2), where the fw_cfg view of
the file size goes out of sync with the compressed image's own view of
its size.

As a fix for issue 1), we explored another solution, which was to
allocate fixed areas in memory for the RNG seed, so that the absolute
address added to setup_data is guaranteed to be correct regardless of
where the compressed image is loaded, but that was shot down for other
reasons, and we ended up enabling this feature only for legacy BIOS
boot. But apparently, this approach has other issues so perhaps it is
better to revisit that solution again.

So instead of appending data to the compressed image and assuming that
it will stay in place, create or extend a memory reservation
elsewhere, and refer to its absolute address in setup_data.

-- 
Ard.


[0] 
https://lore.kernel.org/all/camj1kxfr6bv4_g0-wctu4fp_icrg060nhjx_j2dbnyifjky...@mail.gmail.com/

Re: [PATCH RESEND v3 00/10] migration: introduce dirtylimit capability

[Hyman Huang]

Ping,

Hi, David, how about the commit about live migration:
[PATCH RESEND v3 08/10] migration: Implement dirty-limit convergence algo.

在 2022/12/4 1:09, huang...@chinatelecom.cn 写道:

From: Hyman Huang(黄勇) 

v3(resend):
- fix the syntax error of the topic.

v3:
This version make some modifications inspired by Peter and Markus
as following:
1. Do the code clean up in [PATCH v2 02/11] suggested by Markus
2. Replace the [PATCH v2 03/11] with a much simpler patch posted by
Peter to fix the following bug:
https://bugzilla.redhat.com/show_bug.cgi?id=2124756
3. Fix the error path of migrate_params_check in [PATCH v2 04/11]
pointed out by Markus. Enrich the commit message to explain why
x-vcpu-dirty-limit-period an unstable parameter.
4. Refactor the dirty-limit convergence algo in [PATCH v2 07/11]
suggested by Peter:
a. apply blk_mig_bulk_active check before enable dirty-limit
b. drop the unhelpful check function before enable dirty-limit
c. change the migration_cancel logic, just cancel dirty-limit
   only if dirty-limit capability turned on.
d. abstract a code clean commit [PATCH v3 07/10] to adjust
   the check order before enable auto-converge
5. Change the name of observing indexes during dirty-limit live
migration to make them more easy-understanding. Use the
maximum throttle time of vpus as "dirty-limit-throttle-time-per-full"
6. Fix some grammatical and spelling errors pointed out by Markus
and enrich the document about the dirty-limit live migration
observing indexes "dirty-limit-ring-full-time"
and "dirty-limit-throttle-time-per-full"
7. Change the default value of x-vcpu-dirty-limit-period to 1000ms,
which is optimal value pointed out in cover letter in that
testing environment.
8. Drop the 2 guestperf test commits [PATCH v2 10/11],
[PATCH v2 11/11] and post them with a standalone series in the
future.

Thanks Peter and Markus sincerely for the passionate, efficient
and careful comments and suggestions.

Please review.

Yong

v2:
This version make a little bit modifications comparing with
version 1 as following:
1. fix the overflow issue reported by Peter Maydell
2. add parameter check for hmp "set_vcpu_dirty_limit" command
3. fix the racing issue between dirty ring reaper thread and
Qemu main thread.
4. add migrate parameter check for x-vcpu-dirty-limit-period
and vcpu-dirty-limit.
5. add the logic to forbid hmp/qmp commands set_vcpu_dirty_limit,
cancel_vcpu_dirty_limit during dirty-limit live migration when
implement dirty-limit convergence algo.
6. add capability check to ensure auto-converge and dirty-limit
are mutually exclusive.
7. pre-check if kvm dirty ring size is configured before setting
dirty-limit migrate parameter

A more comprehensive test was done comparing with version 1.

The following are test environment:
-
a. Host hardware info:

CPU:
Intel(R) Xeon(R) Gold 5218 CPU @ 2.30GHz

CPU(s):  64
On-line CPU(s) list: 0-63
Thread(s) per core:  2
Core(s) per socket:  16
Socket(s):   2
NUMA node(s):2

NUMA node0 CPU(s):   0-15,32-47
NUMA node1 CPU(s):   16-31,48-63

Memory:
Hynix  503Gi

Interface:
Intel Corporation Ethernet Connection X722 for 1GbE (rev 09)
Speed: 1000Mb/s

b. Host software info:

OS: ctyunos release 2
Kernel: 4.19.90-2102.2.0.0066.ctl2.x86_64
Libvirt baseline version:  libvirt-6.9.0
Qemu baseline version: qemu-5.0

c. vm scale
CPU: 4
Memory: 4G
-

All the supplementary test data shown as follows are basing on
above test environment.

In version 1, we post a test data from unixbench as follows:

$ taskset -c 8-15 ./Run -i 2 -c 8 {unixbench test item}

host cpu: Intel(R) Xeon(R) Platinum 8378A
host interface speed: 1000Mb/s
   |-+++---|
   | UnixBench test item | Normal | Dirtylimit | Auto-converge |
   |-+++---|
   | dhry2reg| 32800  | 32786  | 25292 |
   | whetstone-double| 10326  | 10315  | 9847  |
   | pipe| 15442  | 15271  | 14506 |
   | context1| 7260   | 6235   | 4514  |
   | spawn   | 3663   | 3317   | 3249  |
   | syscall | 4669   | 4667   | 3841  |
   |-+++---|

In version 2, we post a supplementary test data that do not use
taskset and make the scenario more general, see as follows:

$ ./Run

per-vcpu data:
   |-+++---|
   | UnixBench test item | Normal | Dirtylimit | Auto-converge |
   |-+++---|
   | dhry2reg| 2991