Re: [PATCH 03/11] RISC-V: Adding T-Head SYNC instructions

[Richard Henderson]

On 9/6/22 13:22, Christoph Muellner wrote:

+NOP_PRIVCHECK(th_sfence_vmas, REQUIRE_PRIV_MHS)
+NOP_PRIVCHECK(th_sync, REQUIRE_PRIV_MHSU)
+NOP_PRIVCHECK(th_sync_i, REQUIRE_PRIV_MHSU)
+NOP_PRIVCHECK(th_sync_is, REQUIRE_PRIV_MHSU)
+NOP_PRIVCHECK(th_sync_s, REQUIRE_PRIV_MHSU)


These should not be nops: th_sfence_vmas requires a tlb flush; th.sync{,.i} needs to end 
the current translation block; th.sync.{s,is} needs multiprocessor sync, which involves a 
call to async_safe_run_on_cpu.



r~

Re: [PATCH 10/11] RISC-V: Adding T-Head FMemIdx extension

[Richard Henderson]

On 9/6/22 13:22, Christoph Muellner wrote:

@@ -732,6 +733,7 @@ static int ex_rvc_shifti(DisasContext *ctx, int imm)
  #include "decode-xtheadbs.c.inc"
  #include "decode-xtheadcmo.c.inc"
  #include "decode-xtheadcondmov.c.inc"
+#include "decode-xtheadfmemidx.c.inc"
  #include "decode-xtheadmac.c.inc"
  #include "decode-xtheadmemidx.c.inc"
  #include "decode-xtheadmempair.c.inc"
@@ -1061,6 +1063,7 @@ static void decode_opc(CPURISCVState *env, DisasContext 
*ctx, uint16_t opcode)
  { has_xtheadbs_p, decode_xtheadbs },
  { has_xtheadcmo_p, decode_xtheadcmo },
  { has_xtheadcondmov_p, decode_xtheadcondmov },
+{ has_xtheadfmemidx_p, decode_xtheadfmemidx },
  { has_xtheadmac_p, decode_xtheadmac },
  { has_xtheadmemidx_p, decode_xtheadmemidx },
  { has_xtheadmempair_p, decode_xtheadmempair },


I think you should have a single decoder for all of the xthread extensions, and each 
translate function should test for the individual extension.  You know up-front that these 
extensions do not conflict.



r~

Re: [PATCH 11/11] RISC-V: Add initial support for T-Head C906 and C910 CPUs

[Richard Henderson]

On 9/6/22 13:22, Christoph Muellner wrote:

+DEFINE_CPU(TYPE_RISCV_CPU_THEAD_C906,   rv64_thead_c906_cpu_init),
+DEFINE_CPU(TYPE_RISCV_CPU_THEAD_C910,   rv64_thead_c906_cpu_init),


Why model both if they're identical?


r~

Re: [PATCH 3/3] target/ppc: Merge fsqrt and fsqrts helpers

[Richard Henderson]

On 9/6/22 15:22, Víctor Colombo wrote:

On 05/09/2022 14:20, Richard Henderson wrote:
Well, there would of course be no separate call, but 


I didn't understand what you meant here with 'no separate call'...


We generate a separate call from tcg to helper_reset_fpstatus sometimes.


Right, makes sense. And what about when an invalid operation occurs,
with the corresponding exception enabled bit set?
float_invalid_op_* would stop the execution and do_float_check_status
would not be called, right? So it would require to call
set_float_exception_flags there too, correct?


Correct.


r~

Re: [PATCH v9 06/10] s390x/cpu_topology: resetting the Topology-Change-Report

[Janis Schoetterl-Glausch]

On Fri, 2022-09-02 at 09:55 +0200, Pierre Morel wrote:
> During a subsystem reset the Topology-Change-Report is cleared
> by the machine.
> Let's ask KVM to clear the Modified Topology Change Report (MTCR)
>  bit of the SCA in the case of a subsystem reset.
> 
> Signed-off-by: Pierre Morel 
> ---
>  hw/s390x/cpu-topology.c  | 12 
>  hw/s390x/s390-virtio-ccw.c   |  1 +
>  target/s390x/cpu-sysemu.c|  7 +++
>  target/s390x/cpu.h   |  1 +
>  target/s390x/kvm/kvm.c   | 23 +++
>  target/s390x/kvm/kvm_s390x.h |  1 +
>  6 files changed, 45 insertions(+)

[...]

> diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
> index f96630440b..9c994d27d5 100644
> --- a/target/s390x/kvm/kvm.c
> +++ b/target/s390x/kvm/kvm.c
> @@ -2585,3 +2585,26 @@ int kvm_s390_get_zpci_op(void)
>  {
>  return cap_zpci_op;
>  }
> +
> +int kvm_s390_topology_set_mtcr(uint64_t attr)
> +{
> +struct kvm_device_attr attribute = {
> +.group = KVM_S390_VM_CPU_TOPOLOGY,
> +.attr  = attr,
> +};
> +int ret;
> +
> +if (!s390_has_feat(S390_FEAT_CONFIGURATION_TOPOLOGY)) {
> +return -EFAULT;

Why EFAULT?
The return value is just ignored when resetting, isn't it?
I wonder if it would be better not to.
Is it necessary because you're detecting the feature after you've
already created the S390Topology instance?
And you're doing that because that's just the order in which QEMU does
things? So the machine class is inited before the cpu model?
I wonder if there is a nice way to create the S390Topology only if the
feature is selected.

Anyway:
Reviewed-by: Janis Schoetterl-Glausch 

> +}
> +if (!kvm_vm_check_attr(kvm_state, KVM_S390_VM_CPU_TOPOLOGY, attr)) {
> +return -ENOENT;
> +}
> +
> +ret = kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, &attribute);
> +if (ret) {
> +error_report("Failed to set cpu topology attribute %lu: %s",
> + attr, strerror(-ret));
> +}
> +return ret;
> +}
> 
[...]

[PATCH 1/2] Update linux headers to v6.0-rc4

[Chenyi Qiang]

commit 7e18e42e4b280c85b76967a9106a13ca61c16179

Signed-off-by: Chenyi Qiang 
---
 include/standard-headers/asm-x86/bootparam.h  |   7 +-
 include/standard-headers/drm/drm_fourcc.h |  73 +++-
 include/standard-headers/linux/ethtool.h  |  29 +--
 include/standard-headers/linux/input.h|  12 +-
 include/standard-headers/linux/pci_regs.h |  30 ++-
 include/standard-headers/linux/vhost_types.h  |  17 +-
 include/standard-headers/linux/virtio_9p.h|   2 +-
 .../standard-headers/linux/virtio_config.h|   7 +-
 include/standard-headers/linux/virtio_ids.h   |  14 +-
 include/standard-headers/linux/virtio_net.h   |  34 +++-
 include/standard-headers/linux/virtio_pci.h   |   2 +
 include/standard-headers/linux/virtio_ring.h  |  16 +-
 linux-headers/asm-arm64/kvm.h |  33 +++-
 linux-headers/asm-generic/unistd.h|   4 +-
 linux-headers/asm-riscv/kvm.h |  22 +++
 linux-headers/asm-riscv/unistd.h  |   3 +-
 linux-headers/asm-s390/kvm.h  |   1 +
 linux-headers/asm-x86/kvm.h   |  33 ++--
 linux-headers/asm-x86/mman.h  |  14 --
 linux-headers/linux/kvm.h | 172 +-
 linux-headers/linux/userfaultfd.h |  10 +-
 linux-headers/linux/vduse.h   |  47 +
 linux-headers/linux/vfio.h|   4 +-
 linux-headers/linux/vfio_zdev.h   |   7 +
 linux-headers/linux/vhost.h   |  35 +++-
 25 files changed, 538 insertions(+), 90 deletions(-)

diff --git a/include/standard-headers/asm-x86/bootparam.h 
b/include/standard-headers/asm-x86/bootparam.h
index b2aaad10e5..0b06d2bff1 100644
--- a/include/standard-headers/asm-x86/bootparam.h
+++ b/include/standard-headers/asm-x86/bootparam.h
@@ -10,12 +10,13 @@
 #define SETUP_EFI  4
 #define SETUP_APPLE_PROPERTIES 5
 #define SETUP_JAILHOUSE6
+#define SETUP_CC_BLOB  7
+#define SETUP_IMA  8
 #define SETUP_RNG_SEED 9
+#define SETUP_ENUM_MAX SETUP_RNG_SEED
 
 #define SETUP_INDIRECT (1<<31)
-
-/* SETUP_INDIRECT | max(SETUP_*) */
-#define SETUP_TYPE_MAX (SETUP_INDIRECT | SETUP_JAILHOUSE)
+#define SETUP_TYPE_MAX (SETUP_ENUM_MAX | SETUP_INDIRECT)
 
 /* ram_size flags */
 #define RAMDISK_IMAGE_START_MASK   0x07FF
diff --git a/include/standard-headers/drm/drm_fourcc.h 
b/include/standard-headers/drm/drm_fourcc.h
index 4888f85f69..48b620cbef 100644
--- a/include/standard-headers/drm/drm_fourcc.h
+++ b/include/standard-headers/drm/drm_fourcc.h
@@ -558,7 +558,7 @@ extern "C" {
  *
  * The main surface is Y-tiled and is at plane index 0 whereas CCS is linear
  * and at index 1. The clear color is stored at index 2, and the pitch should
- * be ignored. The clear color structure is 256 bits. The first 128 bits
+ * be 64 bytes aligned. The clear color structure is 256 bits. The first 128 
bits
  * represents Raw Clear Color Red, Green, Blue and Alpha color each represented
  * by 32 bits. The raw clear color is consumed by the 3d engine and generates
  * the converted clear color of size 64 bits. The first 32 bits store the Lower
@@ -571,6 +571,53 @@ extern "C" {
  */
 #define I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC fourcc_mod_code(INTEL, 8)
 
+/*
+ * Intel Tile 4 layout
+ *
+ * This is a tiled layout using 4KB tiles in a row-major layout. It has the 
same
+ * shape as Tile Y at two granularities: 4KB (128B x 32) and 64B (16B x 4). It
+ * only differs from Tile Y at the 256B granularity in between. At this
+ * granularity, Tile Y has a shape of 16B x 32 rows, but this tiling has a 
shape
+ * of 64B x 8 rows.
+ */
+#define I915_FORMAT_MOD_4_TILED fourcc_mod_code(INTEL, 9)
+
+/*
+ * Intel color control surfaces (CCS) for DG2 render compression.
+ *
+ * The main surface is Tile 4 and at plane index 0. The CCS data is stored
+ * outside of the GEM object in a reserved memory area dedicated for the
+ * storage of the CCS data for all RC/RC_CC/MC compressible GEM objects. The
+ * main surface pitch is required to be a multiple of four Tile 4 widths.
+ */
+#define I915_FORMAT_MOD_4_TILED_DG2_RC_CCS fourcc_mod_code(INTEL, 10)
+
+/*
+ * Intel color control surfaces (CCS) for DG2 media compression.
+ *
+ * The main surface is Tile 4 and at plane index 0. For semi-planar formats
+ * like NV12, the Y and UV planes are Tile 4 and are located at plane indices
+ * 0 and 1, respectively. The CCS for all planes are stored outside of the
+ * GEM object in a reserved memory area dedicated for the storage of the
+ * CCS data for all RC/RC_CC/MC compressible GEM objects. The main surface
+ * pitch is required to be a multiple of four Tile 4 widths.
+ */
+#define I915_FORMAT_MOD_4_TILED_DG2_MC_CCS fourcc_mod_code(INTEL, 11)
+
+/*
+ * Intel Color Control Surface with Clear Color (CCS) for DG2 render 
compression.
+ *
+ * The ma

[PATCH 2/2] configure: Add -Wno-gnu-variable-sized-type-not-at-end

[Chenyi Qiang]

After updating linux headers to v6.0-rc, clang build on x86 target would
generate warnings like:

target/i386/kvm/kvm.c:470:25: error: field 'info' with variable sized
type 'struct kvm_msrs' not at the end of a struct or class is a GNU
extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
struct kvm_msrs info;
^
target/i386/kvm/kvm.c:1701:27: error: field 'cpuid' with variable sized
type 'struct kvm_cpuid2' not at the end of a struct or class is a GNU
extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
struct kvm_cpuid2 cpuid;
  ^
target/i386/kvm/kvm.c:2868:25: error: field 'info' with variable sized
type 'struct kvm_msrs' not at the end of a struct or class is a GNU
extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
struct kvm_msrs info;
^

Considering that it is OK to use GNU extension in QEMU (e.g. g_auto stuff),
it is acceptable to turn off this warning, which is only relevant to people
striving for fully portable C code.

Suggested-by: Daniel P. Berrangé 
Signed-off-by: Chenyi Qiang 
---
 configure | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configure b/configure
index 575dde1c1f..7e0a1a4187 100755
--- a/configure
+++ b/configure
@@ -1258,6 +1258,7 @@ add_to nowarn_flags -Wno-string-plus-int
 add_to nowarn_flags -Wno-typedef-redefinition
 add_to nowarn_flags -Wno-tautological-type-limit-compare
 add_to nowarn_flags -Wno-psabi
+add_to nowarn_flags -Wno-gnu-variable-sized-type-not-at-end
 
 gcc_flags="$warn_flags $nowarn_flags"
 
-- 
2.17.1

[PATCH 0/2] Update linux headers to v6.0-rc4 and fix the clang build error

[Chenyi Qiang]

After updating linux headers to v6.0-rc, clang build on x86 target would
generate warnings related to -Wgnu-variable-sized-type-not-at-end.

Simply turn off this warning in this patch set. If not suitable to turn it
off, any comments or suggestions are welcome.

Chenyi Qiang (2):
  Update linux headers to v6.0-rc4
  configure: Add -Wno-gnu-variable-sized-type-not-at-end

 configure |   1 +
 include/standard-headers/asm-x86/bootparam.h  |   7 +-
 include/standard-headers/drm/drm_fourcc.h |  73 +++-
 include/standard-headers/linux/ethtool.h  |  29 +--
 include/standard-headers/linux/input.h|  12 +-
 include/standard-headers/linux/pci_regs.h |  30 ++-
 include/standard-headers/linux/vhost_types.h  |  17 +-
 include/standard-headers/linux/virtio_9p.h|   2 +-
 .../standard-headers/linux/virtio_config.h|   7 +-
 include/standard-headers/linux/virtio_ids.h   |  14 +-
 include/standard-headers/linux/virtio_net.h   |  34 +++-
 include/standard-headers/linux/virtio_pci.h   |   2 +
 include/standard-headers/linux/virtio_ring.h  |  16 +-
 linux-headers/asm-arm64/kvm.h |  33 +++-
 linux-headers/asm-generic/unistd.h|   4 +-
 linux-headers/asm-riscv/kvm.h |  22 +++
 linux-headers/asm-riscv/unistd.h  |   3 +-
 linux-headers/asm-s390/kvm.h  |   1 +
 linux-headers/asm-x86/kvm.h   |  33 ++--
 linux-headers/asm-x86/mman.h  |  14 --
 linux-headers/linux/kvm.h | 172 +-
 linux-headers/linux/userfaultfd.h |  10 +-
 linux-headers/linux/vduse.h   |  47 +
 linux-headers/linux/vfio.h|   4 +-
 linux-headers/linux/vfio_zdev.h   |   7 +
 linux-headers/linux/vhost.h   |  35 +++-
 26 files changed, 539 insertions(+), 90 deletions(-)

-- 
2.17.1

Re: [PATCH v2] audio: add help option for -audio and -audiodev

[Claudio Fontana]

On 9/8/22 07:55, Volker Rümelin wrote:
> Am 07.09.22 um 19:30 schrieb Claudio Fontana:
> 
>> add a simple help option for -audio and -audiodev
>> to show the list of available drivers, and document them.
>>
>> Signed-off-by: Claudio Fontana 
>> ---
>>   audio/audio.c   | 20 
>>   audio/audio.h   |  1 +
>>   qemu-options.hx | 10 ++
>>   softmmu/vl.c|  9 +++--
>>   4 files changed, 34 insertions(+), 6 deletions(-)
>>
>> v1 -> v2: also extend the help to -audio.
>>
>>   -audio help
>>   -audio driver=help
>>   -audiodev help
>>
>> will all show the same results.
>>
>> diff --git a/audio/audio.c b/audio/audio.c
>> index 4f4bb10cce..ffb09ec825 100644
>> --- a/audio/audio.c
>> +++ b/audio/audio.c
>> @@ -32,6 +32,7 @@
>>   #include "qapi/qapi-visit-audio.h"
>>   #include "qemu/cutils.h"
>>   #include "qemu/module.h"
>> +#include "qemu/help_option.h"
>>   #include "sysemu/sysemu.h"
>>   #include "sysemu/replay.h"
>>   #include "sysemu/runstate.h"
>> @@ -2105,10 +2106,29 @@ static void audio_validate_opts(Audiodev *dev, Error 
>> **errp)
>>   }
>>   }
>>   
>> +void audio_help(void)
>> +{
>> +int i;
>> +
>> +printf("Available audio drivers:\n");
>> +printf("none\n");
>> +
>> +for (i = 0; audio_prio_list[i]; i++) {
>> +audio_driver *driver = audio_driver_lookup(audio_prio_list[i]);
> 
> Hi Claudio,
> 
> there is no guarantee that the audio_prio_list contains all audio 
> backend drivers. I would use this
> 
> +    for (i = 0; i < AUDIODEV_DRIVER__MAX; i++) {
> +    const char *name = AudiodevDriver_str(i);
> +    audio_driver *driver = audio_driver_lookup(name);
> 
> to enumerate all audio backend drivers.

Thanks Volker, will update accordingly.

Claudio

> 
> With best regards,
> Volker
> 
>> +if (driver) {
>> +printf("%s\n", driver->name);
>> +}
>> +}
>> +}
>> +
>>
> 

Re: [PATCH 2/3] module: add Error arguments to module_load_one and module_load_qom_one

[Richard Henderson]

On 9/6/22 12:55, Claudio Fontana wrote:

improve error handling during module load, by changing:

bool module_load_one(const char *prefix, const char *lib_name);
void module_load_qom_one(const char *type);

to:

bool module_load_one(const char *prefix, const char *name, Error **errp);
bool module_load_qom_one(const char *type, Error **errp);

module_load_qom_one has been introduced in:

commit 28457744c345 ("module: qom module support"), which built on top of
module_load_one, but discarded the bool return value. Restore it.

Adapt all callers to emit errors, or ignore them, or fail hard,
as appropriate in each context.

Signed-off-by: Claudio Fontana 
---
  audio/audio.c |   6 +-
  block.c   |  12 +++-
  block/dmg.c   |  10 ++-
  hw/core/qdev.c|  10 ++-
  include/qemu/module.h |  10 +--
  qom/object.c  |  15 +++-
  softmmu/qtest.c   |   6 +-
  ui/console.c  |  19 +-
  util/module.c | 155 ++
  9 files changed, 182 insertions(+), 61 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index 76b8735b44..4f4bb10cce 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -72,6 +72,7 @@ void audio_driver_register(audio_driver *drv)
  audio_driver *audio_driver_lookup(const char *name)
  {
  struct audio_driver *d;
+Error *local_err = NULL;
  
  QLIST_FOREACH(d, &audio_drivers, next) {

  if (strcmp(name, d->name) == 0) {
@@ -79,7 +80,10 @@ audio_driver *audio_driver_lookup(const char *name)
  }
  }
  
-audio_module_load_one(name);

+if (!audio_module_load_one(name, &local_err) && local_err) {
+error_report_err(local_err);
+}


Why would local_err not be set on failure?  Is this the NOTDIR thing?  I guess this is 
sufficient to distinguish the two cases...  The urge to bikeshed the return value is 
strong.  :-)



+bool module_load_one(const char *prefix, const char *name, Error **errp);
+bool module_load_qom_one(const char *type, Error **errp);


Doc comments go in the header file.


+/*
+ * module_load_file: attempt to load a dso file
+ *
+ * fname:  full pathname of the file to load
+ * export_symbols: if true, add the symbols to the global name space
+ * errp:   error to set.
+ *
+ * Return value:   0 on success (found and loaded), < 0 on failure.
+ * A return value of -ENOENT or -ENOTDIR means 'not found'.
+ * -EINVAL is used as the generic error condition.
+ *
+ * Error:  If fname is found, but could not be loaded, errp is set
+ * with the error encountered during load.
+ */
+static int module_load_file(const char *fname, bool export_symbols,
+Error **errp)
  {
  GModule *g_module;
  void (*sym)(void);
@@ -152,16 +168,19 @@ static int module_load_file(const char *fname, bool 
export_symbols)
  int len = strlen(fname);
  int suf_len = strlen(dsosuf);
  ModuleEntry *e, *next;
-int ret, flags;
+int flags;
  
  if (len <= suf_len || strcmp(&fname[len - suf_len], dsosuf)) {

-/* wrong suffix */
-ret = -EINVAL;
-goto out;
+error_setg(errp, "wrong filename, missing suffix %s", dsosuf);
+return -EINVAL;
  }
-if (access(fname, F_OK)) {
-ret = -ENOENT;
-goto out;
+if (access(fname, F_OK) != 0) {
+int ret = errno;
+if (ret != ENOENT && ret != ENOTDIR) {
+error_setg_errno(errp, ret, "error trying to access %s", fname);
+}
+/* most likely is EACCES here */
+return -ret;
  }


I looked at this a couple of days ago and came to the conclusion that the split between 
this function and its caller is wrong.


The directory path probe loop is currently split between the two functions.  I think the 
probe loop should be in the caller (i.e. the access call here moved out).  I think 
module_load_file should only be called once the file is known to exist.  Which then 
simplifies the set of errors to be returned from here.


(I might even go so far as to say module_load_file should be moved into module_load_one, 
because there's not really much here, and it would reduce ifdefs.)



r~

Re: [PULL 00/44] riscv-to-apply queue

[Alistair Francis]

On Wed, Sep 7, 2022 at 4:18 PM Stefan Hajnoczi  wrote:
>
> On Wed, 7 Sept 2022 at 04:32, Alistair Francis via
>  wrote:
> >
> > The following changes since commit 946e9bccf12f2bcc3ca471b820738fb22d14fc80:
> >
> >   Merge tag 'samuel-thibault' of https://people.debian.org/~sthibault/qemu 
> > into staging (2022-09-06 08:31:24 -0400)
> >
> > are available in the Git repository at:
> >
> >   g...@github.com:alistair23/qemu.git tags/pull-riscv-to-apply-20220907
>
> Hi Alistair,
> Please update .git/config to separate the push URL from the fetch URL:
>
> [remote "github"]
> url = https://github.com/alistair23/qemu.git
> pushUrl = g...@gitlab.com:alistair23/qemu.git
>
> That way future pull requests will include an https URL that allows
> fetches without ssh or a GitHub account. Thanks!

Done! Sorry about that

Alistair

>
> Stefan

[PATCH v3] audio: add help option for -audio and -audiodev

[Claudio Fontana]

add a simple help option for -audio and -audiodev
to show the list of available drivers, and document them.

Signed-off-by: Claudio Fontana 
---
 audio/audio.c   | 19 +++
 audio/audio.h   |  1 +
 qemu-options.hx | 10 ++
 softmmu/vl.c|  9 +++--
 4 files changed, 33 insertions(+), 6 deletions(-)

v2 -> v3: use AudiodevDriver_str and AUDIODEV_DRIVER__MAX
  to loop over drivers instead of audio_prio_list (Volker).

v1 -> v2: also extend the help to -audio.

 -audio help
 -audio driver=help
 -audiodev help

will all show the same results.


diff --git a/audio/audio.c b/audio/audio.c
index 4f4bb10cce..6647ed5b1e 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -32,6 +32,7 @@
 #include "qapi/qapi-visit-audio.h"
 #include "qemu/cutils.h"
 #include "qemu/module.h"
+#include "qemu/help_option.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/replay.h"
 #include "sysemu/runstate.h"
@@ -2105,10 +2106,28 @@ static void audio_validate_opts(Audiodev *dev, Error 
**errp)
 }
 }
 
+void audio_help(void)
+{
+int i;
+
+printf("Available audio drivers:\n");
+
+for (i = 0; i < AUDIODEV_DRIVER__MAX; i++) {
+audio_driver *driver = audio_driver_lookup(AudiodevDriver_str(i));
+if (driver) {
+printf("%s\n", driver->name);
+}
+}
+}
+
 void audio_parse_option(const char *opt)
 {
 Audiodev *dev = NULL;
 
+if (is_help_option(opt)) {
+audio_help();
+exit(EXIT_SUCCESS);
+}
 Visitor *v = qobject_input_visitor_new_str(opt, "driver", &error_fatal);
 visit_type_Audiodev(v, NULL, &dev, &error_fatal);
 visit_free(v);
diff --git a/audio/audio.h b/audio/audio.h
index 27e67079a0..01bdc567fb 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -171,6 +171,7 @@ void audio_sample_from_uint64(void *samples, int pos,
 void audio_define(Audiodev *audio);
 void audio_parse_option(const char *opt);
 bool audio_init_audiodevs(void);
+void audio_help(void);
 void audio_legacy_help(void);
 
 AudioState *audio_state_by_name(const char *name);
diff --git a/qemu-options.hx b/qemu-options.hx
index 31c04f7eea..04cd4dacfc 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -704,10 +704,11 @@ SRST
 ``-audio [driver=]driver,model=value[,prop[=value][,...]]``
 This option is a shortcut for configuring both the guest audio
 hardware and the host audio backend in one go.
-The host backend options are the same as with the corresponding
-``-audiodev`` options below. The guest hardware model can be set with
-``model=modelname``. Use ``model=help`` to list the available device
-types.
+The driver option is the same as with the corresponding ``-audiodev`` 
option below.
+The guest hardware model can be set with ``model=modelname``.
+
+Use ``driver=help`` to list the available drivers,
+and ``model=help`` to list the available device types.
 
 The following two example do exactly the same, to show how ``-audio``
 can be used to shorten the command line length:
@@ -721,6 +722,7 @@ ERST
 DEF("audiodev", HAS_ARG, QEMU_OPTION_audiodev,
 "-audiodev [driver=]driver,id=id[,prop[=value][,...]]\n"
 "specifies the audio backend to use\n"
+"Use ``-audiodev help`` to list the available drivers\n"
 "id= identifier of the backend\n"
 "timer-period= timer period in microseconds\n"
 "in|out.mixing-engine= use mixing engine to mix streams 
inside QEMU\n"
diff --git a/softmmu/vl.c b/softmmu/vl.c
index dea4005e47..2f8eecf5c1 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2842,11 +2842,16 @@ void qemu_init(int argc, char **argv, char **envp)
 audio_parse_option(optarg);
 break;
 case QEMU_OPTION_audio: {
-QDict *dict = keyval_parse(optarg, "driver", NULL, 
&error_fatal);
+bool help;
 char *model;
 Audiodev *dev = NULL;
 Visitor *v;
-
+QDict *dict = keyval_parse(optarg, "driver", &help, 
&error_fatal);
+if (help || (qdict_haskey(dict, "driver") &&
+ is_help_option(qdict_get_str(dict, "driver" {
+audio_help();
+exit(EXIT_SUCCESS);
+}
 if (!qdict_haskey(dict, "id")) {
 qdict_put_str(dict, "id", "audiodev0");
 }
-- 
2.26.2

Re: [PATCH 11/11] RISC-V: Add initial support for T-Head C906 and C910 CPUs

[Christoph Müllner]

On Thu, Sep 8, 2022 at 9:46 AM Richard Henderson <
richard.hender...@linaro.org> wrote:

> On 9/6/22 13:22, Christoph Muellner wrote:
> > +DEFINE_CPU(TYPE_RISCV_CPU_THEAD_C906,
>  rv64_thead_c906_cpu_init),
> > +DEFINE_CPU(TYPE_RISCV_CPU_THEAD_C910,
>  rv64_thead_c906_cpu_init),
>
> Why model both if they're identical?
>

I figured that users might expect that (existence of "thead-c906" and
"thead-c910").
And using "thead-c9xx" feels like it would be regretted in the future.

Should I drop "thead-c910"?



>
>
> r~
>

Re: [PATCH 2/3] module: add Error arguments to module_load_one and module_load_qom_one

[Claudio Fontana]

Hi Richard, thanks for looking at this,

On 9/8/22 10:11, Richard Henderson wrote:
> On 9/6/22 12:55, Claudio Fontana wrote:
>> improve error handling during module load, by changing:
>>
>> bool module_load_one(const char *prefix, const char *lib_name);
>> void module_load_qom_one(const char *type);
>>
>> to:
>>
>> bool module_load_one(const char *prefix, const char *name, Error **errp);
>> bool module_load_qom_one(const char *type, Error **errp);
>>
>> module_load_qom_one has been introduced in:
>>
>> commit 28457744c345 ("module: qom module support"), which built on top of
>> module_load_one, but discarded the bool return value. Restore it.
>>
>> Adapt all callers to emit errors, or ignore them, or fail hard,
>> as appropriate in each context.
>>
>> Signed-off-by: Claudio Fontana 
>> ---
>>   audio/audio.c |   6 +-
>>   block.c   |  12 +++-
>>   block/dmg.c   |  10 ++-
>>   hw/core/qdev.c|  10 ++-
>>   include/qemu/module.h |  10 +--
>>   qom/object.c  |  15 +++-
>>   softmmu/qtest.c   |   6 +-
>>   ui/console.c  |  19 +-
>>   util/module.c | 155 ++
>>   9 files changed, 182 insertions(+), 61 deletions(-)
>>
>> diff --git a/audio/audio.c b/audio/audio.c
>> index 76b8735b44..4f4bb10cce 100644
>> --- a/audio/audio.c
>> +++ b/audio/audio.c
>> @@ -72,6 +72,7 @@ void audio_driver_register(audio_driver *drv)
>>   audio_driver *audio_driver_lookup(const char *name)
>>   {
>>   struct audio_driver *d;
>> +Error *local_err = NULL;
>>   
>>   QLIST_FOREACH(d, &audio_drivers, next) {
>>   if (strcmp(name, d->name) == 0) {
>> @@ -79,7 +80,10 @@ audio_driver *audio_driver_lookup(const char *name)
>>   }
>>   }
>>   
>> -audio_module_load_one(name);
>> +if (!audio_module_load_one(name, &local_err) && local_err) {
>> +error_report_err(local_err);
>> +}
> 
> Why would local_err not be set on failure?  Is this the NOTDIR thing?  I 
> guess this is 

Right, we need to distinguish the case where the module does not exist from the 
case where
the module exists but an error has occured while trying to load it.

In the distros we build one time with all the modules enabled, but then may 
deliver those modules
or not deliver them in a specific distro, or the user may or may not have 
installed the packages
containing those DSOs.

So failing to find a module may be just fine, while we want to report errors if 
a load goes wrong.
We just encountered a bug where the actual cause was hidden by the fact that no 
error was produced on module load.


> sufficient to distinguish the two cases...  The urge to bikeshed the return 
> value is 
> strong.  :-)

If you find a better way let me know, this is just the first thing that came to 
mind to distinguish the cases, ie:

return value true -> loaded successfully

return value false -> not loaded:
  local_err set   -> error during load
  local_err unset -> no error, just not present

> 
>> +bool module_load_one(const char *prefix, const char *name, Error **errp);
>> +bool module_load_qom_one(const char *type, Error **errp);
> 
> Doc comments go in the header file.
> 
>> +/*
>> + * module_load_file: attempt to load a dso file
>> + *
>> + * fname:  full pathname of the file to load
>> + * export_symbols: if true, add the symbols to the global name space
>> + * errp:   error to set.
>> + *
>> + * Return value:   0 on success (found and loaded), < 0 on failure.
>> + * A return value of -ENOENT or -ENOTDIR means 'not found'.
>> + * -EINVAL is used as the generic error condition.
>> + *
>> + * Error:  If fname is found, but could not be loaded, errp is set
>> + * with the error encountered during load.
>> + */
>> +static int module_load_file(const char *fname, bool export_symbols,
>> +Error **errp)
>>   {
>>   GModule *g_module;
>>   void (*sym)(void);
>> @@ -152,16 +168,19 @@ static int module_load_file(const char *fname, bool 
>> export_symbols)
>>   int len = strlen(fname);
>>   int suf_len = strlen(dsosuf);
>>   ModuleEntry *e, *next;
>> -int ret, flags;
>> +int flags;
>>   
>>   if (len <= suf_len || strcmp(&fname[len - suf_len], dsosuf)) {
>> -/* wrong suffix */
>> -ret = -EINVAL;
>> -goto out;
>> +error_setg(errp, "wrong filename, missing suffix %s", dsosuf);
>> +return -EINVAL;
>>   }
>> -if (access(fname, F_OK)) {
>> -ret = -ENOENT;
>> -goto out;
>> +if (access(fname, F_OK) != 0) {
>> +int ret = errno;
>> +if (ret != ENOENT && ret != ENOTDIR) {
>> +error_setg_errno(errp, ret, "error trying to access %s", fname);
>> +}
>> +/* most likely is EACCES here */
>> +return -ret;
>>   }
> 
> I looked at this a couple of days ago and came to the 

Re: [PATCH v5 00/13] Instantiate VT82xx functions in host device

[Bernhard Beschow]

Am 1. September 2022 11:41:14 UTC schrieb Bernhard Beschow :
>v5:
>
>* Add patch "Inline vt82c686b_southbridge_init() and remove it" (Zoltan)
>
>* Use machine parameter when creating rtc-time alias (Zoltan)
>
>
>
>Testing done: Same as in v3.
>
>
>
>v4:
>
>* Fix in comment: AC97 Modem -> MC97 Modem (Zoltan)
>
>* Introduce TYPE_VT82C686B_USB_UHCI define (Zoltan)
>
>* Introduce TYPE_VIA_IDE define (for consistency)
>
>
>
>v3:
>
>* Replace pre increment by post increment in for loop (Zoltan)
>
>* Move class defines close to where the class is defined (Zoltan)
>
>
>
>Testing done:
>
>* `make check-avocado`
>
>  Passes for boot_linux_console.py for mips64el_fuloong2e
>
>* `qemu-system-ppc -machine pegasos2 -rtc base=localtime -device 
>ati-vga,guest_hwcursor=true,romfile="" -cdrom morphos-3.17.iso -kernel 
>morphos-3.17/boot.img`
>
>  Boots successfully and it is possible to open games and tools.
>
>
>
>v2:
>
>* Keep the call to pci_ide_create_devs() in board code for consistency (Zoltan)
>
>* Create rtc-time alias in board rather than in south bridge code
>
>* Remove stale comments about PCI functions (Zoltan)
>
>
>
>v1:
>
>This series instantiates all PCI functions of the VT82xx south bridges in the 
>south bridges themselves.
>
>For the IDE function this is especially important since its interrupt routing 
>is configured in the
>
>ISA function, hence doesn't make sense to instantiate it as a "Frankenstein" 
>device. The interrupt
>
>routing is currently hardcoded and changing that is currently not in the scope 
>of this series.
>
>
>
>Testing done:
>
>* `qemu-system-ppc -machine pegasos2 -rtc base=localtime -device 
>ati-vga,guest_hwcursor=true,romfile="" -cdrom morphos-3.17.iso -kernel 
>morphos-3.17/boot.img`
>
>  Boots successfully and it is possible to open games and tools.
>
>
>
>* I was unable to test the fuloong2e board even before this series since it 
>seems to be unfinished [1].
>
>  A buildroot-baked kernel [2] booted but doesn't find its root partition, 
> though the issues could be in the buildroot receipt I created.
>
>
>
>[1] https://osdn.net/projects/qmiga/wiki/SubprojectPegasos2
>
>[2] https://github.com/shentok/buildroot/commits/fuloong2e
>

Ping

Zoltan, would you mind giving your Reviewed-by for 'hw/mips/fuloong2e: Inline 
vt82c686b_southbridge_init() and remove it' explicitly? Perhaps I was too eager 
to omit it since I didn't want to put words in your mouth.

What else is missing? Who would do the pull request?

Thanks,
Bernhard
>
>
>Bernhard Beschow (13):
>
>  hw/isa/vt82c686: Resolve chip-specific realize methods
>
>  hw/isa/vt82c686: Resolve unneeded attribute
>
>  hw/isa/vt82c686: Prefer pci_address_space() over get_system_memory()
>
>  hw/isa/vt82c686: Reuse errp
>
>  hw/isa/vt82c686: Introduce TYPE_VIA_IDE define
>
>  hw/isa/vt82c686: Instantiate IDE function in host device
>
>  hw/isa/vt82c686: Introduce TYPE_VT82C686B_USB_UHCI define
>
>  hw/isa/vt82c686: Instantiate USB functions in host device
>
>  hw/isa/vt82c686: Instantiate PM function in host device
>
>  hw/isa/vt82c686: Instantiate AC97 and MC97 functions in host device
>
>  hw/mips/fuloong2e: Inline vt82c686b_southbridge_init() and remove it
>
>  hw/isa/vt82c686: Embed RTCState in host device
>
>  hw/isa/vt82c686: Create rtc-time alias in boards instead
>
>
>
> configs/devices/mips64el-softmmu/default.mak |   1 -
>
> hw/ide/via.c |   2 +-
>
> hw/isa/Kconfig   |   1 +
>
> hw/isa/vt82c686.c| 120 +++
>
> hw/mips/fuloong2e.c  |  39 +++---
>
> hw/ppc/Kconfig   |   1 -
>
> hw/ppc/pegasos2.c|  25 ++--
>
> hw/usb/vt82c686-uhci-pci.c   |   4 +-
>
> include/hw/isa/vt82c686.h|   4 +-
>
> 9 files changed, 126 insertions(+), 71 deletions(-)
>
>
>
>-- >
>2.37.3
>
>
>

Re: [PATCH 00/42] Consolidate PIIX south bridges

[Bernhard Beschow]

Am 1. September 2022 16:25:31 UTC schrieb Bernhard Beschow :
>This series consolidates the implementations of the PIIX3 and PIIX4 south
>
>bridges and is an extended version of [1]. The motivation is to share as much
>
>code as possible and to bring both device models to feature parity such that
>
>perhaps PIIX4 can become a drop-in-replacement for PIIX3 in the pc machine. 
>This
>
>could resolve the "Frankenstein" PIIX4-PM problem in PIIX3 discussed on this
>
>list before.
>
>
>
>The series is structured as follows: First, PIIX3 is changed to instantiate
>
>internal devices itself, like PIIX4 does already. Second, PIIX3 gets prepared
>
>for the merge with PIIX4 which includes some fixes, cleanups, and renamings.
>
>Third, the same is done for PIIX4. In step four the implementations are merged.
>
>Since some consolidations could be done easier with merged implementations, the
>
>consolidation continues in step five which concludes the series.
>
>
>
>One particular challenge in this series was that the PIC of PIIX3 used to be
>
>instantiated outside of the south bridge while some sub functions require a PIC
>
>with populated qemu_irqs. This has been solved by introducing a proxy PIC which
>
>furthermore allows PIIX3 to be agnostic towards the virtualization technology
>
>used (KVM, TCG, Xen). Due to consolidation PIIX4 gained the PIC as well,
>
>possibly allowing the Malta board to gain KVM capabilities in the future.
>

Ping

Never mind the comment about Malta. I think it supports KVM just fine.

>
>
>Another challenge was dealing with optional devices where Peter already gave
>
>advice in [1] which this series implements.
>
>
>
>An unsolved problem still is PCI interrupt handling. The first function
>
>passed to pci_bus_irqs() is device-specific while the second one seems
>
>board-specific. This causes both PIIX device models to be coupled to a
>
>particular board. Any advice how to resolve this would be highly appreaciated.
>
>
>
>Last but not least there might be some opportunity to consolidate VM state
>
>handling, probably by reusing the one from PIIX3. Since I'm not very familiar
>
>with the requirements I didn't touch it so far.
>
>
>
>Testing done:
>
>* make check
>
>* Boot live CD:
>
>  * `qemu-system-x86_64 -M pc -m 2G -accel kvm -cpu host -cdrom
>
>manjaro-kde-21.3.2-220704-linux515.iso`
>
>  * `qemu-system-x86_64 -M q35 -m 2G -accel kvm -cpu host -cdrom
>
>manjaro-kde-21.3.2-220704-linux515.iso`
>
>
>
>[1] https://lists.nongnu.org/archive/html/qemu-devel/2022-07/msg02348.html
>
>
>
>Bernhard Beschow (42):
>
>  hw/i386/pc: Create DMA controllers in south bridges
>
>  hw/i386/pc: Create RTC controllers in south bridges
>
>  hw/i386/pc: No need for rtc_state to be an out-parameter
>
>  hw/i386/pc_piix: Allow for setting properties before realizing PIIX3
>
>south bridge
>
>  hw/isa/piix3: Create USB controller in host device
>
>  hw/isa/piix3: Create power management controller in host device
>
>  hw/intc/i8259: Introduce i8259 proxy "isa-pic"
>
>  hw/isa/piix3: Create ISA PIC in host device
>
>  hw/isa/piix3: Create IDE controller in host device
>
>  hw/isa/piix3: Wire up ACPI interrupt internally
>
>  hw/isa/piix3: Remove extra ';' outside of functions
>
>  hw/isa/piix3: Remove unused include
>
>  hw/isa/piix3: Add size constraints to rcr_ops
>
>  hw/isa/piix3: Modernize reset handling
>
>  hw/isa/piix3: Prefer pci_address_space() over get_system_memory()
>
>  hw/isa/piix3: Allow board to provide PCI interrupt routes
>
>  hw/isa/piix3: Resolve redundant PIIX_NUM_PIC_IRQS
>
>  hw/isa/piix3: Rename pci_piix3_props for sharing with PIIX4
>
>  hw/isa/piix3: Rename piix3_reset() for sharing with PIIX4
>
>  hw/isa/piix3: Prefix pci_slot_get_pirq() with "piix3_"
>
>  hw/isa/piix3: Rename typedef PIIX3State to PIIXState
>
>  hw/mips/malta: Reuse dev variable
>
>  meson: Fix dependencies of piix4 southbridge
>
>  hw/isa/piix4: Add missing initialization
>
>  hw/isa/piix4: Move pci_ide_create_devs() call to board code
>
>  hw/isa/piix4: Make PIIX4's ACPI and USB functions optional
>
>  hw/isa/piix4: Allow board to provide PCI interrupt routes
>
>  hw/isa/piix4: Remove unused code
>
>  hw/isa/piix4: Use ISA PIC device
>
>  hw/isa/piix4: Reuse struct PIIXState from PIIX3
>
>  hw/isa/piix4: Rename reset control operations to match PIIX3
>
>  hw/isa/piix4: Rename wrongly named method
>
>  hw/isa/piix4: Prefix pci_slot_get_pirq() with "piix4_"
>
>  hw/isa/piix3: Merge hw/isa/piix4.c
>
>  hw/isa/piix: Harmonize names of reset control memory regions
>
>  hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4
>
>  hw/isa/piix: Rename functions to be shared for interrupt triggering
>
>  hw/isa/piix: Consolidate IRQ triggering
>
>  hw/isa/piix: Unexport PIIXState
>
>  hw/isa/piix: Share PIIX3 base class with PIIX4
>
>  hw/isa/piix: Drop the "3" from the PIIX base class
>
>  hw/i386/acpi-build: Resolve PIIX ISA bridge rather than ACPI
>
>controller
>
>
>
> MAINTAINERS |   6 +

Re: [PATCH 2/2] configure: Add -Wno-gnu-variable-sized-type-not-at-end

[Peter Maydell]

On Thu, 8 Sept 2022 at 09:08, Chenyi Qiang  wrote:
>
> After updating linux headers to v6.0-rc, clang build on x86 target would
> generate warnings like:
>
> target/i386/kvm/kvm.c:470:25: error: field 'info' with variable sized
> type 'struct kvm_msrs' not at the end of a struct or class is a GNU
> extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
> struct kvm_msrs info;
> ^
> target/i386/kvm/kvm.c:1701:27: error: field 'cpuid' with variable sized
> type 'struct kvm_cpuid2' not at the end of a struct or class is a GNU
> extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
> struct kvm_cpuid2 cpuid;
>   ^
> target/i386/kvm/kvm.c:2868:25: error: field 'info' with variable sized
> type 'struct kvm_msrs' not at the end of a struct or class is a GNU
> extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
> struct kvm_msrs info;
> ^
>
> Considering that it is OK to use GNU extension in QEMU (e.g. g_auto stuff),
> it is acceptable to turn off this warning, which is only relevant to people
> striving for fully portable C code.

Can we get the kernel folks to fix their headers not to
use GCC extensions like this ? It's not a big deal for us
I guess, but in general it doesn't seem great that the
kernel headers rely on userspace to silence warnings...

-- PMM

Re: [PATCH 2/2] configure: Add -Wno-gnu-variable-sized-type-not-at-end

[Richard Henderson]

On 9/8/22 09:07, Chenyi Qiang wrote:

After updating linux headers to v6.0-rc, clang build on x86 target would
generate warnings like:

target/i386/kvm/kvm.c:470:25: error: field 'info' with variable sized
type 'struct kvm_msrs' not at the end of a struct or class is a GNU
extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
 struct kvm_msrs info;
 ^
target/i386/kvm/kvm.c:1701:27: error: field 'cpuid' with variable sized
type 'struct kvm_cpuid2' not at the end of a struct or class is a GNU
extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
 struct kvm_cpuid2 cpuid;
   ^
target/i386/kvm/kvm.c:2868:25: error: field 'info' with variable sized
type 'struct kvm_msrs' not at the end of a struct or class is a GNU
extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
 struct kvm_msrs info;
 ^

Considering that it is OK to use GNU extension in QEMU (e.g. g_auto stuff),
it is acceptable to turn off this warning, which is only relevant to people
striving for fully portable C code.

Suggested-by: Daniel P. Berrangé
Signed-off-by: Chenyi Qiang
---
  configure | 1 +
  1 file changed, 1 insertion(+)


Reviewed-by: Richard Henderson 

I'm surprised this is on with -std=gnu11, as opposed to -std=c11.


r~

Re: [PATCH 11/11] RISC-V: Add initial support for T-Head C906 and C910 CPUs

[Richard Henderson]

On 9/8/22 09:23, Christoph Müllner wrote:



On Thu, Sep 8, 2022 at 9:46 AM Richard Henderson > wrote:


On 9/6/22 13:22, Christoph Muellner wrote:
 > +    DEFINE_CPU(TYPE_RISCV_CPU_THEAD_C906,       
rv64_thead_c906_cpu_init),
 > +    DEFINE_CPU(TYPE_RISCV_CPU_THEAD_C910,       
rv64_thead_c906_cpu_init),

Why model both if they're identical?


I figured that users might expect that (existence of "thead-c906" and 
"thead-c910").
And using "thead-c9xx" feels like it would be regretted in the future.

Should I drop "thead-c910"?


Quite possibly.  For Arm, we don't try to supply every cpu model, only those that differ 
in some substantial way.



r~

Re: [PATCH v5 0/2] block: add missed block_acct_setup with new block device init procedure

[Kevin Wolf]

Am 07.09.2022 um 19:25 hat Denis V. Lunev geschrieben:
> ping

I'll get to it (and quite a few other small series on the list that
should be quick to review), but probably only after KVM Forum. So I'll
have to ask you to be patient for a little longer.

Kevin

Re: [PATCH 11/11] RISC-V: Add initial support for T-Head C906 and C910 CPUs

[Christoph Müllner]

On Thu, Sep 8, 2022 at 10:56 AM Richard Henderson <
richard.hender...@linaro.org> wrote:

> On 9/8/22 09:23, Christoph Müllner wrote:
> >
> >
> > On Thu, Sep 8, 2022 at 9:46 AM Richard Henderson <
> richard.hender...@linaro.org
> > > wrote:
> >
> > On 9/6/22 13:22, Christoph Muellner wrote:
> >  > +DEFINE_CPU(TYPE_RISCV_CPU_THEAD_C906,
>  rv64_thead_c906_cpu_init),
> >  > +DEFINE_CPU(TYPE_RISCV_CPU_THEAD_C910,
>  rv64_thead_c906_cpu_init),
> >
> > Why model both if they're identical?
> >
> >
> > I figured that users might expect that (existence of "thead-c906" and
> "thead-c910").
> > And using "thead-c9xx" feels like it would be regretted in the future.
> >
> > Should I drop "thead-c910"?
>
> Quite possibly.  For Arm, we don't try to supply every cpu model, only
> those that differ
> in some substantial way.
>

Ok, will do.

Thanks!


>
>
> r~
>

Re: [PATCH 2/2] configure: Add -Wno-gnu-variable-sized-type-not-at-end

[Cornelia Huck]

On Thu, Sep 08 2022, Peter Maydell  wrote:

> On Thu, 8 Sept 2022 at 09:08, Chenyi Qiang  wrote:
>>
>> After updating linux headers to v6.0-rc, clang build on x86 target would
>> generate warnings like:
>>
>> target/i386/kvm/kvm.c:470:25: error: field 'info' with variable sized
>> type 'struct kvm_msrs' not at the end of a struct or class is a GNU
>> extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
>> struct kvm_msrs info;
>> ^
>> target/i386/kvm/kvm.c:1701:27: error: field 'cpuid' with variable sized
>> type 'struct kvm_cpuid2' not at the end of a struct or class is a GNU
>> extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
>> struct kvm_cpuid2 cpuid;
>>   ^
>> target/i386/kvm/kvm.c:2868:25: error: field 'info' with variable sized
>> type 'struct kvm_msrs' not at the end of a struct or class is a GNU
>> extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
>> struct kvm_msrs info;
>> ^
>>
>> Considering that it is OK to use GNU extension in QEMU (e.g. g_auto stuff),
>> it is acceptable to turn off this warning, which is only relevant to people
>> striving for fully portable C code.
>
> Can we get the kernel folks to fix their headers not to
> use GCC extensions like this ? It's not a big deal for us
> I guess, but in general it doesn't seem great that the
> kernel headers rely on userspace to silence warnings...

We only get the warnings once we define structures that include the
imported structures, so I guess the header per se is not broken (and the
kernel itself probably does not use them that way, given that there's an
effort to build it with clang as well.)

Re: [PATCH v4 0/4] hw/ssi: ibex_spi: cleanup and fixup bugs

[Alistair Francis]

On Tue, Aug 23, 2022 at 8:12 AM Wilfred Mallawa
 wrote:
>
> From: Wilfred Mallawa 
>
> Patch V4 fixes up:
> - Fixup missing register field clearing on tx/rx_fifo_reset() in [2/4]
>
> Testing:
> - Tested with Opentitan unit tests for TockOS...[OK]
>
> Wilfred Mallawa (4):
>   hw/ssi: ibex_spi: fixup typos in ibex_spi_host
>   hw/ssi: ibex_spi: fixup coverity issue
>   hw/ssi: ibex_spi: fixup/add rw1c functionality
>   hw/ssi: ibex_spi: update reg addr

Thanks!

Applied to riscv-to-apply.next

Alistair

>
>  hw/ssi/ibex_spi_host.c | 174 -
>  include/hw/ssi/ibex_spi_host.h |   4 +-
>  2 files changed, 106 insertions(+), 72 deletions(-)
>
> --
> 2.37.2
>
>

Re: [PATCH 2/2] configure: Add -Wno-gnu-variable-sized-type-not-at-end

[Daniel P . Berrangé]

On Thu, Sep 08, 2022 at 09:53:44AM +0100, Peter Maydell wrote:
> On Thu, 8 Sept 2022 at 09:08, Chenyi Qiang  wrote:
> >
> > After updating linux headers to v6.0-rc, clang build on x86 target would
> > generate warnings like:
> >
> > target/i386/kvm/kvm.c:470:25: error: field 'info' with variable sized
> > type 'struct kvm_msrs' not at the end of a struct or class is a GNU
> > extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
> > struct kvm_msrs info;
> > ^
> > target/i386/kvm/kvm.c:1701:27: error: field 'cpuid' with variable sized
> > type 'struct kvm_cpuid2' not at the end of a struct or class is a GNU
> > extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
> > struct kvm_cpuid2 cpuid;
> >   ^
> > target/i386/kvm/kvm.c:2868:25: error: field 'info' with variable sized
> > type 'struct kvm_msrs' not at the end of a struct or class is a GNU
> > extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
> > struct kvm_msrs info;
> > ^
> >
> > Considering that it is OK to use GNU extension in QEMU (e.g. g_auto stuff),
> > it is acceptable to turn off this warning, which is only relevant to people
> > striving for fully portable C code.
> 
> Can we get the kernel folks to fix their headers not to
> use GCC extensions like this ? It's not a big deal for us
> I guess, but in general it doesn't seem great that the
> kernel headers rely on userspace to silence warnings...

The kernel headers are fine actually - it is C99 compliant to have
a unsized array field in structs. eg 

The problem is in the QEMU code which is taking the kernel declared
struct, and then embedding it inside another struct. e.g. the
kernel exposes:

  struct kvm_msrs {
__u32 nmsrs; /* number of msrs in entries */
__u32 pad;

struct kvm_msr_entry entries[];
  };


which we then use as:

  uint64_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t index)
  {
struct {
struct kvm_msrs info;
struct kvm_msr_entry entries[1];
} msr_data = {};

'kvm_msrs info' is variable in size, so offset of 'entries[1]' is
undefined by C99. I presume the GNU defined semantics are that the
variable length 'entries[]' field in 'info' is zero-sized, in order
to give predictable offset for 'entries[1]' in the local msr_data.

IOW, our locally defined struct is just there to force a stack
allocation large enough for 1 kvm_msr_entry. A clever trick, but
requires that we turn off the CLang portability warning

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

Re: [PATCH V4 1/3] hw/arm, loongarch: Move load_image_to_fw_cfg() to common location

[Alistair Francis]

On Tue, Sep 6, 2022 at 11:38 AM Sunil V L  wrote:
>
> load_image_to_fw_cfg() is duplicated by both arm and loongarch. The same
> function will be required by riscv too. So, it's time to refactor and
> move this function to a common path.
>
> Signed-off-by: Sunil V L 
> Reviewed-by: Andrew Jones 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  hw/arm/boot.c | 49 ---
>  hw/loongarch/virt.c   | 33 --
>  hw/nvram/fw_cfg.c | 32 +
>  include/hw/nvram/fw_cfg.h | 21 +
>  4 files changed, 53 insertions(+), 82 deletions(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index ada2717f76..704f368d9c 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -818,55 +818,6 @@ static void do_cpu_reset(void *opaque)
>  }
>  }
>
> -/**
> - * load_image_to_fw_cfg() - Load an image file into an fw_cfg entry 
> identified
> - *  by key.
> - * @fw_cfg: The firmware config instance to store the data in.
> - * @size_key:   The firmware config key to store the size of the loaded
> - *  data under, with fw_cfg_add_i32().
> - * @data_key:   The firmware config key to store the loaded data under,
> - *  with fw_cfg_add_bytes().
> - * @image_name: The name of the image file to load. If it is NULL, the
> - *  function returns without doing anything.
> - * @try_decompress: Whether the image should be decompressed (gunzipped) 
> before
> - *  adding it to fw_cfg. If decompression fails, the image is
> - *  loaded as-is.
> - *
> - * In case of failure, the function prints an error message to stderr and the
> - * process exits with status 1.
> - */
> -static void load_image_to_fw_cfg(FWCfgState *fw_cfg, uint16_t size_key,
> - uint16_t data_key, const char *image_name,
> - bool try_decompress)
> -{
> -size_t size = -1;
> -uint8_t *data;
> -
> -if (image_name == NULL) {
> -return;
> -}
> -
> -if (try_decompress) {
> -size = load_image_gzipped_buffer(image_name,
> - LOAD_IMAGE_MAX_GUNZIP_BYTES, &data);
> -}
> -
> -if (size == (size_t)-1) {
> -gchar *contents;
> -gsize length;
> -
> -if (!g_file_get_contents(image_name, &contents, &length, NULL)) {
> -error_report("failed to load \"%s\"", image_name);
> -exit(1);
> -}
> -size = length;
> -data = (uint8_t *)contents;
> -}
> -
> -fw_cfg_add_i32(fw_cfg, size_key, size);
> -fw_cfg_add_bytes(fw_cfg, data_key, data, size);
> -}
> -
>  static int do_arm_linux_init(Object *obj, void *opaque)
>  {
>  if (object_dynamic_cast(obj, TYPE_ARM_LINUX_BOOT_IF)) {
> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
> index 5cc0b05538..eee2c193c0 100644
> --- a/hw/loongarch/virt.c
> +++ b/hw/loongarch/virt.c
> @@ -542,39 +542,6 @@ static void reset_load_elf(void *opaque)
>  }
>  }
>
> -/* Load an image file into an fw_cfg entry identified by key. */
> -static void load_image_to_fw_cfg(FWCfgState *fw_cfg, uint16_t size_key,
> - uint16_t data_key, const char *image_name,
> - bool try_decompress)
> -{
> -size_t size = -1;
> -uint8_t *data;
> -
> -if (image_name == NULL) {
> -return;
> -}
> -
> -if (try_decompress) {
> -size = load_image_gzipped_buffer(image_name,
> - LOAD_IMAGE_MAX_GUNZIP_BYTES, &data);
> -}
> -
> -if (size == (size_t)-1) {
> -gchar *contents;
> -gsize length;
> -
> -if (!g_file_get_contents(image_name, &contents, &length, NULL)) {
> -error_report("failed to load \"%s\"", image_name);
> -exit(1);
> -}
> -size = length;
> -data = (uint8_t *)contents;
> -}
> -
> -fw_cfg_add_i32(fw_cfg, size_key, size);
> -fw_cfg_add_bytes(fw_cfg, data_key, data, size);
> -}
> -
>  static void fw_cfg_add_kernel_info(FWCfgState *fw_cfg)
>  {
>  /*
> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> index d605f3f45a..371a45dfe2 100644
> --- a/hw/nvram/fw_cfg.c
> +++ b/hw/nvram/fw_cfg.c
> @@ -41,6 +41,7 @@
>  #include "qapi/error.h"
>  #include "hw/acpi/aml-build.h"
>  #include "hw/pci/pci_bus.h"
> +#include "hw/loader.h"
>
>  #define FW_CFG_FILE_SLOTS_DFLT 0x20
>
> @@ -1221,6 +1222,37 @@ FWCfgState *fw_cfg_find(void)
>  return FW_CFG(object_resolve_path_type("", TYPE_FW_CFG, NULL));
>  }
>
> +void load_image_to_fw_cfg(FWCfgState *fw_cfg, uint16_t size_key,
> +  uint16_t data_key, const char *image_name,
> +  bool try_decompress)
> +{
> +size_t size = -1;
> +uint8_t *data;
> +
> +if (image_name == NULL) {
> +return;
> +

Re: [RFC PATCH] docs/system: clean up code escape for riscv virt platform

[Alistair Francis]

On Tue, Sep 6, 2022 at 2:26 PM Alex Bennée  wrote:
>
>
> Alistair Francis  writes:
>
> > On Mon, Sep 5, 2022 at 6:39 PM Alex Bennée  wrote:
> >>
> >> The example code is rendered slightly mangled due to missing code
> >> block. Properly escape the code block and add shell prompt and qemu to
> >> fit in with the other examples on the page.
> >>
> >> Signed-off-by: Alex Bennée 
> >
> > Reviewed-by: Alistair Francis 
>
> Are you going to queue via your tree?

Yep! It's applied now, was just catching up on the first 7.2 PR

Alistair

>
> --
> Alex Bennée

Re: [PATCH V4 2/3] hw/riscv: virt: Move create_fw_cfg() prior to loading kernel

[Alistair Francis]

On Tue, Sep 6, 2022 at 11:44 AM Sunil V L  wrote:
>
> To enable both -kernel and -pflash options, the fw_cfg needs to be
> created prior to loading the kernel.
>
> Signed-off-by: Sunil V L 
> Reviewed-by: Andrew Jones 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  hw/riscv/virt.c | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index ff8c0df5cd..b6bbf03f61 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -1251,6 +1251,13 @@ static void virt_machine_done(Notifier *notifier, void 
> *data)
>  RISCV64_BIOS_BIN, start_addr, NULL);
>  }
>
> +/*
> + * Init fw_cfg.  Must be done before riscv_load_fdt, otherwise the device
> + * tree cannot be altered and we get FDT_ERR_NOSPACE.
> + */
> +s->fw_cfg = create_fw_cfg(machine);
> +rom_set_fw(s->fw_cfg);
> +
>  if (machine->kernel_filename) {
>  kernel_start_addr = riscv_calc_kernel_start_addr(&s->soc[0],
>   firmware_end_addr);
> @@ -1284,13 +1291,6 @@ static void virt_machine_done(Notifier *notifier, void 
> *data)
>  start_addr = virt_memmap[VIRT_FLASH].base;
>  }
>
> -/*
> - * Init fw_cfg.  Must be done before riscv_load_fdt, otherwise the device
> - * tree cannot be altered and we get FDT_ERR_NOSPACE.
> - */
> -s->fw_cfg = create_fw_cfg(machine);
> -rom_set_fw(s->fw_cfg);
> -
>  /* Compute the fdt load address in dram */
>  fdt_load_addr = riscv_load_fdt(memmap[VIRT_DRAM].base,
> machine->ram_size, machine->fdt);
> --
> 2.25.1
>
>

Re: [PATCH] target/riscv: Remove sideleg and sedeleg

[Alistair Francis]

On Wed, Aug 24, 2022 at 4:54 PM Rahul Pathak  wrote:
>
> sideleg and sedeleg csrs are not part of riscv isa spec
> anymore, these csrs were part of N extension which
> is removed from the riscv isa specification.
>
> These commits removed all traces of these csrs from
> riscv spec (https://github.com/riscv/riscv-isa-manual) -
>
> commit f8d27f805b65 ("Remove or downgrade more references to N extension 
> (#674)")
> commit b6cade07034d ("Remove N extension chapter for now")
>
> Signed-off-by: Rahul Pathak 
> Reviewed-by: Andrew Jones 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  disas/riscv.c   | 2 --
>  target/riscv/cpu_bits.h | 2 --
>  2 files changed, 4 deletions(-)
>
> diff --git a/disas/riscv.c b/disas/riscv.c
> index 7af6afc8fa..a709d66167 100644
> --- a/disas/riscv.c
> +++ b/disas/riscv.c
> @@ -1304,8 +1304,6 @@ static const char *csr_name(int csrno)
>  case 0x0043: return "utval";
>  case 0x0044: return "uip";
>  case 0x0100: return "sstatus";
> -case 0x0102: return "sedeleg";
> -case 0x0103: return "sideleg";
>  case 0x0104: return "sie";
>  case 0x0105: return "stvec";
>  case 0x0106: return "scounteren";
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index 6be5a9e9f0..7251121218 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -190,8 +190,6 @@
>
>  /* Supervisor Trap Setup */
>  #define CSR_SSTATUS 0x100
> -#define CSR_SEDELEG 0x102
> -#define CSR_SIDELEG 0x103
>  #define CSR_SIE 0x104
>  #define CSR_STVEC   0x105
>  #define CSR_SCOUNTEREN  0x106
> --
> 2.34.1
>
>

Re: [PATCH 2/2] target/riscv: fence: reconcile with specification

[Alistair Francis]

On Fri, Aug 12, 2022 at 4:19 PM Philipp Tomsich
 wrote:
>
> Happy to lower it back into the decode file.
> However, I initially pulled it up into the trans-function to more
> closely match the ISA specification: there is only one FENCE
> instruction with 3 arguments (FM, PRED, and SUCC).
> One might argue that the decode table for "RV32I Base Instruction Set"
> in the specification lists FENCE.TSO as a separate instruction, but
> the normative text doesn't (and FENCE overlaps FENCE.TSO in the
> tabular representation) — so I would consider the table as
> informative.
>
> I'll wait until we see what consensus emerges from the discussion.

>From the discussion on patch 1 it seems that QEMU ignoring these
fields (current behaviour) is correct

Alistair

>
> Philipp.
>
> On Fri, 12 Aug 2022 at 15:21, Peter Maydell  wrote:
> >
> > On Fri, 12 Aug 2022 at 14:17, Philipp Tomsich  
> > wrote:
> > >
> > > Our decoding of fence-instructions is problematic in respect to the
> > > RISC-V ISA specification:
> > > - rs and rd are ignored, but need to be 0
> > > - fm is ignored
> > >
> > > This change adjusts the decode pattern to enfore rs and rd being 0,
> > > and validates the fm-field (together with pred/succ for FENCE.TSO) to
> > > determine whether a reserved instruction is specified.
> > >
> > > While the specification allows UNSPECIFIED behaviour for reserved
> > > instructions, we now always raise an illegal instruction exception.
> > >
> > > Signed-off-by: Philipp Tomsich 
> > >
> > > ---
> > >
> > >  target/riscv/insn32.decode  |  2 +-
> > >  target/riscv/insn_trans/trans_rvi.c.inc | 19 ++-
> > >  2 files changed, 19 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
> > > index 089128c3dc..4e53df1b62 100644
> > > --- a/target/riscv/insn32.decode
> > > +++ b/target/riscv/insn32.decode
> > > @@ -150,7 +150,7 @@ srl  000 .. 101 . 0110011 @r
> > >  sra  010 .. 101 . 0110011 @r
> > >  or   000 .. 110 . 0110011 @r
> > >  and  000 .. 111 . 0110011 @r
> > > -fence pred:4 succ:4 - 000 - 000
> > > +fencefm:4 pred:4 succ:4 0 000 0 000
> > >  fence_i   0 001 0 000
> > >  csrrw . 001 . 1110011 @csr
> > >  csrrs . 010 . 1110011 @csr
> > > diff --git a/target/riscv/insn_trans/trans_rvi.c.inc 
> > > b/target/riscv/insn_trans/trans_rvi.c.inc
> > > index ca8e3d1ea1..515bb3b22a 100644
> > > --- a/target/riscv/insn_trans/trans_rvi.c.inc
> > > +++ b/target/riscv/insn_trans/trans_rvi.c.inc
> > > @@ -795,7 +795,24 @@ static bool trans_srad(DisasContext *ctx, arg_srad 
> > > *a)
> > >
> > >  static bool trans_fence(DisasContext *ctx, arg_fence *a)
> > >  {
> > > -/* FENCE is a full memory barrier. */
> > > +switch (a->fm) {
> > > +case 0b:
> > > + /* normal fence */
> > > + break;
> > > +
> > > +case 0b0001:
> > > + /* FENCE.TSO requires PRED and SUCC to be RW */
> > > + if (a->pred != 0xb0011 || a->succ != 0b0011) {
> > > +return false;
> > > + }
> > > + break;
> > > +
> > > +default:
> > > +/* reserved for future use */
> > > +return false;
> > > +}
> >
> > I think it would be neater to do this decode in the
> > .decode file, rather than by hand in the trans function.
> >
> > thanks
> > -- PMM
>

Re: [PATCH v3] target/riscv: fix csr check for cycle{h}, instret{h}, time{h}, hpmcounter3-31{h}

[Alistair Francis]

On Wed, Aug 17, 2022 at 10:39 AM Weiwei Li  wrote:
>
> - modify check for mcounteren to work in all less-privilege mode
> - modify check for scounteren to work only when S mode is enabled
> - distinguish the exception type raised by check for scounteren between U
> and VU mode
>
> Signed-off-by: Weiwei Li 
> Signed-off-by: Junqiang Wang 

Reviewed-by: Alistair Francis 

Alistair

> ---
> v3:
>  - remove unnecessary ()'s
>
> v2:
>  - Rebase on patches v13 for "Improve PMU support"
>
>  target/riscv/csr.c | 13 +
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 2dcd4e5b2d..ca72b5df98 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -98,17 +98,22 @@ static RISCVException ctr(CPURISCVState *env, int csrno)
>
>  skip_ext_pmu_check:
>
> -if (((env->priv == PRV_S) && (!get_field(env->mcounteren, ctr_mask))) ||
> -((env->priv == PRV_U) && (!get_field(env->scounteren, ctr_mask {
> +if (env->priv < PRV_M && !get_field(env->mcounteren, ctr_mask)) {
>  return RISCV_EXCP_ILLEGAL_INST;
>  }
>
>  if (riscv_cpu_virt_enabled(env)) {
> -if (!get_field(env->hcounteren, ctr_mask) &&
> -get_field(env->mcounteren, ctr_mask)) {
> +if (!get_field(env->hcounteren, ctr_mask) ||
> +(env->priv == PRV_U && !get_field(env->scounteren, ctr_mask))) {
>  return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
>  }
>  }
> +
> +if (riscv_has_ext(env, RVS) && env->priv == PRV_U &&
> +!get_field(env->scounteren, ctr_mask)) {
> +return RISCV_EXCP_ILLEGAL_INST;
> +}
> +
>  #endif
>  return RISCV_EXCP_NONE;
>  }
> --
> 2.17.1
>
>

Re: [PATCH 2/2] target/riscv: fence: reconcile with specification

[Philipp Tomsich]

On Thu, 8 Sept 2022 at 11:25, Alistair Francis  wrote:
>
> On Fri, Aug 12, 2022 at 4:19 PM Philipp Tomsich
>  wrote:
> >
> > Happy to lower it back into the decode file.
> > However, I initially pulled it up into the trans-function to more
> > closely match the ISA specification: there is only one FENCE
> > instruction with 3 arguments (FM, PRED, and SUCC).
> > One might argue that the decode table for "RV32I Base Instruction Set"
> > in the specification lists FENCE.TSO as a separate instruction, but
> > the normative text doesn't (and FENCE overlaps FENCE.TSO in the
> > tabular representation) — so I would consider the table as
> > informative.
> >
> > I'll wait until we see what consensus emerges from the discussion.
>
> From the discussion on patch 1 it seems that QEMU ignoring these
> fields (current behaviour) is correct


Yes, this is an accurate reading of the situation.

Philipp.

Re: [PATCH v3] audio: add help option for -audio and -audiodev

[Paolo Bonzini]

Queued, thanks.

Paolo

[PATCH RESEND] hw/microblaze: pass random seed to fdt

[Jason A. Donenfeld]

If the FDT contains /chosen/rng-seed, then the Linux RNG will use it to
initialize early. Set this using the usual guest random number
generation function. This FDT node is part of the DT specification.

Reviewed-by: Edgar E. Iglesias 
Signed-off-by: Jason A. Donenfeld 
---
 hw/microblaze/boot.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c
index 8b92a9801a..25ad54754e 100644
--- a/hw/microblaze/boot.c
+++ b/hw/microblaze/boot.c
@@ -30,6 +30,7 @@
 #include "qemu/option.h"
 #include "qemu/config-file.h"
 #include "qemu/error-report.h"
+#include "qemu/guest-random.h"
 #include "sysemu/device_tree.h"
 #include "sysemu/reset.h"
 #include "hw/boards.h"
@@ -75,6 +76,7 @@ static int microblaze_load_dtb(hwaddr addr,
 int fdt_size;
 void *fdt = NULL;
 int r;
+uint8_t rng_seed[32];
 
 if (dtb_filename) {
 fdt = load_device_tree(dtb_filename, &fdt_size);
@@ -83,6 +85,9 @@ static int microblaze_load_dtb(hwaddr addr,
 return 0;
 }
 
+qemu_guest_getrandom_nofail(rng_seed, sizeof(rng_seed));
+qemu_fdt_setprop(fdt, "/chosen", "rng-seed", rng_seed, sizeof(rng_seed));
+
 if (kernel_cmdline) {
 r = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
 kernel_cmdline);
-- 
2.37.3

[PATCH v1 6/9] hw/loongarch: Add RAMFB to dynamic_sysbus_devices list

[Xiaojuan Yang]

Add RAMFB device to dynamic_sysbus_devices list so that it can be
hotpluged to the machine.

Signed-off-by: Xiaojuan Yang 
---
 hw/loongarch/virt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index a3dd35d579..1e1dc699ef 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -39,6 +39,7 @@
 #include 
 #include "hw/core/sysbus-fdt.h"
 #include "hw/platform-bus.h"
+#include "hw/display/ramfb.h"
 
 static void create_fdt(LoongArchMachineState *lams)
 {
@@ -853,6 +854,7 @@ static void loongarch_class_init(ObjectClass *oc, void 
*data)
 NULL, NULL);
 object_class_property_set_description(oc, "acpi",
 "Enable ACPI");
+machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
 }
 
 static const TypeInfo loongarch_machine_types[] = {
-- 
2.31.1

[PATCH v1 0/9] Fix bugs and improve functions for LoongArch

[Xiaojuan Yang]

These patches integrate all previous patches, including
'[PATCH v1 0/2] Add mem hotplug and improve acpi dsdt (26 Aug )', 
'[PATCH v1] hw/loongarch: Fix acpi ged irq number in dsdt table (19 
Aug)',
'[PATCH v1 0/6] Add funtions for LoongArch virt machine (11 Aug)'.
As none of the patches has not been reviewed, so we integrate them for 
more convenient reviewing.

Changes for v1: 
1. Remove vga device when loongarch init
2. Support fw_cfg dma function
3. Add interrupt information to FDT table
4. Add platform bus support
5. Add hotplug handler for machine
6. Add RAMFB to dynamic_sysbus_devices list
7. Fix acpi ged irq number in dsdt table
8. Support memory hotplug
9. Improve acpi dsdt table

Xiaojuan Yang (9):
  hw/loongarch: Remove vga device when loongarch init
  hw/loongarch: Support fw_cfg dma function
  hw/loongarch: Add interrupt information to FDT table
  hw/loongarch: Add platform bus support
  hw/loongarch: Add hotplug handler for machine
  hw/loongarch: Add RAMFB to dynamic_sysbus_devices list
  hw/loongarch: Fix acpi ged irq number in dsdt table
  hw/loongarch: Support memory hotplug
  hw/loongarch: Improve acpi dsdt table

 hw/loongarch/Kconfig|   5 +-
 hw/loongarch/acpi-build.c   | 191 -
 hw/loongarch/fw_cfg.c   |   3 +-
 hw/loongarch/virt.c | 205 +++-
 include/hw/loongarch/virt.h |   2 +
 include/hw/pci-host/ls7a.h  |   5 +
 6 files changed, 259 insertions(+), 152 deletions(-)

-- 
2.31.1

Re: [PATCH 3/4] msmouse: Add pnp data

[Marc-André Lureau]

Hi

On Wed, Sep 7, 2022 at 2:05 AM Arwed Meyer  wrote:

> Make msmouse send serial pnp data.
> Enables you to see nice qemu device name in Win9x.
>
> Signed-off-by: Arwed Meyer 
> ---
>  chardev/msmouse.c | 101 +++---
>  1 file changed, 68 insertions(+), 33 deletions(-)
>
> diff --git a/chardev/msmouse.c b/chardev/msmouse.c
> index 0ecf26a436..b4ddaee778 100644
> --- a/chardev/msmouse.c
> +++ b/chardev/msmouse.c
> @@ -24,6 +24,7 @@
>
>  #include "qemu/osdep.h"
>  #include "qemu/module.h"
> +#include "qemu/fifo8.h"
>  #include "chardev/char.h"
>  #include "chardev/char-serial.h"
>  #include "ui/console.h"
> @@ -34,6 +35,25 @@
>  #define MSMOUSE_HI2(n)  (((n) & 0xc0) >> 6)
>  #define MSMOUSE_PWR(cm) (cm & (CHR_TIOCM_RTS | CHR_TIOCM_DTR))
>
> +/* Serial PnP for 6 bit devices/mice sends all ASCII chars - 0x20 */
> +#define M(c) (c - 0x20)
> +/* Serial fifo size. */
> +#define MSMOUSE_BUF_SZ 64
> +
> +/* Mouse ID: Send "M3" cause we behave like a 3 button logitech mouse. */
> +const uint8_t mouse_id[] = {'M', '3'};
> +/*
> + * PnP start "(", PnP version (1.0), vendor ID, product ID, '\\',
> + * serial ID (omitted), '\\', MS class name, '\\', driver ID (omitted),
> '\\',
> + * product description, checksum, ")"
> + * Missing parts are inserted later.
> + */
> +const uint8_t pnp_data[] = {M('('), 1, '$', M('Q'), M('M'), M('U'),
> + M('0'), M('0'), M('0'), M('1'),
> + M('\\'), M('\\'),
> + M('M'), M('O'), M('U'), M('S'), M('E'),
> + M('\\'), M('\\')};
> +
>  struct MouseChardev {
>  Chardev parent;
>
> @@ -42,8 +62,7 @@ struct MouseChardev {
>  int axis[INPUT_AXIS__MAX];
>  bool btns[INPUT_BUTTON__MAX];
>  bool btnc[INPUT_BUTTON__MAX];
> -uint8_t outbuf[32];
> -int outlen;
> +Fifo8 outbuf;
>

Could you make this outbuf replacement a different patch?


>  };
>  typedef struct MouseChardev MouseChardev;
>
> @@ -54,21 +73,15 @@ DECLARE_INSTANCE_CHECKER(MouseChardev, MOUSE_CHARDEV,
>  static void msmouse_chr_accept_input(Chardev *chr)
>  {
>  MouseChardev *mouse = MOUSE_CHARDEV(chr);
> -int len;
> +uint32_t len_out, len;
>
> -len = qemu_chr_be_can_write(chr);
> -if (len > mouse->outlen) {
> -len = mouse->outlen;
> -}
> -if (!len) {
> +len_out = qemu_chr_be_can_write(chr);
> +if (!len_out || fifo8_is_empty(&mouse->outbuf)) {
>  return;
>  }
> -
> -qemu_chr_be_write(chr, mouse->outbuf, len);
> -mouse->outlen -= len;
> -if (mouse->outlen) {
> -memmove(mouse->outbuf, mouse->outbuf + len, mouse->outlen);
> -}
> +len = MIN(fifo8_num_used(&mouse->outbuf), len_out);
> +qemu_chr_be_write(chr, fifo8_pop_buf(&mouse->outbuf, len, &len_out),
> +len_out);
>  }
>
>  static void msmouse_queue_event(MouseChardev *mouse)
> @@ -94,12 +107,11 @@ static void msmouse_queue_event(MouseChardev *mouse)
>  mouse->btnc[INPUT_BUTTON_MIDDLE]) {
>  bytes[3] |= (mouse->btns[INPUT_BUTTON_MIDDLE] ? 0x20 : 0x00);
>  mouse->btnc[INPUT_BUTTON_MIDDLE] = false;
> -count = 4;
> +count++;
>  }
>
> -if (mouse->outlen <= sizeof(mouse->outbuf) - count) {
> -memcpy(mouse->outbuf + mouse->outlen, bytes, count);
> -mouse->outlen += count;
> +if (fifo8_num_free(&mouse->outbuf) >= count) {
> +fifo8_push_all(&mouse->outbuf, bytes, count);
>  } else {
>  /* queue full -> drop event */
>  }
> @@ -155,11 +167,22 @@ static int msmouse_chr_write(struct Chardev *s,
> const uint8_t *buf, int len)
>  return len;
>  }
>
> +static QemuInputHandler msmouse_handler = {
> +.name  = "QEMU Microsoft Mouse",
> +.mask  = INPUT_EVENT_MASK_BTN | INPUT_EVENT_MASK_REL,
> +.event = msmouse_input_event,
> +.sync  = msmouse_input_sync,
> +};
> +
>  static int msmouse_ioctl(Chardev *chr, int cmd, void *arg)
>  {
>  MouseChardev *mouse = MOUSE_CHARDEV(chr);
> -int c;
> +int c, i, j;
> +uint8_t bytes[MSMOUSE_BUF_SZ / 2];
>  int *targ = (int *)arg;
> +const uint8_t hexchr[16] = {M('0'), M('1'), M('2'), M('3'), M('4'),
> M('5'),
> + M('6'), M('7'), M('8'), M('9'), M('A'),
> M('B'),
> + M('C'), M('D'), M('E'), M('F')};
>
>  switch (cmd) {
>  case CHR_IOCTL_SERIAL_SET_TIOCM:
> @@ -168,13 +191,30 @@ static int msmouse_ioctl(Chardev *chr, int cmd, void
> *arg)
>  if (MSMOUSE_PWR(mouse->tiocm)) {
>  if (!MSMOUSE_PWR(c)) {
>  /*
> - * Power on after reset: send "M3"
> - * cause we behave like a 3 button logitech
> - * mouse.
> + * Power on after reset: Send ID and PnP data
> + * No need to check fifo space as it is empty at this
> point.
> + */
> +fifo8_push_all(&mouse->outbuf, mouse_id,
> sizeof(mou

Re: [PATCH 2/4] chardev: src buffer const for write functions

[Marc-André Lureau]

Hi


On Wed, Sep 7, 2022 at 2:05 AM Arwed Meyer  wrote:

> Make source buffers const for char be write functions.
> This allows using buffers returned by fifo as buf parameter and source
> buffer
> should not be changed by write functions anyway.
>
> Signed-off-by: Arwed Meyer 
>

Reviewed-by: Marc-André Lureau 

---
>  chardev/char.c  | 4 ++--
>  include/chardev/char.h  | 4 ++--
>  include/sysemu/replay.h | 2 +-
>  replay/replay-char.c| 2 +-
>  stubs/replay-tools.c| 2 +-
>  5 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/chardev/char.c b/chardev/char.c
> index 0169d8dde4..b005df3ccf 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -193,7 +193,7 @@ int qemu_chr_be_can_write(Chardev *s)
>  return be->chr_can_read(be->opaque);
>  }
>
> -void qemu_chr_be_write_impl(Chardev *s, uint8_t *buf, int len)
> +void qemu_chr_be_write_impl(Chardev *s, const uint8_t *buf, int len)
>  {
>  CharBackend *be = s->be;
>
> @@ -202,7 +202,7 @@ void qemu_chr_be_write_impl(Chardev *s, uint8_t *buf,
> int len)
>  }
>  }
>
> -void qemu_chr_be_write(Chardev *s, uint8_t *buf, int len)
> +void qemu_chr_be_write(Chardev *s, const uint8_t *buf, int len)
>  {
>  if (qemu_chr_replay(s)) {
>  if (replay_mode == REPLAY_MODE_PLAY) {
> diff --git a/include/chardev/char.h b/include/chardev/char.h
> index a319b5fdff..44cd82e405 100644
> --- a/include/chardev/char.h
> +++ b/include/chardev/char.h
> @@ -186,7 +186,7 @@ int qemu_chr_be_can_write(Chardev *s);
>   * the caller should call @qemu_chr_be_can_write to determine how much
> data
>   * the front end can currently accept.
>   */
> -void qemu_chr_be_write(Chardev *s, uint8_t *buf, int len);
> +void qemu_chr_be_write(Chardev *s, const uint8_t *buf, int len);
>
>  /**
>   * qemu_chr_be_write_impl:
> @@ -195,7 +195,7 @@ void qemu_chr_be_write(Chardev *s, uint8_t *buf, int
> len);
>   *
>   * Implementation of back end writing. Used by replay module.
>   */
> -void qemu_chr_be_write_impl(Chardev *s, uint8_t *buf, int len);
> +void qemu_chr_be_write_impl(Chardev *s, const uint8_t *buf, int len);
>
>  /**
>   * qemu_chr_be_update_read_handlers:
> diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
> index 73dee9ccdf..7ec0882b50 100644
> --- a/include/sysemu/replay.h
> +++ b/include/sysemu/replay.h
> @@ -198,7 +198,7 @@ uint64_t blkreplay_next_id(void);
>  /*! Registers char driver to save it's events */
>  void replay_register_char_driver(struct Chardev *chr);
>  /*! Saves write to char device event to the log */
> -void replay_chr_be_write(struct Chardev *s, uint8_t *buf, int len);
> +void replay_chr_be_write(struct Chardev *s, const uint8_t *buf, int len);
>  /*! Writes char write return value to the replay log. */
>  void replay_char_write_event_save(int res, int offset);
>  /*! Reads char write return value from the replay log. */
> diff --git a/replay/replay-char.c b/replay/replay-char.c
> index d2025948cf..a31aded032 100644
> --- a/replay/replay-char.c
> +++ b/replay/replay-char.c
> @@ -48,7 +48,7 @@ void replay_register_char_driver(Chardev *chr)
>  char_drivers[drivers_count++] = chr;
>  }
>
> -void replay_chr_be_write(Chardev *s, uint8_t *buf, int len)
> +void replay_chr_be_write(Chardev *s, const uint8_t *buf, int len)
>  {
>  CharEvent *event = g_new0(CharEvent, 1);
>
> diff --git a/stubs/replay-tools.c b/stubs/replay-tools.c
> index f2e72bb225..3e8ca3212d 100644
> --- a/stubs/replay-tools.c
> +++ b/stubs/replay-tools.c
> @@ -53,7 +53,7 @@ void replay_register_char_driver(struct Chardev *chr)
>  {
>  }
>
> -void replay_chr_be_write(struct Chardev *s, uint8_t *buf, int len)
> +void replay_chr_be_write(struct Chardev *s, const uint8_t *buf, int len)
>  {
>  abort();
>  }
> --
> 2.34.1
>
>
>

-- 
Marc-André Lureau

[PATCH v1 8/9] hw/loongarch: Support memory hotplug

[Xiaojuan Yang]

Add hotplug/unplug interface for memory device.

Signed-off-by: Xiaojuan Yang 
---
 hw/loongarch/Kconfig  |   2 +
 hw/loongarch/acpi-build.c |  32 +---
 hw/loongarch/virt.c   | 105 +-
 3 files changed, 132 insertions(+), 7 deletions(-)

diff --git a/hw/loongarch/Kconfig b/hw/loongarch/Kconfig
index fef55c5638..17d15b6c90 100644
--- a/hw/loongarch/Kconfig
+++ b/hw/loongarch/Kconfig
@@ -4,6 +4,7 @@ config LOONGARCH_VIRT
 select PCI_EXPRESS_GENERIC_BRIDGE
 imply VIRTIO_VGA
 imply PCI_DEVICES
+imply NVDIMM
 select ISA_BUS
 select SERIAL
 select SERIAL_ISA
@@ -18,3 +19,4 @@ config LOONGARCH_VIRT
 select ACPI_PCI
 select ACPI_HW_REDUCED
 select FW_CFG_DMA
+select DIMM
diff --git a/hw/loongarch/acpi-build.c b/hw/loongarch/acpi-build.c
index 95e30975a8..92ee62c11a 100644
--- a/hw/loongarch/acpi-build.c
+++ b/hw/loongarch/acpi-build.c
@@ -186,6 +186,12 @@ build_srat(GArray *table_data, BIOSLinker *linker, 
MachineState *machine)
 build_srat_memory(table_data, VIRT_HIGHMEM_BASE, machine->ram_size - 
VIRT_LOWMEM_SIZE,
   0, MEM_AFFINITY_ENABLED);
 
+if (ms->device_memory) {
+build_srat_memory(table_data, ms->device_memory->base,
+  memory_region_size(&ms->device_memory->mr),
+  0, MEM_AFFINITY_HOTPLUGGABLE | MEM_AFFINITY_ENABLED);
+}
+
 acpi_table_end(linker, &table);
 }
 
@@ -335,6 +341,25 @@ static void build_uart_device_aml(Aml *table)
 aml_append(table, scope);
 }
 
+static void
+build_la_ged_aml(Aml *dsdt, MachineState *machine)
+{
+uint32_t event;
+LoongArchMachineState *lams = LOONGARCH_MACHINE(machine);
+
+build_ged_aml(dsdt, "\\_SB."GED_DEVICE,
+  HOTPLUG_HANDLER(lams->acpi_ged),
+  VIRT_SCI_IRQ, AML_SYSTEM_MEMORY,
+  VIRT_GED_EVT_ADDR);
+event = object_property_get_uint(OBJECT(lams->acpi_ged),
+ "ged-event", &error_abort);
+if (event & ACPI_GED_MEM_HOTPLUG_EVT) {
+build_memory_hotplug_aml(dsdt, machine->ram_slots, "\\_SB", NULL,
+ AML_SYSTEM_MEMORY,
+ VIRT_GED_MEM_ADDR);
+}
+}
+
 /* build DSDT */
 static void
 build_dsdt(GArray *table_data, BIOSLinker *linker, MachineState *machine)
@@ -364,12 +389,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, 
MachineState *machine)
 
 build_gpex_pci0_int(dsdt);
 build_uart_device_aml(dsdt);
-if (lams->acpi_ged) {
-build_ged_aml(dsdt, "\\_SB."GED_DEVICE,
-  HOTPLUG_HANDLER(lams->acpi_ged),
-  VIRT_SCI_IRQ, AML_SYSTEM_MEMORY,
-  VIRT_GED_EVT_ADDR);
-}
+build_la_ged_aml(dsdt, machine);
 
 scope = aml_scope("\\_SB.PCI0");
 /* Build PCI0._CRS */
diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index 1e1dc699ef..a81db29384 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -40,6 +40,7 @@
 #include "hw/core/sysbus-fdt.h"
 #include "hw/platform-bus.h"
 #include "hw/display/ramfb.h"
+#include "hw/mem/pc-dimm.h"
 
 static void create_fdt(LoongArchMachineState *lams)
 {
@@ -719,6 +720,35 @@ static void loongarch_init(MachineState *machine)
  machine->ram, offset, highram_size);
 memory_region_add_subregion(address_space_mem, 0x9000, &lams->highmem);
 memmap_add_entry(0x9000, highram_size, 1);
+
+/* initialize device memory address space */
+if (machine->ram_size < machine->maxram_size) {
+machine->device_memory = g_malloc0(sizeof(*machine->device_memory));
+ram_addr_t device_mem_size = machine->maxram_size - machine->ram_size;
+
+if (machine->ram_slots > ACPI_MAX_RAM_SLOTS) {
+error_report("unsupported amount of memory slots: %"PRIu64,
+ machine->ram_slots);
+exit(EXIT_FAILURE);
+}
+
+if (QEMU_ALIGN_UP(machine->maxram_size,
+  TARGET_PAGE_SIZE) != machine->maxram_size) {
+error_report("maximum memory size must by aligned to multiple of "
+ "%d bytes", TARGET_PAGE_SIZE);
+exit(EXIT_FAILURE);
+}
+/* device memory base is the top of high memory address. */
+machine->device_memory->base = 0x9000 + highram_size;
+machine->device_memory->base =
+ROUND_UP(machine->device_memory->base, 1 * GiB);
+
+memory_region_init(&machine->device_memory->mr, OBJECT(lams),
+   "device-memory", device_mem_size);
+memory_region_add_subregion(address_space_mem, 
machine->device_memory->base,
+&machine->device_memory->mr);
+}
+
 /* Add isa io region */
 memory_region_init_alias(&lams->isa_io, NULL, "isa-io",
  get_system_io(), 0, VIRT_ISA_IO_SIZE);
@@ -8

Re: [PATCH 1/4] msmouse: Handle mouse reset

[Marc-André Lureau]

Hi

On Wed, Sep 7, 2022 at 2:03 AM Arwed Meyer  wrote:

> Detect mouse reset via RTS or DTR line:
> Don't send or process anything while in reset.
> When coming out of reset, send ID sequence first thing.
> This allows msmouse to be detected by common mouse drivers.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/77
> Signed-off-by: Arwed Meyer 
> ---
>  chardev/msmouse.c | 65 +--
>  1 file changed, 63 insertions(+), 2 deletions(-)
>
> diff --git a/chardev/msmouse.c b/chardev/msmouse.c
> index eb9231dcdb..0ecf26a436 100644
> --- a/chardev/msmouse.c
> +++ b/chardev/msmouse.c
> @@ -25,17 +25,20 @@
>  #include "qemu/osdep.h"
>  #include "qemu/module.h"
>  #include "chardev/char.h"
> +#include "chardev/char-serial.h"
>  #include "ui/console.h"
>  #include "ui/input.h"
>  #include "qom/object.h"
>
> -#define MSMOUSE_LO6(n) ((n) & 0x3f)
> -#define MSMOUSE_HI2(n) (((n) & 0xc0) >> 6)
> +#define MSMOUSE_LO6(n)  ((n) & 0x3f)
> +#define MSMOUSE_HI2(n)  (((n) & 0xc0) >> 6)
> +#define MSMOUSE_PWR(cm) (cm & (CHR_TIOCM_RTS | CHR_TIOCM_DTR))
>
>  struct MouseChardev {
>  Chardev parent;
>
>  QemuInputHandlerState *hs;
> +int tiocm;
>  int axis[INPUT_AXIS__MAX];
>  bool btns[INPUT_BUTTON__MAX];
>  bool btnc[INPUT_BUTTON__MAX];
> @@ -109,6 +112,11 @@ static void msmouse_input_event(DeviceState *dev,
> QemuConsole *src,
>  InputMoveEvent *move;
>  InputBtnEvent *btn;
>
> +/* Ignore events if serial mouse powered down. */
> +if (!MSMOUSE_PWR(mouse->tiocm)) {
> +return;
> +}
> +
>  switch (evt->type) {
>  case INPUT_EVENT_KIND_REL:
>  move = evt->u.rel.data;
> @@ -132,6 +140,11 @@ static void msmouse_input_sync(DeviceState *dev)
>  MouseChardev *mouse = MOUSE_CHARDEV(dev);
>  Chardev *chr = CHARDEV(dev);
>
> +/* Ignore events if serial mouse powered down. */
> +if (!MSMOUSE_PWR(mouse->tiocm)) {
> +return;
> +}
> +
>  msmouse_queue_event(mouse);
>  msmouse_chr_accept_input(chr);
>  }
> @@ -142,6 +155,52 @@ static int msmouse_chr_write(struct Chardev *s, const
> uint8_t *buf, int len)
>  return len;
>  }
>
> +static int msmouse_ioctl(Chardev *chr, int cmd, void *arg)
> +{
> +MouseChardev *mouse = MOUSE_CHARDEV(chr);
> +int c;
> +int *targ = (int *)arg;
> +
> +switch (cmd) {
> +case CHR_IOCTL_SERIAL_SET_TIOCM:
> +c = mouse->tiocm;
> +mouse->tiocm = *(int *)arg;
> +if (MSMOUSE_PWR(mouse->tiocm)) {
> +if (!MSMOUSE_PWR(c)) {
> +/*
> + * Power on after reset: send "M3"
> + * cause we behave like a 3 button logitech
> + * mouse.
> + */
> +mouse->outbuf[0] = 'M';
> +mouse->outbuf[1] = '3';
> +mouse->outlen = 2;
> +/* Start sending data to serial. */
> +msmouse_chr_accept_input(chr);
> +}
> +break;
> +}
> +/*
> + * Reset mouse buffers on power down.
> + * Mouse won't send anything without power.
> + */
> +mouse->outlen = 0;
> +memset(mouse->axis, 0, sizeof(mouse->axis));
> +for (c = INPUT_BUTTON__MAX - 1; c >= 0; c--) {
> +mouse->btns[c] = false;
> +mouse->btnc[c] = false;
> +}
>

Why not memset those fields as well?


> +break;
> +case CHR_IOCTL_SERIAL_GET_TIOCM:
> +/* Remember line control status. */
> +*targ = mouse->tiocm;
> +break;
> +default:
> +return -ENOTSUP;
> +}
> +return 0;
> +}
> +
>  static void char_msmouse_finalize(Object *obj)
>  {
>  MouseChardev *mouse = MOUSE_CHARDEV(obj);
> @@ -166,6 +225,7 @@ static void msmouse_chr_open(Chardev *chr,
>  *be_opened = false;
>  mouse->hs = qemu_input_handler_register((DeviceState *)mouse,
>  &msmouse_handler);
> +mouse->tiocm = 0;
>  }
>
>  static void char_msmouse_class_init(ObjectClass *oc, void *data)
> @@ -175,6 +235,7 @@ static void char_msmouse_class_init(ObjectClass *oc,
> void *data)
>  cc->open = msmouse_chr_open;
>  cc->chr_write = msmouse_chr_write;
>  cc->chr_accept_input = msmouse_chr_accept_input;
> +cc->chr_ioctl = msmouse_ioctl;
>  }
>
>  static const TypeInfo char_msmouse_type_info = {
> --
> 2.34.1
>
>
>
lgtm otherwise,
Reviewed-by: Marc-André Lureau 

-- 
Marc-André Lureau

[PATCH v1 9/9] hw/loongarch: Improve acpi dsdt table

[Xiaojuan Yang]

Cleanup the previous pci information in acpi dsdt table.
And using the common acpi_dsdt_add_gpex function to build
the gpex and pci information.

Signed-off-by: Xiaojuan Yang 
---
 hw/loongarch/acpi-build.c   | 159 +---
 hw/loongarch/virt.c |   1 +
 include/hw/loongarch/virt.h |   1 +
 3 files changed, 21 insertions(+), 140 deletions(-)

diff --git a/hw/loongarch/acpi-build.c b/hw/loongarch/acpi-build.c
index 92ee62c11a..378a6d9d38 100644
--- a/hw/loongarch/acpi-build.c
+++ b/hw/loongarch/acpi-build.c
@@ -30,6 +30,7 @@
 #include "qom/qom-qobject.h"
 
 #include "hw/acpi/generic_event_device.h"
+#include "hw/pci-host/gpex.h"
 
 #define ACPI_BUILD_ALIGN_SIZE 0x1000
 #define ACPI_BUILD_TABLE_SIZE 0x2
@@ -206,108 +207,6 @@ struct AcpiBuildState {
 MemoryRegion *linker_mr;
 } AcpiBuildState;
 
-static void build_gpex_pci0_int(Aml *table)
-{
-Aml *sb_scope = aml_scope("_SB");
-Aml *pci0_scope = aml_scope("PCI0");
-Aml *prt_pkg = aml_varpackage(128);
-int slot, pin;
-
-for (slot = 0; slot < PCI_SLOT_MAX; slot++) {
-for (pin = 0; pin < PCI_NUM_PINS; pin++) {
-Aml *pkg = aml_package(4);
-aml_append(pkg, aml_int((slot << 16) | 0x));
-aml_append(pkg, aml_int(pin));
-aml_append(pkg, aml_int(0));
-aml_append(pkg, aml_int(80 + (slot + pin) % 4));
-aml_append(prt_pkg, pkg);
-}
-}
-aml_append(pci0_scope, aml_name_decl("_PRT", prt_pkg));
-aml_append(sb_scope, pci0_scope);
-aml_append(table, sb_scope);
-}
-
-static void build_dbg_aml(Aml *table)
-{
-Aml *field;
-Aml *method;
-Aml *while_ctx;
-Aml *scope = aml_scope("\\");
-Aml *buf = aml_local(0);
-Aml *len = aml_local(1);
-Aml *idx = aml_local(2);
-
-aml_append(scope,
-   aml_operation_region("DBG", AML_SYSTEM_IO, aml_int(0x0402), 0x01));
-field = aml_field("DBG", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
-aml_append(field, aml_named_field("DBGB", 8));
-aml_append(scope, field);
-
-method = aml_method("DBUG", 1, AML_NOTSERIALIZED);
-
-aml_append(method, aml_to_hexstring(aml_arg(0), buf));
-aml_append(method, aml_to_buffer(buf, buf));
-aml_append(method, aml_subtract(aml_sizeof(buf), aml_int(1), len));
-aml_append(method, aml_store(aml_int(0), idx));
-
-while_ctx = aml_while(aml_lless(idx, len));
-aml_append(while_ctx,
-aml_store(aml_derefof(aml_index(buf, idx)), aml_name("DBGB")));
-aml_append(while_ctx, aml_increment(idx));
-aml_append(method, while_ctx);
-aml_append(method, aml_store(aml_int(0x0A), aml_name("DBGB")));
-aml_append(scope, method);
-aml_append(table, scope);
-}
-
-static Aml *build_osc_method(void)
-{
-Aml *if_ctx;
-Aml *if_ctx2;
-Aml *else_ctx;
-Aml *method;
-Aml *a_cwd1 = aml_name("CDW1");
-Aml *a_ctrl = aml_local(0);
-
-method = aml_method("_OSC", 4, AML_NOTSERIALIZED);
-aml_append(method, aml_create_dword_field(aml_arg(3), aml_int(0), "CDW1"));
-
-if_ctx = aml_if(aml_equal(
-aml_arg(0), aml_touuid("33DB4D5B-1FF7-401C-9657-7441C03DD766")));
-aml_append(if_ctx, aml_create_dword_field(aml_arg(3), aml_int(4), "CDW2"));
-aml_append(if_ctx, aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3"));
-aml_append(if_ctx, aml_store(aml_name("CDW3"), a_ctrl));
-
-/*
- * Always allow native PME, AER (no dependencies)
- * Allow SHPC (PCI bridges can have SHPC controller)
- */
-aml_append(if_ctx, aml_and(a_ctrl, aml_int(0x1F), a_ctrl));
-
-if_ctx2 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(1;
-/* Unknown revision */
-aml_append(if_ctx2, aml_or(a_cwd1, aml_int(0x08), a_cwd1));
-aml_append(if_ctx, if_ctx2);
-
-if_ctx2 = aml_if(aml_lnot(aml_equal(aml_name("CDW3"), a_ctrl)));
-/* Capabilities bits were masked */
-aml_append(if_ctx2, aml_or(a_cwd1, aml_int(0x10), a_cwd1));
-aml_append(if_ctx, if_ctx2);
-
-/* Update DWORD3 in the buffer */
-aml_append(if_ctx, aml_store(a_ctrl, aml_name("CDW3")));
-aml_append(method, if_ctx);
-
-else_ctx = aml_else();
-/* Unrecognized UUID */
-aml_append(else_ctx, aml_or(a_cwd1, aml_int(4), a_cwd1));
-aml_append(method, else_ctx);
-
-aml_append(method, aml_return(aml_arg(3)));
-return method;
-}
-
 static void build_uart_device_aml(Aml *table)
 {
 Aml *dev;
@@ -360,57 +259,37 @@ build_la_ged_aml(Aml *dsdt, MachineState *machine)
 }
 }
 
+static void build_pci_device_aml(Aml *scope, LoongArchMachineState *lams)
+{
+struct GPEXConfig cfg = {
+.mmio64.base = VIRT_PCI_MEM_BASE,
+.mmio64.size = VIRT_PCI_MEM_SIZE,
+.pio.base= VIRT_PCI_IO_BASE,
+.pio.size= VIRT_PCI_IO_SIZE,
+.ecam.base   = VIRT_PCI_CFG_BASE,
+.ecam.size   = VIRT_PCI_CFG_SIZE,
+.irq = PCH_PIC_IRQ_OFFSET + VIRT_DEVICE_IRQS,
+.bus = lams->pc

[PATCH v1 4/9] hw/loongarch: Add platform bus support

[Xiaojuan Yang]

Add platform bus support and add the bus information such as address,
size, irq number to FDT table.

Signed-off-by: Xiaojuan Yang 
---
 hw/loongarch/Kconfig|  1 +
 hw/loongarch/virt.c | 33 +
 include/hw/loongarch/virt.h |  1 +
 include/hw/pci-host/ls7a.h  |  5 +
 4 files changed, 40 insertions(+)

diff --git a/hw/loongarch/Kconfig b/hw/loongarch/Kconfig
index 1deea83626..fef55c5638 100644
--- a/hw/loongarch/Kconfig
+++ b/hw/loongarch/Kconfig
@@ -8,6 +8,7 @@ config LOONGARCH_VIRT
 select SERIAL
 select SERIAL_ISA
 select VIRTIO_PCI
+select PLATFORM_BUS
 select LOONGARCH_IPI
 select LOONGARCH_PCH_PIC
 select LOONGARCH_PCH_MSI
diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index 561b05d404..3976e8a058 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -37,6 +37,8 @@
 #include "hw/mem/nvdimm.h"
 #include "sysemu/device_tree.h"
 #include 
+#include "hw/core/sysbus-fdt.h"
+#include "hw/platform-bus.h"
 
 static void create_fdt(LoongArchMachineState *lams)
 {
@@ -346,6 +348,31 @@ static DeviceState *create_acpi_ged(DeviceState *pch_pic, 
LoongArchMachineState
 return dev;
 }
 
+static DeviceState *create_platform_bus(DeviceState *pch_pic)
+{
+DeviceState *dev;
+SysBusDevice *sysbus;
+int i, irq;
+MemoryRegion *sysmem = get_system_memory();
+
+dev = qdev_new(TYPE_PLATFORM_BUS_DEVICE);
+dev->id = g_strdup(TYPE_PLATFORM_BUS_DEVICE);
+qdev_prop_set_uint32(dev, "num_irqs", VIRT_PLATFORM_BUS_NUM_IRQS);
+qdev_prop_set_uint32(dev, "mmio_size", VIRT_PLATFORM_BUS_SIZE);
+sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
+
+sysbus = SYS_BUS_DEVICE(dev);
+for (i = 0; i < VIRT_PLATFORM_BUS_NUM_IRQS; i++) {
+irq = VIRT_PLATFORM_BUS_IRQ - PCH_PIC_IRQ_OFFSET + i;
+sysbus_connect_irq(sysbus, i, qdev_get_gpio_in(pch_pic, irq));
+}
+
+memory_region_add_subregion(sysmem,
+VIRT_PLATFORM_BUS_BASEADDRESS,
+sysbus_mmio_get_region(sysbus, 0));
+return dev;
+}
+
 static void loongarch_devices_init(DeviceState *pch_pic, LoongArchMachineState 
*lams)
 {
 DeviceState *gpex_dev;
@@ -421,6 +448,8 @@ static void loongarch_devices_init(DeviceState *pch_pic, 
LoongArchMachineState *
 memory_region_add_subregion(get_system_memory(), PM_BASE, pm_mem);
 /* acpi ged */
 lams->acpi_ged = create_acpi_ged(pch_pic, lams);
+/* platform bus */
+lams->platform_bus_dev = create_platform_bus(pch_pic);
 }
 
 static void loongarch_irq_init(LoongArchMachineState *lams)
@@ -726,6 +755,10 @@ static void loongarch_init(MachineState *machine)
 /* Initialize the IO interrupt subsystem */
 loongarch_irq_init(lams);
 fdt_add_irqchip_node(lams);
+platform_bus_add_all_fdt_nodes(machine->fdt, "/intc",
+   VIRT_PLATFORM_BUS_BASEADDRESS,
+   VIRT_PLATFORM_BUS_SIZE,
+   VIRT_PLATFORM_BUS_IRQ);
 lams->machine_done.notify = virt_machine_done;
 qemu_add_machine_init_done_notifier(&lams->machine_done);
 fdt_add_pcie_node(lams);
diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
index 92b84de1c5..64c90b80d2 100644
--- a/include/hw/loongarch/virt.h
+++ b/include/hw/loongarch/virt.h
@@ -49,6 +49,7 @@ struct LoongArchMachineState {
 char *oem_table_id;
 DeviceState  *acpi_ged;
 int  fdt_size;
+DeviceState *platform_bus_dev;
 };
 
 #define TYPE_LOONGARCH_MACHINE  MACHINE_TYPE_NAME("virt")
diff --git a/include/hw/pci-host/ls7a.h b/include/hw/pci-host/ls7a.h
index cdde0af1f8..9bd875ca8b 100644
--- a/include/hw/pci-host/ls7a.h
+++ b/include/hw/pci-host/ls7a.h
@@ -42,4 +42,9 @@
 #define VIRT_RTC_REG_BASE(VIRT_MISC_REG_BASE + 0x00050100)
 #define VIRT_RTC_LEN 0x100
 #define VIRT_SCI_IRQ (PCH_PIC_IRQ_OFFSET + 4)
+
+#define VIRT_PLATFORM_BUS_BASEADDRESS   0x1600
+#define VIRT_PLATFORM_BUS_SIZE  0x200
+#define VIRT_PLATFORM_BUS_NUM_IRQS  2
+#define VIRT_PLATFORM_BUS_IRQ   69
 #endif
-- 
2.31.1

[PATCH v1 2/9] hw/loongarch: Support fw_cfg dma function

[Xiaojuan Yang]

Support fw_cfg dma function for LoongArch virt machine.

Signed-off-by: Xiaojuan Yang 
---
 hw/loongarch/Kconfig  | 1 +
 hw/loongarch/fw_cfg.c | 3 ++-
 hw/loongarch/virt.c   | 2 +-
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/loongarch/Kconfig b/hw/loongarch/Kconfig
index 73c52b093e..1deea83626 100644
--- a/hw/loongarch/Kconfig
+++ b/hw/loongarch/Kconfig
@@ -16,3 +16,4 @@ config LOONGARCH_VIRT
 select SMBIOS
 select ACPI_PCI
 select ACPI_HW_REDUCED
+select FW_CFG_DMA
diff --git a/hw/loongarch/fw_cfg.c b/hw/loongarch/fw_cfg.c
index f6503d5607..f15a17416c 100644
--- a/hw/loongarch/fw_cfg.c
+++ b/hw/loongarch/fw_cfg.c
@@ -23,7 +23,8 @@ FWCfgState *loongarch_fw_cfg_init(ram_addr_t ram_size, 
MachineState *ms)
 int max_cpus = ms->smp.max_cpus;
 int smp_cpus = ms->smp.cpus;
 
-fw_cfg = fw_cfg_init_mem_wide(VIRT_FWCFG_BASE + 8, VIRT_FWCFG_BASE, 8, 0, 
NULL);
+fw_cfg = fw_cfg_init_mem_wide(VIRT_FWCFG_BASE + 8, VIRT_FWCFG_BASE, 8,
+  VIRT_FWCFG_BASE + 16, &address_space_memory);
 fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)max_cpus);
 fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
 fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index b56820ecda..4f833a2044 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -118,7 +118,7 @@ static void fdt_add_fw_cfg_node(const LoongArchMachineState 
*lams)
 qemu_fdt_setprop_string(ms->fdt, nodename,
 "compatible", "qemu,fw-cfg-mmio");
 qemu_fdt_setprop_sized_cells(ms->fdt, nodename, "reg",
- 2, base, 2, 0x8);
+ 2, base, 2, 0x18);
 qemu_fdt_setprop(ms->fdt, nodename, "dma-coherent", NULL, 0);
 g_free(nodename);
 }
-- 
2.31.1

[PATCH v1 5/9] hw/loongarch: Add hotplug handler for machine

[Xiaojuan Yang]

Add hotplug handler for LoongArch virt machine and now only support
the dynamic sysbus device.

Signed-off-by: Xiaojuan Yang 
---
 hw/loongarch/virt.c | 32 
 1 file changed, 32 insertions(+)

diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index 3976e8a058..a3dd35d579 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -804,9 +804,35 @@ static void loongarch_machine_initfn(Object *obj)
 lams->oem_table_id = g_strndup(ACPI_BUILD_APPNAME8, 8);
 }
 
+static void loongarch_machine_device_plug_cb(HotplugHandler *hotplug_dev,
+DeviceState *dev, Error **errp)
+{
+LoongArchMachineState *lams = LOONGARCH_MACHINE(hotplug_dev);
+MachineClass *mc = MACHINE_GET_CLASS(lams);
+
+if (device_is_dynamic_sysbus(mc, dev)) {
+if (lams->platform_bus_dev) {
+
platform_bus_link_device(PLATFORM_BUS_DEVICE(lams->platform_bus_dev),
+ SYS_BUS_DEVICE(dev));
+}
+}
+}
+
+static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine,
+DeviceState *dev)
+{
+MachineClass *mc = MACHINE_GET_CLASS(machine);
+
+if (device_is_dynamic_sysbus(mc, dev)) {
+return HOTPLUG_HANDLER(machine);
+}
+return NULL;
+}
+
 static void loongarch_class_init(ObjectClass *oc, void *data)
 {
 MachineClass *mc = MACHINE_CLASS(oc);
+HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
 
 mc->desc = "Loongson-3A5000 LS7A1000 machine";
 mc->init = loongarch_init;
@@ -819,6 +845,8 @@ static void loongarch_class_init(ObjectClass *oc, void 
*data)
 mc->block_default_type = IF_VIRTIO;
 mc->default_boot_order = "c";
 mc->no_cdrom = 1;
+mc->get_hotplug_handler = virt_machine_get_hotplug_handler;
+hc->plug = loongarch_machine_device_plug_cb;
 
 object_class_property_add(oc, "acpi", "OnOffAuto",
 loongarch_get_acpi, loongarch_set_acpi,
@@ -834,6 +862,10 @@ static const TypeInfo loongarch_machine_types[] = {
 .instance_size  = sizeof(LoongArchMachineState),
 .class_init = loongarch_class_init,
 .instance_init = loongarch_machine_initfn,
+.interfaces = (InterfaceInfo[]) {
+ { TYPE_HOTPLUG_HANDLER },
+ { }
+},
 }
 };
 
-- 
2.31.1

[PATCH v1 3/9] hw/loongarch: Add interrupt information to FDT table

[Xiaojuan Yang]

Add interrupt information to FDT table, such as interrupt
controller info, compatiable info, etc.

Signed-off-by: Xiaojuan Yang 
---
 hw/loongarch/virt.c | 29 +
 1 file changed, 29 insertions(+)

diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index 4f833a2044..561b05d404 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -158,6 +158,34 @@ static void fdt_add_pcie_node(const LoongArchMachineState 
*lams)
 qemu_fdt_dumpdtb(ms->fdt, lams->fdt_size);
 }
 
+static void fdt_add_irqchip_node(LoongArchMachineState *lams)
+{
+MachineState *ms = MACHINE(lams);
+char *nodename;
+uint32_t irqchip_phandle;
+
+irqchip_phandle = qemu_fdt_alloc_phandle(ms->fdt);
+qemu_fdt_setprop_cell(ms->fdt, "/", "interrupt-parent", irqchip_phandle);
+
+nodename = g_strdup_printf("/intc@%" PRIx64,
+  VIRT_IOAPIC_REG_BASE);
+qemu_fdt_add_subnode(ms->fdt, nodename);
+qemu_fdt_setprop_cell(ms->fdt, nodename, "#interrupt-cells", 3);
+qemu_fdt_setprop(ms->fdt, nodename, "interrupt-controller", NULL, 0);
+qemu_fdt_setprop_cell(ms->fdt, nodename, "#address-cells", 0x2);
+qemu_fdt_setprop_cell(ms->fdt, nodename, "#size-cells", 0x2);
+qemu_fdt_setprop(ms->fdt, nodename, "ranges", NULL, 0);
+
+qemu_fdt_setprop_string(ms->fdt, nodename, "compatible",
+"loongarch,ls7a");
+
+qemu_fdt_setprop_sized_cells(ms->fdt, nodename, "reg",
+ 2, VIRT_IOAPIC_REG_BASE,
+ 2, PCH_PIC_ROUTE_ENTRY_OFFSET);
+
+qemu_fdt_setprop_cell(ms->fdt, nodename, "phandle", irqchip_phandle);
+g_free(nodename);
+}
 
 #define PM_BASE 0x1008
 #define PM_SIZE 0x100
@@ -697,6 +725,7 @@ static void loongarch_init(MachineState *machine)
 }
 /* Initialize the IO interrupt subsystem */
 loongarch_irq_init(lams);
+fdt_add_irqchip_node(lams);
 lams->machine_done.notify = virt_machine_done;
 qemu_add_machine_init_done_notifier(&lams->machine_done);
 fdt_add_pcie_node(lams);
-- 
2.31.1

[PATCH v1 1/9] hw/loongarch: Remove vga device when loongarch init

[Xiaojuan Yang]

Remove the vga device when loongarch machine init and
we will support other display device in the future.

Signed-off-by: Xiaojuan Yang 
---
 hw/loongarch/Kconfig | 1 -
 hw/loongarch/virt.c  | 3 ---
 2 files changed, 4 deletions(-)

diff --git a/hw/loongarch/Kconfig b/hw/loongarch/Kconfig
index a99aa387c3..73c52b093e 100644
--- a/hw/loongarch/Kconfig
+++ b/hw/loongarch/Kconfig
@@ -2,7 +2,6 @@ config LOONGARCH_VIRT
 bool
 select PCI
 select PCI_EXPRESS_GENERIC_BRIDGE
-imply VGA_PCI
 imply VIRTIO_VGA
 imply PCI_DEVICES
 select ISA_BUS
diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index 5cc0b05538..b56820ecda 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -378,9 +378,6 @@ static void loongarch_devices_init(DeviceState *pch_pic, 
LoongArchMachineState *
 pci_nic_init_nofail(nd, pci_bus, nd->model, NULL);
 }
 
-/* VGA setup */
-pci_vga_init(pci_bus);
-
 /*
  * There are some invalid guest memory access.
  * Create some unimplemented devices to emulate this.
-- 
2.31.1

[PATCH v1 7/9] hw/loongarch: Fix acpi ged irq number in dsdt table

[Xiaojuan Yang]

In dsdt, acpi ged irq should use gsi number, and the
VIRT_SCI_IRQ means it.

Signed-off-by: Xiaojuan Yang 
---
 hw/loongarch/acpi-build.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/loongarch/acpi-build.c b/hw/loongarch/acpi-build.c
index d0f01a6485..95e30975a8 100644
--- a/hw/loongarch/acpi-build.c
+++ b/hw/loongarch/acpi-build.c
@@ -367,7 +367,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, 
MachineState *machine)
 if (lams->acpi_ged) {
 build_ged_aml(dsdt, "\\_SB."GED_DEVICE,
   HOTPLUG_HANDLER(lams->acpi_ged),
-  VIRT_SCI_IRQ - PCH_PIC_IRQ_OFFSET, AML_SYSTEM_MEMORY,
+  VIRT_SCI_IRQ, AML_SYSTEM_MEMORY,
   VIRT_GED_EVT_ADDR);
 }
 
-- 
2.31.1

Re: [PATCH 4/4] serial: Allow unaligned i/o access

[Marc-André Lureau]

Hi

On Wed, Sep 7, 2022 at 2:03 AM Arwed Meyer  wrote:

> Unaligned i/o access on serial UART works on real PCs.
> This is used for example by FreeDOS CTMouse driver. Without this it
> can't reset and detect serial mice.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/77
> Signed-off-by: Arwed Meyer 
> ---
>  hw/char/serial.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/hw/char/serial.c b/hw/char/serial.c
> index 7061aacbce..41b5e61977 100644
> --- a/hw/char/serial.c
> +++ b/hw/char/serial.c
> @@ -961,6 +961,9 @@ void serial_set_frequency(SerialState *s, uint32_t
> frequency)
>  const MemoryRegionOps serial_io_ops = {
>  .read = serial_ioport_read,
>  .write = serial_ioport_write,
> +.valid = {
> +.unaligned = 1,
> +},
>

I don't get how this can help if both min_access_size & max_access_size are
1.


>  .impl = {
>  .min_access_size = 1,
>  .max_access_size = 1,
> --
> 2.34.1
>
>
>

-- 
Marc-André Lureau

Re: [PATCH v5 06/13] hw/isa/vt82c686: Instantiate IDE function in host device

[Daniel Henrique Barboza]

On 9/1/22 08:41, Bernhard Beschow wrote:

The IDE function is closely tied to the ISA function (e.g. the IDE
interrupt routing happens there), so it makes sense that the IDE
function is instantiated within the south bridge itself.

Signed-off-by: Bernhard Beschow 
---


Acked-by: Daniel Henrique Barboza 


  configs/devices/mips64el-softmmu/default.mak |  1 -
  hw/isa/Kconfig   |  1 +
  hw/isa/vt82c686.c| 17 +
  hw/mips/fuloong2e.c  |  8 
  hw/ppc/Kconfig   |  1 -
  hw/ppc/pegasos2.c|  9 -
  6 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/configs/devices/mips64el-softmmu/default.mak 
b/configs/devices/mips64el-softmmu/default.mak
index c610749ac1..d5188f7ea5 100644
--- a/configs/devices/mips64el-softmmu/default.mak
+++ b/configs/devices/mips64el-softmmu/default.mak
@@ -1,7 +1,6 @@
  # Default configuration for mips64el-softmmu
  
  include ../mips-softmmu/common.mak

-CONFIG_IDE_VIA=y
  CONFIG_FULOONG=y
  CONFIG_LOONGSON3V=y
  CONFIG_ATI_VGA=y
diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
index d42143a991..20de7e9294 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -53,6 +53,7 @@ config VT82C686
  select I8254
  select I8257
  select I8259
+select IDE_VIA
  select MC146818RTC
  select PARALLEL
  
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c

index 37e37b3855..63c1e3b8ce 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -17,6 +17,7 @@
  #include "hw/isa/vt82c686.h"
  #include "hw/pci/pci.h"
  #include "hw/qdev-properties.h"
+#include "hw/ide/pci.h"
  #include "hw/isa/isa.h"
  #include "hw/isa/superio.h"
  #include "hw/intc/i8259.h"
@@ -544,6 +545,7 @@ struct ViaISAState {
  qemu_irq cpu_intr;
  qemu_irq *isa_irqs;
  ViaSuperIOState via_sio;
+PCIIDEState ide;
  };
  
  static const VMStateDescription vmstate_via = {

@@ -556,10 +558,18 @@ static const VMStateDescription vmstate_via = {
  }
  };
  
+static void via_isa_init(Object *obj)

+{
+ViaISAState *s = VIA_ISA(obj);
+
+object_initialize_child(obj, "ide", &s->ide, TYPE_VIA_IDE);
+}
+
  static const TypeInfo via_isa_info = {
  .name  = TYPE_VIA_ISA,
  .parent= TYPE_PCI_DEVICE,
  .instance_size = sizeof(ViaISAState),
+.instance_init = via_isa_init,
  .abstract  = true,
  .interfaces= (InterfaceInfo[]) {
  { INTERFACE_CONVENTIONAL_PCI_DEVICE },
@@ -583,6 +593,7 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
  {
  ViaISAState *s = VIA_ISA(d);
  DeviceState *dev = DEVICE(d);
+PCIBus *pci_bus = pci_get_bus(d);
  qemu_irq *isa_irq;
  ISABus *isa_bus;
  int i;
@@ -612,6 +623,12 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
  if (!qdev_realize(DEVICE(&s->via_sio), BUS(isa_bus), errp)) {
  return;
  }
+
+/* Function 1: IDE */
+qdev_prop_set_int32(DEVICE(&s->ide), "addr", d->devfn + 1);
+if (!qdev_realize(DEVICE(&s->ide), BUS(pci_bus), errp)) {
+return;
+}
  }
  
  /* TYPE_VT82C686B_ISA */

diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 44225fbe33..32605901e7 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -199,13 +199,13 @@ static void main_cpu_reset(void *opaque)
  static void vt82c686b_southbridge_init(PCIBus *pci_bus, int slot, qemu_irq 
intc,
 I2CBus **i2c_bus)
  {
-PCIDevice *dev;
+PCIDevice *dev, *via;
  
-dev = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(slot, 0), true,

+via = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(slot, 0), true,
TYPE_VT82C686B_ISA);
-qdev_connect_gpio_out(DEVICE(dev), 0, intc);
+qdev_connect_gpio_out(DEVICE(via), 0, intc);
  
-dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 1), TYPE_VIA_IDE);

+dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
  pci_ide_create_devs(dev);
  
  pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");

diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
index 400511c6b7..18565e966b 100644
--- a/hw/ppc/Kconfig
+++ b/hw/ppc/Kconfig
@@ -74,7 +74,6 @@ config PEGASOS2
  bool
  select MV64361
  select VT82C686
-select IDE_VIA
  select SMBUS_EEPROM
  select VOF
  # This should come with VT82C686
diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index 8039775f80..8bc528a560 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -102,7 +102,7 @@ static void pegasos2_init(MachineState *machine)
  CPUPPCState *env;
  MemoryRegion *rom = g_new(MemoryRegion, 1);
  PCIBus *pci_bus;
-PCIDevice *dev;
+PCIDevice *dev, *via;
  I2CBus *i2c_bus;
  const char *fwname = machine->firmware ?: PROM_FILENAME;
  char *filename;
@@ -160,13 +160,12 @@ static void pegasos2_init(MachineState

Re: [PATCH v5 05/13] hw/isa/vt82c686: Introduce TYPE_VIA_IDE define

[Daniel Henrique Barboza]

On 9/1/22 08:41, Bernhard Beschow wrote:

Establishes consistency with other (VIA) devices.

Signed-off-by: Bernhard Beschow 
---


Acked-by: Daniel Henrique Barboza 


  hw/ide/via.c  | 2 +-
  hw/mips/fuloong2e.c   | 2 +-
  hw/ppc/pegasos2.c | 2 +-
  include/hw/isa/vt82c686.h | 1 +
  4 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/ide/via.c b/hw/ide/via.c
index 82def819c4..e1a429405d 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -230,7 +230,7 @@ static void via_ide_class_init(ObjectClass *klass, void 
*data)
  }
  
  static const TypeInfo via_ide_info = {

-.name  = "via-ide",
+.name  = TYPE_VIA_IDE,
  .parent= TYPE_PCI_IDE,
  .class_init= via_ide_class_init,
  };
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 5ee546f5f6..44225fbe33 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -205,7 +205,7 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int 
slot, qemu_irq intc,
TYPE_VT82C686B_ISA);
  qdev_connect_gpio_out(DEVICE(dev), 0, intc);
  
-dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 1), "via-ide");

+dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 1), TYPE_VIA_IDE);
  pci_ide_create_devs(dev);
  
  pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");

diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index 61f4263953..8039775f80 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -166,7 +166,7 @@ static void pegasos2_init(MachineState *machine)
qdev_get_gpio_in_named(pm->mv, "gpp", 31));
  
  /* VT8231 function 1: IDE Controller */

-dev = pci_create_simple(pci_bus, PCI_DEVFN(12, 1), "via-ide");
+dev = pci_create_simple(pci_bus, PCI_DEVFN(12, 1), TYPE_VIA_IDE);
  pci_ide_create_devs(dev);
  
  /* VT8231 function 2-3: USB Ports */

diff --git a/include/hw/isa/vt82c686.h b/include/hw/isa/vt82c686.h
index 56ac141be3..87aca3e5bb 100644
--- a/include/hw/isa/vt82c686.h
+++ b/include/hw/isa/vt82c686.h
@@ -8,6 +8,7 @@
  #define TYPE_VT8231_ISA "vt8231-isa"
  #define TYPE_VT8231_PM "vt8231-pm"
  #define TYPE_VIA_AC97 "via-ac97"
+#define TYPE_VIA_IDE "via-ide"
  #define TYPE_VIA_MC97 "via-mc97"
  
  void via_isa_set_irq(PCIDevice *d, int n, int level);

Re: [PATCH v5 08/13] hw/isa/vt82c686: Instantiate USB functions in host device

[Daniel Henrique Barboza]

On 9/1/22 08:41, Bernhard Beschow wrote:

The USB functions can be enabled/disabled through the ISA function. Also
its interrupt routing can be influenced there.

Signed-off-by: Bernhard Beschow 
---


Acked-by: Daniel Henrique Barboza 


  hw/isa/vt82c686.c   | 12 
  hw/mips/fuloong2e.c |  3 ---
  hw/ppc/pegasos2.c   |  4 
  3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 63c1e3b8ce..f05fd9948a 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -23,6 +23,7 @@
  #include "hw/intc/i8259.h"
  #include "hw/irq.h"
  #include "hw/dma/i8257.h"
+#include "hw/usb/hcd-uhci.h"
  #include "hw/timer/i8254.h"
  #include "hw/rtc/mc146818rtc.h"
  #include "migration/vmstate.h"
@@ -546,6 +547,7 @@ struct ViaISAState {
  qemu_irq *isa_irqs;
  ViaSuperIOState via_sio;
  PCIIDEState ide;
+UHCIState uhci[2];
  };
  
  static const VMStateDescription vmstate_via = {

@@ -563,6 +565,8 @@ static void via_isa_init(Object *obj)
  ViaISAState *s = VIA_ISA(obj);
  
  object_initialize_child(obj, "ide", &s->ide, TYPE_VIA_IDE);

+object_initialize_child(obj, "uhci1", &s->uhci[0], 
TYPE_VT82C686B_USB_UHCI);
+object_initialize_child(obj, "uhci2", &s->uhci[1], 
TYPE_VT82C686B_USB_UHCI);
  }
  
  static const TypeInfo via_isa_info = {

@@ -629,6 +633,14 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
  if (!qdev_realize(DEVICE(&s->ide), BUS(pci_bus), errp)) {
  return;
  }
+
+/* Functions 2-3: USB Ports */
+for (i = 0; i < ARRAY_SIZE(s->uhci); i++) {
+qdev_prop_set_int32(DEVICE(&s->uhci[i]), "addr", d->devfn + 2 + i);
+if (!qdev_realize(DEVICE(&s->uhci[i]), BUS(pci_bus), errp)) {
+return;
+}
+}
  }
  
  /* TYPE_VT82C686B_ISA */

diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 6b7370f2aa..dc92223b76 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -208,9 +208,6 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int 
slot, qemu_irq intc,
  dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
  pci_ide_create_devs(dev);
  
-pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), TYPE_VT82C686B_USB_UHCI);

-pci_create_simple(pci_bus, PCI_DEVFN(slot, 3), TYPE_VT82C686B_USB_UHCI);
-
  dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 4), TYPE_VT82C686B_PM);
  *i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c"));
  
diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c

index 70776558c9..85cca6f7a6 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -168,10 +168,6 @@ static void pegasos2_init(MachineState *machine)
  dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
  pci_ide_create_devs(dev);
  
-/* VT8231 function 2-3: USB Ports */

-pci_create_simple(pci_bus, PCI_DEVFN(12, 2), TYPE_VT82C686B_USB_UHCI);
-pci_create_simple(pci_bus, PCI_DEVFN(12, 3), TYPE_VT82C686B_USB_UHCI);
-
  /* VT8231 function 4: Power Management Controller */
  dev = pci_create_simple(pci_bus, PCI_DEVFN(12, 4), TYPE_VT8231_PM);
  i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c"));

Re: [PATCH v5 07/13] hw/isa/vt82c686: Introduce TYPE_VT82C686B_USB_UHCI define

[Daniel Henrique Barboza]

On 9/1/22 08:41, Bernhard Beschow wrote:

Suggested-by: BALATON Zoltan 
Signed-off-by: Bernhard Beschow 
---


Acked-by: Daniel Henrique Barboza 


  hw/mips/fuloong2e.c| 4 ++--
  hw/ppc/pegasos2.c  | 4 ++--
  hw/usb/vt82c686-uhci-pci.c | 4 ++--
  include/hw/isa/vt82c686.h  | 1 +
  4 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 32605901e7..6b7370f2aa 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -208,8 +208,8 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int 
slot, qemu_irq intc,
  dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
  pci_ide_create_devs(dev);
  
-pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");

-pci_create_simple(pci_bus, PCI_DEVFN(slot, 3), "vt82c686b-usb-uhci");
+pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), TYPE_VT82C686B_USB_UHCI);
+pci_create_simple(pci_bus, PCI_DEVFN(slot, 3), TYPE_VT82C686B_USB_UHCI);
  
  dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 4), TYPE_VT82C686B_PM);

  *i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c"));
diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index 8bc528a560..70776558c9 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -169,8 +169,8 @@ static void pegasos2_init(MachineState *machine)
  pci_ide_create_devs(dev);
  
  /* VT8231 function 2-3: USB Ports */

-pci_create_simple(pci_bus, PCI_DEVFN(12, 2), "vt82c686b-usb-uhci");
-pci_create_simple(pci_bus, PCI_DEVFN(12, 3), "vt82c686b-usb-uhci");
+pci_create_simple(pci_bus, PCI_DEVFN(12, 2), TYPE_VT82C686B_USB_UHCI);
+pci_create_simple(pci_bus, PCI_DEVFN(12, 3), TYPE_VT82C686B_USB_UHCI);
  
  /* VT8231 function 4: Power Management Controller */

  dev = pci_create_simple(pci_bus, PCI_DEVFN(12, 4), TYPE_VT8231_PM);
diff --git a/hw/usb/vt82c686-uhci-pci.c b/hw/usb/vt82c686-uhci-pci.c
index 0bf2b72ff0..46a901f56f 100644
--- a/hw/usb/vt82c686-uhci-pci.c
+++ b/hw/usb/vt82c686-uhci-pci.c
@@ -31,7 +31,7 @@ static void usb_uhci_vt82c686b_realize(PCIDevice *dev, Error 
**errp)
  
  static UHCIInfo uhci_info[] = {

  {
-.name  = "vt82c686b-usb-uhci",
+.name  = TYPE_VT82C686B_USB_UHCI,
  .vendor_id = PCI_VENDOR_ID_VIA,
  .device_id = PCI_DEVICE_ID_VIA_UHCI,
  .revision  = 0x01,
@@ -45,7 +45,7 @@ static UHCIInfo uhci_info[] = {
  
  static const TypeInfo vt82c686b_usb_uhci_type_info = {

  .parent = TYPE_UHCI,
-.name   = "vt82c686b-usb-uhci",
+.name   = TYPE_VT82C686B_USB_UHCI,
  .class_init = uhci_data_class_init,
  .class_data = uhci_info,
  };
diff --git a/include/hw/isa/vt82c686.h b/include/hw/isa/vt82c686.h
index 87aca3e5bb..e6f6dd4d43 100644
--- a/include/hw/isa/vt82c686.h
+++ b/include/hw/isa/vt82c686.h
@@ -5,6 +5,7 @@
  
  #define TYPE_VT82C686B_ISA "vt82c686b-isa"

  #define TYPE_VT82C686B_PM "vt82c686b-pm"
+#define TYPE_VT82C686B_USB_UHCI "vt82c686b-usb-uhci"
  #define TYPE_VT8231_ISA "vt8231-isa"
  #define TYPE_VT8231_PM "vt8231-pm"
  #define TYPE_VIA_AC97 "via-ac97"

Re: [PATCH v5 09/13] hw/isa/vt82c686: Instantiate PM function in host device

[Daniel Henrique Barboza]

On 9/1/22 08:41, Bernhard Beschow wrote:

The PM controller has activity bits which monitor activity of other
built-in devices in the host device.

Signed-off-by: Bernhard Beschow 
---


Acked-by: Daniel Henrique Barboza 


  hw/isa/vt82c686.c | 13 +
  hw/mips/fuloong2e.c   |  2 +-
  hw/ppc/pegasos2.c |  3 +--
  include/hw/isa/vt82c686.h |  2 --
  4 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index f05fd9948a..d048607079 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -250,6 +250,8 @@ static const ViaPMInitInfo vt82c686b_pm_init_info = {
  .device_id = PCI_DEVICE_ID_VIA_82C686B_PM,
  };
  
+#define TYPE_VT82C686B_PM "vt82c686b-pm"

+
  static const TypeInfo vt82c686b_pm_info = {
  .name  = TYPE_VT82C686B_PM,
  .parent= TYPE_VIA_PM,
@@ -261,6 +263,8 @@ static const ViaPMInitInfo vt8231_pm_init_info = {
  .device_id = PCI_DEVICE_ID_VIA_8231_PM,
  };
  
+#define TYPE_VT8231_PM "vt8231-pm"

+
  static const TypeInfo vt8231_pm_info = {
  .name  = TYPE_VT8231_PM,
  .parent= TYPE_VIA_PM,
@@ -548,6 +552,7 @@ struct ViaISAState {
  ViaSuperIOState via_sio;
  PCIIDEState ide;
  UHCIState uhci[2];
+ViaPMState pm;
  };
  
  static const VMStateDescription vmstate_via = {

@@ -641,6 +646,12 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
  return;
  }
  }
+
+/* Function 4: Power Management */
+qdev_prop_set_int32(DEVICE(&s->pm), "addr", d->devfn + 4);
+if (!qdev_realize(DEVICE(&s->pm), BUS(pci_bus), errp)) {
+return;
+}
  }
  
  /* TYPE_VT82C686B_ISA */

@@ -683,6 +694,7 @@ static void vt82c686b_init(Object *obj)
  ViaISAState *s = VIA_ISA(obj);
  
  object_initialize_child(obj, "sio", &s->via_sio, TYPE_VT82C686B_SUPERIO);

+object_initialize_child(obj, "pm", &s->pm, TYPE_VT82C686B_PM);
  }
  
  static void vt82c686b_class_init(ObjectClass *klass, void *data)

@@ -746,6 +758,7 @@ static void vt8231_init(Object *obj)
  ViaISAState *s = VIA_ISA(obj);
  
  object_initialize_child(obj, "sio", &s->via_sio, TYPE_VT8231_SUPERIO);

+object_initialize_child(obj, "pm", &s->pm, TYPE_VT8231_PM);
  }
  
  static void vt8231_class_init(ObjectClass *klass, void *data)

diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index dc92223b76..377108d313 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -208,7 +208,7 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int 
slot, qemu_irq intc,
  dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
  pci_ide_create_devs(dev);
  
-dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 4), TYPE_VT82C686B_PM);

+dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "pm"));
  *i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c"));
  
  /* Audio support */

diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index 85cca6f7a6..e32944ee2b 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -168,8 +168,7 @@ static void pegasos2_init(MachineState *machine)
  dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
  pci_ide_create_devs(dev);
  
-/* VT8231 function 4: Power Management Controller */

-dev = pci_create_simple(pci_bus, PCI_DEVFN(12, 4), TYPE_VT8231_PM);
+dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "pm"));
  i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c"));
  spd_data = spd_data_generate(DDR, machine->ram_size);
  smbus_eeprom_init_one(i2c_bus, 0x57, spd_data);
diff --git a/include/hw/isa/vt82c686.h b/include/hw/isa/vt82c686.h
index e6f6dd4d43..eaa07881c5 100644
--- a/include/hw/isa/vt82c686.h
+++ b/include/hw/isa/vt82c686.h
@@ -4,10 +4,8 @@
  #include "hw/pci/pci.h"
  
  #define TYPE_VT82C686B_ISA "vt82c686b-isa"

-#define TYPE_VT82C686B_PM "vt82c686b-pm"
  #define TYPE_VT82C686B_USB_UHCI "vt82c686b-usb-uhci"
  #define TYPE_VT8231_ISA "vt8231-isa"
-#define TYPE_VT8231_PM "vt8231-pm"
  #define TYPE_VIA_AC97 "via-ac97"
  #define TYPE_VIA_IDE "via-ide"
  #define TYPE_VIA_MC97 "via-mc97"

Re: [PATCH V4 0/3] hw/riscv: virt: Enable booting S-mode firmware from pflashy

[Sunil V L]

On Wed, Sep 07, 2022 at 09:10:37AM +0200, Gerd Hoffmann wrote:
> On Tue, Sep 06, 2022 at 06:02:00PM +0530, Sunil V L wrote:
> > Hi Gerd,
> > 
> > On Tue, Sep 06, 2022 at 12:41:28PM +0200, Gerd Hoffmann wrote:
> > >   Hi,
> > > 
> > > > 3)Make the EDK2 image size to match with what qemu flash expects
> > > > truncate -s 32M Build/RiscVVirt/DEBUG_GCC5/FV/RISCV_VIRT.fd
> > > 
> > > Hmm, we have that kind of padding on arm too (64M for code and 64M for
> > > vars) and only a fraction of the space is actually used, which isn't
> > > exactly ideal.  So not sure it is a good plan to repeat that on riscv.
> > 
> > Yeah.. but it looks like limitation from qemu flash emulation. Do you mean
> > this limitation exists for arm in general on real flash also?
> 
> Well, at least on x86 flash devices can have odd sizes.  I don't think
> the qemu pflash emulation dictates anything here.
> 
> I think the underlying problem we actually have in qemu is that the
> flash size indirectly dictates the memory layout.  We pack the flash
> devices next to each other, on x86 downwards from 4G, on arm upwards
> from zero, not sure what risc-v is dong here.
> 
> edk2 arm code expects the variable store being mapped at 64m.  So
> QEMU_EFI.fd (which is actually 2M in size) gets padded to 64m, which
> has the desired effect that the next flash device (the varstore) is
> mapped at 64m.  But also has the side effect that we map 62m of zeros
> into the guest address space ...
> 
> The vars file is padded to 64m for consistency with the code.  Not
> padding the vars file should have no bad side effects I think, except
> for live migration where the flash size change might cause
> compatibility problems.
> 
> Not padding the code file needs some alternative way to specify the
> memory layout, to make sure the vars flash continues to be mapped at
> 64m even when the code flash is smaller.  Cc'ed Pawel who investigates
> that right now.
> 
> One possible option is to just hard-code the flash memory layout per
> machine type or per architecture.  Another option would be to add
> some way to configure that on the command line.

Thanks Gerd. One question: Is it possible to have separate code + vars
even when there is TF-A? My understanding is, TF-A will take one drive
and will be hidden from the non-secure word. So, there is only one flash
left for edk2. Is that correct?

In RISC-V, I think we have the this situation always since M-mode is
mandatory. The first flash is always reserved for secure fw. So, we will
have to increase the number of flash supported to 3 to support edk2 use
case.

I have a fix RISC-V which resolves truncate issue leveraging logic from
x86. It also creates 2 flash drives within non-secure space.
EDK2 also needs to be modified to work with smaller code flash. But
because the patch takes care of the actual size, it allows to have
bigger code and smaller var images.
Same thing can be adopted to arm also since both seem to follow the same
logic. But I think that will be a separate patch than this series. I
will run that as a separate RFC patch. Is that fine?

Thanks!
Sunil
> 
> take care,
>   Gerd
> 

Re: [PATCH v5 00/13] Instantiate VT82xx functions in host device

[Daniel Henrique Barboza]

On 9/8/22 05:34, Bernhard Beschow wrote:

Am 1. September 2022 11:41:14 UTC schrieb Bernhard Beschow :

v5:

* Add patch "Inline vt82c686b_southbridge_init() and remove it" (Zoltan)

* Use machine parameter when creating rtc-time alias (Zoltan)



Testing done: Same as in v3.



v4:

* Fix in comment: AC97 Modem -> MC97 Modem (Zoltan)

* Introduce TYPE_VT82C686B_USB_UHCI define (Zoltan)

* Introduce TYPE_VIA_IDE define (for consistency)



v3:

* Replace pre increment by post increment in for loop (Zoltan)

* Move class defines close to where the class is defined (Zoltan)



Testing done:

* `make check-avocado`

  Passes for boot_linux_console.py for mips64el_fuloong2e

* `qemu-system-ppc -machine pegasos2 -rtc base=localtime -device 
ati-vga,guest_hwcursor=true,romfile="" -cdrom morphos-3.17.iso -kernel 
morphos-3.17/boot.img`

  Boots successfully and it is possible to open games and tools.



v2:

* Keep the call to pci_ide_create_devs() in board code for consistency (Zoltan)

* Create rtc-time alias in board rather than in south bridge code

* Remove stale comments about PCI functions (Zoltan)



v1:

This series instantiates all PCI functions of the VT82xx south bridges in the 
south bridges themselves.

For the IDE function this is especially important since its interrupt routing 
is configured in the

ISA function, hence doesn't make sense to instantiate it as a "Frankenstein" 
device. The interrupt

routing is currently hardcoded and changing that is currently not in the scope 
of this series.



Testing done:

* `qemu-system-ppc -machine pegasos2 -rtc base=localtime -device 
ati-vga,guest_hwcursor=true,romfile="" -cdrom morphos-3.17.iso -kernel 
morphos-3.17/boot.img`

  Boots successfully and it is possible to open games and tools.



* I was unable to test the fuloong2e board even before this series since it 
seems to be unfinished [1].

  A buildroot-baked kernel [2] booted but doesn't find its root partition, 
though the issues could be in the buildroot receipt I created.



[1] https://osdn.net/projects/qmiga/wiki/SubprojectPegasos2

[2] https://github.com/shentok/buildroot/commits/fuloong2e



Ping

Zoltan, would you mind giving your Reviewed-by for 'hw/mips/fuloong2e: Inline 
vt82c686b_southbridge_init() and remove it' explicitly? Perhaps I was too eager 
to omit it since I didn't want to put words in your mouth.

What else is missing? Who would do the pull request?



The bulk of the changes were done in hw/isa/vt82c686.c and hw/mips/fuloong2e.c.
The Fuloong 2E maintainers are already CCed, so I believe they're already
aware of this series.

I did my test round with the PowerPC test suit with this series and it didn't
break anything, so I acked all patches that changed hw/ppc/pegasos2.c. Feel
free to push those changes in the Fuloong 2E pull request.


Thanks,


Daniel




Thanks,
Bernhard



Bernhard Beschow (13):

  hw/isa/vt82c686: Resolve chip-specific realize methods

  hw/isa/vt82c686: Resolve unneeded attribute

  hw/isa/vt82c686: Prefer pci_address_space() over get_system_memory()

  hw/isa/vt82c686: Reuse errp

  hw/isa/vt82c686: Introduce TYPE_VIA_IDE define

  hw/isa/vt82c686: Instantiate IDE function in host device

  hw/isa/vt82c686: Introduce TYPE_VT82C686B_USB_UHCI define

  hw/isa/vt82c686: Instantiate USB functions in host device

  hw/isa/vt82c686: Instantiate PM function in host device

  hw/isa/vt82c686: Instantiate AC97 and MC97 functions in host device

  hw/mips/fuloong2e: Inline vt82c686b_southbridge_init() and remove it

  hw/isa/vt82c686: Embed RTCState in host device

  hw/isa/vt82c686: Create rtc-time alias in boards instead



configs/devices/mips64el-softmmu/default.mak |   1 -

hw/ide/via.c |   2 +-

hw/isa/Kconfig   |   1 +

hw/isa/vt82c686.c| 120 +++

hw/mips/fuloong2e.c  |  39 +++---

hw/ppc/Kconfig   |   1 -

hw/ppc/pegasos2.c|  25 ++--

hw/usb/vt82c686-uhci-pci.c   |   4 +-

include/hw/isa/vt82c686.h|   4 +-

9 files changed, 126 insertions(+), 71 deletions(-)



-- >
2.37.3

Re: [PATCH v5 13/13] hw/isa/vt82c686: Create rtc-time alias in boards instead

[Daniel Henrique Barboza]

On 9/1/22 08:41, Bernhard Beschow wrote:

According to good QOM practice, an object should only deal with objects
of its own sub tree. Having devices create an alias on the machine
object doesn't respect this good practice. To resolve this, create the
alias in the machine's code.

Signed-off-by: Bernhard Beschow 
---


Acked-by: Daniel Henrique Barboza 


  hw/isa/vt82c686.c   | 2 --
  hw/mips/fuloong2e.c | 4 
  hw/ppc/pegasos2.c   | 4 
  3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 48cd4d0036..3f9bd0c04d 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -632,8 +632,6 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
  if (!qdev_realize(DEVICE(&s->rtc), BUS(isa_bus), errp)) {
  return;
  }
-object_property_add_alias(qdev_get_machine(), "rtc-time", OBJECT(&s->rtc),
-  "date");
  isa_connect_gpio_out(ISA_DEVICE(&s->rtc), 0, s->rtc.isairq);
  
  for (i = 0; i < PCI_CONFIG_HEADER_SIZE; i++) {

diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 3c46215616..b478483706 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -295,6 +295,10 @@ static void mips_fuloong2e_init(MachineState *machine)
  pci_dev = pci_create_simple_multifunction(pci_bus,
PCI_DEVFN(FULOONG2E_VIA_SLOT, 
0),
true, TYPE_VT82C686B_ISA);
+object_property_add_alias(OBJECT(machine), "rtc-time",
+  object_resolve_path_component(OBJECT(pci_dev),
+"rtc"),
+  "date");
  qdev_connect_gpio_out(DEVICE(pci_dev), 0, env->irq[5]);
  
  dev = DEVICE(object_resolve_path_component(OBJECT(pci_dev), "ide"));

diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index 09fdb7557f..49b753c7cc 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -161,6 +161,10 @@ static void pegasos2_init(MachineState *machine)
  /* VIA VT8231 South Bridge (multifunction PCI device) */
  via = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(12, 0), true,
TYPE_VT8231_ISA);
+object_property_add_alias(OBJECT(machine), "rtc-time",
+  object_resolve_path_component(OBJECT(via),
+"rtc"),
+  "date");
  qdev_connect_gpio_out(DEVICE(via), 0,
qdev_get_gpio_in_named(pm->mv, "gpp", 31));
  

Re: [PATCH v5 10/13] hw/isa/vt82c686: Instantiate AC97 and MC97 functions in host device

[Daniel Henrique Barboza]

On 9/1/22 08:41, Bernhard Beschow wrote:

The AC97 function's wakeup status is wired to the PM function and both
the AC97 and MC97 interrupt routing is determined by the ISA function.

Signed-off-by: Bernhard Beschow 
---


Acked-by: Daniel Henrique Barboza 


  hw/isa/vt82c686.c   | 16 
  hw/mips/fuloong2e.c |  4 
  hw/ppc/pegasos2.c   |  5 -
  3 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index d048607079..91686e9570 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -553,6 +553,8 @@ struct ViaISAState {
  PCIIDEState ide;
  UHCIState uhci[2];
  ViaPMState pm;
+PCIDevice ac97;
+PCIDevice mc97;
  };
  
  static const VMStateDescription vmstate_via = {

@@ -572,6 +574,8 @@ static void via_isa_init(Object *obj)
  object_initialize_child(obj, "ide", &s->ide, TYPE_VIA_IDE);
  object_initialize_child(obj, "uhci1", &s->uhci[0], 
TYPE_VT82C686B_USB_UHCI);
  object_initialize_child(obj, "uhci2", &s->uhci[1], 
TYPE_VT82C686B_USB_UHCI);
+object_initialize_child(obj, "ac97", &s->ac97, TYPE_VIA_AC97);
+object_initialize_child(obj, "mc97", &s->mc97, TYPE_VIA_MC97);
  }
  
  static const TypeInfo via_isa_info = {

@@ -652,6 +656,18 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
  if (!qdev_realize(DEVICE(&s->pm), BUS(pci_bus), errp)) {
  return;
  }
+
+/* Function 5: AC97 Audio */
+qdev_prop_set_int32(DEVICE(&s->ac97), "addr", d->devfn + 5);
+if (!qdev_realize(DEVICE(&s->ac97), BUS(pci_bus), errp)) {
+return;
+}
+
+/* Function 6: MC97 Modem */
+qdev_prop_set_int32(DEVICE(&s->mc97), "addr", d->devfn + 6);
+if (!qdev_realize(DEVICE(&s->mc97), BUS(pci_bus), errp)) {
+return;
+}
  }
  
  /* TYPE_VT82C686B_ISA */

diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 377108d313..2d8723ab74 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -210,10 +210,6 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, 
int slot, qemu_irq intc,
  
  dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "pm"));

  *i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c"));
-
-/* Audio support */
-pci_create_simple(pci_bus, PCI_DEVFN(slot, 5), TYPE_VIA_AC97);
-pci_create_simple(pci_bus, PCI_DEVFN(slot, 6), TYPE_VIA_MC97);
  }
  
  /* Network support */

diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index e32944ee2b..09fdb7557f 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -159,7 +159,6 @@ static void pegasos2_init(MachineState *machine)
  pci_bus = mv64361_get_pci_bus(pm->mv, 1);
  
  /* VIA VT8231 South Bridge (multifunction PCI device) */

-/* VT8231 function 0: PCI-to-ISA Bridge */
  via = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(12, 0), true,
TYPE_VT8231_ISA);
  qdev_connect_gpio_out(DEVICE(via), 0,
@@ -173,10 +172,6 @@ static void pegasos2_init(MachineState *machine)
  spd_data = spd_data_generate(DDR, machine->ram_size);
  smbus_eeprom_init_one(i2c_bus, 0x57, spd_data);
  
-/* VT8231 function 5-6: AC97 Audio & Modem */

-pci_create_simple(pci_bus, PCI_DEVFN(12, 5), TYPE_VIA_AC97);
-pci_create_simple(pci_bus, PCI_DEVFN(12, 6), TYPE_VIA_MC97);
-
  /* other PC hardware */
  pci_vga_init(pci_bus);
  

Re: [PATCH v5 00/13] Instantiate VT82xx functions in host device

[BALATON Zoltan]

On Thu, 8 Sep 2022, Bernhard Beschow wrote:

Am 1. September 2022 11:41:14 UTC schrieb Bernhard Beschow :

v5:

* Add patch "Inline vt82c686b_southbridge_init() and remove it" (Zoltan)

* Use machine parameter when creating rtc-time alias (Zoltan)



Testing done: Same as in v3.



v4:

* Fix in comment: AC97 Modem -> MC97 Modem (Zoltan)

* Introduce TYPE_VT82C686B_USB_UHCI define (Zoltan)

* Introduce TYPE_VIA_IDE define (for consistency)



v3:

* Replace pre increment by post increment in for loop (Zoltan)

* Move class defines close to where the class is defined (Zoltan)



Testing done:

* `make check-avocado`

 Passes for boot_linux_console.py for mips64el_fuloong2e

* `qemu-system-ppc -machine pegasos2 -rtc base=localtime -device 
ati-vga,guest_hwcursor=true,romfile="" -cdrom morphos-3.17.iso -kernel 
morphos-3.17/boot.img`

 Boots successfully and it is possible to open games and tools.



v2:

* Keep the call to pci_ide_create_devs() in board code for consistency (Zoltan)

* Create rtc-time alias in board rather than in south bridge code

* Remove stale comments about PCI functions (Zoltan)



v1:

This series instantiates all PCI functions of the VT82xx south bridges in the 
south bridges themselves.

For the IDE function this is especially important since its interrupt routing 
is configured in the

ISA function, hence doesn't make sense to instantiate it as a "Frankenstein" 
device. The interrupt

routing is currently hardcoded and changing that is currently not in the scope 
of this series.



Testing done:

* `qemu-system-ppc -machine pegasos2 -rtc base=localtime -device 
ati-vga,guest_hwcursor=true,romfile="" -cdrom morphos-3.17.iso -kernel 
morphos-3.17/boot.img`

 Boots successfully and it is possible to open games and tools.



* I was unable to test the fuloong2e board even before this series since it 
seems to be unfinished [1].

 A buildroot-baked kernel [2] booted but doesn't find its root partition, 
though the issues could be in the buildroot receipt I created.



[1] https://osdn.net/projects/qmiga/wiki/SubprojectPegasos2

[2] https://github.com/shentok/buildroot/commits/fuloong2e



Ping

Zoltan, would you mind giving your Reviewed-by for 'hw/mips/fuloong2e: 
Inline vt82c686b_southbridge_init() and remove it' explicitly? Perhaps I 
was too eager to omit it since I didn't want to put words in your mouth.


You said in your follow up message that all except this patch has my R-b 
which is correct. This one already has Suggested-by from me so I agree 
with it with or without an explicit Reviewed-by.



What else is missing? Who would do the pull request?


It was Philippe before who merged these maybe needs his attention or give 
some Ack to go via smoe other tree? My mails don't seem to reach him 
though due to bouncing as spam so not sure he sees this.


Regards,
BALATON Zoltan


Thanks,
Bernhard



Bernhard Beschow (13):

 hw/isa/vt82c686: Resolve chip-specific realize methods

 hw/isa/vt82c686: Resolve unneeded attribute

 hw/isa/vt82c686: Prefer pci_address_space() over get_system_memory()

 hw/isa/vt82c686: Reuse errp

 hw/isa/vt82c686: Introduce TYPE_VIA_IDE define

 hw/isa/vt82c686: Instantiate IDE function in host device

 hw/isa/vt82c686: Introduce TYPE_VT82C686B_USB_UHCI define

 hw/isa/vt82c686: Instantiate USB functions in host device

 hw/isa/vt82c686: Instantiate PM function in host device

 hw/isa/vt82c686: Instantiate AC97 and MC97 functions in host device

 hw/mips/fuloong2e: Inline vt82c686b_southbridge_init() and remove it

 hw/isa/vt82c686: Embed RTCState in host device

 hw/isa/vt82c686: Create rtc-time alias in boards instead



configs/devices/mips64el-softmmu/default.mak |   1 -

hw/ide/via.c |   2 +-

hw/isa/Kconfig   |   1 +

hw/isa/vt82c686.c| 120 +++

hw/mips/fuloong2e.c  |  39 +++---

hw/ppc/Kconfig   |   1 -

hw/ppc/pegasos2.c|  25 ++--

hw/usb/vt82c686-uhci-pci.c   |   4 +-

include/hw/isa/vt82c686.h|   4 +-

9 files changed, 126 insertions(+), 71 deletions(-)



-- >
2.37.3

[PATCH] migration: support file: uri for source migration

[Nikolay Borisov]

This is a prototype of supporting a 'file:' based uri protocol for
writing out the migration stream of qemu. Currently the code always
opens the file in DIO mode and adheres to an alignment of 64k to be
generic enough. However this comes with a problem - it requires copying
all data that we are writing (qemu metadata + guest ram pages) to a
bounce buffer so that we adhere to this alignment. With this code I get
the following performance results:

  DIO  exec: cat > file virsh --bypass-cache
  8277  
81
  8278  
80
  8080  
82
  8282  
77
  7779  
77

AVG:  80.6  79.2
79.4
stddev: 1.959   1.720   
2.05

All numbers are in seconds.

Those results are somewhat surprising to me as I'd expected doing the
writeout directly within qemu and avoiding copying between qemu and
virsh's iohelper process would result in a speed up. Clearly that's not
the case, I attribute this to the fact that all memory pages have to be
copied into the bounce buffer. There are more measurements/profiling
work that I'd have to do in order to (dis)prove this hypotheses and will
report back when I have the data.

However I'm sending the code now as I'd like to facilitate a discussion
as to whether this is an approach that would be acceptable to upstream
merging. Any ideas/comments would be much appreciated.

Signed-off-by: Nikolay Borisov 
---
 include/io/channel-file.h |   1 +
 include/io/channel.h  |   1 +
 io/channel-file.c |  17 +++
 migration/meson.build |   1 +
 migration/migration.c |   4 ++
 migration/migration.h |   2 +
 migration/qemu-file.c | 104 +-
 7 files changed, 129 insertions(+), 1 deletion(-)

diff --git a/include/io/channel-file.h b/include/io/channel-file.h
index 50e8eb113868..6cb0b698c62c 100644
--- a/include/io/channel-file.h
+++ b/include/io/channel-file.h
@@ -89,4 +89,5 @@ qio_channel_file_new_path(const char *path,
   mode_t mode,
   Error **errp);

+void qio_channel_file_disable_dio(QIOChannelFile *ioc);
 #endif /* QIO_CHANNEL_FILE_H */
diff --git a/include/io/channel.h b/include/io/channel.h
index c680ee748021..6127ff6c0626 100644
--- a/include/io/channel.h
+++ b/include/io/channel.h
@@ -41,6 +41,7 @@ enum QIOChannelFeature {
 QIO_CHANNEL_FEATURE_SHUTDOWN,
 QIO_CHANNEL_FEATURE_LISTEN,
 QIO_CHANNEL_FEATURE_WRITE_ZERO_COPY,
+QIO_CHANNEL_FEATURE_DIO,
 };


diff --git a/io/channel-file.c b/io/channel-file.c
index b67687c2aa64..5c7211b128f1 100644
--- a/io/channel-file.c
+++ b/io/channel-file.c
@@ -59,6 +59,10 @@ qio_channel_file_new_path(const char *path,
 return NULL;
 }

+if (flags & O_DIRECT) {
+   qio_channel_set_feature(QIO_CHANNEL(ioc), QIO_CHANNEL_FEATURE_DIO);
+}
+
 trace_qio_channel_file_new_path(ioc, path, flags, mode, ioc->fd);

 return ioc;
@@ -109,6 +113,19 @@ static ssize_t qio_channel_file_readv(QIOChannel *ioc,
 return ret;
 }

+
+void qio_channel_file_disable_dio(QIOChannelFile *ioc)
+{
+   int flags = fcntl(ioc->fd, F_GETFL);
+   if (flags == -1) {
+   //handle failure
+   }
+
+   if (fcntl(ioc->fd, F_SETFL, (flags & ~O_DIRECT)) == -1) {
+   error_report("Can't disable O_DIRECT");
+   }
+}
+
 static ssize_t qio_channel_file_writev(QIOChannel *ioc,
const struct iovec *iov,
size_t niov,
diff --git a/migration/meson.build b/migration/meson.build
index 690487cf1a81..30a8392701c3 100644
--- a/migration/meson.build
+++ b/migration/meson.build
@@ -17,6 +17,7 @@ softmmu_ss.add(files(
   'colo.c',
   'exec.c',
   'fd.c',
+  'file.c',
   'global_state.c',
   'migration.c',
   'multifd.c',
diff --git a/migration/migration.c b/migration/migration.c
index bb8bbddfe467..e7e84ae12066 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -20,6 +20,7 @@
 #include "migration/blocker.h"
 #include "exec.h"
 #include "fd.h"
+#include "file.h"
 #include "socket.h"
 #include "sysemu/runstate.h"
 #include "sysemu/sysemu.h"
@@ -2414,6 +2415,8 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
 exec_start_outgoing_migration(s, p, &local_err);
 } else if (strstart(uri, "fd:", &p)) {
 fd_start_outgoing_migration(s, p, &local_err);
+} else if (strstart(uri, "file:", &p)) {
+   file_start_outgoing_migration(s, p, &local_err);
 } else {

Re: [PATCH] chardev: fix segfault in finalize

[Vladimir Sementsov-Ogievskiy]

On 8/25/22 19:52, Maksim Davydov wrote:

If finalize chardev-msmouse or chardev-wctable is called immediately after
init it cases QEMU to crash with segfault. This happens because of
QTAILQ_REMOVE in qemu_input_handler_unregister tries to dereference
NULL pointer.
For instance, this error can be reproduced via `qom-list-properties`
command.

Signed-off-by: Maksim Davydov



Reviewed-by: Vladimir Sementsov-Ogievskiy 

--
Best regards,
Vladimir

[PATCH 2/4] qmp: add dump machine type compatible properties

[Maksim Davydov]

To control that creating new machine type doesn't affect the previous
types (their compat_props) and to check complex compat_props inheritance
we need qmp command to print machine type compatible properties.
This patch adds the ability to get list of all the compat_props of the
corresponding supported machines for their comparison via new optional
argument of "query-machines" command.

Signed-off-by: Maksim Davydov 
---
 hw/core/machine-qmp-cmds.c  | 22 ++-
 qapi/machine.json   | 54 +++--
 tests/qtest/fuzz/qos_fuzz.c |  2 +-
 3 files changed, 74 insertions(+), 4 deletions(-)

diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
index 4f4ab30f8c..a6fc387439 100644
--- a/hw/core/machine-qmp-cmds.c
+++ b/hw/core/machine-qmp-cmds.c
@@ -74,7 +74,8 @@ CpuInfoFastList *qmp_query_cpus_fast(Error **errp)
 return head;
 }
 
-MachineInfoList *qmp_query_machines(Error **errp)
+MachineInfoList *qmp_query_machines(bool has_compat_props, bool compat_props,
+Error **errp)
 {
 GSList *el, *machines = object_class_get_list(TYPE_MACHINE, false);
 MachineInfoList *mach_list = NULL;
@@ -107,6 +108,25 @@ MachineInfoList *qmp_query_machines(Error **errp)
 info->default_ram_id = g_strdup(mc->default_ram_id);
 info->has_default_ram_id = true;
 }
+if (compat_props && mc->compat_props) {
+int i;
+info->compat_props = NULL;
+CompatPropertyList **tail = &(info->compat_props);
+info->has_compat_props = true;
+
+for (i = 0; i < mc->compat_props->len; i++) {
+GlobalProperty *mt_prop = g_ptr_array_index(mc->compat_props,
+i);
+CompatProperty *prop;
+
+prop = g_malloc0(sizeof(*prop));
+prop->driver = g_strdup(mt_prop->driver);
+prop->property = g_strdup(mt_prop->property);
+prop->value = g_strdup(mt_prop->value);
+
+QAPI_LIST_APPEND(tail, prop);
+}
+}
 
 QAPI_LIST_PREPEND(mach_list, info);
 }
diff --git a/qapi/machine.json b/qapi/machine.json
index 6afd1936b0..b5477224a8 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -127,6 +127,25 @@
 ##
 { 'command': 'query-cpus-fast', 'returns': [ 'CpuInfoFast' ] }
 
+##
+# @CompatProperty:
+#
+# Machine type compatible property. It's based on GlobalProperty and created
+# for machine type compat properties (see scripts)
+#
+# @driver: name of the driver that has GlobalProperty
+#
+# @property: global property name
+#
+# @value: global property value
+#
+# Since: 7.2
+##
+{ 'struct': 'CompatProperty',
+  'data': { 'driver': 'str',
+'property': 'str',
+'value': 'str' } }
+
 ##
 # @MachineInfo:
 #
@@ -155,6 +174,9 @@
 #
 # @default-ram-id: the default ID of initial RAM memory backend (since 5.2)
 #
+# @compat-props: List of compatible properties that defines machine type
+#(since 7.2)
+#
 # Since: 1.2
 ##
 { 'struct': 'MachineInfo',
@@ -162,18 +184,46 @@
 '*is-default': 'bool', 'cpu-max': 'int',
 'hotpluggable-cpus': 'bool',  'numa-mem-supported': 'bool',
 'deprecated': 'bool', '*default-cpu-type': 'str',
-'*default-ram-id': 'str' } }
+'*default-ram-id': 'str', '*compat-props': ['CompatProperty'] } }
 
 ##
 # @query-machines:
 #
 # Return a list of supported machines
 #
+# @compat-props: if true return will contain information about machine type
+#compatible properties (since 7.2)
+#
 # Returns: a list of MachineInfo
 #
 # Since: 1.2
+#
+# Example:
+#
+# -> { "execute": "query-machines", "arguments": { "compat-props": true } }
+# <- { "return": [
+#  {
+#  "hotpluggable-cpus": true,
+#  "name": "pc-q35-6.2",
+#  "compat-props": [
+#  {
+#  "driver": "virtio-mem",
+#  "property": "unplugged-inaccessible",
+#  "value": "off"
+#   }
+#   ],
+#   "numa-mem-supported": false,
+#   "default-cpu-type": "qemu64-x86_64-cpu",
+#   "cpu-max": 288,
+#   "deprecated": false,
+#   "default-ram-id": "pc.ram"
+#   },
+#   ...
+#}
 ##
-{ 'command': 'query-machines', 'returns': ['MachineInfo'] }
+{ 'command': 'query-machines',
+  'data': { '*compat-props': 'bool' },
+  'returns': ['MachineInfo'] }
 
 ##
 # @CurrentMachineParams:
diff --git a/tests/qtest/fuzz/qos_fuzz.c b/tests/qtest/fuzz/qos_fuzz.c
index c856d3d500..f0c9ed4c4b 100644
--- a/tests/qtest/fuzz/qos_fuzz.c
+++ b/tests/qtest/fuzz/qos_fuzz.c
@@ -46,7 +46,7 @@ static void qos_set_machines_devices_available(void)
 MachineInfoList *mach_info;
 ObjectTypeInfoList *type_info;
 
-mach_info = qmp_qu

Re: [PATCH v6 14/14] qmp/hmp, device_tree.c: introduce dumpdtb

[Dr. David Alan Gilbert]

* Daniel Henrique Barboza (danielhb...@gmail.com) wrote:
> To save the FDT blob we have the '-machine dumpdtb=' property.
> With this property set, the machine saves the FDT in  and exit.
> The created file can then be converted to plain text dts format using
> 'dtc'.
> 
> There's nothing particularly sophisticated into saving the FDT that
> can't be done with the machine at any state, as long as the machine has
> a valid FDT to be saved.
> 
> The 'dumpdtb' command receives a 'filename' paramenter and, if a valid
> FDT is available, it'll save it in a file 'filename'. In short, this is
> a '-machine dumpdtb' that can be fired on demand via QMP/HMP.
> 
> A valid FDT consists of a FDT that was created using libfdt being
> retrieved via 'current_machine->fdt' in device_tree.c. This condition is
> met by most FDT users in QEMU.
> 
> This command will always be executed in-band (i.e. holding BQL),
> avoiding potential race conditions with machines that might change the
> FDT during runtime (e.g. PowerPC 'pseries' machine).
> 
> Cc: Dr. David Alan Gilbert 
> Cc: Markus Armbruster 
> Cc: Alistair Francis 
> Cc: David Gibson 
> Signed-off-by: Daniel Henrique Barboza 
> ---
>  hmp-commands.hx  | 15 +++
>  include/sysemu/device_tree.h |  1 +
>  monitor/misc.c   |  1 +
>  qapi/machine.json| 18 ++
>  softmmu/device_tree.c| 31 +++
>  5 files changed, 66 insertions(+)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 182e639d14..9a3e57504f 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1800,3 +1800,18 @@ ERST
>"\n\t\t\t\t\t limit on a specified virtual cpu",
>  .cmd= hmp_cancel_vcpu_dirty_limit,
>  },
> +
> +#if defined(CONFIG_FDT)
> +SRST
> +``dumpdtb`` *filename*
> +  Save the FDT in the 'filename' file to be decoded using dtc.
> +  Requires 'libfdt' support.
> +ERST

Can you please put the SRST/ERST below the { }'s please;  most of the
hmp-commands.hx has it that way around but it looks like the diry
rate/limit set at the bottom have them flipped which we need to fix.

> +{
> +.name   = "dumpdtb",
> +.args_type  = "filename:F",
> +.params = "filename",
> +.help   = "save the FDT in the 'filename' file to be decoded 
> using dtc",
> +.cmd= hmp_dumpdtb,
> +},
> +#endif

Other than the flip,

Acked-by: Dr. David Alan Gilbert 

> diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
> index ef060a9759..e7c5441f56 100644
> --- a/include/sysemu/device_tree.h
> +++ b/include/sysemu/device_tree.h
> @@ -136,6 +136,7 @@ int qemu_fdt_add_path(void *fdt, const char *path);
>  } while (0)
>  
>  void qemu_fdt_dumpdtb(void *fdt, int size);
> +void hmp_dumpdtb(Monitor *mon, const QDict *qdict);
>  
>  /**
>   * qemu_fdt_setprop_sized_cells_from_array:
> diff --git a/monitor/misc.c b/monitor/misc.c
> index 3d2312ba8d..e7dd63030b 100644
> --- a/monitor/misc.c
> +++ b/monitor/misc.c
> @@ -49,6 +49,7 @@
>  #include "sysemu/blockdev.h"
>  #include "sysemu/sysemu.h"
>  #include "sysemu/tpm.h"
> +#include "sysemu/device_tree.h"
>  #include "qapi/qmp/qdict.h"
>  #include "qapi/qmp/qerror.h"
>  #include "qapi/qmp/qstring.h"
> diff --git a/qapi/machine.json b/qapi/machine.json
> index 6afd1936b0..f968a5d343 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -1664,3 +1664,21 @@
>   '*size': 'size',
>   '*max-size': 'size',
>   '*slots': 'uint64' } }
> +
> +##
> +# @dumpdtb:
> +#
> +# Save the FDT in dtb format. Requires 'libfdt' support.
> +#
> +# @filename: name of the FDT file to be created
> +#
> +# Since: 7.2
> +#
> +# Example:
> +#   {"execute": "dumpdtb"}
> +#"arguments": { "filename": "/tmp/fdt.dtb" } }
> +#
> +##
> +{ 'command': 'dumpdtb',
> +  'data': { 'filename': 'str' },
> +  'if': 'CONFIG_FDT' }
> diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c
> index 6ca3fad285..cdd41b6de6 100644
> --- a/softmmu/device_tree.c
> +++ b/softmmu/device_tree.c
> @@ -26,6 +26,9 @@
>  #include "hw/loader.h"
>  #include "hw/boards.h"
>  #include "qemu/config-file.h"
> +#include "qapi/qapi-commands-machine.h"
> +#include "qapi/qmp/qdict.h"
> +#include "monitor/hmp.h"
>  
>  #include 
>  
> @@ -643,3 +646,31 @@ out:
>  g_free(propcells);
>  return ret;
>  }
> +
> +void qmp_dumpdtb(const char *filename, Error **errp)
> +{
> +g_autoptr(GError) err = NULL;
> +int size;
> +
> +if (!current_machine->fdt) {
> +error_setg(errp, "Unable to find the machine FDT");
> +return;
> +}
> +
> +size = fdt_totalsize(current_machine->fdt);
> +
> +if (!g_file_set_contents(filename, current_machine->fdt, size, &err)) {
> +error_setg(errp, "Error saving FDT to file %s: %s",
> +   filename, err->message);
> +}
> +}
> +
> +void hmp_dumpdtb(Monitor *mon, const QDict *qdict)
> +{
> +const char *filename = qd

[PATCH 1/4] qom: add devault value

[Maksim Davydov]

qmp_qom_list_properties can print default values if they are available
as qmp_device_list_properties does, because both of them use the
ObjectPropertyInfo structure with default_value field. This can be useful
when working with "not device" types.

Signed-off-by: Maksim Davydov 
---
 qom/qom-qmp-cmds.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c
index 2e63a4c184..1d7867dc19 100644
--- a/qom/qom-qmp-cmds.c
+++ b/qom/qom-qmp-cmds.c
@@ -217,6 +217,8 @@ ObjectPropertyInfoList *qmp_qom_list_properties(const char 
*typename,
 info->type = g_strdup(prop->type);
 info->has_description = !!prop->description;
 info->description = g_strdup(prop->description);
+info->default_value = qobject_ref(prop->defval);
+info->has_default_value = !!info->default_value;
 
 QAPI_LIST_PREPEND(prop_list, info);
 }
-- 
2.25.1

[PATCH 0/4] compare machine type compat_props

[Maksim Davydov]

This script is necessary to choose the best machine type in the
appropriate cases. Also we have to check compat_props of the old MT 
after changes to be sure that they haven't broken old the MT. For 
example, pc_compat_3_1 of pc-q35-3.1 has Icelake-Client which was
removed in March.

changes:
* new default value print concept
* QEMU python library is used to collect qmp data
* remove auxiliary patches (that was used to fix ->get sematics)
* print compat_props in the correct order
* delete `absract` field to reduce output JSON size

Maksim Davydov (4):
  qom: add devault value
  qmp: add dump machine type compatible properties
  python/qmp: increase read buffer size
  scripts: add script to compare compatible properties

 hw/core/machine-qmp-cmds.c|  22 +-
 python/qemu/qmp/qmp_client.py |   4 +-
 qapi/machine.json |  54 -
 qom/qom-qmp-cmds.c|   2 +
 scripts/compare_mt.py | 370 ++
 tests/qtest/fuzz/qos_fuzz.c   |   2 +-
 6 files changed, 448 insertions(+), 6 deletions(-)
 create mode 100755 scripts/compare_mt.py

-- 
2.25.1

[PATCH 3/4] python/qmp: increase read buffer size

[Maksim Davydov]

After modification of "query-machines" command the buffer size should be
more than 452kB to contain output with compat-props.

Signed-off-by: Maksim Davydov 
---
 python/qemu/qmp/qmp_client.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/python/qemu/qmp/qmp_client.py b/python/qemu/qmp/qmp_client.py
index 5dcda04a75..659fe4d98c 100644
--- a/python/qemu/qmp/qmp_client.py
+++ b/python/qemu/qmp/qmp_client.py
@@ -197,8 +197,8 @@ async def run(self, address='/tmp/qemu.socket'):
 #: Logger object used for debugging messages.
 logger = logging.getLogger(__name__)
 
-# Read buffer limit; large enough to accept query-qmp-schema
-_limit = (256 * 1024)
+# Read buffer limit; large enough to accept query-machines
+_limit = (512 * 1024)
 
 # Type alias for pending execute() result items
 _PendingT = Union[Message, ExecInterruptedError]
-- 
2.25.1

Re: [PATCH V4 0/3] hw/riscv: virt: Enable booting S-mode firmware from pflash

[Sunil V L]

> Same thing can be adopted to arm also since both seem to follow the same
> logic. But I think that will be a separate patch than this series. I
> will run that as a separate RFC patch. Is that fine?

Just to be clear, I meant RISC-V fix as separate RFC patch. Not the ARM.

Thanks
Sunil

[PATCH 4/4] scripts: add script to compare compatible properties

[Maksim Davydov]

This script run QEMU to obtain compat_props of machines and default
values of different types and produce appropriate table. This table
can be used to compare machine types to choose the most suitable
machine. Also table in json or csv format should be used to check that
new machine doesn't affect previous ones via comparisin tables with and
without new machine.
Default values of properties are needed to fill "holes" in the table (one
machine has these properties and another not).

Notes:
* some init values from the devices can't be available like properties
from virtio-9p when configure has --disable-virtfs. This situations will
be seen in the table as "unavailable driver".
* Default values can be get can be obtained in an unobvious way, like
x86 features. If the script doesn't know how to get property default value
to compare one machine with another it fills "holes" with "unavailable
method". This is done because script uses whitelist model to get default
values of different types. It means that the method that can't be applied
to a new type that can crash this script. It is better to get an
"unavailable driver" when creating a new machine with new compatible
properties than to break this script. So it turns out a more stable and
generic script.
* If the default value can't be obtained because this property doesn't
exist or because this property can't have default value, appropriate
"hole" will be filled by "unknown property" or "no default value"
* If the property is applied to the abstract class, the script collects
default values from all child classes (set of default values)

Example:

scripts/compare_mt.py --MT pc-q35-3.1 pc-q35-2.12

╒╤═══╤═══╕
││  pc-q35-2.12  │  pc-q35-3.1   │
╞╪═══╪═══╡
│EPYC-IBPB-x86_64-cpu-xlevel │  0x800a   │  2147483678   │
├┼───┼───┤
│   EPYC-x86_64-cpu-xlevel   │  0x800a   │  2147483678   │
├┼───┼───┤
│ Skylake-Server-IBRS-x86_64-cpu-pku │ False │ True  │
├┼───┼───┤
│   Skylake-Server-x86_64-cpu-pku│ False │ True  │
├┼───┼───┤
│ VGA-global-vmstate │ True  │ False │
├┼───┼───┤
│ cirrus-vga-global-vmstate  │ True  │ False │
├┼───┼───┤
│hda-audio-use-timer │ False │ True  │
├┼───┼───┤
│  migration-decompress-error-check  │ False │ True  │
├┼───┼───┤
│   qxl-vga-global-vmstate   │ True  │ False │
├┼───┼───┤
│ vmware-svga-global-vmstate │ True  │ False │
├┼───┼───┤
│  x86_64-cpu-legacy-cache   │ True  │ [True, False] │
├┼───┼───┤
│ x86_64-cpu-topoext │ False │ [True, False] │
├┼───┼───┤
│   x86_64-cpu-x-hv-synic-kvm-only   │ True  │ False │
╘╧═══╧═══╛

Signed-off-by: Maksim Davydov 
---
 scripts/compare_mt.py | 370 ++
 1 file changed, 370 insertions(+)
 create mode 100755 scripts/compare_mt.py

diff --git a/scripts/compare_mt.py b/scripts/compare_mt.py
new file mode 100755
index 00..a063c79682
--- /dev/null
+++ b/scripts/compare_mt.py
@@ -0,0 +1,370 @@
+#!/usr/bin/env python3
+#
+# Copyright (c) Yandex Technologies LLC, 2022
+#
+# Script to compare machine type compatible properties
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, see .
+
+from tabulate import tabulate
+import json
+import sys
+from os import path
+from argparse import ArgumentParser, RawTextHelpForma

Re: [PATCH 2/2] configure: Add -Wno-gnu-variable-sized-type-not-at-end

[Peter Maydell]

On Thu, 8 Sept 2022 at 10:09, Daniel P. Berrangé  wrote:
>
> On Thu, Sep 08, 2022 at 09:53:44AM +0100, Peter Maydell wrote:
> > On Thu, 8 Sept 2022 at 09:08, Chenyi Qiang  wrote:
> > >
> > > After updating linux headers to v6.0-rc, clang build on x86 target would
> > > generate warnings like:
> > >
> > > target/i386/kvm/kvm.c:470:25: error: field 'info' with variable sized
> > > type 'struct kvm_msrs' not at the end of a struct or class is a GNU
> > > extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
> > > struct kvm_msrs info;
> > > ^
> > > target/i386/kvm/kvm.c:1701:27: error: field 'cpuid' with variable sized
> > > type 'struct kvm_cpuid2' not at the end of a struct or class is a GNU
> > > extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
> > > struct kvm_cpuid2 cpuid;
> > >   ^
> > > target/i386/kvm/kvm.c:2868:25: error: field 'info' with variable sized
> > > type 'struct kvm_msrs' not at the end of a struct or class is a GNU
> > > extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
> > > struct kvm_msrs info;
> > > ^
> > >
> > > Considering that it is OK to use GNU extension in QEMU (e.g. g_auto 
> > > stuff),
> > > it is acceptable to turn off this warning, which is only relevant to 
> > > people
> > > striving for fully portable C code.
> >
> > Can we get the kernel folks to fix their headers not to
> > use GCC extensions like this ? It's not a big deal for us
> > I guess, but in general it doesn't seem great that the
> > kernel headers rely on userspace to silence warnings...
>
> The kernel headers are fine actually - it is C99 compliant to have
> a unsized array field in structs. eg
>
> The problem is in the QEMU code which is taking the kernel declared
> struct, and then embedding it inside another struct. e.g. the
> kernel exposes:
>
>   struct kvm_msrs {
> __u32 nmsrs; /* number of msrs in entries */
> __u32 pad;
>
> struct kvm_msr_entry entries[];
>   };
>
> which we then use as:
>
>   uint64_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t index)
>   {
> struct {
> struct kvm_msrs info;
> struct kvm_msr_entry entries[1];
> } msr_data = {};

Hmm, and the kernel used to define the 'entries' field as 'entries[0]',
which is the GNU 'zero-length-array' extension. So the kernel has
switched to the C-standard-defined flexible array member. Those
(unlike the GNU zero-length-array) have some extra restrictions
like this "can't put the struct into another struct" one. So
effectively we've moved from a GCC extension that clang doesn't
complain about to one that it does complain about.

> IOW, our locally defined struct is just there to force a stack
> allocation large enough for 1 kvm_msr_entry. A clever trick, but
> requires that we turn off the CLang portability warning

I guess that's the most reasonable thing. Might be worth
expanding on some of this in the commit message, though.

Also, this patch disabling the warning needs to come before
the patch where the headers are updated; otherwise this will
break bisection with clang.

thanks
-- PMM

Re: [PULL 00/10] QAPI patches patches for 2022-09-07

[Kevin Wolf]

Am 07.09.2022 um 17:03 hat Markus Armbruster geschrieben:
> The following changes since commit 946e9bccf12f2bcc3ca471b820738fb22d14fc80:
> 
>   Merge tag 'samuel-thibault' of https://people.debian.org/~sthibault/qemu 
> into staging (2022-09-06 08:31:24 -0400)
> 
> are available in the Git repository at:
> 
>   git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2022-09-07
> 
> for you to fetch changes up to 6e7a37ffc230d06852f1a8893097331d39df77c9:
> 
>   qapi: fix examples of events missing timestamp (2022-09-07 15:10:13 +0200)
> 
> 
> QAPI patches patches for 2022-09-07
> 
> 
> Victor Toso (10):
>   qapi: fix example of query-ballon command
>   qapi: fix example of query-vnc command
>   qapi: fix example of query-dump-guest-memory-capability command
>   qapi: fix example of BLOCK_JOB_READY event
>   qapi: fix example of NIC_RX_FILTER_CHANGED event
>   qapi: fix example of DEVICE_UNPLUG_GUEST_ERROR event
>   qapi: fix example of MEM_UNPLUG_ERROR event
>   qapi: fix examples of blockdev-add with qcow2

NACK, this patch is wrong.

'file' is a required member (defined in BlockdevOptionsGenericFormat),
removing it makes the example invalid. 'data-file' is only an additional
optional member to be used for external data files (i.e. when the guest
data is kept separate from the metadata in the .qcow2 file).

Kevin

>   qapi: fix example of query-hotpluggable-cpus command
>   qapi: fix examples of events missing timestamp
> 
>  qapi/block-core.json | 12 ++--
>  qapi/dump.json   |  2 +-
>  qapi/machine.json|  8 
>  qapi/migration.json  | 27 +++
>  qapi/net.json|  1 -
>  qapi/qdev.json   |  3 +--
>  qapi/ui.json |  4 ++--
>  7 files changed, 37 insertions(+), 20 deletions(-)
> 
> -- 
> 2.37.2
> 
> 

Re: [PATCH 4/4] serial: Allow unaligned i/o access

[Michael S. Tsirkin]

On Thu, Sep 08, 2022 at 02:11:28PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Wed, Sep 7, 2022 at 2:03 AM Arwed Meyer  wrote:
> 
> Unaligned i/o access on serial UART works on real PCs.
> This is used for example by FreeDOS CTMouse driver. Without this it
> can't reset and detect serial mice.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/77
> Signed-off-by: Arwed Meyer 
> ---
>  hw/char/serial.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/hw/char/serial.c b/hw/char/serial.c
> index 7061aacbce..41b5e61977 100644
> --- a/hw/char/serial.c
> +++ b/hw/char/serial.c
> @@ -961,6 +961,9 @@ void serial_set_frequency(SerialState *s, uint32_t
> frequency)
>  const MemoryRegionOps serial_io_ops = {
>      .read = serial_ioport_read,
>      .write = serial_ioport_write,
> +    .valid = {
> +        .unaligned = 1,
> +    },
> 
> 
> I don't get how this can help if both min_access_size & max_access_size are 1.
>  
> 
>      .impl = {
>          .min_access_size = 1,
>          .max_access_size = 1,
> --
> 2.34.1


Because that's .impl. If access is invalid we don't get as far
as breaking it up to chunks.

> 
> 
> 
> 
> --
> Marc-André Lureau

Re: [PATCH V4 0/3] hw/riscv: virt: Enable booting S-mode firmware from pflashy

[Gerd Hoffmann]

  Hi,

> Thanks Gerd. One question: Is it possible to have separate code + vars
> even when there is TF-A? My understanding is, TF-A will take one drive
> and will be hidden from the non-secure word. So, there is only one flash
> left for edk2. Is that correct?

Yes.

> In RISC-V, I think we have the this situation always since M-mode is
> mandatory. The first flash is always reserved for secure fw. So, we will
> have to increase the number of flash supported to 3 to support edk2 use
> case.

Well.  Adding one more flash is certainly the easiest approach.  Another
possible option would be to have the secure world store the efi variables.
That might be needed anyway for secure boot support.

> I have a fix RISC-V which resolves truncate issue leveraging logic from
> x86. It also creates 2 flash drives within non-secure space.
> EDK2 also needs to be modified to work with smaller code flash. But
> because the patch takes care of the actual size, it allows to have
> bigger code and smaller var images.

The size of the code flash and thereby the address of the varstore is
known at compile time too, so yes, that approach should work fine.

Note that changing the flash size can lead to compatibility problems
(for example when live migrating), so I'd suggest to be generous with
code/vars sizes.  On x64 the upstream default flash size is 2M and when
all compile-time options are enabled things don't fit any more.

So, assuming size figures are roughly the same for risc-v, I'd suggest
to go for 4M or 8M code flash and 1M vars flash.

> Same thing can be adopted to arm also since both seem to follow the same
> logic.

Yes.  The tricky part here is dealing with backward compatibility and
the transition from padded to non-padded images.  I suspect at the end
of the day keeping the vars flash mapping fixed at 64M (and have a hole
between code and vars) will be easier.

> But I think that will be a separate patch than this series. I
> will run that as a separate RFC patch. Is that fine?

No objections.

take care,
  Gerd

[PATCH v3] 9pfs: use GHashTable for fid table

[Linus Heckemann]

The previous implementation would iterate over the fid table for
lookup operations, resulting in an operation with O(n) complexity on
the number of open files and poor cache locality -- for every open,
stat, read, write, etc operation.

This change uses a hashtable for this instead, significantly improving
the performance of the 9p filesystem. The runtime of NixOS's simple
installer test, which copies ~122k files totalling ~1.8GiB from 9p,
decreased by a factor of about 10.

Signed-off-by: Linus Heckemann 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Greg Kurz 
---

Greg Kurz writes:
> The comment above should be adapted to the new situation : no need

I've removed it completely, since the logic is simple enough that only
the shortened comment below remains necessary.

> With the new logic, this should just be:

now is :)

> g_hash_table_steal_extended() [1] actually allows to do just that.

g_hash_table_steal_extended unfortunately isn't available since it was
introduced in glib 2.58 and we're maintaining compatibility to 2.56.

> You could just call g_hash_table_iter_remove(&iter) here

Applied this suggestion, thanks!


> Well... finding at least one clunked fid state in this table is
> definitely a bug so I'll keep the BUG_ON() anyway.

Christian Schoenebeck writes:
> Yeah, I think you are right, it would feel odd. Just drop BUG_ON() for
> now.

I still prefer dropping it, but if we were to keep it I think it should
be in v9fs_reclaim_fd where we iterate and can thus check the whole
table.


Greg Kurz and Philippe Mathieu-Daudé write:
> [patch versioning]

Whoops. I used -v2 on git send-email, which just ignored the option,
rather than git format-patch, by accident. This one _should_ now be v3!


 hw/9pfs/9p.c | 140 +--
 hw/9pfs/9p.h |   2 +-
 2 files changed, 70 insertions(+), 72 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index aebadeaa03..98a475e560 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -282,33 +282,31 @@ static V9fsFidState *coroutine_fn get_fid(V9fsPDU *pdu, 
int32_t fid)
 V9fsFidState *f;
 V9fsState *s = pdu->s;
 
-QSIMPLEQ_FOREACH(f, &s->fid_list, next) {
-BUG_ON(f->clunked);
-if (f->fid == fid) {
-/*
- * Update the fid ref upfront so that
- * we don't get reclaimed when we yield
- * in open later.
- */
-f->ref++;
-/*
- * check whether we need to reopen the
- * file. We might have closed the fd
- * while trying to free up some file
- * descriptors.
- */
-err = v9fs_reopen_fid(pdu, f);
-if (err < 0) {
-f->ref--;
-return NULL;
-}
-/*
- * Mark the fid as referenced so that the LRU
- * reclaim won't close the file descriptor
- */
-f->flags |= FID_REFERENCED;
-return f;
+f = g_hash_table_lookup(s->fids, GINT_TO_POINTER(fid));
+if (f) {
+/*
+ * Update the fid ref upfront so that
+ * we don't get reclaimed when we yield
+ * in open later.
+ */
+f->ref++;
+/*
+ * check whether we need to reopen the
+ * file. We might have closed the fd
+ * while trying to free up some file
+ * descriptors.
+ */
+err = v9fs_reopen_fid(pdu, f);
+if (err < 0) {
+f->ref--;
+return NULL;
 }
+/*
+ * Mark the fid as referenced so that the LRU
+ * reclaim won't close the file descriptor
+ */
+f->flags |= FID_REFERENCED;
+return f;
 }
 return NULL;
 }
@@ -317,12 +315,9 @@ static V9fsFidState *alloc_fid(V9fsState *s, int32_t fid)
 {
 V9fsFidState *f;
 
-QSIMPLEQ_FOREACH(f, &s->fid_list, next) {
+if (g_hash_table_contains(s->fids, GINT_TO_POINTER(fid))) {
 /* If fid is already there return NULL */
-BUG_ON(f->clunked);
-if (f->fid == fid) {
-return NULL;
-}
+return NULL;
 }
 f = g_new0(V9fsFidState, 1);
 f->fid = fid;
@@ -333,7 +328,7 @@ static V9fsFidState *alloc_fid(V9fsState *s, int32_t fid)
  * reclaim won't close the file descriptor
  */
 f->flags |= FID_REFERENCED;
-QSIMPLEQ_INSERT_TAIL(&s->fid_list, f, next);
+g_hash_table_insert(s->fids, GINT_TO_POINTER(fid), f);
 
 v9fs_readdir_init(s->proto_version, &f->fs.dir);
 v9fs_readdir_init(s->proto_version, &f->fs_reclaim.dir);
@@ -424,12 +419,11 @@ static V9fsFidState *clunk_fid(V9fsState *s, int32_t fid)
 {
 V9fsFidState *fidp;
 
-QSIMPLEQ_FOREACH(fidp, &s->fid_list, next) {
-if (fidp->fid == fid) {
-QSIMPLEQ_REMOVE(&s->fid_list, fidp, V9fsFidState, next);
-fidp->clunked = true;
-return fidp;
-}
+fidp = g_hash_table_lookup(s->fids,

Re: [PATCH v2 1/2] x86: only modify setup_data if the boot protocol indicates safety

[Laszlo Ersek]

On 09/06/22 13:33, Daniel P. Berrangé wrote:
> On Tue, Sep 06, 2022 at 01:14:50PM +0200, Ard Biesheuvel wrote:
>> (cc Laszlo)
>>
>> On Tue, 6 Sept 2022 at 12:45, Michael S. Tsirkin  wrote:
>>>
>>> On Tue, Sep 06, 2022 at 12:43:55PM +0200, Jason A. Donenfeld wrote:
 On Tue, Sep 6, 2022 at 12:40 PM Michael S. Tsirkin  wrote:
>
> On Tue, Sep 06, 2022 at 12:36:56PM +0200, Jason A. Donenfeld wrote:
>> It's only safe to modify the setup_data pointer on newer kernels where
>> the EFI stub loader will ignore it. So condition setting that offset on
>> the newer boot protocol version. While we're at it, gate this on SEV too.
>> This depends on the kernel commit linked below going upstream.
>>
>> Cc: Gerd Hoffmann 
>> Cc: Laurent Vivier 
>> Cc: Michael S. Tsirkin 
>> Cc: Paolo Bonzini 
>> Cc: Peter Maydell 
>> Cc: Philippe Mathieu-Daudé 
>> Cc: Richard Henderson 
>> Cc: Ard Biesheuvel 
>> Link: 
>> https://lore.kernel.org/linux-efi/20220904165321.1140894-1-ja...@zx2c4.com/
>> Signed-off-by: Jason A. Donenfeld 
>
> BTW what does it have to do with SEV?
> Is this because SEV is not going to trust the data to be random anyway?

 Daniel (now CC'd) pointed out in one of the previous threads that this
 breaks SEV, because the image hash changes.

 Jason
>>>
>>> Oh I see. I'd add a comment maybe and definitely mention this
>>> in the commit log.
>>>
>>
>> This does raise the question (as I mentioned before) how things like
>> secure boot and measured boot are affected when combined with direct
>> kernel boot: AIUI, libvirt uses direct kernel boot at guest
>> installation time, and modifying setup_data will corrupt the image
>> signature.
> 
> IIUC, qemu already modifies setup_data when using direct kernel boot.
> 
> It put in logic to skip this if SEV is enabled, to avoid interfering
> with SEV hashes over the kernel, but there's nothing doing this more
> generally for non-SEV cases using UEFI. So potentially use of SecureBoot
> may already be impacted when using direct kernel boot.

Yes,

https://github.com/tianocore/edk2/commit/82808b422617

Laszlo

> I haven't formally
> tested this myself though. I just saw that earlier versions of this
> RNG patch broke SEV hashes and later versions addressed that problem
> for SEV when the code was re-arranged.
> 
> With regards,
> Daniel
> 

Re: [PATCH v4 2/4] hw/ssi: ibex_spi: fixup coverity issue

[Alistair Francis]

On Tue, Aug 23, 2022 at 8:13 AM Wilfred Mallawa
 wrote:
>
> From: Wilfred Mallawa 
>
> This patch addresses the coverity issues specified in [1],
> as suggested, `FIELD_DP32()`/`FIELD_EX32()` macros have been
> implemented to clean up the code.
>
> [1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg887713.html
>
> Fixes: Coverity CID 1488107
>
> Signed-off-by: Wilfred Mallawa 
> Reviewed-by: Alistair Francis 
> Reviewed-by: Andrew Jones 
> ---
>  hw/ssi/ibex_spi_host.c | 132 +
>  1 file changed, 68 insertions(+), 64 deletions(-)
>
> diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c
> index 601041d719..d52b193a1a 100644
> --- a/hw/ssi/ibex_spi_host.c
> +++ b/hw/ssi/ibex_spi_host.c
> @@ -108,18 +108,22 @@ static inline uint8_t div4_round_up(uint8_t dividend)
>
>  static void ibex_spi_rxfifo_reset(IbexSPIHostState *s)
>  {
> +uint32_t data = s->regs[IBEX_SPI_HOST_STATUS];
>  /* Empty the RX FIFO and assert RXEMPTY */
>  fifo8_reset(&s->rx_fifo);
> -s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_RXFULL_MASK;
> -s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_RXEMPTY_MASK;
> +data = FIELD_DP32(data, STATUS, RXFULL, 0);
> +data = FIELD_DP32(data, STATUS, RXEMPTY, 1);
> +s->regs[IBEX_SPI_HOST_STATUS] = data;
>  }
>
>  static void ibex_spi_txfifo_reset(IbexSPIHostState *s)
>  {
> +uint32_t data = s->regs[IBEX_SPI_HOST_STATUS];
>  /* Empty the TX FIFO and assert TXEMPTY */
>  fifo8_reset(&s->tx_fifo);
> -s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_TXFULL_MASK;
> -s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_TXEMPTY_MASK;
> +data = FIELD_DP32(data, STATUS, TXFULL, 0);
> +data = FIELD_DP32(data, STATUS, TXEMPTY, 1);
> +s->regs[IBEX_SPI_HOST_STATUS] = data;
>  }
>
>  static void ibex_spi_host_reset(DeviceState *dev)
> @@ -162,37 +166,38 @@ static void ibex_spi_host_reset(DeviceState *dev)
>   */
>  static void ibex_spi_host_irq(IbexSPIHostState *s)
>  {
> -bool error_en = s->regs[IBEX_SPI_HOST_INTR_ENABLE]
> -& R_INTR_ENABLE_ERROR_MASK;
> -bool event_en = s->regs[IBEX_SPI_HOST_INTR_ENABLE]
> -& R_INTR_ENABLE_SPI_EVENT_MASK;
> -bool err_pending = s->regs[IBEX_SPI_HOST_INTR_STATE]
> -& R_INTR_STATE_ERROR_MASK;
> -bool status_pending = s->regs[IBEX_SPI_HOST_INTR_STATE]
> -& R_INTR_STATE_SPI_EVENT_MASK;
> +uint32_t intr_test_reg = s->regs[IBEX_SPI_HOST_INTR_TEST];
> +uint32_t intr_en_reg = s->regs[IBEX_SPI_HOST_INTR_ENABLE];
> +uint32_t intr_state_reg = s->regs[IBEX_SPI_HOST_INTR_STATE];
> +
> +uint32_t err_en_reg = s->regs[IBEX_SPI_HOST_ERROR_ENABLE];
> +uint32_t event_en_reg = s->regs[IBEX_SPI_HOST_EVENT_ENABLE];
> +uint32_t err_status_reg = s->regs[IBEX_SPI_HOST_ERROR_STATUS];
> +uint32_t status_reg = s->regs[IBEX_SPI_HOST_STATUS];
> +
> +
> +bool error_en = FIELD_EX32(intr_en_reg, INTR_ENABLE, ERROR);
> +bool event_en = FIELD_EX32(intr_en_reg, INTR_ENABLE, SPI_EVENT);
> +bool err_pending = FIELD_EX32(intr_state_reg, INTR_STATE, ERROR);
> +bool status_pending = FIELD_EX32(intr_state_reg, INTR_STATE, SPI_EVENT);
> +
>  int err_irq = 0, event_irq = 0;
>
>  /* Error IRQ enabled and Error IRQ Cleared */
>  if (error_en && !err_pending) {
>  /* Event enabled, Interrupt Test Error */
> -if (s->regs[IBEX_SPI_HOST_INTR_TEST] & R_INTR_TEST_ERROR_MASK) {
> +if (FIELD_EX32(intr_test_reg, INTR_TEST,  ERROR)) {
>  err_irq = 1;
> -} else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE]
> -&  R_ERROR_ENABLE_CMDBUSY_MASK) &&
> -s->regs[IBEX_SPI_HOST_ERROR_STATUS]
> -& R_ERROR_STATUS_CMDBUSY_MASK) {
> +} else if (FIELD_EX32(err_en_reg, ERROR_ENABLE,  CMDBUSY) &&
> +   FIELD_EX32(err_status_reg, ERROR_STATUS,  CMDBUSY)) {
>  /* Wrote to COMMAND when not READY */
>  err_irq = 1;
> -} else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE]
> -&  R_ERROR_ENABLE_CMDINVAL_MASK) &&
> -s->regs[IBEX_SPI_HOST_ERROR_STATUS]
> -& R_ERROR_STATUS_CMDINVAL_MASK) {
> +} else if (FIELD_EX32(err_en_reg, ERROR_ENABLE,  CMDINVAL)  &&
> +   FIELD_EX32(err_status_reg, ERROR_STATUS,  CMDINVAL)) {
>  /* Invalid command segment */
>  err_irq = 1;
> -} else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE]
> -& R_ERROR_ENABLE_CSIDINVAL_MASK) &&
> -s->regs[IBEX_SPI_HOST_ERROR_STATUS]
> -& R_ERROR_STATUS_CSIDINVAL_MASK) {
> +} else if (FIELD_EX32(err_en_reg, ERROR_ENABLE,  CSIDINVAL) &&
> +   FIELD_EX32(err_status_reg, ERROR_STATUS,  CSIDINVAL)) {
>  /* Invalid value for CSID */
>  err_irq = 1;
>  }
> @@ -204,22 +209,19 @@ static void ibex_sp

[PATCH v2 1/2] [temporary] reserve bit KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID

[Gerd Hoffmann]

The KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID bit hints to the guest
that the size of the physical address space as advertised by CPUID
leaf 0x8008 is actually valid and can be used.

Unfortunately this is not the case today with qemu.  Default behavior is
to advertise 40 address bits (which I think comes from the very first x64
opteron processors).  There are lots of intel desktop processors around
which support less than that (36 or 39 depending on age), and when trying
to use the full 40 bit address space on those things go south quickly.

This renders the physical address size information effectively useless
for guests.  This patch paves the way to fix that by adding a hint for
the guest so it knows whenever the physical address size is usable or
not.

The plan for qemu is to set the bit when the physical address size is
valid.  That is the case when qemu is started with the host-phys-bits=on
option set for the cpu.  Eventually qemu can also flip the default for
that option from off to on, unfortunately that isn't easy for backward
compatibility reasons.

The plan for the firmware is to check that bit and when it is set just
query and use the available physical address space.  When the bit is not
set be conservative and try not exceed 36 bits (aka 64G) address space.
The latter is what the firmware does today unconditionally.

[ Temporary qemu patch for RfC patch and testing.  This change must
  actually be done in the linux kernel, then picked up by qemu via
  header file sync ].

Signed-off-by: Gerd Hoffmann 
---
 include/standard-headers/asm-x86/kvm_para.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/standard-headers/asm-x86/kvm_para.h 
b/include/standard-headers/asm-x86/kvm_para.h
index f0235e58a1d3..962dabcfdb68 100644
--- a/include/standard-headers/asm-x86/kvm_para.h
+++ b/include/standard-headers/asm-x86/kvm_para.h
@@ -37,7 +37,8 @@
 #define KVM_FEATURE_HC_MAP_GPA_RANGE   16
 #define KVM_FEATURE_MIGRATION_CONTROL  17
 
-#define KVM_HINTS_REALTIME  0
+#define KVM_HINTS_REALTIME  0
+#define KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID  1
 
 /* The last 8 bits are used to indicate how to interpret the flags field
  * in pvclock structure. If no bits are set, all flags are ignored.
-- 
2.37.3

[PATCH v2 2/2] [RfC] expose host-phys-bits to guest

[Gerd Hoffmann]

Move "host-phys-bits" property from cpu->host_phys_bits to
cpu->env.features[FEAT_KVM_HINTS] (KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID bit).

This has the effect that the guest can see whenever host-phys-bits
is turned on or not and act accordingly.

Signed-off-by: Gerd Hoffmann 
---
 target/i386/cpu.h  | 3 ---
 hw/i386/microvm.c  | 7 ++-
 target/i386/cpu.c  | 3 +--
 target/i386/host-cpu.c | 5 -
 target/i386/kvm/kvm.c  | 1 +
 5 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 82004b65b944..b9c6d3d9cac6 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1898,9 +1898,6 @@ struct ArchCPU {
 /* if true fill the top bits of the MTRR_PHYSMASKn variable range */
 bool fill_mtrr_mask;
 
-/* if true override the phys_bits value with a value read from the host */
-bool host_phys_bits;
-
 /* if set, limit maximum value for phys_bits when host_phys_bits is true */
 uint8_t host_phys_bits_limit;
 
diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index 52cafa003d8a..316bbc8ef946 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -54,6 +54,8 @@
 #include "kvm/kvm_i386.h"
 #include "hw/xen/start_info.h"
 
+#include "standard-headers/asm-x86/kvm_para.h"
+
 #define MICROVM_QBOOT_FILENAME "qboot.rom"
 #define MICROVM_BIOS_FILENAME  "bios-microvm.bin"
 
@@ -424,7 +426,10 @@ static void microvm_device_pre_plug_cb(HotplugHandler 
*hotplug_dev,
 {
 X86CPU *cpu = X86_CPU(dev);
 
-cpu->host_phys_bits = true; /* need reliable phys-bits */
+/* need reliable phys-bits */
+cpu->env.features[FEAT_KVM_HINTS] |=
+(1 << KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID);
+
 x86_cpu_pre_plug(hotplug_dev, dev, errp);
 }
 
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 1db1278a599b..d60f4498a3c3 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -778,7 +778,7 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
 [FEAT_KVM_HINTS] = {
 .type = CPUID_FEATURE_WORD,
 .feat_names = {
-"kvm-hint-dedicated", NULL, NULL, NULL,
+"kvm-hint-dedicated", "host-phys-bits", NULL, NULL,
 NULL, NULL, NULL, NULL,
 NULL, NULL, NULL, NULL,
 NULL, NULL, NULL, NULL,
@@ -7016,7 +7016,6 @@ static Property x86_cpu_properties[] = {
 DEFINE_PROP_BOOL("x-force-features", X86CPU, force_features, false),
 DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true),
 DEFINE_PROP_UINT32("phys-bits", X86CPU, phys_bits, 0),
-DEFINE_PROP_BOOL("host-phys-bits", X86CPU, host_phys_bits, false),
 DEFINE_PROP_UINT8("host-phys-bits-limit", X86CPU, host_phys_bits_limit, 0),
 DEFINE_PROP_BOOL("fill-mtrr-mask", X86CPU, fill_mtrr_mask, true),
 DEFINE_PROP_UINT32("level-func7", X86CPU, env.cpuid_level_func7,
diff --git a/target/i386/host-cpu.c b/target/i386/host-cpu.c
index 10f8aba86e53..a1d6b3ac962e 100644
--- a/target/i386/host-cpu.c
+++ b/target/i386/host-cpu.c
@@ -13,6 +13,8 @@
 #include "qapi/error.h"
 #include "sysemu/sysemu.h"
 
+#include "standard-headers/asm-x86/kvm_para.h"
+
 /* Note: Only safe for use on x86(-64) hosts */
 static uint32_t host_cpu_phys_bits(void)
 {
@@ -68,7 +70,8 @@ static uint32_t host_cpu_adjust_phys_bits(X86CPU *cpu)
 warned = true;
 }
 
-if (cpu->host_phys_bits) {
+if (cpu->env.features[FEAT_KVM_HINTS] &
+(1 << KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID)) {
 /* The user asked for us to use the host physical bits */
 phys_bits = host_phys_bits;
 if (cpu->host_phys_bits_limit &&
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index a1fd1f53791d..3335c57b21b2 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -459,6 +459,7 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t 
function,
 }
 } else if (function == KVM_CPUID_FEATURES && reg == R_EDX) {
 ret |= 1U << KVM_HINTS_REALTIME;
+ret |= 1U << KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID;
 }
 
 return ret;
-- 
2.37.3

[PATCH v2 0/2] expose host-phys-bits to guest

[Gerd Hoffmann]

When the guest (firmware specifically) knows how big
the address space actually is it can be used better.

Some more background:
  https://bugzilla.redhat.com/show_bug.cgi?id=2084533

This is a RfC series exposes the information via cpuid.

v2:
 - change fvm hint name.
 - better commit message.

take care,
  Gerd

Gerd Hoffmann (2):
  [temporary] reserve bit KVM_HINTS_PHYS_ADDRESS_SIZE_DATA_VALID
  [RfC] expose host-phys-bits to guest

 include/standard-headers/asm-x86/kvm_para.h | 3 ++-
 target/i386/cpu.h   | 3 ---
 hw/i386/microvm.c   | 7 ++-
 target/i386/cpu.c   | 3 +--
 target/i386/host-cpu.c  | 5 -
 target/i386/kvm/kvm.c   | 1 +
 6 files changed, 14 insertions(+), 8 deletions(-)

-- 
2.37.3

Re: [PATCH 2/2] configure: Add -Wno-gnu-variable-sized-type-not-at-end

[Richard Henderson]

On 9/8/22 10:09, Daniel P. Berrangé wrote:

'kvm_msrs info' is variable in size, so offset of 'entries[1]' is
undefined by C99. I presume the GNU defined semantics are that the
variable length 'entries[]' field in 'info' is zero-sized, in order
to give predictable offset for 'entries[1]' in the local msr_data.


Correct.  I invented this gcc extension for the benefit of glibc, which wanted to append N 
entries to that header, in static storage no less.


I still find it odd that clang warns about a gnu extension when gnu extensions are 
requested via -std=gnu*.



r~

Re: [PULL 00/10] QAPI patches patches for 2022-09-07

[Markus Armbruster]

Markus Armbruster  writes:

> Kevin Wolf  writes:
>
>> Am 07.09.2022 um 17:03 hat Markus Armbruster geschrieben:
>>> The following changes since commit 946e9bccf12f2bcc3ca471b820738fb22d14fc80:
>>> 
>>>   Merge tag 'samuel-thibault' of https://people.debian.org/~sthibault/qemu 
>>> into staging (2022-09-06 08:31:24 -0400)
>>> 
>>> are available in the Git repository at:
>>> 
>>>   git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2022-09-07
>>> 
>>> for you to fetch changes up to 6e7a37ffc230d06852f1a8893097331d39df77c9:
>>> 
>>>   qapi: fix examples of events missing timestamp (2022-09-07 15:10:13 +0200)
>>> 
>>> 
>>> QAPI patches patches for 2022-09-07
>>> 
>>> 
>>> Victor Toso (10):
>>>   qapi: fix example of query-ballon command
>>>   qapi: fix example of query-vnc command
>>>   qapi: fix example of query-dump-guest-memory-capability command
>>>   qapi: fix example of BLOCK_JOB_READY event
>>>   qapi: fix example of NIC_RX_FILTER_CHANGED event
>>>   qapi: fix example of DEVICE_UNPLUG_GUEST_ERROR event
>>>   qapi: fix example of MEM_UNPLUG_ERROR event
>>>   qapi: fix examples of blockdev-add with qcow2
>>
>> NACK, this patch is wrong.
>>
>> 'file' is a required member (defined in BlockdevOptionsGenericFormat),
>> removing it makes the example invalid. 'data-file' is only an additional
>> optional member to be used for external data files (i.e. when the guest
>> data is kept separate from the metadata in the .qcow2 file).
>
> I'll respin with #8 dropped.  Thank you!

Too late, it's already merged.

Victor, could you fix on top?  Or would you like me to revert the patch?

Re: [PULL 00/10] QAPI patches patches for 2022-09-07

[Markus Armbruster]

Kevin Wolf  writes:

> Am 07.09.2022 um 17:03 hat Markus Armbruster geschrieben:
>> The following changes since commit 946e9bccf12f2bcc3ca471b820738fb22d14fc80:
>> 
>>   Merge tag 'samuel-thibault' of https://people.debian.org/~sthibault/qemu 
>> into staging (2022-09-06 08:31:24 -0400)
>> 
>> are available in the Git repository at:
>> 
>>   git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2022-09-07
>> 
>> for you to fetch changes up to 6e7a37ffc230d06852f1a8893097331d39df77c9:
>> 
>>   qapi: fix examples of events missing timestamp (2022-09-07 15:10:13 +0200)
>> 
>> 
>> QAPI patches patches for 2022-09-07
>> 
>> 
>> Victor Toso (10):
>>   qapi: fix example of query-ballon command
>>   qapi: fix example of query-vnc command
>>   qapi: fix example of query-dump-guest-memory-capability command
>>   qapi: fix example of BLOCK_JOB_READY event
>>   qapi: fix example of NIC_RX_FILTER_CHANGED event
>>   qapi: fix example of DEVICE_UNPLUG_GUEST_ERROR event
>>   qapi: fix example of MEM_UNPLUG_ERROR event
>>   qapi: fix examples of blockdev-add with qcow2
>
> NACK, this patch is wrong.
>
> 'file' is a required member (defined in BlockdevOptionsGenericFormat),
> removing it makes the example invalid. 'data-file' is only an additional
> optional member to be used for external data files (i.e. when the guest
> data is kept separate from the metadata in the .qcow2 file).

I'll respin with #8 dropped.  Thank you!

[...]

Re: [PATCH] Use QMP command object-add instead of object_add for memory hotplugin

[Markus Armbruster]

liuhaiwei  writes:

> From: liuhaiwei 
>
> Signed-off-by: liuhaiwei 
> ---
>  docs/memory-hotplug.txt | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/docs/memory-hotplug.txt b/docs/memory-hotplug.txt
> index 6aa5e17e26..85ed4d8f3d 100644
> --- a/docs/memory-hotplug.txt
> +++ b/docs/memory-hotplug.txt
> @@ -34,15 +34,15 @@ hotplugged by using any combination of the available 
> memory slots.
>  
>  Two monitor commands are used to hotplug memory:
>  
> - - "object_add": creates a memory backend object
> + - "object-add": creates a memory backend object
>   - "device_add": creates a front-end pc-dimm device and inserts it
>   into the first empty slot
>  
>  For example, the following commands add another 1GB to the guest
>  discussed earlier:
>  
> -  (qemu) object_add memory-backend-ram,id=mem1,size=1G
> -  (qemu) device_add pc-dimm,id=dimm1,memdev=mem1
> +  (qemu) object-add qom-type=memory-backend-ram id=mem1 size=1073741824
> +  (qemu) device_add driver=pc-dimm id=dimm1 memdev=mem1

This is HMP, where the command is spelled object_add.  Your patch is
wrong.

>  
>  Using the file backend
>  --
> @@ -55,7 +55,7 @@ For example, assuming that the host has 1GB hugepages 
> available in
>  the /mnt/hugepages-1GB directory, a 1GB hugepage could be hotplugged
>  into the guest from the previous section with the following commands:
>  
> -  (qemu) object_add 
> memory-backend-file,id=mem1,size=1G,mem-path=/mnt/hugepages-1GB

Likewise.

> +  (qemu) object-add qom-type=memory-backend-file id=mem1  size=1073741824 
> mem-path=/mnt/hugepages-1GB 
>(qemu) device_add pc-dimm,id=dimm1,memdev=mem1
>  
>  It's also possible to start a guest with memory cold-plugged into the

Re: [PATCH 1/4] hw/watchdog: wdt_ibex_aon.c: Implement the watchdog for the OpenTitan

[Alistair Francis]

On Fri, Sep 2, 2022 at 3:29 AM Tyler Ng  wrote:
>
> This commit adds most of an implementation of the OpenTitan Always-On
> Timer. The documentation for this timer is found here:
>
> https://docs.opentitan.org/hw/ip/aon_timer/doc/
>
> The implementation includes most of the watchdog features; it does not
> implement the wakeup timer.
>
> An important note: the OpenTitan board uses the sifive_plic. The plic
> will not be able to claim the bark interrupt (158) because the sifive
> plic sets priority[158], but checks priority[157] for the priority, so
> it thinks that the interrupt's priority is 0 (effectively disabled).
>
> Changed Files:
> hw/riscv/Kconfig: Add configuration for the watchdog.
> hw/riscv/opentitan.c: Connect AON Timer to the OpenTitan board.
>
> hw/watchdog/Kconfig: Configuration for the watchdog.
> hw/watchdog/meson.build: Compile the watchdog.
> hw/watchdog/wdt_ibex_aon.c: The watchdog itself.
>
> include/hw/riscv/opentitan.h: Add watchdog bark/wakeup IRQs and timer.
> include/hw/watchdog/wdt_ibex_aon.h: Add watchdog.
>
> tests/qtest/ibex-aon-timer-test.c: Ibex Timer test.
> tests/qtest/meson.build: Build the timer test.
>
> Signed-off-by: Tyler Ng 
> ---
>  hw/riscv/Kconfig   |   4 +
>  hw/riscv/opentitan.c   |  22 +-
>  hw/watchdog/Kconfig|   3 +
>  hw/watchdog/meson.build|   1 +
>  hw/watchdog/wdt_ibex_aon.c | 432 +
>  include/hw/riscv/opentitan.h   |   9 +-
>  include/hw/watchdog/wdt_ibex_aon.h |  67 +
>  tests/qtest/ibex-aon-timer-test.c  | 189 +
>  tests/qtest/meson.build|   3 +
>  9 files changed, 725 insertions(+), 5 deletions(-)
>  create mode 100644 hw/watchdog/wdt_ibex_aon.c
>  create mode 100644 include/hw/watchdog/wdt_ibex_aon.h
>  create mode 100644 tests/qtest/ibex-aon-timer-test.c
>
> diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
> index 79ff61c464..72094010be 100644
> --- a/hw/riscv/Kconfig
> +++ b/hw/riscv/Kconfig
> @@ -4,6 +4,9 @@ config RISCV_NUMA
>  config IBEX
>  bool
>
> +config IBEX_AON
> +bool
> +
>  config MICROCHIP_PFSOC
>  bool
>  select CADENCE_SDHCI
> @@ -20,6 +23,7 @@ config MICROCHIP_PFSOC
>  config OPENTITAN
>  bool
>  select IBEX
> +select IBEX_AON
>  select UNIMP
>
>  config SHAKTI_C
> diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
> index 4495a2c039..10834b831f 100644
> --- a/hw/riscv/opentitan.c
> +++ b/hw/riscv/opentitan.c
> @@ -1,4 +1,5 @@
>  /*
> +ptimer_
>   * QEMU RISC-V Board Compatible with OpenTitan FPGA platform
>   *
>   * Copyright (c) 2020 Western Digital
> @@ -47,7 +48,7 @@ static const MemMapEntry ibex_memmap[] = {
>  [IBEX_DEV_RSTMGR] = {  0x4041,  0x1000  },
>  [IBEX_DEV_CLKMGR] = {  0x4042,  0x1000  },
>  [IBEX_DEV_PINMUX] = {  0x4046,  0x1000  },
> -[IBEX_DEV_PADCTRL] ={  0x4047,  0x1000  },

What about the pad controller?

> +[IBEX_DEV_AON_TIMER] =  {  0x4047,  0x1000  },
>  [IBEX_DEV_FLASH_CTRL] = {  0x4100,  0x1000  },
>  [IBEX_DEV_AES] ={  0x4110,  0x1000  },
>  [IBEX_DEV_HMAC] =   {  0x4111,  0x1000  },
> @@ -121,6 +122,8 @@ static void lowrisc_ibex_soc_init(Object *obj)
>
>  object_initialize_child(obj, "timer", &s->timer, TYPE_IBEX_TIMER);
>
> +object_initialize_child(obj, "aon_timer", &s->aon_timer,
> TYPE_IBEX_AON_TIMER);
> +
>  for (int i = 0; i < OPENTITAN_NUM_SPI_HOSTS; i++) {
>  object_initialize_child(obj, "spi_host[*]", &s->spi_host[i],
>  TYPE_IBEX_SPI_HOST);
> @@ -205,6 +208,7 @@ static void lowrisc_ibex_soc_realize(DeviceState
> *dev_soc, Error **errp)
> 3, qdev_get_gpio_in(DEVICE(&s->plic),
> IBEX_UART0_RX_OVERFLOW_IRQ));
>
> +/* RV Timer */
>  if (!sysbus_realize(SYS_BUS_DEVICE(&s->timer), errp)) {
>  return;
>  }
> @@ -215,6 +219,20 @@ static void lowrisc_ibex_soc_realize(DeviceState
> *dev_soc, Error **errp)
>  qdev_connect_gpio_out(DEVICE(&s->timer), 0,
>qdev_get_gpio_in(DEVICE(qemu_get_cpu(0)),
> IRQ_M_TIMER));
> +
> +/* AON Timer */
> +if (!sysbus_realize(SYS_BUS_DEVICE(&s->aon_timer), errp)) {
> +return;
> +}
> +sysbus_mmio_map(SYS_BUS_DEVICE(&s->aon_timer), 0,
> memmap[IBEX_DEV_AON_TIMER].base);
> +sysbus_connect_irq(SYS_BUS_DEVICE(&s->aon_timer),
> +   0, qdev_get_gpio_in(DEVICE(&s->plic),
> +   IBEX_AONTIMER_WDOG_BARK));
> +/*
> + * Note: There should be a line to pwrmgr but it's not implemented.
> + * TODO: Needs a line connected in, "counter-run" (stop the watchdog if
> + * debugging)
> + */
>
>  /* SPI-Hosts */
>  for (int i = 0; i < OPENTITAN_NUM_SPI_HOSTS; ++i) {
> @@ -261,8 +279,6 @@ static void lowrisc_ibex_soc_realize(Dev

Re: [PATCH 3/4] hw/timer: ibex_timer.c: Update register addresses

[Alistair Francis]

On Fri, Sep 2, 2022 at 3:24 AM Tyler Ng  wrote:
>
> Updates the register addresses to match the OpenTitan spec.
>
> These changes were made in this commit:
> https://github.com/lowRISC/opentitan/commit/a25e162b8f91bd0ca32258c83d1d480f93327204

Thanks for the patch

We try to keep all OpenTitan devices in sync with each other. QEMU
currently supports OT commit 217a0168ba118503c166a9587819e3811eeb0c0c

We don't want to update a single device without updating all of them.
If you want you are welcome to update all devices to a newer commit

Also, the commits QEMU supports are generally driven by Tock, as
that's the software running on QEMU OT. Have a look here for the board
https://github.com/tock/tock/tree/master/boards/opentitan or here for
the latest update (which QEMU already supports)
https://github.com/tock/tock/pull/3056

Alistair

>
> Signed-off-by: Tyler Ng 
> ---
>  hw/timer/ibex_timer.c | 20 ++--
>  1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/hw/timer/ibex_timer.c b/hw/timer/ibex_timer.c
> index 8c2ca364da..9ffd4821e8 100644
> --- a/hw/timer/ibex_timer.c
> +++ b/hw/timer/ibex_timer.c
> @@ -38,19 +38,19 @@ REG32(ALERT_TEST, 0x00)
>  FIELD(ALERT_TEST, FATAL_FAULT, 0, 1)
>  REG32(CTRL, 0x04)
>  FIELD(CTRL, ACTIVE, 0, 1)
> -REG32(CFG0, 0x100)
> -FIELD(CFG0, PRESCALE, 0, 12)
> -FIELD(CFG0, STEP, 16, 8)
> -REG32(LOWER0, 0x104)
> -REG32(UPPER0, 0x108)
> -REG32(COMPARE_LOWER0, 0x10C)
> -REG32(COMPARE_UPPER0, 0x110)
> -REG32(INTR_ENABLE, 0x114)
> +REG32(INTR_ENABLE, 0x100)
>  FIELD(INTR_ENABLE, IE_0, 0, 1)
> -REG32(INTR_STATE, 0x118)
> +REG32(INTR_STATE, 0x104)
>  FIELD(INTR_STATE, IS_0, 0, 1)
> -REG32(INTR_TEST, 0x11C)
> +REG32(INTR_TEST, 0x108)
>  FIELD(INTR_TEST, T_0, 0, 1)
> +REG32(CFG0, 0x10C)
> +FIELD(CFG0, PRESCALE, 0, 12)
> +FIELD(CFG0, STEP, 16, 8)
> +REG32(LOWER0, 0x110)
> +REG32(UPPER0, 0x114)
> +REG32(COMPARE_LOWER0, 0x118)
> +REG32(COMPARE_UPPER0, 0x11C)
>
>  static uint64_t cpu_riscv_read_rtc(uint32_t timebase_freq)
>  {
> --
> 2.30.2
>

Re: [PATCH v4 2/9] target/arm: Change gen_goto_tb to work on displacements

[Richard Henderson]

On 9/6/22 13:52, Philippe Mathieu-Daudé wrote:

@@ -1399,9 +1401,9 @@ static void disas_comp_b_imm(DisasContext *s, uint32_t 
insn)
  tcg_gen_brcondi_i64(op ? TCG_COND_NE : TCG_COND_EQ,
  tcg_cmp, 0, label_match);
-    gen_goto_tb(s, 0, s->base.pc_next);
+    gen_goto_tb(s, 0, 4);


Why not use curr_insn_len() here?


I guess I could, but it's always 4 for aarch64.


r~

Re: [PATCH v2 02/23] target/i386: Return bool from disas_insn

[Richard Henderson]

On 9/6/22 15:42, Philippe Mathieu-Daudé wrote:

On 6/9/22 12:09, Richard Henderson wrote:

Instead of returning the new pc, which is present in
DisasContext, return true if an insn was translated.
This is false when we detect a page crossing and must
undo the insn under translation.

Signed-off-by: Richard Henderson 
---
  target/i386/tcg/translate.c | 42 +++--
  1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 1e24bb2985..46300ffd91 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -4665,7 +4665,7 @@ static void gen_sse(CPUX86State *env, DisasContext *s, 
int b)
  /* convert one instruction. s->base.is_jmp is set if the translation must
 be stopped. Return the next pc value */
-static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
+static bool disas_insn(DisasContext *s, CPUState *cpu)
  {
  CPUX86State *env = cpu->env_ptr;
  int b, prefixes;
@@ -4695,12 +4695,13 @@ static target_ulong disas_insn(DisasContext *s, 
CPUState *cpu)
  return s->pc;


Shouldn't we return 'true' here?


Whoops, yes.


r~

Re: [PATCH v3] 9pfs: use GHashTable for fid table

[Greg Kurz]

On Thu,  8 Sep 2022 13:23:53 +0200
Linus Heckemann  wrote:

> The previous implementation would iterate over the fid table for
> lookup operations, resulting in an operation with O(n) complexity on
> the number of open files and poor cache locality -- for every open,
> stat, read, write, etc operation.
> 
> This change uses a hashtable for this instead, significantly improving
> the performance of the 9p filesystem. The runtime of NixOS's simple
> installer test, which copies ~122k files totalling ~1.8GiB from 9p,
> decreased by a factor of about 10.
> 
> Signed-off-by: Linus Heckemann 
> Reviewed-by: Philippe Mathieu-Daudé 
> Reviewed-by: Greg Kurz 
> ---
> 
> Greg Kurz writes:
> > The comment above should be adapted to the new situation : no need
> 
> I've removed it completely, since the logic is simple enough that only
> the shortened comment below remains necessary.
> 
> > With the new logic, this should just be:
> 
> now is :)
> 
> > g_hash_table_steal_extended() [1] actually allows to do just that.
> 
> g_hash_table_steal_extended unfortunately isn't available since it was
> introduced in glib 2.58 and we're maintaining compatibility to 2.56.
> 

Ha... this could be addressed through conditional compilation, e.g.:

static V9fsFidState *clunk_fid(V9fsState *s, int32_t fid)
{
V9fsFidState *fidp;

#if GLIB_CHECK_VERSION(2,56,0)
if (!g_hash_table_steal_extended(s->fids, GINT_TO_POINTER(fid),
 NULL, (gpointer *) &fidp)) {
return NULL;
}
#else
fidp = g_hash_table_lookup(s->fids, GINT_TO_POINTER(fid));
if (fidp) {
g_hash_table_remove(s->fids, GINT_TO_POINTER(fid));
} else {
return NULL;
}
#endif

fidp->clunked = true;
return fidp;
}

or simply leave a TODO comment so that we don't forget.


> > You could just call g_hash_table_iter_remove(&iter) here
> 
> Applied this suggestion, thanks!
> 
> 
> > Well... finding at least one clunked fid state in this table is
> > definitely a bug so I'll keep the BUG_ON() anyway.
> 
> Christian Schoenebeck writes:
> > Yeah, I think you are right, it would feel odd. Just drop BUG_ON() for
> > now.
> 
> I still prefer dropping it, but if we were to keep it I think it should
> be in v9fs_reclaim_fd where we iterate and can thus check the whole
> table.
> 

IMO the relevant aspect isn't really about checking the whole table, but
rather not to get a clunked fid out of this table and pass it over.

> 
> Greg Kurz and Philippe Mathieu-Daudé write:
> > [patch versioning]
> 
> Whoops. I used -v2 on git send-email, which just ignored the option,
> rather than git format-patch, by accident. This one _should_ now be v3!
> 
> 

v3 it is and LGTM ! No big deal with the BUG_ON(), given the improvement.

My R-b stands. Thanks Linus !

>  hw/9pfs/9p.c | 140 +--
>  hw/9pfs/9p.h |   2 +-
>  2 files changed, 70 insertions(+), 72 deletions(-)
> 
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index aebadeaa03..98a475e560 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -282,33 +282,31 @@ static V9fsFidState *coroutine_fn get_fid(V9fsPDU *pdu, 
> int32_t fid)
>  V9fsFidState *f;
>  V9fsState *s = pdu->s;
>  
> -QSIMPLEQ_FOREACH(f, &s->fid_list, next) {
> -BUG_ON(f->clunked);
> -if (f->fid == fid) {
> -/*
> - * Update the fid ref upfront so that
> - * we don't get reclaimed when we yield
> - * in open later.
> - */
> -f->ref++;
> -/*
> - * check whether we need to reopen the
> - * file. We might have closed the fd
> - * while trying to free up some file
> - * descriptors.
> - */
> -err = v9fs_reopen_fid(pdu, f);
> -if (err < 0) {
> -f->ref--;
> -return NULL;
> -}
> -/*
> - * Mark the fid as referenced so that the LRU
> - * reclaim won't close the file descriptor
> - */
> -f->flags |= FID_REFERENCED;
> -return f;
> +f = g_hash_table_lookup(s->fids, GINT_TO_POINTER(fid));
> +if (f) {
> +/*
> + * Update the fid ref upfront so that
> + * we don't get reclaimed when we yield
> + * in open later.
> + */
> +f->ref++;
> +/*
> + * check whether we need to reopen the
> + * file. We might have closed the fd
> + * while trying to free up some file
> + * descriptors.
> + */
> +err = v9fs_reopen_fid(pdu, f);
> +if (err < 0) {
> +f->ref--;
> +return NULL;
>  }
> +/*
> + * Mark the fid as referenced so that the LRU
> + * reclaim won't close the file descriptor
> + */
> +f->flags |= FID_REFERENCED;
> +return f;
>  }
>  return NULL;
>  }
> @@ -317,12 +315,9 @

Re: [PATCH 4/4] hw/timer: ibex_timer.c: Add support for writes to mtime

[Alistair Francis]

On Fri, Sep 2, 2022 at 3:28 AM Tyler Ng  wrote:
>
> 1. Adds fields to hold the value of mtime in timer_upper0 and timer_lower0.
>
> 2. Changes the read and write functions to use the mtime fields.
>
> 3. Updates the value of mtime in update_mtime() by extrapolating the
> time elapsed. This will need to change if/when the prescalar is
> implemented.
>
> Signed-off-by: Tyler Ng 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  hw/timer/ibex_timer.c | 94 +--
>  include/hw/timer/ibex_timer.h |  5 ++
>  2 files changed, 73 insertions(+), 26 deletions(-)
>
> diff --git a/hw/timer/ibex_timer.c b/hw/timer/ibex_timer.c
> index 9ffd4821e8..7d0ea2db1e 100644
> --- a/hw/timer/ibex_timer.c
> +++ b/hw/timer/ibex_timer.c
> @@ -52,30 +52,59 @@ REG32(UPPER0, 0x114)
>  REG32(COMPARE_LOWER0, 0x118)
>  REG32(COMPARE_UPPER0, 0x11C)
>
> -static uint64_t cpu_riscv_read_rtc(uint32_t timebase_freq)
> +
> +/*
> + * The goal of this function is to:
> + * 1. Check if the timer is enabled. If not, return false,
> + * 2. Calculate the amount of time that has passed since.
> + * 3. Extrapolate the number of ticks that have passed, and add it to 
> `mtime`.
> + * 4. Return true.
> + */
> +static bool update_mtime(IbexTimerState *s)
> +{
> +if (!(R_CTRL & R_CTRL_ACTIVE_MASK)) {
> +return false;
> +}
> +/* Get the time then extrapolate the number of ticks that have elapsed */
> +uint64_t mtime = (s->timer_lower0) | ((uint64_t) s->timer_upper0 << 32);
> +int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> +int64_t elapsed = now - s->timer_last_update;
> +if (elapsed < 0) {
> +/* We jumped back in time. */
> +mtime -= muldiv64((uint64_t)(-elapsed), s->timebase_freq,
> +   NANOSECONDS_PER_SECOND);
> +} else {
> +mtime += muldiv64(elapsed, s->timebase_freq, NANOSECONDS_PER_SECOND);
> +}
> +s->timer_lower0 = mtime & 0x;
> +s->timer_upper0 = (mtime >> 32) & 0x;
> +/* update last-checkpoint timestamp */
> +s->timer_last_update = now;
> +return true;
> +}
> +
> +static inline uint64_t cpu_riscv_read_rtc(void *opaque)
>  {
> -return muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
> -timebase_freq, NANOSECONDS_PER_SECOND);
> +IbexTimerState *s = opaque;
> +return (s->timer_lower0) | ((uint64_t) s->timer_upper0 << 32);
>  }
>
>  static void ibex_timer_update_irqs(IbexTimerState *s)
>  {
>  CPUState *cs = qemu_get_cpu(0);
>  RISCVCPU *cpu = RISCV_CPU(cs);
> -uint64_t value = s->timer_compare_lower0 |
> +uint64_t mtimecmp = s->timer_compare_lower0 |
>   ((uint64_t)s->timer_compare_upper0 << 32);
> -uint64_t next, diff;
> -uint64_t now = cpu_riscv_read_rtc(s->timebase_freq);
> -
> -if (!(s->timer_ctrl & R_CTRL_ACTIVE_MASK)) {
> -/* Timer isn't active */
> +if (!update_mtime(s)) {
> +/* Timer is not enabled? */
>  return;
>  }
> +uint64_t mtime = cpu_riscv_read_rtc(s);
>
>  /* Update the CPUs mtimecmp */
> -cpu->env.timecmp = value;
> +cpu->env.timecmp = mtimecmp;
>
> -if (cpu->env.timecmp <= now) {
> +if (cpu->env.timecmp <= mtime) {
>  /*
>   * If the mtimecmp was in the past raise the interrupt now.
>   */
> @@ -86,18 +115,18 @@ static void ibex_timer_update_irqs(IbexTimerState *s)
>  }
>  return;
>  }
> -
> +/* Update timers */
> +int64_t next;
> +uint64_t diff;
>  /* Setup a timer to trigger the interrupt in the future */
>  qemu_irq_lower(s->m_timer_irq);
>  qemu_set_irq(s->irq, false);
> -
> -diff = cpu->env.timecmp - now;
> -next = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
> - muldiv64(diff,
> -  NANOSECONDS_PER_SECOND,
> -  s->timebase_freq);
> -
> -if (next < qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)) {
> +/* Compute the difference, and set a timer for the next update. */
> +diff = mtimecmp - mtime;
> +const uint64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> +const uint64_t towait = muldiv64(diff, NANOSECONDS_PER_SECOND,
> s->timebase_freq);
> +next = now + towait;
> +if (next < now) {
>  /* We overflowed the timer, just set it as large as we can */
>  timer_mod(cpu->env.timer, 0x7FFF);
>  } else {
> @@ -128,11 +157,13 @@ static void ibex_timer_reset(DeviceState *dev)
>
>  s->timer_ctrl = 0x;
>  s->timer_cfg0 = 0x0001;
> +s->timer_lower0 = 0x;
> +s->timer_upper0 = 0x;
>  s->timer_compare_lower0 = 0x;
>  s->timer_compare_upper0 = 0x;
>  s->timer_intr_enable = 0x;
>  s->timer_intr_state = 0x;
> -
> +s->timer_last_update = 0x;
>  ibex_timer_update_irqs(s);
>  }
>
> @@ -140,7 +171,6 @@ static

Re: [PATCH 1/2] target/riscv: remove fflags, frm, and fcsr from riscv-*-fpu.xml

[Alistair Francis]

On Wed, Aug 31, 2022 at 10:43 AM Andrew Burgess  wrote:
>
> While testing some changes to GDB's handling for the RISC-V registers
> fcsr, fflags, and frm, I spotted that QEMU includes these registers
> twice in the target description it sends to GDB, once in the fpu
> feature, and once in the csr feature.
>
> Right now things basically work OK, QEMU maps these registers onto two
> different register numbers, e.g. fcsr maps to both 68 and 73, and GDB
> can use either of these to access the register.
>
> However, GDB's target descriptions don't really work this way, each
> register should appear just once in a target description, mapping the
> register name onto the number GDB should use when accessing the
> register on the target.  Duplicate register names actually result in
> duplicate registers on the GDB side, however, as the registers have
> the same name, the user can only access one of these registers.
>
> Currently GDB has a hack in place, specifically for RISC-V, to spot
> the duplicate copies of these three registers, and hide them from the
> user, ensuring the user only ever sees a single copy of each.
>
> In this commit I propose fixing this issue on the QEMU side, and in
> the process, simplify the fpu register handling a little.
>
> I think we should, remove fflags, frm, and fcsr from the two (32-bit
> and 64-bit) fpu feature xml files.  These files will only contain the
> 32 core floating point register f0 to f31.  The fflags, frm, and fcsr
> registers will continue to be advertised in the csr feature as they
> currently are.
>
> With that change made, I will simplify riscv_gdb_get_fpu and
> riscv_gdb_set_fpu, removing the extra handling for the 3 status
> registers.
>
> Signed-off-by: Andrew Burgess 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  gdb-xml/riscv-32bit-fpu.xml |  4 
>  gdb-xml/riscv-64bit-fpu.xml |  4 
>  target/riscv/gdbstub.c  | 32 ++--
>  3 files changed, 2 insertions(+), 38 deletions(-)
>
> diff --git a/gdb-xml/riscv-32bit-fpu.xml b/gdb-xml/riscv-32bit-fpu.xml
> index 1eaae9119e..84a44ba8df 100644
> --- a/gdb-xml/riscv-32bit-fpu.xml
> +++ b/gdb-xml/riscv-32bit-fpu.xml
> @@ -43,8 +43,4 @@
>
>
>
> -
> -  
> -  
> -  
>  
> diff --git a/gdb-xml/riscv-64bit-fpu.xml b/gdb-xml/riscv-64bit-fpu.xml
> index 794854cc01..9856a9d1d3 100644
> --- a/gdb-xml/riscv-64bit-fpu.xml
> +++ b/gdb-xml/riscv-64bit-fpu.xml
> @@ -49,8 +49,4 @@
>
>
>
> -
> -  
> -  
> -  
>  
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index 9ed049c29e..9974b7aac6 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -114,20 +114,6 @@ static int riscv_gdb_get_fpu(CPURISCVState *env, 
> GByteArray *buf, int n)
>  if (env->misa_ext & RVF) {
>  return gdb_get_reg32(buf, env->fpr[n]);
>  }
> -/* there is hole between ft11 and fflags in fpu.xml */
> -} else if (n < 36 && n > 32) {
> -target_ulong val = 0;
> -int result;
> -/*
> - * CSR_FFLAGS is at index 1 in csr_register, and gdb says it is FP
> - * register 33, so we recalculate the map index.
> - * This also works for CSR_FRM and CSR_FCSR.
> - */
> -result = riscv_csrrw_debug(env, n - 32, &val,
> -   0, 0);
> -if (result == RISCV_EXCP_NONE) {
> -return gdb_get_regl(buf, val);
> -}
>  }
>  return 0;
>  }
> @@ -137,20 +123,6 @@ static int riscv_gdb_set_fpu(CPURISCVState *env, uint8_t 
> *mem_buf, int n)
>  if (n < 32) {
>  env->fpr[n] = ldq_p(mem_buf); /* always 64-bit */
>  return sizeof(uint64_t);
> -/* there is hole between ft11 and fflags in fpu.xml */
> -} else if (n < 36 && n > 32) {
> -target_ulong val = ldtul_p(mem_buf);
> -int result;
> -/*
> - * CSR_FFLAGS is at index 1 in csr_register, and gdb says it is FP
> - * register 33, so we recalculate the map index.
> - * This also works for CSR_FRM and CSR_FCSR.
> - */
> -result = riscv_csrrw_debug(env, n - 32, NULL,
> -   val, -1);
> -if (result == RISCV_EXCP_NONE) {
> -return sizeof(target_ulong);
> -}
>  }
>  return 0;
>  }
> @@ -404,10 +376,10 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState 
> *cs)
>  CPURISCVState *env = &cpu->env;
>  if (env->misa_ext & RVD) {
>  gdb_register_coprocessor(cs, riscv_gdb_get_fpu, riscv_gdb_set_fpu,
> - 36, "riscv-64bit-fpu.xml", 0);
> + 32, "riscv-64bit-fpu.xml", 0);
>  } else if (env->misa_ext & RVF) {
>  gdb_register_coprocessor(cs, riscv_gdb_get_fpu, riscv_gdb_set_fpu,
> - 36, "riscv-32bit-fpu.xml", 0);
> + 32, "riscv-32bit-fpu.xml", 0);
>  }
>  if (env->misa_ext & RVV) {
> 

Re: [PATCH] target/riscv: Remove sideleg and sedeleg

[Alistair Francis]

On Wed, Aug 24, 2022 at 4:54 PM Rahul Pathak  wrote:
>
> sideleg and sedeleg csrs are not part of riscv isa spec
> anymore, these csrs were part of N extension which
> is removed from the riscv isa specification.
>
> These commits removed all traces of these csrs from
> riscv spec (https://github.com/riscv/riscv-isa-manual) -
>
> commit f8d27f805b65 ("Remove or downgrade more references to N extension 
> (#674)")
> commit b6cade07034d ("Remove N extension chapter for now")
>
> Signed-off-by: Rahul Pathak 
> Reviewed-by: Andrew Jones 

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  disas/riscv.c   | 2 --
>  target/riscv/cpu_bits.h | 2 --
>  2 files changed, 4 deletions(-)
>
> diff --git a/disas/riscv.c b/disas/riscv.c
> index 7af6afc8fa..a709d66167 100644
> --- a/disas/riscv.c
> +++ b/disas/riscv.c
> @@ -1304,8 +1304,6 @@ static const char *csr_name(int csrno)
>  case 0x0043: return "utval";
>  case 0x0044: return "uip";
>  case 0x0100: return "sstatus";
> -case 0x0102: return "sedeleg";
> -case 0x0103: return "sideleg";
>  case 0x0104: return "sie";
>  case 0x0105: return "stvec";
>  case 0x0106: return "scounteren";
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index 6be5a9e9f0..7251121218 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -190,8 +190,6 @@
>
>  /* Supervisor Trap Setup */
>  #define CSR_SSTATUS 0x100
> -#define CSR_SEDELEG 0x102
> -#define CSR_SIDELEG 0x103
>  #define CSR_SIE 0x104
>  #define CSR_STVEC   0x105
>  #define CSR_SCOUNTEREN  0x106
> --
> 2.34.1
>
>

Re: [PATCH v2 1/2] x86: only modify setup_data if the boot protocol indicates safety

[Ard Biesheuvel]

On Thu, 8 Sept 2022 at 13:30, Laszlo Ersek  wrote:
>
> On 09/06/22 13:33, Daniel P. Berrangé wrote:
> > On Tue, Sep 06, 2022 at 01:14:50PM +0200, Ard Biesheuvel wrote:
> >> (cc Laszlo)
> >>
> >> On Tue, 6 Sept 2022 at 12:45, Michael S. Tsirkin  wrote:
> >>>
> >>> On Tue, Sep 06, 2022 at 12:43:55PM +0200, Jason A. Donenfeld wrote:
>  On Tue, Sep 6, 2022 at 12:40 PM Michael S. Tsirkin  
>  wrote:
> >
> > On Tue, Sep 06, 2022 at 12:36:56PM +0200, Jason A. Donenfeld wrote:
> >> It's only safe to modify the setup_data pointer on newer kernels where
> >> the EFI stub loader will ignore it. So condition setting that offset on
> >> the newer boot protocol version. While we're at it, gate this on SEV 
> >> too.
> >> This depends on the kernel commit linked below going upstream.
> >>
> >> Cc: Gerd Hoffmann 
> >> Cc: Laurent Vivier 
> >> Cc: Michael S. Tsirkin 
> >> Cc: Paolo Bonzini 
> >> Cc: Peter Maydell 
> >> Cc: Philippe Mathieu-Daudé 
> >> Cc: Richard Henderson 
> >> Cc: Ard Biesheuvel 
> >> Link: 
> >> https://lore.kernel.org/linux-efi/20220904165321.1140894-1-ja...@zx2c4.com/
> >> Signed-off-by: Jason A. Donenfeld 
> >
> > BTW what does it have to do with SEV?
> > Is this because SEV is not going to trust the data to be random anyway?
> 
>  Daniel (now CC'd) pointed out in one of the previous threads that this
>  breaks SEV, because the image hash changes.
> 
>  Jason
> >>>
> >>> Oh I see. I'd add a comment maybe and definitely mention this
> >>> in the commit log.
> >>>
> >>
> >> This does raise the question (as I mentioned before) how things like
> >> secure boot and measured boot are affected when combined with direct
> >> kernel boot: AIUI, libvirt uses direct kernel boot at guest
> >> installation time, and modifying setup_data will corrupt the image
> >> signature.
> >
> > IIUC, qemu already modifies setup_data when using direct kernel boot.
> >
> > It put in logic to skip this if SEV is enabled, to avoid interfering
> > with SEV hashes over the kernel, but there's nothing doing this more
> > generally for non-SEV cases using UEFI. So potentially use of SecureBoot
> > may already be impacted when using direct kernel boot.
>
> Yes,
>
> https://github.com/tianocore/edk2/commit/82808b422617
>

Ah yes, thanks for jogging my memory.

So virt-install --network already ignores secure boot failures on
direct kernel boot, so this is not going to make it any worse.

Re: [PATCH v3] target/riscv: fix csr check for cycle{h}, instret{h}, time{h}, hpmcounter3-31{h}

[Alistair Francis]

On Wed, Aug 17, 2022 at 10:39 AM Weiwei Li  wrote:
>
> - modify check for mcounteren to work in all less-privilege mode
> - modify check for scounteren to work only when S mode is enabled
> - distinguish the exception type raised by check for scounteren between U
> and VU mode
>
> Signed-off-by: Weiwei Li 
> Signed-off-by: Junqiang Wang 

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
> v3:
>  - remove unnecessary ()'s
>
> v2:
>  - Rebase on patches v13 for "Improve PMU support"
>
>  target/riscv/csr.c | 13 +
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 2dcd4e5b2d..ca72b5df98 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -98,17 +98,22 @@ static RISCVException ctr(CPURISCVState *env, int csrno)
>
>  skip_ext_pmu_check:
>
> -if (((env->priv == PRV_S) && (!get_field(env->mcounteren, ctr_mask))) ||
> -((env->priv == PRV_U) && (!get_field(env->scounteren, ctr_mask {
> +if (env->priv < PRV_M && !get_field(env->mcounteren, ctr_mask)) {
>  return RISCV_EXCP_ILLEGAL_INST;
>  }
>
>  if (riscv_cpu_virt_enabled(env)) {
> -if (!get_field(env->hcounteren, ctr_mask) &&
> -get_field(env->mcounteren, ctr_mask)) {
> +if (!get_field(env->hcounteren, ctr_mask) ||
> +(env->priv == PRV_U && !get_field(env->scounteren, ctr_mask))) {
>  return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
>  }
>  }
> +
> +if (riscv_has_ext(env, RVS) && env->priv == PRV_U &&
> +!get_field(env->scounteren, ctr_mask)) {
> +return RISCV_EXCP_ILLEGAL_INST;
> +}
> +
>  #endif
>  return RISCV_EXCP_NONE;
>  }
> --
> 2.17.1
>
>

Re: [PATCH] Use QMP command object-add instead of object_add for memory hotplugin

[Markus Armbruster]

liuhaiwei9699   writes:

> why the hmp using the object_add , qmp using the object-add command?
> can't we use the same command ?

Command names differ between HMP and QMP for historical reasons.

QMP is a stable interface, and changing names there is no go.

HMP is not a stable interface, but changing names would still
inconvenience users.  We don't do that without really compelling
reasons.

I think HMP could fold '_' and '-' together in command names, so that
both object_add and object-add work.  Best to check with the HMP
maintainer before you start coding.

Re: [PATCH 2/2] target/riscv: remove fixed numbering from GDB xml feature files

[Alistair Francis]

On Wed, Aug 31, 2022 at 10:45 AM Andrew Burgess  wrote:
>
> The fixed register numbering in the various GDB feature files for
> RISC-V only exists because these files were originally copied from the
> GDB source tree.
>
> However, the fixed numbering only exists in the GDB source tree so
> that GDB, when it connects to a target that doesn't provide a target
> description, will use a specific numbering scheme.
>
> That numbering scheme is designed to be compatible with the first
> versions of QEMU (for RISC-V), that didn't send a target description,
> and relied on a fixed numbering scheme.
>
> Because of the way that QEMU manages its target descriptions,
> recording the number of registers in each feature, and just relying on
> GDB's numbering starting from 0, then I propose that we remove all the
> fixed numbering from the RISC-V feature xml files, and just rely on
> the standard numbering scheme.  Plenty of other targets manage their
> xml files this way, e.g. ARM, AArch64, Loongarch, m68k, rx, and s390.
>
> Signed-off-by: Andrew Burgess 

Acked-by: Alistair Francis 

Alistair

> ---
>  gdb-xml/riscv-32bit-cpu.xml | 6 +-
>  gdb-xml/riscv-32bit-fpu.xml | 6 +-
>  gdb-xml/riscv-64bit-cpu.xml | 6 +-
>  gdb-xml/riscv-64bit-fpu.xml | 6 +-
>  4 files changed, 4 insertions(+), 20 deletions(-)
>
> diff --git a/gdb-xml/riscv-32bit-cpu.xml b/gdb-xml/riscv-32bit-cpu.xml
> index 0d07aaec85..466f2c0648 100644
> --- a/gdb-xml/riscv-32bit-cpu.xml
> +++ b/gdb-xml/riscv-32bit-cpu.xml
> @@ -5,13 +5,9 @@
>   are permitted in any medium without royalty provided the copyright
>   notice and this notice are preserved.  -->
>
> -
> -
>  
>  
> -  
> +  
>
>
>
> diff --git a/gdb-xml/riscv-32bit-fpu.xml b/gdb-xml/riscv-32bit-fpu.xml
> index 84a44ba8df..24aa087031 100644
> --- a/gdb-xml/riscv-32bit-fpu.xml
> +++ b/gdb-xml/riscv-32bit-fpu.xml
> @@ -5,13 +5,9 @@
>   are permitted in any medium without royalty provided the copyright
>   notice and this notice are preserved.  -->
>
> -
> -
>  
>  
> -  
> +  
>
>
>
> diff --git a/gdb-xml/riscv-64bit-cpu.xml b/gdb-xml/riscv-64bit-cpu.xml
> index b8aa424ae4..c4d83de09b 100644
> --- a/gdb-xml/riscv-64bit-cpu.xml
> +++ b/gdb-xml/riscv-64bit-cpu.xml
> @@ -5,13 +5,9 @@
>   are permitted in any medium without royalty provided the copyright
>   notice and this notice are preserved.  -->
>
> -
> -
>  
>  
> -  
> +  
>
>
>
> diff --git a/gdb-xml/riscv-64bit-fpu.xml b/gdb-xml/riscv-64bit-fpu.xml
> index 9856a9d1d3..d0f17f9984 100644
> --- a/gdb-xml/riscv-64bit-fpu.xml
> +++ b/gdb-xml/riscv-64bit-fpu.xml
> @@ -5,10 +5,6 @@
>   are permitted in any medium without royalty provided the copyright
>   notice and this notice are preserved.  -->
>
> -
> -
>  
>  
>
> @@ -17,7 +13,7 @@
>  
>
>
> -  
> +  
>
>
>
> --
> 2.25.4
>
>

Re: [PATCH v2 1/2] x86: only modify setup_data if the boot protocol indicates safety

[Daniel P . Berrangé]

On Thu, Sep 08, 2022 at 02:28:29PM +0200, Ard Biesheuvel wrote:
> On Thu, 8 Sept 2022 at 13:30, Laszlo Ersek  wrote:
> >
> > On 09/06/22 13:33, Daniel P. Berrangé wrote:
> > > On Tue, Sep 06, 2022 at 01:14:50PM +0200, Ard Biesheuvel wrote:
> > >> (cc Laszlo)
> > >>
> > >> On Tue, 6 Sept 2022 at 12:45, Michael S. Tsirkin  wrote:
> > >>>
> > >>> On Tue, Sep 06, 2022 at 12:43:55PM +0200, Jason A. Donenfeld wrote:
> >  On Tue, Sep 6, 2022 at 12:40 PM Michael S. Tsirkin  
> >  wrote:
> > >
> > > On Tue, Sep 06, 2022 at 12:36:56PM +0200, Jason A. Donenfeld wrote:
> > >> It's only safe to modify the setup_data pointer on newer kernels 
> > >> where
> > >> the EFI stub loader will ignore it. So condition setting that offset 
> > >> on
> > >> the newer boot protocol version. While we're at it, gate this on SEV 
> > >> too.
> > >> This depends on the kernel commit linked below going upstream.
> > >>
> > >> Cc: Gerd Hoffmann 
> > >> Cc: Laurent Vivier 
> > >> Cc: Michael S. Tsirkin 
> > >> Cc: Paolo Bonzini 
> > >> Cc: Peter Maydell 
> > >> Cc: Philippe Mathieu-Daudé 
> > >> Cc: Richard Henderson 
> > >> Cc: Ard Biesheuvel 
> > >> Link: 
> > >> https://lore.kernel.org/linux-efi/20220904165321.1140894-1-ja...@zx2c4.com/
> > >> Signed-off-by: Jason A. Donenfeld 
> > >
> > > BTW what does it have to do with SEV?
> > > Is this because SEV is not going to trust the data to be random 
> > > anyway?
> > 
> >  Daniel (now CC'd) pointed out in one of the previous threads that this
> >  breaks SEV, because the image hash changes.
> > 
> >  Jason
> > >>>
> > >>> Oh I see. I'd add a comment maybe and definitely mention this
> > >>> in the commit log.
> > >>>
> > >>
> > >> This does raise the question (as I mentioned before) how things like
> > >> secure boot and measured boot are affected when combined with direct
> > >> kernel boot: AIUI, libvirt uses direct kernel boot at guest
> > >> installation time, and modifying setup_data will corrupt the image
> > >> signature.
> > >
> > > IIUC, qemu already modifies setup_data when using direct kernel boot.
> > >
> > > It put in logic to skip this if SEV is enabled, to avoid interfering
> > > with SEV hashes over the kernel, but there's nothing doing this more
> > > generally for non-SEV cases using UEFI. So potentially use of SecureBoot
> > > may already be impacted when using direct kernel boot.
> >
> > Yes,
> >
> > https://github.com/tianocore/edk2/commit/82808b422617
> >
> 
> Ah yes, thanks for jogging my memory.
> 
> So virt-install --network already ignores secure boot failures on
> direct kernel boot, so this is not going to make it any worse.

And in a cloud world this isn't too much of a problem to start
with. The cloud disks images will be built offline in trusted
infrastructure, so lack of SecureBoot isn't a show stopper. When
later deployed to the public cloud, SecureBoot (and/or Confidential
Boot) will be fully operational, where it matters most.

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

[PATCH] migrate block dirty bitmap: Fix the block dirty bitmap can't to migration_completion when pending size larger than threshold size : 1、dirty bitmap size big enough (such as 8MB) when block si

[liuhaiwei]

From: liuhaiwei 

so we set the fake pending size when pending size > threshold size

Signed-off-by: liuhaiwei 
Signed-off-by: liuhaiwei 
---
 migration/block-dirty-bitmap.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index 9aba7d9c22..6086d8d1c3 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -782,7 +782,10 @@ static void dirty_bitmap_save_pending(QEMUFile *f, void 
*opaque,
 }
 
 qemu_mutex_unlock_iothread();
-
+/*we set the fake pending size  when the dirty bitmap size more than 
max_size(bandwith of speed) */
+if(pending > max_size && max_size == 0){
+pending = max_size - 1;
+}
 trace_dirty_bitmap_save_pending(pending, max_size);
 
 *res_postcopy_only += pending;
-- 
2.27.0

Access DomU shared memory in Dom0 Kernel

[duslabo]

Hi,

I am mapping DomU DMA Address in Qemu source using dma_memory_map API. But my 
Dom0's kernel is unable to access the VA assigned to dma_memory_map API in Qemu.

Any Suggestion?

Sent with [Proton Mail](https://proton.me/) secure email.

[PATCH] Use QMP command object-add instead of object_add for memory hotplugin

[liuhaiwei]

From: liuhaiwei 

Signed-off-by: liuhaiwei 
---
 docs/memory-hotplug.txt | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/docs/memory-hotplug.txt b/docs/memory-hotplug.txt
index 6aa5e17e26..85ed4d8f3d 100644
--- a/docs/memory-hotplug.txt
+++ b/docs/memory-hotplug.txt
@@ -34,15 +34,15 @@ hotplugged by using any combination of the available memory 
slots.
 
 Two monitor commands are used to hotplug memory:
 
- - "object_add": creates a memory backend object
+ - "object-add": creates a memory backend object
  - "device_add": creates a front-end pc-dimm device and inserts it
  into the first empty slot
 
 For example, the following commands add another 1GB to the guest
 discussed earlier:
 
-  (qemu) object_add memory-backend-ram,id=mem1,size=1G
-  (qemu) device_add pc-dimm,id=dimm1,memdev=mem1
+  (qemu) object-add qom-type=memory-backend-ram id=mem1 size=1073741824
+  (qemu) device_add driver=pc-dimm id=dimm1 memdev=mem1
 
 Using the file backend
 --
@@ -55,7 +55,7 @@ For example, assuming that the host has 1GB hugepages 
available in
 the /mnt/hugepages-1GB directory, a 1GB hugepage could be hotplugged
 into the guest from the previous section with the following commands:
 
-  (qemu) object_add 
memory-backend-file,id=mem1,size=1G,mem-path=/mnt/hugepages-1GB
+  (qemu) object-add qom-type=memory-backend-file id=mem1  size=1073741824 
mem-path=/mnt/hugepages-1GB 
   (qemu) device_add pc-dimm,id=dimm1,memdev=mem1
 
 It's also possible to start a guest with memory cold-plugged into the
-- 
2.27.0

[PATCH] qemu-img: Wean documentation and help output off '?' for help

[Markus Armbruster]

'?' for help is deprecated since commit c8057f951d "Support 'help' as
a synonym for '?' in command line options", v1.2.0.  We neglected to
update output of qemu-img --help and the manual.  Do that now.

Signed-off-by: Markus Armbruster 
---
 docs/tools/qemu-img.rst | 2 +-
 qemu-img.c  | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst
index 85a6e05b35..15aeddc6d8 100644
--- a/docs/tools/qemu-img.rst
+++ b/docs/tools/qemu-img.rst
@@ -57,7 +57,7 @@ cases. See below for a description of the supported disk 
formats.
 *OUTPUT_FMT* is the destination format.
 
 *OPTIONS* is a comma separated list of format specific options in a
-name=value format. Use ``-o ?`` for an overview of the options supported
+name=value format. Use ``-o help`` for an overview of the options supported
 by the used format or see the format descriptions below for details.
 
 *SNAPSHOT_PARAM* is param used for internal snapshot, format is
diff --git a/qemu-img.c b/qemu-img.c
index 7d4b33b3da..cab9776f42 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -164,8 +164,8 @@ void help(void)
"  'output_filename' is the destination disk image filename\n"
"  'output_fmt' is the destination format\n"
"  'options' is a comma separated list of format specific options 
in a\n"
-   "name=value format. Use -o ? for an overview of the options 
supported by the\n"
-   "used format\n"
+   "name=value format. Use -o help for an overview of the options 
supported by\n"
+   "the used format\n"
"  'snapshot_param' is param used for internal snapshot, format\n"
"is 'snapshot.id=[ID],snapshot.name=[NAME]', or\n"
"'[ID_OR_NAME]'\n"
-- 
2.37.2

Re:Re: [PATCH] Use QMP command object-add instead of object_add for memory hotplugin

[liuhaiwei9699]

why the hmp using the object_add , qmp using the object-add command?
can't we use the same command ?

















At 2022-09-08 19:41:33, "Markus Armbruster"  wrote:
>liuhaiwei  writes:
>
>> From: liuhaiwei 
>>
>> Signed-off-by: liuhaiwei 
>> ---
>>  docs/memory-hotplug.txt | 8 
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/docs/memory-hotplug.txt b/docs/memory-hotplug.txt
>> index 6aa5e17e26..85ed4d8f3d 100644
>> --- a/docs/memory-hotplug.txt
>> +++ b/docs/memory-hotplug.txt
>> @@ -34,15 +34,15 @@ hotplugged by using any combination of the available 
>> memory slots.
>>  
>>  Two monitor commands are used to hotplug memory:
>>  
>> - - "object_add": creates a memory backend object
>> + - "object-add": creates a memory backend object
>>   - "device_add": creates a front-end pc-dimm device and inserts it
>>   into the first empty slot
>>  
>>  For example, the following commands add another 1GB to the guest
>>  discussed earlier:
>>  
>> -  (qemu) object_add memory-backend-ram,id=mem1,size=1G
>> -  (qemu) device_add pc-dimm,id=dimm1,memdev=mem1
>> +  (qemu) object-add qom-type=memory-backend-ram id=mem1 size=1073741824
>> +  (qemu) device_add driver=pc-dimm id=dimm1 memdev=mem1
>
>This is HMP, where the command is spelled object_add.  Your patch is
>wrong.
>
>>  
>>  Using the file backend
>>  --
>> @@ -55,7 +55,7 @@ For example, assuming that the host has 1GB hugepages 
>> available in
>>  the /mnt/hugepages-1GB directory, a 1GB hugepage could be hotplugged
>>  into the guest from the previous section with the following commands:
>>  
>> -  (qemu) object_add 
>> memory-backend-file,id=mem1,size=1G,mem-path=/mnt/hugepages-1GB
>
>Likewise.
>
>> +  (qemu) object-add qom-type=memory-backend-file id=mem1  size=1073741824 
>> mem-path=/mnt/hugepages-1GB 
>>(qemu) device_add pc-dimm,id=dimm1,memdev=mem1
>>  
>>  It's also possible to start a guest with memory cold-plugged into the

Re: [PATCH] Use QMP command object-add instead of object_add for memory hotplugin

[Dr. David Alan Gilbert]

* Markus Armbruster (arm...@redhat.com) wrote:
> liuhaiwei9699   writes:
> 
> > why the hmp using the object_add , qmp using the object-add command?
> > can't we use the same command ?
> 
> Command names differ between HMP and QMP for historical reasons.
> 
> QMP is a stable interface, and changing names there is no go.
> 
> HMP is not a stable interface, but changing names would still
> inconvenience users.  We don't do that without really compelling
> reasons.
> 
> I think HMP could fold '_' and '-' together in command names, so that
> both object_add and object-add work.  Best to check with the HMP
> maintainer before you start coding.

Yes, I'd be up for having folding on _/- - I never remember which I need
in any case.

Dave

-- 
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

[PATCH 0/7] nsis: gitlab-ci: Improve QEMU Windows installer packaging

[Bin Meng]

At present packaging the required DLLs of QEMU executables is a
manual process, and error prone.

Improve scripts/nsis.py by adding a logic to automatically package
required DLLs of QEMU executables.

'make installer' is tested in the cross-build on Linux in CI, but
not in the Windows native build. Update CI to test the installer
generation on Windows too.

During testing a 32-bit build issue was exposed in block/nfs.c and
the fix is included in this series.


Bin Meng (7):
  scripts/nsis.py: Drop the unnecessary path separator
  scripts/nsis.py: Fix destination directory name when invoked on
Windows
  scripts/nsis.py: Automatically package required DLLs of QEMU
executables
  .gitlab-ci.d/windows.yml: Drop the sed processing in the 64-bit build
  block/nfs: Fix 32-bit Windows build
  .gitlab-ci.d/windows.yml: Unify the prerequisite packages
  .gitlab-ci.d/windows.yml: Test 'make installer' in the CI

 meson.build  |  1 +
 block/nfs.c  |  8 ++
 .gitlab-ci.d/windows.yml | 40 ---
 scripts/nsis.py  | 60 +---
 4 files changed, 89 insertions(+), 20 deletions(-)

-- 
2.34.1