Re: [PATCH v11 11/16] target/riscv: Add orc.b instruction for Zbb, removing gorc/gorci
On Sun, Sep 12, 2021 at 12:07 AM Philipp Tomsich
wrote:
>
> The 1.0.0 version of Zbb does not contain gorc/gorci. Instead, a
> orc.b instruction (equivalent to the orc.b pseudo-instruction built on
> gorci from pre-0.93 draft-B) is available, mainly targeting
> string-processing workloads.
>
> This commit adds the new orc.b instruction and removed gorc/gorci.
>
> Signed-off-by: Philipp Tomsich
> Reviewed-by: Richard Henderson
> Reviewed-by: Alistair Francis
I'm seeing this warning when building with gcc (GCC) 11.2.1
/var/mnt/scratch/alistair/software/qemu/include/tcg/tcg.h:1267:5:
warning: overflow in conversion from ‘long long unsigned int’ to
‘int32_t’ {aka ‘int’} changes value from ‘72340172838076673’ to
‘16843009’ [-Woverflow]
1267 | (__builtin_constant_p(VECE)\
| ^~~~
1268 | ? ( (VECE) == MO_8 ? 0x0101010101010101ull * (uint8_t)(C) \
| ~~~
1269 | : (VECE) == MO_16 ? 0x0001000100010001ull * (uint16_t)(C) \
|
1270 | : (VECE) == MO_32 ? 0x00010001ull * (uint32_t)(C) \
|
1271 | : (VECE) == MO_64 ? (uint64_t)(C) \
|
1272 | : (qemu_build_not_reached_always(), 0))\
|
1273 | : dup_const(VECE, C))
| ~
../target/riscv/insn_trans/trans_rvb.c.inc:301:34: note: in expansion
of macro ‘dup_const’
301 | TCGv ones = tcg_constant_tl(dup_const(MO_8, 0x01));
| ^
[78/87] Compiling C object
libqemu-riscv32-linux-user.fa.p/target_riscv_translate.c.o
In file included from
/var/mnt/scratch/alistair/software/qemu/include/tcg/tcg-op.h:28,
from ../target/riscv/translate.c:22:
Alistair
>
> ---
>
> (no changes since v9)
>
> Changes in v9:
> - Picked up Alistair's Reviewed-by, after patman had failed to catch
> it for v8.
>
> Changes in v8:
> - Optimize orc.b further by reordering the shift/and, updating the
> comment to reflect that we put the truth-value into the LSB, and
> putting the (now only) constant in a temporary
> - Fold the final bitwise-not into the second and, using and andc.
>
> Changes in v7:
> - Free TCG temporary in gen_orc_b().
>
> Changes in v6:
> - Fixed orc.b (now passes SPEC w/ optimized string functions) by
> adding the missing final negation.
>
> Changes in v4:
> - Change orc.b to implementation suggested by Richard Henderson
>
> Changes in v3:
> - Moved orc.b and gorc/gorci changes into separate commit.
> - Using the simpler orc.b implementation suggested by Richard Henderson
>
> target/riscv/bitmanip_helper.c | 26 -
> target/riscv/helper.h | 2 --
> target/riscv/insn32.decode | 6 +---
> target/riscv/insn_trans/trans_rvb.c.inc | 39 +++--
> 4 files changed, 18 insertions(+), 55 deletions(-)
>
> diff --git a/target/riscv/bitmanip_helper.c b/target/riscv/bitmanip_helper.c
> index 73be5a81c7..bb48388fcd 100644
> --- a/target/riscv/bitmanip_helper.c
> +++ b/target/riscv/bitmanip_helper.c
> @@ -64,32 +64,6 @@ target_ulong HELPER(grevw)(target_ulong rs1, target_ulong
> rs2)
> return do_grev(rs1, rs2, 32);
> }
>
> -static target_ulong do_gorc(target_ulong rs1,
> -target_ulong rs2,
> -int bits)
> -{
> -target_ulong x = rs1;
> -int i, shift;
> -
> -for (i = 0, shift = 1; shift < bits; i++, shift <<= 1) {
> -if (rs2 & shift) {
> -x |= do_swap(x, adjacent_masks[i], shift);
> -}
> -}
> -
> -return x;
> -}
> -
> -target_ulong HELPER(gorc)(target_ulong rs1, target_ulong rs2)
> -{
> -return do_gorc(rs1, rs2, TARGET_LONG_BITS);
> -}
> -
> -target_ulong HELPER(gorcw)(target_ulong rs1, target_ulong rs2)
> -{
> -return do_gorc(rs1, rs2, 32);
> -}
> -
> target_ulong HELPER(clmul)(target_ulong rs1, target_ulong rs2)
> {
> target_ulong result = 0;
> diff --git a/target/riscv/helper.h b/target/riscv/helper.h
> index 8a318a2dbc..a9bda2c8ac 100644
> --- a/target/riscv/helper.h
> +++ b/target/riscv/helper.h
> @@ -61,8 +61,6 @@ DEF_HELPER_FLAGS_1(fclass_d, TCG_CALL_NO_RWG_SE, tl, i64)
> /* Bitmanip */
> DEF_HELPER_FLAGS_2(grev, TCG_CALL_NO_RWG_SE, tl, tl, tl)
> DEF_HELPER_FLAGS_2(grevw, TCG_CALL_NO_RWG_SE, tl, tl, tl)
> -DEF_HELPER_FLAGS_2(gorc, TCG_CALL_NO_RWG_SE, tl, tl, tl)
> -DEF_HELPER_FLAGS_2(gorcw, TCG_CALL_NO_RWG_SE, tl, tl, tl)
> DEF_HELPER_FLAGS_2(clmul, TCG_CALL_NO_RWG_SE, tl, tl, tl)
> DEF_HELPER_FLAGS_2(clmulr, TCG_CAL
[PATCH] qemu-options: -chardev reconnect=seconds duplicated in help, tidy up
Fixes: 5dd1f02b4bc2f2c2ef3a2adfd8a412c8c8769085
Signed-off-by: Markus Armbruster
---
qemu-options.hx | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qemu-options.hx b/qemu-options.hx
index 8f603cc7e6..27e7b9a77c 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3188,7 +3188,7 @@ DEFHEADING(Character device options:)
DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
"-chardev help\n"
"-chardev null,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
-"-chardev
socket,id=id[,host=host],port=port[,to=to][,ipv4=on|off][,ipv6=on|off][,nodelay=on|off][,reconnect=seconds]\n"
+"-chardev
socket,id=id[,host=host],port=port[,to=to][,ipv4=on|off][,ipv6=on|off][,nodelay=on|off]\n"
"
[,server=on|off][,wait=on|off][,telnet=on|off][,websocket=on|off][,reconnect=seconds][,mux=on|off]\n"
" [,logfile=PATH][,logappend=on|off][,tls-creds=ID][,tls-authz=ID]
(tcp)\n"
"-chardev
socket,id=id,path=path[,server=on|off][,wait=on|off][,telnet=on|off][,websocket=on|off][,reconnect=seconds]\n"
--
2.31.1
Re: [RFC PATCH] spapr/xive: Allocate vCPU IPIs from local context
On Mon, 27 Sep 2021 18:50:40 +0200
Cédric Le Goater wrote:
> On 9/24/21 19:13, Greg Kurz wrote:
> > On Fri, 24 Sep 2021 16:58:00 +0200
> > Cédric Le Goater wrote:
> >
> >> [ ... ]
> >>
> The changes only impact KVM support since we are deferring the IRQ
> initialization at the KVM level. What we have to be careful about is not
> accessing an ESB page of an interrupt that would not have been
> initiliazed
> in the KVM device, else QEMU gets a sigbus.
>
> >>>
> >>> Ok, so this is just needed on a path that leads to xive_esb_rw() or
> >>> kvmppc_xive_esb_trigger(), right ?
> >>>
> >>> It seems that
> >>>
> >>> h_int_esb()
> >>>kvmppc_xive_esb_rw()
> >>>
> >>> can get there with an lisn provided by the guest, and I don't see any
> >>> such check on the way : h_int_esb() only checks xive_eas_is_valid().
> >>
> >> This call is for LSI interrupts (device only) and not vCPU IPIs. see hcall
> >> h_int_get_source_info(). I agree a hcall fuzzer could reach it.
> >>
> >
> > Yes we tell the guest to use H_INT_ESB for LSIs only but I don't have
> > the impression that it is forbidden for the guest to call H_INT_ESB
> > for arbitrary interrupts.
> >
> >> An alternative solution would be to claim the vCPU IPIs on demand, like
> >> we do for the MSIs, and not in spapr_irq_init(). It's a much bigger change
> >> tough, because the H_INT hcalls consider that the interrupt numbers have
> >> been claimed.
> >>
> >
> > Maybe it would be simpler to call xive_source_is_initialized() instead of
> > xive_eas_is_valid() in cases like this, e.g. hcall code since it is shared
> > between emulation and KVM ?
>
> Yes but we need a better check than :
>
> if (lisn < SPAPR_XIRQ_BASE) {
> return !!xive_get_field64(EAS_END_INDEX, xive->eat[lisn].w);
> }
>
> This is more an heuristic than a precise test on the "validity" of
> a source at the KVM level.
>
I guess we should be able to get kvmppc_xive_irq_state::valid from
KVM by making the KVM_DEV_XIVE_GRP_SOURCE attribute readable. Would
that be enough ?
>
> Thanks,
>
> C.
Re: [RFC PATCH 01/11] target/riscv: Add CLIC CSR mintstatus
On Fri, Jul 2, 2021 at 3:17 PM Alistair Francis
wrote:
> On Fri, Jul 2, 2021 at 4:11 PM LIU Zhiwei wrote:
> >
> >
> > On 2021/7/2 下午1:38, Alistair Francis wrote:
> > > On Thu, Jul 1, 2021 at 6:45 PM Frank Chang
> wrote:
> > >> LIU Zhiwei 於 2021年4月20日 週二 上午8:49寫道：
> > >>>
> > >>> On 2021/4/20 上午7:23, Alistair Francis wrote:
> > On Fri, Apr 9, 2021 at 5:52 PM LIU Zhiwei
> wrote:
> > > CSR mintstatus holds the active interrupt level for each supported
> > > privilege mode. sintstatus, and user, uintstatus, provide
> restricted
> > > views of mintstatus.
> > >
> > > Signed-off-by: LIU Zhiwei
> > > ---
> > >target/riscv/cpu.h | 2 ++
> > >target/riscv/cpu_bits.h | 11 +++
> > >target/riscv/csr.c | 26 ++
> > >3 files changed, 39 insertions(+)
> > >
> > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > > index 0a33d387ba..1a44ca62c7 100644
> > > --- a/target/riscv/cpu.h
> > > +++ b/target/riscv/cpu.h
> > > @@ -159,6 +159,7 @@ struct CPURISCVState {
> > >target_ulong mip;
> > >
> > >uint32_t miclaim;
> > > +uint32_t mintstatus; /* clic-spec */
> > >
> > >target_ulong mie;
> > >target_ulong mideleg;
> > > @@ -243,6 +244,7 @@ struct CPURISCVState {
> > >
> > >/* Fields from here on are preserved across CPU reset. */
> > >QEMUTimer *timer; /* Internal timer */
> > > +void *clic; /* clic interrupt controller */
> > This should be the CLIC type.
> > >>> OK.
> > >>>
> > >>> Actually there are many versions of CLIC in my branch as different
> > >>> devices. But it is better to use CLIC type for the upstream version.
> > >>
> > >> Hi Alistair and Zhiwei,
> > >>
> > >> Replacing void *clic with RISCVCLICState *clic may create a circular
> loop
> > >> because CPURISCVState is also referenced in riscv_clic.h.
> > >>
> > >> However, I would like to ask what is the best approach to add
> > >> the reference of CLIC device in CPURISCVState struct?
> > >>
> > >> There may be different kinds of CLIC devices.
> > >> AFAK, there was another RFC patchset trying to add void *eclic
> > >> for Nuclei processor into CPURISCVState struct:
> > >>
> https://patchwork.kernel.org/project/qemu-devel/patch/20210507081654.11056-2-wangjunqi...@iscas.ac.cn/
> > >>
> > >> Is it okay to add the device reference directly into CPURISCVState
> struct like that,
> > >> or we should create some abstraction for these CLIC devices?
> > >> (However, I'm not sure how big the differences are for these CLIC
> devices...)
> > > I would prefer to not have the CLIC in the struct at all.
> > >
> > > Why is the CLIC required from the CPU?
> >
> > In my opinion, the tight coupled interrupt controller, like NVIC in
> > ARM, is much different from other devices.
> > CPU harts need to communicate with it through many ways.
>
> Agreed.
>
> The difference with RISC-V is that we already have multiple tightly
> coupled interrupt controllers. We have the PLIC, CLIC, eCLIC and AIA,
> not to mention possible vendor controllers. Do we really need a CPU
> struct entry for every single one?
>
> I would like to try and keep all of that logic outside of the CPU
> state. It might not be possible (at least without being a mess) in
> which case that's fine, but it's at least worth considering.
>
> >
> > We can store the CLIC instance in the struct CPURISCVState or a global
> > variable. Is that any other way?
>
> We could split the device. So for example the CSRs and other parts
> that relate to the CPU could be in the CPU while the register mappings
> and GPIO lines could be it's own device.
>
> Another option is to use GPIO lines to indicate the status, but for
> anything too complex that will be messy.
>
Hi Alistair,
I would like to know whether:
"Using GPIO lines to indicate the status of CPU"
is the right approach for future RISC-V CPU/peripherals development?
As RISC-V ecosystem is open to everyone and there are more and more
peripherals
being designed by different vendors.
I think there is a rising requirement to have an approach for CPU to notify
those core tightly-coupled peripherals after certain instruction/action has
been completed.
(for this case, CLIC would need to choose the next interrupt to be served
after returning from interrupt in mret/sret)
Is it possible to add something like:
* qemu_irq gpio_out[NUM_OF_GPIOS];*
into *CPURISCVState* struct so that we can connect the GPIO lines
at machine/SoC-level, e.g. *sifive_u* or *sifive_e*?
When certain instruction or event is executed/completed by CPU.
It's possible for CPU to notify the peripheral through these GPIO lines,
and the rest of the tasks can be completed solely by the peripheral itself.
We don't have to add the hacky codes to the generic CPU routines.
This is just the rough scenario I have come up.
There might be some other issues I may not think o
Emulation of IOT hardware
Hello Team, Would you please give me some suggestions on how I should proceed to build an IOT (Internet of Things) emulator ? What aspects do I need to consider ? IOT can be anything like a smart light ,smart bulb ,smart lock ,etc. Here smart means that the device can be controlled via the internet. -- Best Regards, Niraj
Re: [PATCH] qemu-options: -chardev reconnect=seconds duplicated in help, tidy up
On Tue, Sep 28, 2021 at 09:14:49AM +0200, Markus Armbruster wrote: > Fixes: 5dd1f02b4bc2f2c2ef3a2adfd8a412c8c8769085 > Signed-off-by: Markus Armbruster > --- > qemu-options.hx | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Daniel P. Berrangé 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 v5] Prevent vhost-user-blk-test hang
On Mon, Sep 27, 2021 at 05:17:01PM +, Raphael Norwitz wrote:
> In the vhost-user-blk-test, as of now there is nothing stoping
> vhost-user-blk in QEMU writing to the socket right after forking off the
> storage daemon before it has a chance to come up properly, leaving the
> test hanging forever. This intermittently hanging test has caused QEMU
> automation failures reported multiple times on the mailing list [1].
>
> This change makes the storage-daemon notify the vhost-user-blk-test
> that it is fully initialized and ready to handle client connections by
> creating a pidfile on initialiation. This ensures that the storage-daemon
> backend won't miss vhost-user messages and thereby resolves the hang.
>
> [1]
> https://lore.kernel.org/qemu-devel/CAFEAcA8kYpz9LiPNxnWJAPSjc=nv532bedyfynabemeohqb...@mail.gmail.com/
Hi Raphael,
I would like to understand the issue that is being worked around in the
patch.
QEMU should be okay with listen fd passing. The qemu-storage-daemon
documentation even contains example code for this
(docs/tools/qemu-storage-daemon.rst) and that may need to be updated if
listen fd passing is fundamentally broken.
Can you share more details about the problem?
Does "writing to the socket" mean writing vhost-user protocol messages
or does it mean connect(2)?
Could the problem be that vhost-user-blk-test.c creates the listen fds
and does not close them? This means the host network stack doesn't
consider the socket closed after QEMU terminates and therefore the test
process hangs after QEMU is gone? In that case vhost-user-blk-test needs
to close the fds after spawning qemu-storage-daemon.
Stefan
>
> Signed-off-by: Raphael Norwitz
> Reviewed-by: Eric Blake
> ---
> tests/qtest/vhost-user-blk-test.c | 26 +-
> 1 file changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/tests/qtest/vhost-user-blk-test.c
> b/tests/qtest/vhost-user-blk-test.c
> index 6f108a1b62..5fed262da1 100644
> --- a/tests/qtest/vhost-user-blk-test.c
> +++ b/tests/qtest/vhost-user-blk-test.c
> @@ -24,6 +24,7 @@
> #define TEST_IMAGE_SIZE (64 * 1024 * 1024)
> #define QVIRTIO_BLK_TIMEOUT_US (30 * 1000 * 1000)
> #define PCI_SLOT_HP 0x06
> +#define PIDFILE_RETRIES 5
>
> typedef struct {
> pid_t pid;
> @@ -885,7 +886,8 @@ static void start_vhost_user_blk(GString *cmd_line, int
> vus_instances,
> int num_queues)
> {
> const char *vhost_user_blk_bin = qtest_qemu_storage_daemon_binary();
> -int i;
> +int i, retries;
> +char *daemon_pidfile_path;
> gchar *img_path;
> GString *storage_daemon_command = g_string_new(NULL);
> QemuStorageDaemonState *qsd;
> @@ -898,6 +900,8 @@ static void start_vhost_user_blk(GString *cmd_line, int
> vus_instances,
> " -object memory-backend-memfd,id=mem,size=256M,share=on "
> " -M memory-backend=mem -m 256M ");
>
> +daemon_pidfile_path = g_strdup_printf("/tmp/daemon-%d", getpid());
> +
> for (i = 0; i < vus_instances; i++) {
> int fd;
> char *sock_path = create_listen_socket(&fd);
> @@ -914,6 +918,9 @@ static void start_vhost_user_blk(GString *cmd_line, int
> vus_instances,
> i + 1, sock_path);
> }
>
> +g_string_append_printf(storage_daemon_command, "--pidfile %s ",
> + daemon_pidfile_path);
> +
> g_test_message("starting vhost-user backend: %s",
> storage_daemon_command->str);
> pid_t pid = fork();
> @@ -930,7 +937,24 @@ static void start_vhost_user_blk(GString *cmd_line, int
> vus_instances,
> execlp("/bin/sh", "sh", "-c", storage_daemon_command->str, NULL);
> exit(1);
> }
> +
> +/*
> + * Ensure the storage-daemon has come up properly before allowing the
> + * test to proceed.
> + */
> +retries = 0;
> +while (access(daemon_pidfile_path, F_OK) != 0) {
> +g_assert_cmpint(retries, <, PIDFILE_RETRIES);
> +
> +retries++;
> +g_usleep(1000);
> +}
> +
> g_string_free(storage_daemon_command, true);
> +if (access(daemon_pidfile_path, F_OK) == 0) {
> +unlink(daemon_pidfile_path);
> +}
> +g_free(daemon_pidfile_path);
>
> qsd = g_new(QemuStorageDaemonState, 1);
> qsd->pid = pid;
> --
> 2.20.1
>
signature.asc
Description: PGP signature
[PATCH v4] VNC-related HMP/QMP fixes
Since the removal of the generic 'qmp_change' command, one can no longer replace the 'default' VNC display listen address at runtime (AFAIK). For our users who need to set up a secondary VNC access port, this means configuring a second VNC display (in addition to our standard one for web-access), but it turns out one cannot set a password on this second display at the moment, as the 'set_password' call only operates on the 'default' display. Additionally, using secret objects, the password is only read once at startup. This could be considered a bug too, but is not touched in this series and left for a later date. v3 -> v4: * drop previously patch 1, this was fixed here instead: https://lists.gnu.org/archive/html/qemu-devel/2021-09/msg02529.html * patch 1: add Eric's R-b * patch 2: remove if-assignment, use 'deprecated' feature in schema v2 -> v3: * refactor QMP schema for set/expire_password as suggested by Eric Blake and Markus Armbruster v1 -> v2: * add Marc-André's R-b on patch 1 * use '-d' flag as suggested by Eric Blake and Gerd Hoffmann * I didn't see a way to do this yet, so I added a "flags with values" arg type Stefan Reiter (2): monitor/hmp: add support for flag argument with value monitor: refactor set/expire_password and allow VNC display id hmp-commands.hx| 24 --- monitor/hmp-cmds.c | 57 ++- monitor/hmp.c | 17 - monitor/qmp-cmds.c | 62 ++-- qapi/ui.json | 173 + 5 files changed, 251 insertions(+), 82 deletions(-) -- 2.30.2
[PATCH v4 2/2] monitor: refactor set/expire_password and allow VNC display id
It is possible to specify more than one VNC server on the command line,
either with an explicit ID or the auto-generated ones à la "default",
"vnc2", "vnc3", ...
It is not possible to change the password on one of these extra VNC
displays though. Fix this by adding a "display" parameter to the
"set_password" and "expire_password" QMP and HMP commands.
For HMP, the display is specified using the "-d" value flag.
For QMP, the schema is updated to explicitly express the supported
variants of the commands with protocol-discriminated unions.
Suggested-by: Eric Blake
Suggested-by: Markus Armbruster
Signed-off-by: Stefan Reiter
---
hmp-commands.hx| 24 ---
monitor/hmp-cmds.c | 57 ++-
monitor/qmp-cmds.c | 62 ++--
qapi/ui.json | 173 +
4 files changed, 235 insertions(+), 81 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index cf723c69ac..d78e4cfc47 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1514,33 +1514,35 @@ ERST
{
.name = "set_password",
-.args_type = "protocol:s,password:s,connected:s?",
-.params = "protocol password action-if-connected",
+.args_type = "protocol:s,password:s,display:-dS,connected:s?",
+.params = "protocol password [-d display] [action-if-connected]",
.help = "set spice/vnc password",
.cmd= hmp_set_password,
},
SRST
-``set_password [ vnc | spice ] password [ action-if-connected ]``
- Change spice/vnc password. *action-if-connected* specifies what
- should happen in case a connection is established: *fail* makes the
- password change fail. *disconnect* changes the password and
+``set_password [ vnc | spice ] password [ -d display ] [ action-if-connected
]``
+ Change spice/vnc password. *display* can be used with 'vnc' to specify
+ which display to set the password on. *action-if-connected* specifies
+ what should happen in case a connection is established: *fail* makes
+ the password change fail. *disconnect* changes the password and
disconnects the client. *keep* changes the password and keeps the
connection up. *keep* is the default.
ERST
{
.name = "expire_password",
-.args_type = "protocol:s,time:s",
-.params = "protocol time",
+.args_type = "protocol:s,time:s,display:-dS",
+.params = "protocol time [-d display]",
.help = "set spice/vnc password expire-time",
.cmd= hmp_expire_password,
},
SRST
-``expire_password [ vnc | spice ]`` *expire-time*
- Specify when a password for spice/vnc becomes
- invalid. *expire-time* accepts:
+``expire_password [ vnc | spice ] expire-time [ -d display ]``
+ Specify when a password for spice/vnc becomes invalid.
+ *display* behaves the same as in ``set_password``.
+ *expire-time* accepts:
``now``
Invalidate password instantly.
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index b5e71d9e6f..48b3fe4519 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1451,10 +1451,41 @@ void hmp_set_password(Monitor *mon, const QDict *qdict)
{
const char *protocol = qdict_get_str(qdict, "protocol");
const char *password = qdict_get_str(qdict, "password");
+const char *display = qdict_get_try_str(qdict, "display");
const char *connected = qdict_get_try_str(qdict, "connected");
Error *err = NULL;
+DisplayProtocol proto;
-qmp_set_password(protocol, password, !!connected, connected, &err);
+SetPasswordOptions opts = {
+.password = g_strdup(password),
+.u.vnc.display = NULL,
+};
+
+proto = qapi_enum_parse(&DisplayProtocol_lookup, protocol,
+DISPLAY_PROTOCOL_VNC, &err);
+if (err) {
+hmp_handle_error(mon, err);
+return;
+}
+opts.protocol = proto;
+
+if (proto == DISPLAY_PROTOCOL_VNC) {
+opts.u.vnc.has_display = !!display;
+opts.u.vnc.display = g_strdup(display);
+} else if (proto == DISPLAY_PROTOCOL_SPICE) {
+opts.u.spice.has_connected = !!connected;
+opts.u.spice.connected =
+qapi_enum_parse(&SetPasswordAction_lookup, connected,
+SET_PASSWORD_ACTION_KEEP, &err);
+if (err) {
+hmp_handle_error(mon, err);
+return;
+}
+}
+
+qmp_set_password(&opts, &err);
+g_free(opts.password);
+g_free(opts.u.vnc.display);
hmp_handle_error(mon, err);
}
@@ -1462,9 +1493,31 @@ void hmp_expire_password(Monitor *mon, const QDict
*qdict)
{
const char *protocol = qdict_get_str(qdict, "protocol");
const char *whenstr = qdict_get_str(qdict, "time");
+const char *display = qdict_get_try_str(qdict, "display");
Error *err = NULL;
+DisplayProtocol proto;
-qmp_expire_password(protocol, whenstr, &err);
+ExpirePasswordOptions opts = {
+.time
[PATCH v4 1/2] monitor/hmp: add support for flag argument with value
Adds support for the "-xS" parameter type, where "-x" denotes a flag
name and the "S" suffix indicates that this flag is supposed to take an
arbitrary string parameter.
These parameters are always optional, the entry in the qdict will be
omitted if the flag is not given.
Reviewed-by: Eric Blake
Signed-off-by: Stefan Reiter
---
monitor/hmp.c | 17 -
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/monitor/hmp.c b/monitor/hmp.c
index d50c3124e1..a32dce7a35 100644
--- a/monitor/hmp.c
+++ b/monitor/hmp.c
@@ -980,6 +980,7 @@ static QDict *monitor_parse_arguments(Monitor *mon,
{
const char *tmp = p;
int skip_key = 0;
+int ret;
/* option */
c = *typestr++;
@@ -1002,8 +1003,22 @@ static QDict *monitor_parse_arguments(Monitor *mon,
}
if (skip_key) {
p = tmp;
+} else if (*typestr == 'S') {
+/* has option with string value */
+typestr++;
+tmp = p++;
+while (qemu_isspace(*p)) {
+p++;
+}
+ret = get_str(buf, sizeof(buf), &p);
+if (ret < 0) {
+monitor_printf(mon, "%s: value expected for -%c\n",
+ cmd->name, *tmp);
+goto fail;
+}
+qdict_put_str(qdict, key, buf);
} else {
-/* has option */
+/* has boolean option */
p++;
qdict_put_bool(qdict, key, true);
}
--
2.30.2
Re: [PATCH] qemu-options: -chardev reconnect=seconds duplicated in help, tidy up
Le 28/09/2021 à 09:14, Markus Armbruster a écrit :
> Fixes: 5dd1f02b4bc2f2c2ef3a2adfd8a412c8c8769085
> Signed-off-by: Markus Armbruster
> ---
> qemu-options.hx | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 8f603cc7e6..27e7b9a77c 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -3188,7 +3188,7 @@ DEFHEADING(Character device options:)
> DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
> "-chardev help\n"
> "-chardev null,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> -"-chardev
> socket,id=id[,host=host],port=port[,to=to][,ipv4=on|off][,ipv6=on|off][,nodelay=on|off][,reconnect=seconds]\n"
> +"-chardev
> socket,id=id[,host=host],port=port[,to=to][,ipv4=on|off][,ipv6=on|off][,nodelay=on|off]\n"
> "
> [,server=on|off][,wait=on|off][,telnet=on|off][,websocket=on|off][,reconnect=seconds][,mux=on|off]\n"
> "
> [,logfile=PATH][,logappend=on|off][,tls-creds=ID][,tls-authz=ID] (tcp)\n"
> "-chardev
> socket,id=id,path=path[,server=on|off][,wait=on|off][,telnet=on|off][,websocket=on|off][,reconnect=seconds]\n"
>
Applied to my trivial-patches branch.
Thanks,
Laurent
Re: [PATCH] hw/arm/virt: Allow additions to the generated device tree
On Mon, 27 Sept 2021 at 21:12, Simon Glass wrote: > I think you are misunderstanding my patch and that may be the problem here. > > Where QEMU is provided with a dtb (-dtb) it uses that and passes it > on. This is absolutely fine and I have tested that this works well > with U-Boot. No issues. > > Where QEMU creates its own dtb on the fly the -dtb parameter is > actually ignored and there is no way to adjust what QEMU passes on, > without recompiling QEMU. It is quite inflexible, actually. Even just > creating a new device for development purposes is not possible. That > is the problem I am trying to solve. > > There is certainly no intent to combine two bits of dtb with my patch. > We could easily do that externally to QEMU. So what *is* this patch doing? The subject says "Allow additions to the generated device tree", and the patch adds an option to "Merge a device tree binary (dtb) file into the generated device tree". That sounds exactly like "combine two bits of dtb" -- the one the user provides to the -dtbi option and the one QEMU generates internally. > The only current working option is to just pass the U-Boot dtb through > and not use QEMU's on-the-fly-generated dtb at all. But I am assuming > there is a reason why QEMU generates that dtb, so that would not be > desirable? QEMU generates the dtb to tell the guest what hardware is present and where it is. We don't guarantee that that hardware arrangement is necessarily stable between QEMU versions (in practice there are generally additions and not deletions or moves, but in theory there might be). All the guest is supposed to assume is the location of RAM and flash; everything else it must look in the dtb to determine. > One more question...other than dtb, does QEMU typically add support > for data structures needed by particular projects or groups of > projects? It looks like dtb was supported for ARM Linux originally? I > am looking at supporting bloblist as a way of communicating > information between firmware (basically a simple way of packaging > multiple blobs). The answer here is essentially "no, as far as possible". We ideally would not have special case support for any particular guest code. We do have special handling for "directly boot the Linux kernel" for a combination of historical reasons (we've always supported it) and it being a massive use case. But even that is painful to maintain (it starts off seeming easy but gradually requires more weird tweaks and handling as CPU architectures evolve). I really strongly don't want to add additional categories of guests that QEMU has special case support for, which is the main driver behind why I'm so negative about this patchset. Guest software running on the "virt" board needs to know it is running on the "virt" board and deal with the interface and information that QEMU provides. (For instance, Arm Trusted Firmware and UEFI both have done this.) -- PMM
Re: [PATCH 3/3] hw/char: sifive_uart: Register device in 'input' category
On Sun, Sep 26, 2021 at 8:53 PM Bin Meng wrote:
>
> The category of sifive_uart device is not set. Put it into the
> 'input' category.
>
> Signed-off-by: Bin Meng
Thanks!
Applied to riscv-to-apply.next
Alistair
> ---
>
> hw/char/sifive_uart.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/char/sifive_uart.c b/hw/char/sifive_uart.c
> index 278e21c434..1c75f792b3 100644
> --- a/hw/char/sifive_uart.c
> +++ b/hw/char/sifive_uart.c
> @@ -248,6 +248,7 @@ static void sifive_uart_class_init(ObjectClass *oc, void
> *data)
> rc->phases.enter = sifive_uart_reset_enter;
> rc->phases.hold = sifive_uart_reset_hold;
> device_class_set_props(dc, sifive_uart_properties);
> +set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
> }
>
> static const TypeInfo sifive_uart_info = {
> --
> 2.25.1
>
>
Re: [PATCH v4 16/25] linux-user/nios2: Document non-use of setup_sigtramp
On Tue, 28 Sept 2021 at 03:00, Richard Henderson
wrote:
>
> Signed-off-by: Richard Henderson
> ---
> linux-user/nios2/target_signal.h | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/linux-user/nios2/target_signal.h
> b/linux-user/nios2/target_signal.h
> index aebf749f12..fe266c4c51 100644
> --- a/linux-user/nios2/target_signal.h
> +++ b/linux-user/nios2/target_signal.h
> @@ -19,4 +19,7 @@ typedef struct target_sigaltstack {
>
> #include "../generic/signal.h"
>
> +/* Nios2 uses a fixed address on the kuser page for sigreturn. */
> +#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 0
> +
Reviewed-by: Peter Maydell
thanks
-- PMM
[PATCH 2/2] qemu-options: Add missing "sockets=2" to CLI "-smp 2"
There is one example of -smp CLI in qemu-options.hx currently using "-smp 2" and assuming that there will be 2 sockets. However now the actually calculation logic of missing sockets and cores is not immutable, we should use more explicit CLIs like "-smp 2,sockets=2" if we want multiple sockets. Signed-off-by: Yanan Wang --- qemu-options.hx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qemu-options.hx b/qemu-options.hx index dcd9595650..ff8917c5e1 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -395,7 +395,7 @@ SRST -m 2G \ -object memory-backend-ram,size=1G,id=m0 \ -object memory-backend-ram,size=1G,id=m1 \ --smp 2 \ +-smp 2,sockets=2,maxcpus=2 \ -numa node,nodeid=0,memdev=m0 \ -numa node,nodeid=1,memdev=m1,initiator=0 \ -numa cpu,node-id=0,socket-id=0 \ -- 2.19.1
Re: [PATCH v2 2/5] meson_options.txt: Switch the default value for the vnc option to 'auto'
On 03/09/21 10:13, Thomas Huth wrote:
There is no reason why VNC should always be enabled and not be set to
the default value. We already switched the setting in the "configure"
script in commit 3a6a1256d4 ("configure: Allow vnc to get disabled with
--without-default-features"), so let's do that in meson_options.txt now,
too.
Signed-off-by: Thomas Huth
---
meson_options.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/meson_options.txt b/meson_options.txt
index a9a9b8f4c6..2c89e79e8b 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -120,7 +120,7 @@ option('usb_redir', type : 'feature', value : 'auto',
description: 'libusbredir support')
option('virglrenderer', type : 'feature', value : 'auto',
description: 'virgl rendering support')
-option('vnc', type : 'feature', value : 'enabled',
+option('vnc', type : 'feature', value : 'auto',
description: 'VNC server')
option('vnc_jpeg', type : 'feature', value : 'auto',
description: 'JPEG lossy compression for VNC server')
Queued this one for now, thanks.
Paolo
[PATCH 0/2] qemu-options: Trivial doc fixes related to -smp
There are two places found related to -smp that may need to be fixed when reading qemu-options.hx. And after searching other Doc files, there hasn't been any other similar issues. Yanan Wang (2): qemu-options: Tweak [,maxcpus=cpus] to [,maxcpus=maxcpus] qemu-options: Add missing "sockets=2" to CLI "-smp 2" qemu-options.hx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) -- 2.19.1
[PATCH 1/2] qemu-options: Tweak [, maxcpus=cpus] to [, maxcpus=maxcpus]
In qemu-option.hx, there is "-smp [[cpus=]n][,maxcpus=cpus]..." in the
DEF part, and "-smp [[cpus=]n][,maxcpus=maxcpus]..." in the RST part.
Obviously the later is right, let's fix the previous one.
Signed-off-by: Yanan Wang
---
qemu-options.hx | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qemu-options.hx b/qemu-options.hx
index 5c1b0311c0..dcd9595650 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -200,7 +200,7 @@ SRST
ERST
DEF("smp", HAS_ARG, QEMU_OPTION_smp,
-"-smp
[[cpus=]n][,maxcpus=cpus][,sockets=sockets][,dies=dies][,cores=cores][,threads=threads]\n"
+"-smp
[[cpus=]n][,maxcpus=maxcpus][,sockets=sockets][,dies=dies][,cores=cores][,threads=threads]\n"
"set the number of CPUs to 'n' [default=1]\n"
"maxcpus= maximum number of total CPUs, including\n"
"offline CPUs for hotplug, etc\n"
--
2.19.1
Re: [PATCH v4 05/25] linux-user/arm: Implement setup_sigtramp
On Tue, 28 Sept 2021 at 03:00, Richard Henderson
wrote:
>
> Mirror what the kernel does in arch/arm/kernel/signal.h,
> using the old sigframe struct in the rt sigframe struct.
>
> Update the trampoline code to match the kernel: this uses
> sp-relative accesses rather than pc-relative.
>
> Copy the code into frame->retcode from the trampoline page.
> This minimises the different cases wrt arm vs thumb vs fdpic.
>
> Signed-off-by: Richard Henderson
> @@ -225,44 +204,34 @@ setup_return(CPUARMState *env, struct target_sigaction
> *ka,
>
> if (ka->sa_flags & TARGET_SA_RESTORER) {
> if (is_fdpic) {
> -/* For FDPIC we ensure that the restorer is called with a
> - * correct r9 value. For that we need to write code on
> - * the stack that sets r9 and jumps back to restorer
> - * value.
> - */
> -if (thumb) {
> -__put_user(sigreturn_fdpic_thumb_codes[0], rc);
> -__put_user(sigreturn_fdpic_thumb_codes[1], rc + 1);
> -__put_user(sigreturn_fdpic_thumb_codes[2], rc + 2);
> -__put_user((abi_ulong)ka->sa_restorer, rc + 3);
> -} else {
> -__put_user(sigreturn_fdpic_codes[0], rc);
> -__put_user(sigreturn_fdpic_codes[1], rc + 1);
> -__put_user(sigreturn_fdpic_codes[2], rc + 2);
> -__put_user((abi_ulong)ka->sa_restorer, rc + 3);
> -}
> -
> -retcode = rc_addr + thumb;
> +__put_user((abi_ulong)ka->sa_restorer, &frame->retcode[3]);
> +retcode = (sigreturn_fdpic_tramp +
> + retcode_idx * RETCODE_BYTES + thumb);
Here 'retcode' is an interworking-PC value with the LSB indicating
Thumb mode...
> +copy_retcode = true;
> } else {
> retcode = ka->sa_restorer;
> +copy_retcode = false;
> }
> } else {
> -unsigned int idx = thumb;
> +retcode = default_sigreturn + retcode_idx * RETCODE_BYTES + thumb;
> +copy_retcode = true;
> +}
>
> -if (ka->sa_flags & TARGET_SA_SIGINFO) {
> -idx += 2;
> +/* Copy the code to the stack slot for ABI compatibility. */
> +if (copy_retcode) {
> +uint32_t *host_rc = g2h_untagged(retcode);
...but here we treat it as a normal guest address that we can
convert into a host address and dereference. If the signal handler
is being entered in Thumb mode this will be a misaligned pointer.
> +int i;
> +
> +for (i = 0; i < RETCODE_WORDS; ++i) {
> +__put_user(host_rc[i], &frame->retcode[i]);
> }
> -
> -__put_user(retcodes[idx], rc);
> -
> -retcode = rc_addr + thumb;
> }
>
> env->regs[0] = usig;
> if (is_fdpic) {
> env->regs[9] = handler_fdpic_GOT;
> }
> -env->regs[13] = frame_addr;
> +env->regs[13] = sp_addr;
> env->regs[14] = retcode;
> env->regs[15] = handler & (thumb ? ~1 : ~3);
> cpsr_write(env, cpsr, CPSR_IT | CPSR_T | CPSR_E, CPSRWriteByInstr);
Otherwise
Reviewed-by: Peter Maydell
thanks
-- PMM
Re: [PATCH v11 01/14] machine: Deprecate "parameter=0" SMP configurations
On Tue, Sep 28, 2021 at 11:57:42AM +0800, Yanan Wang wrote:
> In the SMP configuration, we should either provide a topology
> parameter with a reasonable value (greater than zero) or just
> omit it and QEMU will compute the missing value.
>
> The users shouldn't provide a configuration with any parameter
> of it specified as zero (e.g. -smp 8,sockets=0) which could
> possibly cause unexpected results in the -smp parsing. So we
> deprecate this kind of configurations since 6.2 by adding the
> explicit sanity check.
>
> Signed-off-by: Yanan Wang
> Reviewed-by: Cornelia Huck
> ---
> docs/about/deprecated.rst | 15 +++
> hw/core/machine.c | 14 ++
> qapi/machine.json | 2 +-
> qemu-options.hx | 12 +++-
> 4 files changed, 37 insertions(+), 6 deletions(-)
>
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index 3c2be84d80..97415dbecd 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -160,6 +160,21 @@ Use ``-display sdl`` instead.
>
> Use ``-display curses`` instead.
>
> +``-smp`` ("parameter=0" SMP configurations) (since 6.2)
> +'''
> +
> +Specified CPU topology parameters must be greater than zero.
> +
> +In the SMP configuration, users should either provide a CPU topology
> +parameter with a reasonable value (greater than zero) or just omit it
> +and QEMU will compute the missing value.
> +
> +However, historically it was implicitly allowed for users to provide
> +a parameter with zero value, which is meaningless and could also possibly
> +cause unexpected results in the -smp parsing. So support for this kind of
> +configurations (e.g. -smp 8,sockets=0) is deprecated since 6.2 and will
> +be removed in the near future, users have to ensure that all the topology
> +members described with -smp are greater than zero.
>
> Plugin argument passing through ``arg=`` (since 6.1)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 067f42b528..711e83db54 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -835,6 +835,20 @@ static void machine_set_smp(Object *obj, Visitor *v,
> const char *name,
> return;
> }
>
> +/*
> + * Specified CPU topology parameters must be greater than zero,
> + * explicit configuration like "cpus=0" is not allowed.
> + */
> +if ((config->has_cpus && config->cpus == 0) ||
> +(config->has_sockets && config->sockets == 0) ||
> +(config->has_dies && config->dies == 0) ||
> +(config->has_cores && config->cores == 0) ||
> +(config->has_threads && config->threads == 0) ||
> +(config->has_maxcpus && config->maxcpus == 0)) {
> +warn_report("Invalid CPU topology deprecated: "
> +"CPU topology parameters must be greater than zero");
> +}
> +
> mc->smp_parse(ms, config, errp);
> if (*errp) {
> goto out_free;
> diff --git a/qapi/machine.json b/qapi/machine.json
> index 32d47f4e35..227e75d8af 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -1331,7 +1331,7 @@
> #
> # @dies: number of dies per socket in the CPU topology
> #
> -# @cores: number of cores per thread in the CPU topology
> +# @cores: number of cores per die in the CPU topology
> #
> # @threads: number of threads per core in the CPU topology
> #
This change is unrelated to the rest of this commit.
It just looks like a simple bug fix and should just be
spun out into its own patch.
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 8f603cc7e6..91d859aa29 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -227,11 +227,13 @@ SRST
> of computing the CPU maximum count.
>
> Either the initial CPU count, or at least one of the topology parameters
> -must be specified. Values for any omitted parameters will be computed
> -from those which are given. Historically preference was given to the
> -coarsest topology parameters when computing missing values (ie sockets
> -preferred over cores, which were preferred over threads), however, this
> -behaviour is considered liable to change.
> +must be specified. The specified parameters must be greater than zero,
> +explicit configuration like "cpus=0" is not allowed. Values for any
> +omitted parameters will be computed from those which are given.
> +Historically preference was given to the coarsest topology parameters
> +when computing missing values (ie sockets preferred over cores, which
> +were preferred over threads), however, this behaviour is considered
> +liable to change.
> ERST
>
> DEF("numa", HAS_ARG, QEMU_OPTION_numa,
If you split the docs fix out into its own patch then you can add
Reviewed-by: Daniel P. Berrangé
to both this patch and the new patch.
Regards,
Daniel
--
|: https://berrange.com -o-https://w
Re: [PATCH v11 02/14] machine: Minor refactor/fix for the smp parsers
On Tue, Sep 28, 2021 at 11:57:43AM +0800, Yanan Wang wrote: > To pave the way for the functional improvement in later patches, > make some refactor/cleanup for the smp parsers, including using > local maxcpus instead of ms->smp.max_cpus in the calculation, > defaulting dies to 0 initially like other members, cleanup the > sanity check for dies. > > We actually also fix a hidden defect by avoiding directly using > the provided *zero value* in the calculation, which could cause > a segment fault (e.g. using dies=0 in the calculation). > > Signed-off-by: Yanan Wang > Reviewed-by: Andrew Jones > --- > hw/core/machine.c | 18 ++ > hw/i386/pc.c | 23 ++- > 2 files changed, 24 insertions(+), 17 deletions(-) Reviewed-by: Daniel P. Berrangé 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 v11 03/14] machine: Uniformly use maxcpus to calculate the omitted parameters
On Tue, Sep 28, 2021 at 11:57:44AM +0800, Yanan Wang wrote: > We are currently using maxcpus to calculate the omitted sockets > but using cpus to calculate the omitted cores/threads. This makes > cmdlines like: > -smp cpus=8,maxcpus=16 > -smp cpus=8,cores=4,maxcpus=16 > -smp cpus=8,threads=2,maxcpus=16 > work fine but the ones like: > -smp cpus=8,sockets=2,maxcpus=16 > -smp cpus=8,sockets=2,cores=4,maxcpus=16 > -smp cpus=8,sockets=2,threads=2,maxcpus=16 > break the sanity check. > > Since we require for a valid config that the product of "sockets * cores > * threads" should equal to the maxcpus, we should uniformly use maxcpus > to calculate their omitted values. > > Also the if-branch of "cpus == 0 || sockets == 0" was split into two > branches of "cpus == 0" and "sockets == 0" so that we can clearly read > that we are parsing the configuration with a preference on cpus over > sockets over cores over threads. > > Note: change in this patch won't affect any existing working cmdlines > but improves consistency and allows more incomplete configs to be valid. > > Signed-off-by: Yanan Wang > Reviewed-by: Andrew Jones > Reviewed-by: Pankaj Gupta > --- > hw/core/machine.c | 30 +++--- > hw/i386/pc.c | 30 +++--- > 2 files changed, 30 insertions(+), 30 deletions(-) Reviewed-by: Daniel P. Berrangé 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 v11 04/14] machine: Set the value of cpus to match maxcpus if it's omitted
On Tue, Sep 28, 2021 at 11:57:45AM +0800, Yanan Wang wrote: > Currently we directly calculate the omitted cpus based on the given > incomplete collection of parameters. This makes some cmdlines like: > -smp maxcpus=16 > -smp sockets=2,maxcpus=16 > -smp sockets=2,dies=2,maxcpus=16 > -smp sockets=2,cores=4,maxcpus=16 > not work. We should probably set the value of cpus to match maxcpus > if it's omitted, which will make above configs start to work. > > So the calculation logic of cpus/maxcpus after this patch will be: > When both maxcpus and cpus are omitted, maxcpus will be calculated > from the given parameters and cpus will be set equal to maxcpus. > When only one of maxcpus and cpus is given then the omitted one > will be set to its counterpart's value. Both maxcpus and cpus may > be specified, but maxcpus must be equal to or greater than cpus. > > Note: change in this patch won't affect any existing working cmdlines > but allows more incomplete configs to be valid. > > Signed-off-by: Yanan Wang > Reviewed-by: Andrew Jones > --- > hw/core/machine.c | 29 - > hw/i386/pc.c | 29 - > qemu-options.hx | 11 --- > 3 files changed, 40 insertions(+), 29 deletions(-) Reviewed-by: Daniel P. Berrangé 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 v11 05/14] machine: Improve the error reporting of smp parsing
On Tue, Sep 28, 2021 at 11:57:46AM +0800, Yanan Wang wrote: > We have two requirements for a valid SMP configuration: > the product of "sockets * cores * threads" must represent all the > possible cpus, i.e., max_cpus, and then must include the initially > present cpus, i.e., smp_cpus. > > So we only need to ensure 1) "sockets * cores * threads == maxcpus" > at first and then ensure 2) "maxcpus >= cpus". With a reasonable > order of the sanity check, we can simplify the error reporting code. > When reporting an error message we also report the exact value of > each topology member to make users easily see what's going on. > > Signed-off-by: Yanan Wang > Reviewed-by: Andrew Jones > Reviewed-by: Pankaj Gupta > --- > hw/core/machine.c | 22 +- > hw/i386/pc.c | 24 ++-- > 2 files changed, 19 insertions(+), 27 deletions(-) Reviewed-by: Daniel P. Berrangé 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 1/1] virtio-gpu: CONTEXT_INIT feature
On 28/09/21 07:13, Gerd Hoffmann wrote:
@@ -212,6 +212,8 @@ virtio_gpu_base_get_features(VirtIODevice *vdev, uint64_t
features,
features |= (1 << VIRTIO_GPU_F_RESOURCE_BLOB);
}
+features |= (1 << VIRTIO_GPU_F_CONTEXT_INIT);
This needs a config option, simliar to the other features. It is a
guest-visible change so we must be able to turn it off for live
migration compatibility reasons. We also need a compat property to
turn it off by default for 6.1 + older machine types.
Could you give me a hint on how to add this compat property?
+if (cc.context_init) {
+virgl_renderer_context_create_with_flags(cc.hdr.ctx_id,
+ cc.context_init,
+ cc.nlen,
+ cc.debug_name);
This requires a minimum virglrenderer version I guess?
Definitely, that is going to be >= 0.9.0
--- a/include/standard-headers/linux/virtio_gpu.h
+++ b/include/standard-headers/linux/virtio_gpu.h
Separate patch please.
Also use scripts/update-linux-headers.sh for this.
Well, then I believe we will need to wait for this patch series:
https://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg367531.html
Cheers,
Antonio
Re: [PATCH v11 06/14] qtest/numa-test: Use detailed -smp CLIs in pc_dynamic_cpu_cfg
On Tue, Sep 28, 2021 at 11:57:47AM +0800, Yanan Wang wrote: > Since commit 80d7835749 (qemu-options: rewrite help for -smp options), > the preference of sockets/cores in -smp parsing is considered liable > to change, and actually we are going to change it in a coming commit. > So it'll be more stable to use detailed -smp CLIs in testing if we > have strong dependency on the parsing results. > > pc_dynamic_cpu_cfg currently assumes/needs that there will be 2 CPU > sockets with "-smp 2". To avoid breaking the test because of parsing > logic change, now explicitly use "-smp 2,sockets=2". > > Cc: Paolo Bonzini > Cc: Igor Mammedov > Signed-off-by: Yanan Wang > Reviewed-by: Andrew Jones > --- > tests/qtest/numa-test.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Reviewed-by: Daniel P. Berrangé 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 v11 07/14] qtest/numa-test: Use detailed -smp CLIs in test_def_cpu_split
On Tue, Sep 28, 2021 at 11:57:48AM +0800, Yanan Wang wrote: > Since commit 80d7835749 (qemu-options: rewrite help for -smp options), > the preference of sockets/cores in -smp parsing is considered liable > to change, and actually we are going to change it in a coming commit. > So it'll be more stable to use detailed -smp CLIs in the testcases > that have strong dependency on the parsing results. > > Currently, test_def_cpu_split use "-smp 8" and will get 8 CPU sockets > based on current parsing rule. But if we change to prefer cores over > sockets we will get one CPU socket with 8 cores, and this testcase > will not get expected numa set by default on x86_64 (Ok on aarch64). > > So now explicitly use "-smp 8,sockets=8" to avoid affect from parsing > logic change. > > Cc: Paolo Bonzini > Cc: Igor Mammedov > Signed-off-by: Yanan Wang > Reviewed-by: Andrew Jones > --- > tests/qtest/numa-test.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Reviewed-by: Daniel P. Berrangé 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 v11 08/14] machine: Prefer cores over sockets in smp parsing since 6.2
On Tue, Sep 28, 2021 at 11:57:49AM +0800, Yanan Wang wrote: > In the real SMP hardware topology world, it's much more likely that > we have high cores-per-socket counts and few sockets totally. While > the current preference of sockets over cores in smp parsing results > in a virtual cpu topology with low cores-per-sockets counts and a > large number of sockets, which is just contrary to the real world. > > Given that it is better to make the virtual cpu topology be more > reflective of the real world and also for the sake of compatibility, > we start to prefer cores over sockets over threads in smp parsing > since machine type 6.2 for different arches. > > In this patch, a boolean "smp_prefer_sockets" is added, and we only > enable the old preference on older machines and enable the new one > since type 6.2 for all arches by using the machine compat mechanism. > > Suggested-by: Daniel P. Berrange > Signed-off-by: Yanan Wang > Acked-by: David Gibson > Acked-by: Cornelia Huck > Reviewed-by: Andrew Jones > Reviewed-by: Pankaj Gupta > --- > hw/arm/virt.c | 1 + > hw/core/machine.c | 35 ++- > hw/i386/pc.c | 35 ++- > hw/i386/pc_piix.c | 1 + > hw/i386/pc_q35.c | 1 + > hw/ppc/spapr.c | 1 + > hw/s390x/s390-virtio-ccw.c | 1 + > include/hw/boards.h| 1 + > qemu-options.hx| 3 ++- > 9 files changed, 60 insertions(+), 19 deletions(-) Reviewed-by: Daniel P. Berrangé 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 v11 10/14] machine: Tweak the order of topology members in struct CpuTopology
On Tue, Sep 28, 2021 at 11:57:51AM +0800, Yanan Wang wrote: > Now that all the possible topology parameters are integrated in struct > CpuTopology, tweak the order of topology members to be "cpus/sockets/ > dies/cores/threads/maxcpus" for readability and consistency. We also > tweak the comment by adding explanation of dies parameter. > > Signed-off-by: Yanan Wang > Reviewed-by: Andrew Jones > Reviewed-by: Pankaj Gupta > --- > hw/core/machine.c | 8 > include/hw/boards.h | 7 --- > 2 files changed, 8 insertions(+), 7 deletions(-) Reviewed-by: Daniel P. Berrangé 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 v11 09/14] machine: Use ms instead of global current_machine in sanity-check
On Tue, Sep 28, 2021 at 11:57:50AM +0800, Yanan Wang wrote: > In the sanity-check of smp_cpus and max_cpus against mc in function > machine_set_smp(), we are now using ms->smp.max_cpus for the check > but using current_machine->smp.max_cpus in the error message. > Tweak this by uniformly using the local ms. > > Signed-off-by: Yanan Wang > Reviewed-by: Andrew Jones > Reviewed-by: Pankaj Gupta > Reviewed-by: Cornelia Huck > --- > hw/core/machine.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Daniel P. Berrangé 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 v11 12/14] machine: Remove smp_parse callback from MachineClass
On Tue, Sep 28, 2021 at 11:57:53AM +0800, Yanan Wang wrote: > Now we have a generic smp parser for all arches, and there will > not be any other arch specific ones, so let's remove the callback > from MachineClass and call the parser directly. > > Signed-off-by: Yanan Wang > Reviewed-by: Andrew Jones > --- > hw/core/machine.c | 3 +-- > include/hw/boards.h | 5 - > 2 files changed, 1 insertion(+), 7 deletions(-) Reviewed-by: Daniel P. Berrangé 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 v11 13/14] machine: Move smp_prefer_sockets to struct SMPCompatProps
On Tue, Sep 28, 2021 at 11:57:54AM +0800, Yanan Wang wrote: > Now we have a common structure SMPCompatProps used to store information > about SMP compatibility stuff, so we can also move smp_prefer_sockets > there for cleaner code. > > No functional change intended. > > Signed-off-by: Yanan Wang > Acked-by: David Gibson > Reviewed-by: Andrew Jones > --- > hw/arm/virt.c | 2 +- > hw/core/machine.c | 2 +- > hw/i386/pc_piix.c | 2 +- > hw/i386/pc_q35.c | 2 +- > hw/ppc/spapr.c | 2 +- > hw/s390x/s390-virtio-ccw.c | 2 +- > include/hw/boards.h| 3 ++- > 7 files changed, 8 insertions(+), 7 deletions(-) Reviewed-by: Daniel P. Berrangé 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 v11 11/14] machine: Make smp_parse generic enough for all arches
On Tue, Sep 28, 2021 at 11:57:52AM +0800, Yanan Wang wrote: > Currently the only difference between smp_parse and pc_smp_parse > is the support of dies parameter and the related error reporting. > With some arch compat variables like "bool dies_supported", we can > make smp_parse generic enough for all arches and the PC specific > one can be removed. > > Making smp_parse() generic enough can reduce code duplication and > ease the code maintenance, and also allows extending the topology > with more arch specific members (e.g., clusters) in the future. > > Suggested-by: Andrew Jones > Suggested-by: Daniel P. Berrange > Signed-off-by: Yanan Wang > --- > hw/core/machine.c | 91 +++-- > hw/i386/pc.c| 84 + > include/hw/boards.h | 9 + > 3 files changed, 81 insertions(+), 103 deletions(-) Reviewed-by: Daniel P. Berrangé 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 v2 0/6] Add vmnet.framework based network backend
ping https://patchew.org/QEMU/20210831192720.33406-1-yaroshchuk2...@gmail.com/ вт, 31 авг. 2021 г. в 22:27, Vladislav Yaroshchuk : > macOS provides networking API for VMs called vmnet.framework. > I tried to add it as a network backend. All three modes are supported: > > -shared: > allows the guest to comminicate with other guests in shared mode and > also with external network (Internet) via NAT > > -host: > allows the guest to communicate with other guests in host mode > > -bridged: > bridges the guest with a physical network interface > > Separate netdev for each vmnet mode was created because they use quite > different settings, especially since macOS 11.0 when vmnet.framework > gets a lot of updates. > > Not sure that I use qemu_mutex_lock_iothread() and > qemu_mutex_unlock_iothread() in correct way while sending packet > from vmnet interface to QEMU. I'll be happy to receive > recomendations how to make this thing better if I done sth wrong. > > Also vmnet.framework requires com.apple.vm.networking entitlement to > run without root priveledges. Ad-hoc signing does not fit there, > so I didn't touch anything related to signing. As a result we should > run qemu-system by a priviledged user: > `$ sudo qemu-system-x86_64 -nic vmnet-shared` > otherwise vmnet fails with 'general failure'. > > But in any way it seems working now, > I tested it within qemu-system-x86-64 on macOS 10.15.7 host, with nic > models: > - e1000-82545em > - virtio-net-pci > > and having such guests: > - macOS 10.15.7 > - Ubuntu Bionic (server cloudimg) > > v1: > Since v1 minor typos were fixed, patches rebased onto latest master, > redudant > changes removed (small commits squashed) > > Vladislav Yaroshchuk (6): > net/vmnet: dependencies setup, initial preparations > net/vmnet: create common netdev state structure > net/vmnet: implement shared mode (vmnet-shared) > net/vmnet: implement host mode (vmnet-host) > net/vmnet: implement bridged mode (vmnet-bridged) > net/vmnet: update qemu-options.hx > > configure | 31 + > meson.build | 5 + > net/clients.h | 11 ++ > net/meson.build | 7 ++ > net/net.c | 10 ++ > net/vmnet-bridged.m | 123 ++ > net/vmnet-common.m | 294 > net/vmnet-host.c| 93 ++ > net/vmnet-shared.c | 94 ++ > net/vmnet_int.h | 48 > qapi/net.json | 99 ++- > qemu-options.hx | 17 +++ > 12 files changed, 830 insertions(+), 2 deletions(-) > create mode 100644 net/vmnet-bridged.m > create mode 100644 net/vmnet-common.m > create mode 100644 net/vmnet-host.c > create mode 100644 net/vmnet-shared.c > create mode 100644 net/vmnet_int.h > > -- > 2.23.0 > >
Re: [PATCH v3] hw/sensor: Add lsm303dlhc magnetometer device
Hi Peter,
On Mon, 27 Sept 2021 at 18:39, Peter Maydell
wrote:
> I thought we'd agreed to implement the whole of the auto-increment
> logic, not just for specific registers ?
>
Thanks again for the feedback. Dealing with one register value at a time
(versus a buffer of response values) does simplify the code flow.
The following code appears to handle multi-byte reads correctly. I just
wanted to confirm this is what you were looking for before moving on to
the test code?
/*
* Callback handler whenever a 'I2C_START_RECV' (read) event is received.
*/
static void lsm303dlhc_mag_read(LSM303DLHCMagState *s)
{
/*
* Set the LOCK bit whenever a new read attempt is made. This will be
* cleared in I2C_FINISH. Note that DRDY is always set to 1 in this
driver.
*/
s->sr = 0x3;
}
/*
* Callback handler whenever a 'I2C_FINISH' event is received.
*/
static void lsm303dlhc_mag_finish(LSM303DLHCMagState *s)
{
/*
* Clear the LOCK bit when the read attempt terminates.
* This bit is initially set in the I2C_START_RECV handler.
*/
s->sr = 0x1;
}
/*
* Low-level slave-to-master transaction handler (read attempts).
*/
static uint8_t lsm303dlhc_mag_recv(I2CSlave *i2c)
{
LSM303DLHCMagState *s = LSM303DLHC_MAG(i2c);
switch (s->pointer) {
case LSM303DLHC_MAG_REG_CRA:
s->buf = s->cra;
break;
case LSM303DLHC_MAG_REG_CRB:
s->buf = s->crb;
break;
case LSM303DLHC_MAG_REG_MR:
s->buf = s->mr;
break;
case LSM303DLHC_MAG_REG_OUT_X_H:
s->buf = (uint8_t)(s->x >> 8);
break;
case LSM303DLHC_MAG_REG_OUT_X_L:
s->buf = (uint8_t)(s->x);
break;
case LSM303DLHC_MAG_REG_OUT_Z_H:
s->buf = (uint8_t)(s->z >> 8);
break;
case LSM303DLHC_MAG_REG_OUT_Z_L:
s->buf = (uint8_t)(s->z);
break;
case LSM303DLHC_MAG_REG_OUT_Y_H:
s->buf = (uint8_t)(s->y >> 8);
break;
case LSM303DLHC_MAG_REG_OUT_Y_L:
s->buf = (uint8_t)(s->y);
break;
case LSM303DLHC_MAG_REG_SR:
s->buf = s->sr;
break;
case LSM303DLHC_MAG_REG_IRA:
s->buf = s->ira;
break;
case LSM303DLHC_MAG_REG_IRB:
s->buf = s->irb;
break;
case LSM303DLHC_MAG_REG_IRC:
s->buf = s->irc;
break;
case LSM303DLHC_MAG_REG_TEMP_OUT_H:
/* Check if the temperature sensor is enabled or not (CRA & 0x80).
*/
if (s->cra & 0x80) {
s->buf = (uint8_t)(s->temperature >> 8);
} else {
s->buf = 0;
}
break;
case LSM303DLHC_MAG_REG_TEMP_OUT_L:
if (s->cra & 0x80) {
s->buf = (uint8_t)(s->temperature & 0xf0);
} else {
s->buf = 0;
}
break;
default:
s->buf = 0;
break;
}
/*
* The address pointer on the LSM303DLHC auto-increments whenever a byte
* is read, without the master device having to request the next
address.
*
* The auto-increment process has the following logic:
*
* - if (s->pointer == 8) then s->pointer = 3
* - else: if (s->pointer >= 12) then s->pointer = 0
* - else: s->pointer += 1
*
* Reading an invalid address return 0.
*/
if (s->pointer == LSM303DLHC_MAG_REG_OUT_Y_L) {
s->pointer = LSM303DLHC_MAG_REG_OUT_X_H;
} else if (s->pointer >= LSM303DLHC_MAG_REG_IRC) {
s->pointer = LSM303DLHC_MAG_REG_CRA;
} else {
s->pointer++;
}
return s->buf;
}
Re: [PATCH v11 14/14] machine: Put all sanity-check in the generic SMP parser
On Tue, Sep 28, 2021 at 11:57:55AM +0800, Yanan Wang wrote: > Put both sanity-check of the input SMP configuration and sanity-check > of the output SMP configuration uniformly in the generic parser. Then > machine_set_smp() will become cleaner, also all the invalid scenarios > can be tested only by calling the parser. > > Signed-off-by: Yanan Wang > Reviewed-by: Andrew Jones > Reviewed-by: Pankaj Gupta > --- > hw/core/machine.c | 63 +++ > 1 file changed, 31 insertions(+), 32 deletions(-) Reviewed-by: Daniel P. Berrangé 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 1/2] qemu-options: Tweak [, maxcpus=cpus] to [, maxcpus=maxcpus]
On 9/28/21 11:31, Yanan Wang wrote:
In qemu-option.hx, there is "-smp [[cpus=]n][,maxcpus=cpus]..." in the
DEF part, and "-smp [[cpus=]n][,maxcpus=maxcpus]..." in the RST part.
Obviously the later is right, let's fix the previous one.
Signed-off-by: Yanan Wang
---
Reviewed-by: Damien Hedde
qemu-options.hx | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qemu-options.hx b/qemu-options.hx
index 5c1b0311c0..dcd9595650 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -200,7 +200,7 @@ SRST
ERST
DEF("smp", HAS_ARG, QEMU_OPTION_smp,
-"-smp
[[cpus=]n][,maxcpus=cpus][,sockets=sockets][,dies=dies][,cores=cores][,threads=threads]\n"
+"-smp
[[cpus=]n][,maxcpus=maxcpus][,sockets=sockets][,dies=dies][,cores=cores][,threads=threads]\n"
"set the number of CPUs to 'n' [default=1]\n"
"maxcpus= maximum number of total CPUs, including\n"
"offline CPUs for hotplug, etc\n"
Re: [PATCH 2/2] qemu-options: Add missing "sockets=2" to CLI "-smp 2"
On 9/28/21 11:31, Yanan Wang wrote: > There is one example of -smp CLI in qemu-options.hx currently > using "-smp 2" and assuming that there will be 2 sockets. > However now the actually calculation logic of missing sockets > and cores is not immutable, we should use more explicit CLIs > like "-smp 2,sockets=2" if we want multiple sockets. > > Signed-off-by: Yanan Wang > --- > qemu-options.hx | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/qemu-options.hx b/qemu-options.hx > index dcd9595650..ff8917c5e1 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -395,7 +395,7 @@ SRST > -m 2G \ > -object memory-backend-ram,size=1G,id=m0 \ > -object memory-backend-ram,size=1G,id=m1 \ > --smp 2 \ > +-smp 2,sockets=2,maxcpus=2 \ Is the addition of "maxcpus=2" intentional? > -numa node,nodeid=0,memdev=m0 \ > -numa node,nodeid=1,memdev=m1,initiator=0 \ > -numa cpu,node-id=0,socket-id=0 \ >
Re: [PATCH v11 05/14] machine: Improve the error reporting of smp parsing
On 9/28/21 05:57, Yanan Wang wrote: > We have two requirements for a valid SMP configuration: > the product of "sockets * cores * threads" must represent all the > possible cpus, i.e., max_cpus, and then must include the initially > present cpus, i.e., smp_cpus. > > So we only need to ensure 1) "sockets * cores * threads == maxcpus" > at first and then ensure 2) "maxcpus >= cpus". With a reasonable > order of the sanity check, we can simplify the error reporting code. > When reporting an error message we also report the exact value of > each topology member to make users easily see what's going on. > > Signed-off-by: Yanan Wang > Reviewed-by: Andrew Jones > Reviewed-by: Pankaj Gupta > --- > hw/core/machine.c | 22 +- > hw/i386/pc.c | 24 ++-- > 2 files changed, 19 insertions(+), 27 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v11 09/14] machine: Use ms instead of global current_machine in sanity-check
On 9/28/21 05:57, Yanan Wang wrote: > In the sanity-check of smp_cpus and max_cpus against mc in function > machine_set_smp(), we are now using ms->smp.max_cpus for the check > but using current_machine->smp.max_cpus in the error message. > Tweak this by uniformly using the local ms. > > Signed-off-by: Yanan Wang > Reviewed-by: Andrew Jones > Reviewed-by: Pankaj Gupta > Reviewed-by: Cornelia Huck > --- > hw/core/machine.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v11 06/14] qtest/numa-test: Use detailed -smp CLIs in pc_dynamic_cpu_cfg
On 9/28/21 05:57, Yanan Wang wrote: > Since commit 80d7835749 (qemu-options: rewrite help for -smp options), > the preference of sockets/cores in -smp parsing is considered liable > to change, and actually we are going to change it in a coming commit. > So it'll be more stable to use detailed -smp CLIs in testing if we > have strong dependency on the parsing results. > > pc_dynamic_cpu_cfg currently assumes/needs that there will be 2 CPU > sockets with "-smp 2". To avoid breaking the test because of parsing > logic change, now explicitly use "-smp 2,sockets=2". > > Cc: Paolo Bonzini > Cc: Igor Mammedov > Signed-off-by: Yanan Wang > Reviewed-by: Andrew Jones > --- > tests/qtest/numa-test.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v11 07/14] qtest/numa-test: Use detailed -smp CLIs in test_def_cpu_split
On 9/28/21 05:57, Yanan Wang wrote: > Since commit 80d7835749 (qemu-options: rewrite help for -smp options), > the preference of sockets/cores in -smp parsing is considered liable > to change, and actually we are going to change it in a coming commit. > So it'll be more stable to use detailed -smp CLIs in the testcases > that have strong dependency on the parsing results. > > Currently, test_def_cpu_split use "-smp 8" and will get 8 CPU sockets > based on current parsing rule. But if we change to prefer cores over > sockets we will get one CPU socket with 8 cores, and this testcase > will not get expected numa set by default on x86_64 (Ok on aarch64). > > So now explicitly use "-smp 8,sockets=8" to avoid affect from parsing > logic change. > > Cc: Paolo Bonzini > Cc: Igor Mammedov > Signed-off-by: Yanan Wang > Reviewed-by: Andrew Jones > --- > tests/qtest/numa-test.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 1/1] hw: aspeed_gpio: Fix pin I/O type declarations
On 9/28/21 05:24, p...@fb.com wrote:
From: Peter Delevoryas
Some of the pin declarations in the Aspeed GPIO module were incorrect,
probably because of confusion over which bits in the input and output
uint32_t's correspond to which groups in the label array. Since the
uint32_t literals are in big endian, it's sort of the opposite of what
would be intuitive. The least significant bit in ast2500_set_props[6]
corresponds to GPIOY0, not GPIOAB7.
GPIOxx indicates input and output capabilities, GPIxx indicates only
input, GPOxx indicates only output.
AST2500:
- Previously had GPIW0..GPIW7 and GPIX0..GPIX7, that's correct.
- Previously had GPIOY0..GPIOY3, should have been GPIOY0..GPIOY7.
- Previously had GPIOAB0..GPIOAB3 and GPIAB4..GPIAB7, should only have
been GPIOAB0..GPIOAB3.
AST2600:
- GPIOT0..GPIOT7 should have been GPIT0..GPIT7.
- GPIOU0..GPIOU7 should have been GPIU0..GPIU7.
- GPIW0..GPIW7 should have been GPIOW0..GPIOW7.
- GPIOY0..GPIOY7 and GPIOZ0...GPIOZ7 were disabled.
Fixes: 4b7f956862dc2db4c5c ("hw/gpio: Add basic Aspeed GPIO model for AST2400 and
AST2500")
Fixes: 36d737ee82b2972167e ("hw/gpio: Add in AST2600 specific implementation")
Signed-off-by: Peter Delevoryas
Reviewed-by: Damien Hedde
---
hw/gpio/aspeed_gpio.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
index dfa6d6cb40..33a40a624a 100644
--- a/hw/gpio/aspeed_gpio.c
+++ b/hw/gpio/aspeed_gpio.c
@@ -796,7 +796,7 @@ static const GPIOSetProperties ast2500_set_props[] = {
[3] = {0x, 0x, {"M", "N", "O", "P"} },
[4] = {0x, 0x, {"Q", "R", "S", "T"} },
[5] = {0x, 0x, {"U", "V", "W", "X"} },
-[6] = {0xff0f, 0x0f0f, {"Y", "Z", "AA", "AB"} },
+[6] = {0x0fff, 0x0fff, {"Y", "Z", "AA", "AB"} },
[7] = {0x00ff, 0x00ff, {"AC"} },
};
@@ -805,9 +805,9 @@ static GPIOSetProperties ast2600_3_3v_set_props[] = {
[1] = {0x, 0x, {"E", "F", "G", "H"} },
[2] = {0x, 0x, {"I", "J", "K", "L"} },
[3] = {0x, 0x, {"M", "N", "O", "P"} },
-[4] = {0x, 0x, {"Q", "R", "S", "T"} },
-[5] = {0x, 0x, {"U", "V", "W", "X"} },
-[6] = {0x, 0x0fff, {"Y", "Z", "", ""} },
+[4] = {0x, 0x00ff, {"Q", "R", "S", "T"} },
+[5] = {0x, 0xff00, {"U", "V", "W", "X"} },
+[6] = {0x, 0x, {"Y", "Z"} },
};
static GPIOSetProperties ast2600_1_8v_set_props[] = {
Re: [PATCH v11 11/14] machine: Make smp_parse generic enough for all arches
On 9/28/21 05:57, Yanan Wang wrote: > Currently the only difference between smp_parse and pc_smp_parse > is the support of dies parameter and the related error reporting. > With some arch compat variables like "bool dies_supported", we can > make smp_parse generic enough for all arches and the PC specific > one can be removed. > > Making smp_parse() generic enough can reduce code duplication and > ease the code maintenance, and also allows extending the topology > with more arch specific members (e.g., clusters) in the future. > > Suggested-by: Andrew Jones > Suggested-by: Daniel P. Berrange > Signed-off-by: Yanan Wang > --- > hw/core/machine.c | 91 +++-- > hw/i386/pc.c| 84 + > include/hw/boards.h | 9 + > 3 files changed, 81 insertions(+), 103 deletions(-) > +/* > + * smp_parse - Generic function used to parse the given SMP configuration > + * > + * Any missing parameter in "cpus/maxcpus/sockets/cores/threads" will be > + * automatically computed based on the provided ones. > + * > + * In the calculation of omitted sockets/cores/threads: we prefer sockets > + * over cores over threads before 6.2, while preferring cores over sockets > + * over threads since 6.2. > + * > + * In the calculation of cpus/maxcpus: When both maxcpus and cpus are > omitted, > + * maxcpus will be computed from the given parameters and cpus will be set > + * equal to maxcpus. When only one of maxcpus and cpus is given then the > + * omitted one will be set to its given counterpart's value. Both maxcpus and > + * cpus may be specified, but maxcpus must be equal to or greater than cpus. > + * > + * For compatibility, apart from the parameters that will be computed, newly > + * introduced topology members which are likely to be target specific should > + * be directly set as 1 if they are omitted (e.g. dies for PC since 4.1). > + */ > static void smp_parse(MachineState *ms, SMPConfiguration *config, Error > **errp) Can we have it return a boolean instead?
Re: [PATCH 2/2] qemu-options: Add missing "sockets=2" to CLI "-smp 2"
On 2021/9/28 18:46, Philippe Mathieu-Daudé wrote: On 9/28/21 11:31, Yanan Wang wrote: There is one example of -smp CLI in qemu-options.hx currently using "-smp 2" and assuming that there will be 2 sockets. However now the actually calculation logic of missing sockets and cores is not immutable, we should use more explicit CLIs like "-smp 2,sockets=2" if we want multiple sockets. Signed-off-by: Yanan Wang --- qemu-options.hx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qemu-options.hx b/qemu-options.hx index dcd9595650..ff8917c5e1 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -395,7 +395,7 @@ SRST -m 2G \ -object memory-backend-ram,size=1G,id=m0 \ -object memory-backend-ram,size=1G,id=m1 \ --smp 2 \ +-smp 2,sockets=2,maxcpus=2 \ Is the addition of "maxcpus=2" intentional? Yes, but it's not necessary IMO. I just wanted to keep consistency with other numa config examples in the Doc. Should I remove it ? Thanks, Yanan -numa node,nodeid=0,memdev=m0 \ -numa node,nodeid=1,memdev=m1,initiator=0 \ -numa cpu,node-id=0,socket-id=0 \ .
Re: [PATCH v11 10/14] machine: Tweak the order of topology members in struct CpuTopology
On 9/28/21 05:57, Yanan Wang wrote: > Now that all the possible topology parameters are integrated in struct > CpuTopology, tweak the order of topology members to be "cpus/sockets/ > dies/cores/threads/maxcpus" for readability and consistency. We also > tweak the comment by adding explanation of dies parameter. > > Signed-off-by: Yanan Wang > Reviewed-by: Andrew Jones > Reviewed-by: Pankaj Gupta > --- > hw/core/machine.c | 8 > include/hw/boards.h | 7 --- > 2 files changed, 8 insertions(+), 7 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v11 11/14] machine: Make smp_parse generic enough for all arches
On Tue, Sep 28, 2021 at 12:57:21PM +0200, Philippe Mathieu-Daudé wrote: > On 9/28/21 05:57, Yanan Wang wrote: > > Currently the only difference between smp_parse and pc_smp_parse > > is the support of dies parameter and the related error reporting. > > With some arch compat variables like "bool dies_supported", we can > > make smp_parse generic enough for all arches and the PC specific > > one can be removed. > > > > Making smp_parse() generic enough can reduce code duplication and > > ease the code maintenance, and also allows extending the topology > > with more arch specific members (e.g., clusters) in the future. > > > > Suggested-by: Andrew Jones > > Suggested-by: Daniel P. Berrange > > Signed-off-by: Yanan Wang > > --- > > hw/core/machine.c | 91 +++-- > > hw/i386/pc.c| 84 + > > include/hw/boards.h | 9 + > > 3 files changed, 81 insertions(+), 103 deletions(-) > > > +/* > > + * smp_parse - Generic function used to parse the given SMP configuration > > + * > > + * Any missing parameter in "cpus/maxcpus/sockets/cores/threads" will be > > + * automatically computed based on the provided ones. > > + * > > + * In the calculation of omitted sockets/cores/threads: we prefer sockets > > + * over cores over threads before 6.2, while preferring cores over sockets > > + * over threads since 6.2. > > + * > > + * In the calculation of cpus/maxcpus: When both maxcpus and cpus are > > omitted, > > + * maxcpus will be computed from the given parameters and cpus will be set > > + * equal to maxcpus. When only one of maxcpus and cpus is given then the > > + * omitted one will be set to its given counterpart's value. Both maxcpus > > and > > + * cpus may be specified, but maxcpus must be equal to or greater than > > cpus. > > + * > > + * For compatibility, apart from the parameters that will be computed, > > newly > > + * introduced topology members which are likely to be target specific > > should > > + * be directly set as 1 if they are omitted (e.g. dies for PC since 4.1). > > + */ > > static void smp_parse(MachineState *ms, SMPConfiguration *config, Error > > **errp) > > Can we have it return a boolean instead? That's unrelated to this change, so ought to be a separate commit if done. 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 v11 12/14] machine: Remove smp_parse callback from MachineClass
On 9/28/21 05:57, Yanan Wang wrote:
> Now we have a generic smp parser for all arches, and there will
> not be any other arch specific ones, so let's remove the callback
> from MachineClass and call the parser directly.
>
> Signed-off-by: Yanan Wang
> Reviewed-by: Andrew Jones
> ---
> hw/core/machine.c | 3 +--
> include/hw/boards.h | 5 -
> 2 files changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index f5dadcbd78..23f77201eb 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -918,7 +918,7 @@ static void machine_set_smp(Object *obj, Visitor *v,
> const char *name,
> "CPU topology parameters must be greater than zero");
> }
>
> -mc->smp_parse(ms, config, errp);
> +smp_parse(ms, config, errp);
> if (*errp) {
If smp_parse() were to return a boolean, this would become:
if (!smp_parse(ms, config, errp)) {
Regardless:
Reviewed-by: Philippe Mathieu-Daudé
> goto out_free;
> }
Re: [PATCH v11 14/14] machine: Put all sanity-check in the generic SMP parser
On 9/28/21 05:57, Yanan Wang wrote:
> Put both sanity-check of the input SMP configuration and sanity-check
> of the output SMP configuration uniformly in the generic parser. Then
> machine_set_smp() will become cleaner, also all the invalid scenarios
> can be tested only by calling the parser.
>
> Signed-off-by: Yanan Wang
> Reviewed-by: Andrew Jones
> Reviewed-by: Pankaj Gupta
> ---
> hw/core/machine.c | 63 +++
> 1 file changed, 31 insertions(+), 32 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index e2a48aa18c..637acd8d42 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -798,6 +798,20 @@ static void smp_parse(MachineState *ms, SMPConfiguration
> *config, Error **errp)
> unsigned threads = config->has_threads ? config->threads : 0;
> unsigned maxcpus = config->has_maxcpus ? config->maxcpus : 0;
>
> +/*
> + * Specified CPU topology parameters must be greater than zero,
> + * explicit configuration like "cpus=0" is not allowed.
> + */
> +if ((config->has_cpus && config->cpus == 0) ||
> +(config->has_sockets && config->sockets == 0) ||
> +(config->has_dies && config->dies == 0) ||
> +(config->has_cores && config->cores == 0) ||
> +(config->has_threads && config->threads == 0) ||
> +(config->has_maxcpus && config->maxcpus == 0)) {
> +warn_report("Invalid CPU topology deprecated: "
Maybe:
"Deprecated CPU topology: "
or
"Deprecated CPU topology (considered invalid): "
> +"CPU topology parameters must be greater than zero");
> +}
Re: [PATCH 2/2] qemu-options: Add missing "sockets=2" to CLI "-smp 2"
On Tue, Sep 28, 2021 at 06:58:20PM +0800, wangyanan (Y) wrote: > > On 2021/9/28 18:46, Philippe Mathieu-Daudé wrote: > > On 9/28/21 11:31, Yanan Wang wrote: > > > There is one example of -smp CLI in qemu-options.hx currently > > > using "-smp 2" and assuming that there will be 2 sockets. > > > However now the actually calculation logic of missing sockets > > > and cores is not immutable, we should use more explicit CLIs > > > like "-smp 2,sockets=2" if we want multiple sockets. > > > > > > Signed-off-by: Yanan Wang > > > --- > > > qemu-options.hx | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/qemu-options.hx b/qemu-options.hx > > > index dcd9595650..ff8917c5e1 100644 > > > --- a/qemu-options.hx > > > +++ b/qemu-options.hx > > > @@ -395,7 +395,7 @@ SRST > > > -m 2G \ > > > -object memory-backend-ram,size=1G,id=m0 \ > > > -object memory-backend-ram,size=1G,id=m1 \ > > > --smp 2 \ > > > +-smp 2,sockets=2,maxcpus=2 \ > > Is the addition of "maxcpus=2" intentional? > Yes, but it's not necessary IMO. I just wanted to keep consistency > with other numa config examples in the Doc. Should I remove it ? I think it makes sense to be explicit, because the numa config works in terms of maxcpus when splitting cpus between nodes 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 v11 11/14] machine: Make smp_parse generic enough for all arches
On 9/28/21 12:58, Daniel P. Berrangé wrote: > On Tue, Sep 28, 2021 at 12:57:21PM +0200, Philippe Mathieu-Daudé wrote: >> On 9/28/21 05:57, Yanan Wang wrote: >>> Currently the only difference between smp_parse and pc_smp_parse >>> is the support of dies parameter and the related error reporting. >>> With some arch compat variables like "bool dies_supported", we can >>> make smp_parse generic enough for all arches and the PC specific >>> one can be removed. >>> >>> Making smp_parse() generic enough can reduce code duplication and >>> ease the code maintenance, and also allows extending the topology >>> with more arch specific members (e.g., clusters) in the future. >>> >>> Suggested-by: Andrew Jones >>> Suggested-by: Daniel P. Berrange >>> Signed-off-by: Yanan Wang >>> --- >>> hw/core/machine.c | 91 +++-- >>> hw/i386/pc.c| 84 + >>> include/hw/boards.h | 9 + >>> 3 files changed, 81 insertions(+), 103 deletions(-) >> >>> +/* >>> + * smp_parse - Generic function used to parse the given SMP configuration >>> + * >>> + * Any missing parameter in "cpus/maxcpus/sockets/cores/threads" will be >>> + * automatically computed based on the provided ones. >>> + * >>> + * In the calculation of omitted sockets/cores/threads: we prefer sockets >>> + * over cores over threads before 6.2, while preferring cores over sockets >>> + * over threads since 6.2. >>> + * >>> + * In the calculation of cpus/maxcpus: When both maxcpus and cpus are >>> omitted, >>> + * maxcpus will be computed from the given parameters and cpus will be set >>> + * equal to maxcpus. When only one of maxcpus and cpus is given then the >>> + * omitted one will be set to its given counterpart's value. Both maxcpus >>> and >>> + * cpus may be specified, but maxcpus must be equal to or greater than >>> cpus. >>> + * >>> + * For compatibility, apart from the parameters that will be computed, >>> newly >>> + * introduced topology members which are likely to be target specific >>> should >>> + * be directly set as 1 if they are omitted (e.g. dies for PC since 4.1). >>> + */ >>> static void smp_parse(MachineState *ms, SMPConfiguration *config, Error >>> **errp) >> >> Can we have it return a boolean instead? > > That's unrelated to this change, so ought to be a separate commit if > done. Sure, fine by me.
Re: [PATCH v11 11/14] machine: Make smp_parse generic enough for all arches
On 2021/9/28 18:58, Daniel P. Berrangé wrote: On Tue, Sep 28, 2021 at 12:57:21PM +0200, Philippe Mathieu-Daudé wrote: On 9/28/21 05:57, Yanan Wang wrote: Currently the only difference between smp_parse and pc_smp_parse is the support of dies parameter and the related error reporting. With some arch compat variables like "bool dies_supported", we can make smp_parse generic enough for all arches and the PC specific one can be removed. Making smp_parse() generic enough can reduce code duplication and ease the code maintenance, and also allows extending the topology with more arch specific members (e.g., clusters) in the future. Suggested-by: Andrew Jones Suggested-by: Daniel P. Berrange Signed-off-by: Yanan Wang --- hw/core/machine.c | 91 +++-- hw/i386/pc.c| 84 + include/hw/boards.h | 9 + 3 files changed, 81 insertions(+), 103 deletions(-) +/* + * smp_parse - Generic function used to parse the given SMP configuration + * + * Any missing parameter in "cpus/maxcpus/sockets/cores/threads" will be + * automatically computed based on the provided ones. + * + * In the calculation of omitted sockets/cores/threads: we prefer sockets + * over cores over threads before 6.2, while preferring cores over sockets + * over threads since 6.2. + * + * In the calculation of cpus/maxcpus: When both maxcpus and cpus are omitted, + * maxcpus will be computed from the given parameters and cpus will be set + * equal to maxcpus. When only one of maxcpus and cpus is given then the + * omitted one will be set to its given counterpart's value. Both maxcpus and + * cpus may be specified, but maxcpus must be equal to or greater than cpus. + * + * For compatibility, apart from the parameters that will be computed, newly + * introduced topology members which are likely to be target specific should + * be directly set as 1 if they are omitted (e.g. dies for PC since 4.1). + */ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp) Can we have it return a boolean instead? That's unrelated to this change, so ought to be a separate commit if done. I agree. I vaguely remember that there was a discussion about this before with Paolo. But anyway, I think the suggested change can be in a separate commit if necessary. :) Thanks, Yanan
Re: [PATCH v11 01/14] machine: Deprecate "parameter=0" SMP configurations
On 2021/9/28 17:58, Daniel P. Berrangé wrote:
On Tue, Sep 28, 2021 at 11:57:42AM +0800, Yanan Wang wrote:
In the SMP configuration, we should either provide a topology
parameter with a reasonable value (greater than zero) or just
omit it and QEMU will compute the missing value.
The users shouldn't provide a configuration with any parameter
of it specified as zero (e.g. -smp 8,sockets=0) which could
possibly cause unexpected results in the -smp parsing. So we
deprecate this kind of configurations since 6.2 by adding the
explicit sanity check.
Signed-off-by: Yanan Wang
Reviewed-by: Cornelia Huck
---
docs/about/deprecated.rst | 15 +++
hw/core/machine.c | 14 ++
qapi/machine.json | 2 +-
qemu-options.hx | 12 +++-
4 files changed, 37 insertions(+), 6 deletions(-)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 3c2be84d80..97415dbecd 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -160,6 +160,21 @@ Use ``-display sdl`` instead.
Use ``-display curses`` instead.
+``-smp`` ("parameter=0" SMP configurations) (since 6.2)
+'''
+
+Specified CPU topology parameters must be greater than zero.
+
+In the SMP configuration, users should either provide a CPU topology
+parameter with a reasonable value (greater than zero) or just omit it
+and QEMU will compute the missing value.
+
+However, historically it was implicitly allowed for users to provide
+a parameter with zero value, which is meaningless and could also possibly
+cause unexpected results in the -smp parsing. So support for this kind of
+configurations (e.g. -smp 8,sockets=0) is deprecated since 6.2 and will
+be removed in the near future, users have to ensure that all the topology
+members described with -smp are greater than zero.
Plugin argument passing through ``arg=`` (since 6.1)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 067f42b528..711e83db54 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -835,6 +835,20 @@ static void machine_set_smp(Object *obj, Visitor *v, const
char *name,
return;
}
+/*
+ * Specified CPU topology parameters must be greater than zero,
+ * explicit configuration like "cpus=0" is not allowed.
+ */
+if ((config->has_cpus && config->cpus == 0) ||
+(config->has_sockets && config->sockets == 0) ||
+(config->has_dies && config->dies == 0) ||
+(config->has_cores && config->cores == 0) ||
+(config->has_threads && config->threads == 0) ||
+(config->has_maxcpus && config->maxcpus == 0)) {
+warn_report("Invalid CPU topology deprecated: "
+"CPU topology parameters must be greater than zero");
+}
+
mc->smp_parse(ms, config, errp);
if (*errp) {
goto out_free;
diff --git a/qapi/machine.json b/qapi/machine.json
index 32d47f4e35..227e75d8af 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1331,7 +1331,7 @@
#
# @dies: number of dies per socket in the CPU topology
#
-# @cores: number of cores per thread in the CPU topology
+# @cores: number of cores per die in the CPU topology
#
# @threads: number of threads per core in the CPU topology
#
This change is unrelated to the rest of this commit.
It just looks like a simple bug fix and should just be
spun out into its own patch.
diff --git a/qemu-options.hx b/qemu-options.hx
index 8f603cc7e6..91d859aa29 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -227,11 +227,13 @@ SRST
of computing the CPU maximum count.
Either the initial CPU count, or at least one of the topology parameters
-must be specified. Values for any omitted parameters will be computed
-from those which are given. Historically preference was given to the
-coarsest topology parameters when computing missing values (ie sockets
-preferred over cores, which were preferred over threads), however, this
-behaviour is considered liable to change.
+must be specified. The specified parameters must be greater than zero,
+explicit configuration like "cpus=0" is not allowed. Values for any
+omitted parameters will be computed from those which are given.
+Historically preference was given to the coarsest topology parameters
+when computing missing values (ie sockets preferred over cores, which
+were preferred over threads), however, this behaviour is considered
+liable to change.
ERST
DEF("numa", HAS_ARG, QEMU_OPTION_numa,
If you split the docs fix out into its own patch then you can add
Ok, I will split it out. Thanks for the review of this series.
Thanks,
Yanan
Reviewed-by: Daniel P. Berrangé
to both this patch and the new patch.
Regards,
Daniel
Re: [PATCH v11 14/14] machine: Put all sanity-check in the generic SMP parser
On 2021/9/28 19:01, Philippe Mathieu-Daudé wrote:
On 9/28/21 05:57, Yanan Wang wrote:
Put both sanity-check of the input SMP configuration and sanity-check
of the output SMP configuration uniformly in the generic parser. Then
machine_set_smp() will become cleaner, also all the invalid scenarios
can be tested only by calling the parser.
Signed-off-by: Yanan Wang
Reviewed-by: Andrew Jones
Reviewed-by: Pankaj Gupta
---
hw/core/machine.c | 63 +++
1 file changed, 31 insertions(+), 32 deletions(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index e2a48aa18c..637acd8d42 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -798,6 +798,20 @@ static void smp_parse(MachineState *ms, SMPConfiguration
*config, Error **errp)
unsigned threads = config->has_threads ? config->threads : 0;
unsigned maxcpus = config->has_maxcpus ? config->maxcpus : 0;
+/*
+ * Specified CPU topology parameters must be greater than zero,
+ * explicit configuration like "cpus=0" is not allowed.
+ */
+if ((config->has_cpus && config->cpus == 0) ||
+(config->has_sockets && config->sockets == 0) ||
+(config->has_dies && config->dies == 0) ||
+(config->has_cores && config->cores == 0) ||
+(config->has_threads && config->threads == 0) ||
+(config->has_maxcpus && config->maxcpus == 0)) {
+warn_report("Invalid CPU topology deprecated: "
Maybe:
"Deprecated CPU topology: "
or
"Deprecated CPU topology (considered invalid): "
Ok, I will chose the second one which I think is clearer.
Thanks,
Yanan
+"CPU topology parameters must be greater than zero");
+}
.
[PATCH] migration/ram: Avoid throttling VM in the first iteration
In some cases for large size VM, memory_global_dirty_log_sync() and
ramblock_sync_dirty_bitmap() combined can take more than 1 sec. As a
result diff of end_time and rs->time_last_bitmap_sync can be more
than 1 sec even when migration_bitmap_sync is called the first time,
setting up the throttle in the first iteration itself. When
migration_bitmap_sync is called the first-time num_dirty_pages_period
is equal to VM total pages and ram_counters.transferred is zero which
forces VM to throttle to 99 from migration start. So we should always
check if dirty_sync_count is greater than 1 before trying throttling.
Signed-off-by: manish.mishra
---
migration/ram.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 7a43bfd7af..9ba1c8b235 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1006,8 +1006,12 @@ static void migration_bitmap_sync(RAMState *rs)
end_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
-/* more than 1 second = 1000 millisecons */
-if (end_time > rs->time_last_bitmap_sync + 1000) {
+/*
+ * more than 1 second = 1000 millisecons
+ * Avoid throttling VM in the first iteration of live migration.
+ */
+if (end_time > rs->time_last_bitmap_sync + 1000 &&
+ram_counters.dirty_sync_count > 1) {
migration_trigger_throttle(rs);
migration_update_rates(rs, end_time);
--
2.22.3
Re: [PATCH 10/29] tcg_funcs: Add tlb_flush to TCGModuleOps
On Thu, Sep 02, 2021 at 03:09:15PM +0200, Richard Henderson wrote:
> On 8/31/21 2:15 PM, Gerd Hoffmann wrote:
> > diff --git a/target/i386/helper.c b/target/i386/helper.c
> > index 533b29cb91b6..100add713c5d 100644
> > --- a/target/i386/helper.c
> > +++ b/target/i386/helper.c
> > @@ -103,7 +103,7 @@ void x86_cpu_set_a20(X86CPU *cpu, int a20_state)
> > /* when a20 is changed, all the MMU mappings are invalid, so
> > we must flush everything */
> > -tlb_flush(cs);
> > +tcg.tlb_flush(cs);
>
> I think this is a mistake.
>
> (1) If tcg module is not enabled, we should be able to make a direct call.
>
> So IMO we want to retain the direct function call syntax in all the uses. I
> think you want to put wrapper functions doing the indirection somewhere --
> possibly tcg-module.c.
Hmm, when we want avoid indirection I guess it makes sense to use inline
wrappers in tcg-module.h
How about the patch below? (proof-of-concept for tlb_flush, on top of
this series)?
thanks,
Gerd
>From 22f5a216f410fccb769d0f7496c3c36f4b131833 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann
Date: Tue, 28 Sep 2021 11:51:12 +0200
Subject: [PATCH] tlb_flush inline wrapper
---
include/exec/exec-all.h | 2 ++
include/tcg/tcg-module.h | 13 +
meson.build | 3 ++-
3 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 608d768a4371..72e4e3b5bb89 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -160,7 +160,9 @@ void tlb_flush_page_all_cpus_synced(CPUState *src,
target_ulong addr);
* so this is generally safe. If more selective flushing is required
* use one of the other functions for efficiency.
*/
+#ifdef TCG_DIRECT_CALL
void tlb_flush(CPUState *cpu);
+#endif
/**
* tlb_flush_all_cpus:
* @cpu: src CPU of the flush
diff --git a/include/tcg/tcg-module.h b/include/tcg/tcg-module.h
index e9c0615b51d9..b3c0f53ea9f3 100644
--- a/include/tcg/tcg-module.h
+++ b/include/tcg/tcg-module.h
@@ -1,6 +1,10 @@
#ifndef TCG_MODULE_H
#define TCG_MODULE_H
+#if defined(CONFIG_TCG_BUILTIN) || defined(TCG_MODULE)
+# define TCG_DIRECT_CALL 1
+#endif
+
#include "exec/exec-all.h"
struct TCGModuleOps {
@@ -21,4 +25,13 @@ struct TCGModuleOps {
};
extern struct TCGModuleOps tcg;
+#ifndef TCG_DIRECT_CALL
+
+static inline void tlb_flush(CPUState *cpu)
+{
+tcg.tlb_flush(cpu);
+}
+
+#endif
+
#endif /* TCG_MODULE_H */
diff --git a/meson.build b/meson.build
index 15ef4d3c4187..afe07e7d59c3 100644
--- a/meson.build
+++ b/meson.build
@@ -2317,8 +2317,9 @@ subdir('tests/qtest/libqos')
subdir('tests/qtest/fuzz')
# accel modules
+tcg_module_cflags = declare_dependency(compile_args: '-DTCG_MODULE=1')
tcg_real_module_ss = ss.source_set()
-tcg_real_module_ss.add_all(when: 'CONFIG_TCG_MODULAR', if_true: tcg_module_ss)
+tcg_real_module_ss.add_all(when: ['CONFIG_TCG_MODULAR', tcg_module_cflags],
if_true: tcg_module_ss)
specific_ss.add_all(when: 'CONFIG_TCG_BUILTIN', if_true: tcg_module_ss)
target_modules += { 'accel' : { 'qtest': qtest_module_ss,
'tcg': tcg_real_module_ss }}
--
2.31.1
Re: [RFC PATCH 00/10] security: Introduce qemu_security_policy_taint() API
On Tuesday, 14 September, 2021, 07:00:27 pm IST, P J P
wrote:
>* Thanks so much for restarting this thread. I've been at it intermittently
>last few
> months, thinking about how could we annotate the source/module objects.
>
> -> [*] https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg04642.html
>
>* Last time we discussed about having both compile and run time options for
>users
> to be able to select the qualified objects/backends/devices as desired.
>
>* To confirm: How/Where is the security policy defined? Is it device/module
>specific OR QEMU project wide?
>
>>> it feels like we need
>> 'secure': 'bool'
>
>* Though we started the (above [*]) discussion with '--security' option in
>mind,
> I wonder if 'secure' annotation is much specific. And if we could widen its
> scope.
>
>
>Source annotations: I've been thinking over following approaches
>===
>
>1) Segregate the QEMU sources under
>
> ../staging/ <= devel/experimental, not for production usage
> ../src/ <= good for production usage, hence security relevant
> ../deprecated/ <= Bad for production usage, not security relevant
>
> - This is similar to Linux staging drivers' tree.
> - Staging drivers are not considered for production usage and hence CVE
> allocation.
> - At build time by default we only build sources under ../src/ tree.
> - But we can still have options to build /staging/ and /deprecated/ trees.
> - It's readily understandable to end users.
>
>2) pkgconfig(1) way:
> - If we could define per device/backend a configuration (.pc) file which is
> then used
> at build/run time to decide which sources are suitable for the build or
> usage.
>
> - I'm trying to experiment with this.
>
>3) We annotate QEMU devices/backends/modules with macros which define its
>status.
> It can then be used at build/run times to decide if it's suitable for usage.
> For ex:
>
> $ cat annotsrc.h
>
> #include
>
> enum SRCSTATUS {
> DEVEL = 0,
> STAGING,
> PRODUCTION,
> DEPRECATED
> };
>
...
>
>
>* Approach 3) above is similar to the _security_policy_taint() API,
> but works at the source/object file level, rather than specific 'struct
> type' field.
>
>* Does adding a field to struct type (ex. DeviceClass) scale to all
>objects/modules/backends etc?
> Does it have any limitations to include/cover other sources/objects?
>
>* I'd really appreciate your feedback/inputs/suggestions.
Ping...!?
---
-P J P
http://feedmug.com
Re: QAPI sync meeting
Hi! On Mon, Sep 27, 2021 at 8:55 PM John Snow wrote: > Hiya, > > I'd like to propose that at least the three of us arrange a time to have a > meeting where we discuss our plans and ideas for QAPI going forward, > including rust, python, and golang extensions to the QAPI generator, what > we hope to accomplish with those projects, and so on. > > What I am hoping to get out of this for myself is a high-level overview of > people's plans for QAPI and to produce some notes on those plans so that I > can have a reference that we've all acknowledged as roughly accurate to be > able to keep the community's design goals for QAPI in mind as I continue my > own development. Ultimately, I'd like some kind of rough draft of a "QAPI > roadmap". > > I know there was a rust meetup during KVM Forum, but I was unable to > attend due to the timing. I'd like to expand the focus a little more > broadly to QAPI in general and discuss our "personal" roadmaps, goals, > queued work, etc so that we can collaboratively formulate a broader vision > of our work. > > I'm posting to qemu-devel in case anyone else has an interest in this area > and would like to eavesdrop or share opinions, but we should probably come > up with an agenda first. So: > > Proposed agenda: > > Current projects, wishlists, and goals for QAPI: > - Markus (~10 min) > - Marc-Andre (~10 min) (Rust, dbus, etc?) > The QAPI Rust binding RFC series aims to provide the QAPI types to Rust with to/from C translations. This is just one brick allowing QEMU to have some parts written in Rust: all other internal/subsystem binding pieces remain to be done. I don't have other plans for QAPI at this point. D-Bus (from the early qapi/rust series) was an experiment, showing that QAPI/QMP can be exposed via "serde" with almost no effort. (it could most likely be over other protocols, such as JSON, in full-Rust implementation). We can imagine sharing canonical QAPI IDLs for daemons/helpers written in different languages. However, the biggest hurdle when binding QAPI to D-Bus or many programming languages (c, python, go and rust foremost) is that it is not a machine/ABI-friendly IDL. QAPI doesn't impose orderdering of fields or arguments, and it is not versioned. I believe this is detrimental, because bindings have to be written and maintained by hand in various languages, with various flavours (some may add abstractions, some may version the schema themself, some may use plain dictionaries everywhere etc). The small flexibility advantage doesn't outweigh the cost. Imho, some of the pains of QAPI/QMP is that it's not so easy to interact with, either from a human point of view (who likes speaking JSON?) or a machine point of view (I don't have good bindings to use from my language of choice). If we provided (and generated) idiomatic client bindings, we would most likely have a few rules to not break them, and end up versionizing the schema (version the commands, version arguments etc, various options are possible). The wire format becomes a detail, and JSON can keep its own flexibility wrt fields ordering etc. - jsnow (~10 min) (Python, golang, etc) > > I am certainly interested to learn your updated plans. > Formulating short-term and long-term roadmaps: > - Open discussion, ~30 min > - Collaboratively produce a summary doc (etherpad?) outlining major work > to be done, separated into near and long terms > - Upload this summary to the QEMU wiki and mail it back out to qemu-devel > - We probably won't exactly finish this bit, but we can resume on the > mailing list afterwards perfectly well. > > (Feel free to propose anything different for the meeting, this is just a > jumping off point for discussion.) > > Proposed time: > > - Any weekday after 13:00 UTC. Wednesdays, Thursdays and Fridays work > particularly well for me at the moment. > That could work for me. - bluejeans and google meeting both work well for me. Open to alternatives. > > > Thanks, > --js >
Re: [PATCH 2/2] qemu-options: Add missing "sockets=2" to CLI "-smp 2"
On 2021/9/28 19:01, Daniel P. Berrangé wrote: On Tue, Sep 28, 2021 at 06:58:20PM +0800, wangyanan (Y) wrote: On 2021/9/28 18:46, Philippe Mathieu-Daudé wrote: On 9/28/21 11:31, Yanan Wang wrote: There is one example of -smp CLI in qemu-options.hx currently using "-smp 2" and assuming that there will be 2 sockets. However now the actually calculation logic of missing sockets and cores is not immutable, we should use more explicit CLIs like "-smp 2,sockets=2" if we want multiple sockets. Signed-off-by: Yanan Wang --- qemu-options.hx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qemu-options.hx b/qemu-options.hx index dcd9595650..ff8917c5e1 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -395,7 +395,7 @@ SRST -m 2G \ -object memory-backend-ram,size=1G,id=m0 \ -object memory-backend-ram,size=1G,id=m1 \ --smp 2 \ +-smp 2,sockets=2,maxcpus=2 \ Is the addition of "maxcpus=2" intentional? Yes, but it's not necessary IMO. I just wanted to keep consistency with other numa config examples in the Doc. Should I remove it ? I think it makes sense to be explicit, because the numa config works in terms of maxcpus when splitting cpus between nodes The statement of this numa config example actually assume that there will be 2 cpus totally. Although based on behavior of the smp parser we will get maxcpus=2, I also tend to keep it explicitly. But I should update subject of this patch and the commit message. Thanks, Yanan
Re: [PATCH 09/29] tcg/module: add tcg-module.[ch] infrastructure
On 8/31/21 14:15, Gerd Hoffmann wrote:
> Add TCGModuleOps struct, empty for now, followup patches will fill it.
> This struct has pointers for tcg functions which are called by core
> qemu.
>
> The struct is initialized (at compile time) with pointers to stub
> functions. When the tcg module loads it will update the function
> pointers to point to the real functions instead.
>
> Signed-off-by: Gerd Hoffmann
> ---
> include/tcg/tcg-module.h | 8
> accel/tcg/tcg-module.c | 5 +
> accel/tcg/meson.build| 4
> 3 files changed, 17 insertions(+)
> create mode 100644 include/tcg/tcg-module.h
> create mode 100644 accel/tcg/tcg-module.c
>
> diff --git a/include/tcg/tcg-module.h b/include/tcg/tcg-module.h
> new file mode 100644
> index ..7e87aecb2357
> --- /dev/null
> +++ b/include/tcg/tcg-module.h
> @@ -0,0 +1,8 @@
> +#ifndef TCG_MODULE_H
> +#define TCG_MODULE_H
> +
> +struct TCGModuleOps {
> +};
> +extern struct TCGModuleOps tcg;
TCG functions taking a CPUState argument should reside in
CPUClass's AccelCPUClass/TCGCPUOps, not a global struct.
Re: [PATCH 2/2] qemu-options: Add missing "sockets=2" to CLI "-smp 2"
On 9/28/21 13:42, wangyanan (Y) wrote: > > On 2021/9/28 19:01, Daniel P. Berrangé wrote: >> On Tue, Sep 28, 2021 at 06:58:20PM +0800, wangyanan (Y) wrote: >>> On 2021/9/28 18:46, Philippe Mathieu-Daudé wrote: On 9/28/21 11:31, Yanan Wang wrote: > There is one example of -smp CLI in qemu-options.hx currently > using "-smp 2" and assuming that there will be 2 sockets. > However now the actually calculation logic of missing sockets > and cores is not immutable, we should use more explicit CLIs > like "-smp 2,sockets=2" if we want multiple sockets. > > Signed-off-by: Yanan Wang > --- > qemu-options.hx | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/qemu-options.hx b/qemu-options.hx > index dcd9595650..ff8917c5e1 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -395,7 +395,7 @@ SRST > -m 2G \ > -object memory-backend-ram,size=1G,id=m0 \ > -object memory-backend-ram,size=1G,id=m1 \ > - -smp 2 \ > + -smp 2,sockets=2,maxcpus=2 \ Is the addition of "maxcpus=2" intentional? >>> Yes, but it's not necessary IMO. I just wanted to keep consistency >>> with other numa config examples in the Doc. Should I remove it ? >> I think it makes sense to be explicit, because the numa config >> works in terms of maxcpus when splitting cpus between nodes > The statement of this numa config example actually assume that > there will be 2 cpus totally. Although based on behavior of the > smp parser we will get maxcpus=2, I also tend to keep it explicitly. > > But I should update subject of this patch and the commit message. Once updated: Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 0/2] 9pfs: iounit cleanup
On Montag, 27. September 2021 17:44:59 CEST Christian Schoenebeck wrote: > Two pure refactoring code cleanup patches regarding iounit calculation, no > behaviour change. > > Christian Schoenebeck (2): > 9pfs: deduplicate iounit code > 9pfs: simplify blksize_to_iounit() > > hw/9pfs/9p.c | 41 +++-- > 1 file changed, 19 insertions(+), 22 deletions(-) Queued on 9p.next: https://github.com/cschoenebeck/qemu/commits/9p.next Thanks! Best regards, Christian Schoenebeck
Re: [PATCH 1/1] virtio-gpu: CONTEXT_INIT feature
Hi,
> > This needs a config option, simliar to the other features. It is a
> > guest-visible change so we must be able to turn it off for live
> > migration compatibility reasons. We also need a compat property to
> > turn it off by default for 6.1 + older machine types.
>
> Could you give me a hint on how to add this compat property?
No need to do that for now, see below. But "git log" or "git blame"
should find the patches doing the same for the other features).
> > > +if (cc.context_init) {
> > > +virgl_renderer_context_create_with_flags(cc.hdr.ctx_id,
> > > + cc.context_init,
> > > + cc.nlen,
> > > + cc.debug_name);
> >
> > This requires a minimum virglrenderer version I guess?
>
> Definitely, that is going to be >= 0.9.0
... because we can hardly enable that by default if it isn't even
released. We'll need #ifdefs so qemu continues to build with older
virglrenderer versions for a while. It also must stay disabled by
default so you don't get different qemu behavior depending on the
version compiled against.
Then, in 1-2 years, when distributions have picked up the new version,
we can consider to raise the minimal required version to 0.9.0 and flip
the default to enabled.
> > > --- a/include/standard-headers/linux/virtio_gpu.h
> > > +++ b/include/standard-headers/linux/virtio_gpu.h
> >
> > Separate patch please.
> > Also use scripts/update-linux-headers.sh for this.
> Well, then I believe we will need to wait for this patch series:
>
> https://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg367531.html
Ah, right. Too much stuff on my todo list :(
take care,
Gerd
Re: [PATCH v3] hw/sensor: Add lsm303dlhc magnetometer device
On Tue, 28 Sept 2021 at 11:36, Kevin Townsend wrote:
>
> Hi Peter,
>
> On Mon, 27 Sept 2021 at 18:39, Peter Maydell wrote:
>>
>> I thought we'd agreed to implement the whole of the auto-increment
>> logic, not just for specific registers ?
>
>
> Thanks again for the feedback. Dealing with one register value at a time
> (versus a buffer of response values) does simplify the code flow.
>
> The following code appears to handle multi-byte reads correctly. I just
> wanted to confirm this is what you were looking for before moving on to
> the test code?
>
> /*
> * Callback handler whenever a 'I2C_START_RECV' (read) event is received.
> */
> static void lsm303dlhc_mag_read(LSM303DLHCMagState *s)
> {
> /*
> * Set the LOCK bit whenever a new read attempt is made. This will be
> * cleared in I2C_FINISH. Note that DRDY is always set to 1 in this
> driver.
> */
> s->sr = 0x3;
> }
>
> /*
> * Callback handler whenever a 'I2C_FINISH' event is received.
> */
> static void lsm303dlhc_mag_finish(LSM303DLHCMagState *s)
> {
> /*
> * Clear the LOCK bit when the read attempt terminates.
> * This bit is initially set in the I2C_START_RECV handler.
> */
> s->sr = 0x1;
> }
I would just inline these in the event lsm303dlhc_mag_event()
function. You might also #define some constants for the
register bits.
>
> /*
> * Low-level slave-to-master transaction handler (read attempts).
> */
> static uint8_t lsm303dlhc_mag_recv(I2CSlave *i2c)
> {
> LSM303DLHCMagState *s = LSM303DLHC_MAG(i2c);
>
> switch (s->pointer) {
> case LSM303DLHC_MAG_REG_CRA:
> s->buf = s->cra;
> break;
> case LSM303DLHC_MAG_REG_CRB:
> s->buf = s->crb;
> break;
> case LSM303DLHC_MAG_REG_MR:
> s->buf = s->mr;
> break;
> case LSM303DLHC_MAG_REG_OUT_X_H:
> s->buf = (uint8_t)(s->x >> 8);
> break;
> case LSM303DLHC_MAG_REG_OUT_X_L:
> s->buf = (uint8_t)(s->x);
> break;
> case LSM303DLHC_MAG_REG_OUT_Z_H:
> s->buf = (uint8_t)(s->z >> 8);
> break;
> case LSM303DLHC_MAG_REG_OUT_Z_L:
> s->buf = (uint8_t)(s->z);
> break;
> case LSM303DLHC_MAG_REG_OUT_Y_H:
> s->buf = (uint8_t)(s->y >> 8);
> break;
> case LSM303DLHC_MAG_REG_OUT_Y_L:
> s->buf = (uint8_t)(s->y);
> break;
> case LSM303DLHC_MAG_REG_SR:
> s->buf = s->sr;
> break;
> case LSM303DLHC_MAG_REG_IRA:
> s->buf = s->ira;
> break;
> case LSM303DLHC_MAG_REG_IRB:
> s->buf = s->irb;
> break;
> case LSM303DLHC_MAG_REG_IRC:
> s->buf = s->irc;
> break;
> case LSM303DLHC_MAG_REG_TEMP_OUT_H:
> /* Check if the temperature sensor is enabled or not (CRA & 0x80). */
> if (s->cra & 0x80) {
> s->buf = (uint8_t)(s->temperature >> 8);
> } else {
> s->buf = 0;
> }
> break;
> case LSM303DLHC_MAG_REG_TEMP_OUT_L:
> if (s->cra & 0x80) {
> s->buf = (uint8_t)(s->temperature & 0xf0);
> } else {
> s->buf = 0;
> }
> break;
> default:
> s->buf = 0;
> break;
> }
>
> /*
> * The address pointer on the LSM303DLHC auto-increments whenever a byte
> * is read, without the master device having to request the next address.
> *
> * The auto-increment process has the following logic:
> *
> * - if (s->pointer == 8) then s->pointer = 3
> * - else: if (s->pointer >= 12) then s->pointer = 0
> * - else: s->pointer += 1
> *
> * Reading an invalid address return 0.
> */
> if (s->pointer == LSM303DLHC_MAG_REG_OUT_Y_L) {
> s->pointer = LSM303DLHC_MAG_REG_OUT_X_H;
> } else if (s->pointer >= LSM303DLHC_MAG_REG_IRC) {
> s->pointer = LSM303DLHC_MAG_REG_CRA;
> } else {
> s->pointer++;
> }
>
> return s->buf;
I think you don't need to write the value to s->buf, you can just
use a local variable and return that. Nothing should be able to read
the value back out of s->buf later. I think you should also implement
the actual lock part, to avoid wrong values in the case of
* read starts, reads X_H
* s->x updated via the QOM property setter
* read continues, reads X_L
Basically just capture x,y,z,temp at the point of lock, and then
return those values in the recv function.
> }
thanks
-- PMM
Re: [PULL v2 00/22] Integration testing patches for 2021-09-27
On Mon, 27 Sept 2021 at 18:43, Philippe Mathieu-Daudé wrote: > > The following changes since commit de8ed1055c2ce18c95f597eb10df360dcb534f99: > > Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2021-09-25-v2' > into staging (2021-09-27 15:03:42 +0100) > > are available in the Git repository at: > > https://github.com/philmd/qemu.git tags/integration-testing-20210927 > > for you to fetch changes up to 4c5fc0c5fc496c147adb15536e4ac808feccf2cf: > > tests/acceptance: Test powernv machines (2021-09-27 19:21:37 +0200) > > > Integration testing patches > > - More Linux kernel record/replay tests (Pavel Dovgalyuk) > - Various fixes (Willian Rampazzo, Cleber Rosa) > - Split machine_ppc.py per machine (David Gibson) > - Add AVOCADO_TESTS command line environment variable (Willian Rampazzo) > - Test PowerPC PowerNV 8/9 machines (Cédric Le Goater) > > v2: > - Added missing test from Cédric. Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/6.2 for any user-visible changes. -- PMM
[PATCH v2 2/2] qemu-options: Add missing "sockets=2, maxcpus=2" to CLI "-smp 2"
There is one numa config example in qemu-options.hx currently using "-smp 2" and assuming that there will be 2 sockets and 2 cpus totally. However now the actual calculation logic of missing sockets and cores is not immutable and is considered liable to change. Although we will get maxcpus=2 finally based on current parser, it's always stable to specify it explicitly. So "-smp 2,sockets=2,maxcpus=2" will be optimal when we expect multiple sockets and 2 cpus totally. Signed-off-by: Yanan Wang Reviewed-by: Philippe Mathieu-Daude --- qemu-options.hx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qemu-options.hx b/qemu-options.hx index dcd9595650..ff8917c5e1 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -395,7 +395,7 @@ SRST -m 2G \ -object memory-backend-ram,size=1G,id=m0 \ -object memory-backend-ram,size=1G,id=m1 \ --smp 2 \ +-smp 2,sockets=2,maxcpus=2 \ -numa node,nodeid=0,memdev=m0 \ -numa node,nodeid=1,memdev=m1,initiator=0 \ -numa cpu,node-id=0,socket-id=0 \ -- 2.19.1
[PATCH v2 1/2] qemu-options: Tweak [, maxcpus=cpus] to [, maxcpus=maxcpus]
In qemu-option.hx, there is "-smp [[cpus=]n][,maxcpus=cpus]..." in the
DEF part, and "-smp [[cpus=]n][,maxcpus=maxcpus]..." in the RST part.
Obviously the later is right, let's fix the previous one.
Signed-off-by: Yanan Wang
Reviewed-by: Damien Hedde
---
qemu-options.hx | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qemu-options.hx b/qemu-options.hx
index 5c1b0311c0..dcd9595650 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -200,7 +200,7 @@ SRST
ERST
DEF("smp", HAS_ARG, QEMU_OPTION_smp,
-"-smp
[[cpus=]n][,maxcpus=cpus][,sockets=sockets][,dies=dies][,cores=cores][,threads=threads]\n"
+"-smp
[[cpus=]n][,maxcpus=maxcpus][,sockets=sockets][,dies=dies][,cores=cores][,threads=threads]\n"
"set the number of CPUs to 'n' [default=1]\n"
"maxcpus= maximum number of total CPUs, including\n"
"offline CPUs for hotplug, etc\n"
--
2.19.1
Re: [PATCH 09/29] tcg/module: add tcg-module.[ch] infrastructure
Hi,
> > +struct TCGModuleOps {
> > +};
> > +extern struct TCGModuleOps tcg;
>
> TCG functions taking a CPUState argument should reside in
> CPUClass's AccelCPUClass/TCGCPUOps, not a global struct.
It's not that easy. Tried that first, but using TCGCPUOps outside cpu
code doesn't work.
take care,
Gerd
[PATCH v2 0/2] qemu-options: Trivial doc fixes related to -smp
v1->v2: - update the subject and commit message of patch 2/2. - add R-bs from Damien and Philippe. Thanks. There are two places found related to -smp that may need to be fixed when reading qemu-options.hx. And after searching other Doc files, there hasn't been any other similar issues. Yanan Wang (2): qemu-options: Tweak [,maxcpus=cpus] to [,maxcpus=maxcpus] qemu-options: Add missing "sockets=2,maxcpus=2" to CLI "-smp 2" qemu-options.hx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) -- 2.19.1
Re: [PATCH v11 11/14] machine: Make smp_parse generic enough for all arches
Philippe Mathieu-Daudé writes: > On 9/28/21 05:57, Yanan Wang wrote: >> Currently the only difference between smp_parse and pc_smp_parse >> is the support of dies parameter and the related error reporting. >> With some arch compat variables like "bool dies_supported", we can >> make smp_parse generic enough for all arches and the PC specific >> one can be removed. >> >> Making smp_parse() generic enough can reduce code duplication and >> ease the code maintenance, and also allows extending the topology >> with more arch specific members (e.g., clusters) in the future. >> >> Suggested-by: Andrew Jones >> Suggested-by: Daniel P. Berrange >> Signed-off-by: Yanan Wang >> --- >> hw/core/machine.c | 91 +++-- >> hw/i386/pc.c| 84 + >> include/hw/boards.h | 9 + >> 3 files changed, 81 insertions(+), 103 deletions(-) > >> +/* >> + * smp_parse - Generic function used to parse the given SMP configuration >> + * >> + * Any missing parameter in "cpus/maxcpus/sockets/cores/threads" will be >> + * automatically computed based on the provided ones. >> + * >> + * In the calculation of omitted sockets/cores/threads: we prefer sockets >> + * over cores over threads before 6.2, while preferring cores over sockets >> + * over threads since 6.2. >> + * >> + * In the calculation of cpus/maxcpus: When both maxcpus and cpus are >> omitted, >> + * maxcpus will be computed from the given parameters and cpus will be set >> + * equal to maxcpus. When only one of maxcpus and cpus is given then the >> + * omitted one will be set to its given counterpart's value. Both maxcpus >> and >> + * cpus may be specified, but maxcpus must be equal to or greater than cpus. >> + * >> + * For compatibility, apart from the parameters that will be computed, newly >> + * introduced topology members which are likely to be target specific should >> + * be directly set as 1 if they are omitted (e.g. dies for PC since 4.1). >> + */ >> static void smp_parse(MachineState *ms, SMPConfiguration *config, Error >> **errp) > > Can we have it return a boolean instead? Yes, please. From error.h's big comment: * = Rules = [...] * - Whenever practical, also return a value that indicates success / * failure. This can make the error checking more concise, and can * avoid useless error object creation and destruction. Note that * we still have many functions returning void. We recommend * • bool-valued functions return true on success / false on failure, * • pointer-valued functions return non-null / null pointer, and * • integer-valued functions return non-negative / negative.
Re: [PATCH v3 0/5] introduce QArray
On Montag, 27. September 2021 12:59:40 CEST Greg Kurz wrote: > On Mon, 27 Sep 2021 12:35:16 +0200 > > Christian Schoenebeck wrote: > > On Dienstag, 31. August 2021 14:25:04 CEST Christian Schoenebeck wrote: > > > On Dienstag, 31. August 2021 13:58:02 CEST Greg Kurz wrote: > > > > On Thu, 26 Aug 2021 14:47:26 +0200 > > > > > > > > Christian Schoenebeck wrote: > > > > > Patches 1 and 2 introduce include/qemu/qarray.h which implements a > > > > > deep > > > > > auto free mechanism for arrays. See commit log of patch 1 for a > > > > > detailed > > > > > explanation and motivation for introducing QArray. > > > > > > > > > > Patches 3..5 are provided (e.g. as example) for 9p being the first > > > > > user > > > > > of > > > > > this new QArray API. These particular patches 3..5 are rebased on my > > > > > current 9p queue: > > > > > https://github.com/cschoenebeck/qemu/commits/9p.next > > > > > > > > > which are basically just the following two queued patches: > > > > This looks nice indeed but I have the impression the same could be > > > > achieved using glib's g_autoptr framework with less code being added > > > > to QEMU (at the cost of being less generic maybe). > > > > > > I haven't seen a way doing this with glib, except of GArray which has > > > some > > > disadvantages. But who knows, maybe I was missing something. > > > > Ping > > > > Let's do this? > > Hi Christian, > > Sorry I don't have enough bandwidth to review or to look for an alternate > way... :-\ So I suggest you just go forward with this series. Hopefully > you can get some reviews from Markus and/or Richard. Ok, then I wait for few more days, and if there are no repsonses, nor vetos then I'll queue these patches anyway. Best regards, Christian Schoenebeck
Re: [PATCH v4 05/25] linux-user/arm: Implement setup_sigtramp
On 9/28/21 5:31 AM, Peter Maydell wrote: +uint32_t *host_rc = g2h_untagged(retcode); ...but here we treat it as a normal guest address that we can convert into a host address and dereference. If the signal handler is being entered in Thumb mode this will be a misaligned pointer. Oops, yes. I've no idea why the kernel works so hard to match the mode of the signal handler to the mode of the trampoline, but I presume it's ABI at this point. r~
Re: [PATCH v2 2/2] qemu-options: Add missing "sockets=2,maxcpus=2" to CLI "-smp 2"
On Tue, Sep 28, 2021 at 08:11:34PM +0800, Yanan Wang wrote: > There is one numa config example in qemu-options.hx currently > using "-smp 2" and assuming that there will be 2 sockets and > 2 cpus totally. However now the actual calculation logic of > missing sockets and cores is not immutable and is considered > liable to change. Although we will get maxcpus=2 finally based > on current parser, it's always stable to specify it explicitly. > > So "-smp 2,sockets=2,maxcpus=2" will be optimal when we expect > multiple sockets and 2 cpus totally. > > Signed-off-by: Yanan Wang > Reviewed-by: Philippe Mathieu-Daude > --- > qemu-options.hx | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Daniel P. Berrangé 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 v2 1/2] qemu-options: Tweak [,maxcpus=cpus] to [,maxcpus=maxcpus]
On Tue, Sep 28, 2021 at 08:11:33PM +0800, Yanan Wang wrote: > In qemu-option.hx, there is "-smp [[cpus=]n][,maxcpus=cpus]..." in the > DEF part, and "-smp [[cpus=]n][,maxcpus=maxcpus]..." in the RST part. > Obviously the later is right, let's fix the previous one. > > Signed-off-by: Yanan Wang > Reviewed-by: Damien Hedde > --- > qemu-options.hx | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Daniel P. Berrangé 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 10/29] tcg_funcs: Add tlb_flush to TCGModuleOps
On 9/28/21 7:32 AM, Gerd Hoffmann wrote:
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 608d768a4371..72e4e3b5bb89 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -160,7 +160,9 @@ void tlb_flush_page_all_cpus_synced(CPUState *src,
target_ulong addr);
* so this is generally safe. If more selective flushing is required
* use one of the other functions for efficiency.
*/
+#ifdef TCG_DIRECT_CALL
void tlb_flush(CPUState *cpu);
+#endif
I'm pretty sure you can drop these ifdefs. Just because there's a regular declaration for
a function doesn't mean a subsequent inline definition does not apply.
And even if that didn't work, I'd be willing to trade inline expansion for not adding lots
of ifdefs...
+static inline void tlb_flush(CPUState *cpu)
+{
+tcg.tlb_flush(cpu);
+}
... these could just as well be out-of-line.
r~
[PULL 02/33] Kconfig: Add CONFIG_SGX support
From: Yang Zhong Add new CONFIG_SGX for sgx support in the Qemu, and the Kconfig default enable sgx in the i386 platform. Signed-off-by: Yang Zhong Message-Id: <20210719112136.57018-32-yang.zh...@intel.com> Signed-off-by: Paolo Bonzini --- configs/devices/i386-softmmu/default.mak | 1 + hw/i386/Kconfig | 5 + 2 files changed, 6 insertions(+) diff --git a/configs/devices/i386-softmmu/default.mak b/configs/devices/i386-softmmu/default.mak index 84d1a2487c..598c6646df 100644 --- a/configs/devices/i386-softmmu/default.mak +++ b/configs/devices/i386-softmmu/default.mak @@ -22,6 +22,7 @@ #CONFIG_TPM_CRB=n #CONFIG_TPM_TIS_ISA=n #CONFIG_VTD=n +#CONFIG_SGX=n # Boards: # diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig index ddedcef0b2..962d2c981b 100644 --- a/hw/i386/Kconfig +++ b/hw/i386/Kconfig @@ -6,6 +6,10 @@ config SEV select X86_FW_OVMF depends on KVM +config SGX +bool +depends on KVM + config PC bool imply APPLESMC @@ -21,6 +25,7 @@ config PC imply PVPANIC_ISA imply QXL imply SEV +imply SGX imply SGA imply TEST_DEVICES imply TPM_CRB -- 2.31.1
[PULL 01/33] memory: Add RAM_PROTECTED flag to skip IOMMU mappings
From: Sean Christopherson
Add a new RAMBlock flag to denote "protected" memory, i.e. memory that
looks and acts like RAM but is inaccessible via normal mechanisms,
including DMA. Use the flag to skip protected memory regions when
mapping RAM for DMA in VFIO.
Signed-off-by: Sean Christopherson
Signed-off-by: Yang Zhong
Signed-off-by: Paolo Bonzini
---
hw/vfio/common.c | 1 +
include/exec/memory.h | 15 ++-
softmmu/memory.c | 5 +
softmmu/physmem.c | 3 ++-
4 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 8728d4d5c2..1289cfa8be 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -562,6 +562,7 @@ static bool
vfio_listener_skipped_section(MemoryRegionSection *section)
{
return (!memory_region_is_ram(section->mr) &&
!memory_region_is_iommu(section->mr)) ||
+ memory_region_is_protected(section->mr) ||
/*
* Sizing an enabled 64-bit BAR can cause spurious mappings to
* addresses in the upper part of the 64-bit address space. These
diff --git a/include/exec/memory.h b/include/exec/memory.h
index c3d417d317..9446874d21 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -190,6 +190,9 @@ typedef struct IOMMUTLBEvent {
*/
#define RAM_NORESERVE (1 << 7)
+/* RAM that isn't accessible through normal means. */
+#define RAM_PROTECTED (1 << 8)
+
static inline void iommu_notifier_init(IOMMUNotifier *n, IOMMUNotify fn,
IOMMUNotifierFlag flags,
hwaddr start, hwaddr end,
@@ -1267,7 +1270,7 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
* @name: the name of the region.
* @size: size of the region.
* @ram_flags: RamBlock flags. Supported flags: RAM_SHARED, RAM_PMEM,
- * RAM_NORESERVE.
+ * RAM_NORESERVE, RAM_PROTECTED.
* @fd: the fd to mmap.
* @offset: offset within the file referenced by fd
* @errp: pointer to Error*, to store an error if it happens.
@@ -1568,6 +1571,16 @@ static inline bool memory_region_is_romd(MemoryRegion
*mr)
return mr->rom_device && mr->romd_mode;
}
+/**
+ * memory_region_is_protected: check whether a memory region is protected
+ *
+ * Returns %true if a memory region is protected RAM and cannot be accessed
+ * via standard mechanisms, e.g. DMA.
+ *
+ * @mr: the memory region being queried
+ */
+bool memory_region_is_protected(MemoryRegion *mr);
+
/**
* memory_region_get_iommu: check whether a memory region is an iommu
*
diff --git a/softmmu/memory.c b/softmmu/memory.c
index bfedaf9c4d..54cd0e9824 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -1811,6 +1811,11 @@ bool memory_region_is_ram_device(MemoryRegion *mr)
return mr->ram_device;
}
+bool memory_region_is_protected(MemoryRegion *mr)
+{
+return mr->ram && (mr->ram_block->flags & RAM_PROTECTED);
+}
+
uint8_t memory_region_get_dirty_log_mask(MemoryRegion *mr)
{
uint8_t mask = mr->dirty_log_mask;
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 23e77cb771..088660d973 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -2055,7 +2055,8 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size,
MemoryRegion *mr,
int64_t file_size, file_align;
/* Just support these ram flags by now. */
-assert((ram_flags & ~(RAM_SHARED | RAM_PMEM | RAM_NORESERVE)) == 0);
+assert((ram_flags & ~(RAM_SHARED | RAM_PMEM | RAM_NORESERVE |
+ RAM_PROTECTED)) == 0);
if (xen_enabled()) {
error_setg(errp, "-mem-path not supported with Xen");
--
2.31.1
[PULL 03/33] hostmem: Add hostmem-epc as a backend for SGX EPC
From: Sean Christopherson
EPC (Enclave Page Cahe) is a specialized type of memory used by Intel
SGX (Software Guard Extensions). The SDM desribes EPC as:
The Enclave Page Cache (EPC) is the secure storage used to store
enclave pages when they are a part of an executing enclave. For an
EPC page, hardware performs additional access control checks to
restrict access to the page. After the current page access checks
and translations are performed, the hardware checks that the EPC
page is accessible to the program currently executing. Generally an
EPC page is only accessed by the owner of the executing enclave or
an instruction which is setting up an EPC page.
Because of its unique requirements, Linux manages EPC separately from
normal memory. Similar to memfd, the device /dev/sgx_vepc can be
opened to obtain a file descriptor which can in turn be used to mmap()
EPC memory.
Signed-off-by: Sean Christopherson
Signed-off-by: Yang Zhong
Message-Id: <20210719112136.57018-3-yang.zh...@intel.com>
Signed-off-by: Paolo Bonzini
---
backends/hostmem-epc.c| 82 +++
backends/meson.build | 1 +
include/hw/i386/hostmem-epc.h | 28
3 files changed, 111 insertions(+)
create mode 100644 backends/hostmem-epc.c
create mode 100644 include/hw/i386/hostmem-epc.h
diff --git a/backends/hostmem-epc.c b/backends/hostmem-epc.c
new file mode 100644
index 00..b47f98b6a3
--- /dev/null
+++ b/backends/hostmem-epc.c
@@ -0,0 +1,82 @@
+/*
+ * QEMU host SGX EPC memory backend
+ *
+ * Copyright (C) 2019 Intel Corporation
+ *
+ * Authors:
+ * Sean Christopherson
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#include
+
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "qom/object_interfaces.h"
+#include "qapi/error.h"
+#include "sysemu/hostmem.h"
+#include "hw/i386/hostmem-epc.h"
+
+static void
+sgx_epc_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
+{
+uint32_t ram_flags;
+char *name;
+int fd;
+
+if (!backend->size) {
+error_setg(errp, "can't create backend with size 0");
+return;
+}
+
+fd = qemu_open_old("/dev/sgx_vepc", O_RDWR);
+if (fd < 0) {
+error_setg_errno(errp, errno,
+ "failed to open /dev/sgx_vepc to alloc SGX EPC");
+return;
+}
+
+name = object_get_canonical_path(OBJECT(backend));
+ram_flags = (backend->share ? RAM_SHARED : 0) | RAM_PROTECTED;
+memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend),
+ name, backend->size, ram_flags,
+ fd, 0, errp);
+g_free(name);
+}
+
+static void sgx_epc_backend_instance_init(Object *obj)
+{
+HostMemoryBackend *m = MEMORY_BACKEND(obj);
+
+m->share = true;
+m->merge = false;
+m->dump = false;
+}
+
+static void sgx_epc_backend_class_init(ObjectClass *oc, void *data)
+{
+HostMemoryBackendClass *bc = MEMORY_BACKEND_CLASS(oc);
+
+bc->alloc = sgx_epc_backend_memory_alloc;
+}
+
+static const TypeInfo sgx_epc_backed_info = {
+.name = TYPE_MEMORY_BACKEND_EPC,
+.parent = TYPE_MEMORY_BACKEND,
+.instance_init = sgx_epc_backend_instance_init,
+.class_init = sgx_epc_backend_class_init,
+.instance_size = sizeof(HostMemoryBackendEpc),
+};
+
+static void register_types(void)
+{
+int fd = qemu_open_old("/dev/sgx_vepc", O_RDWR);
+if (fd >= 0) {
+close(fd);
+
+type_register_static(&sgx_epc_backed_info);
+}
+}
+
+type_init(register_types);
diff --git a/backends/meson.build b/backends/meson.build
index d4221831fc..6e68945528 100644
--- a/backends/meson.build
+++ b/backends/meson.build
@@ -16,5 +16,6 @@ softmmu_ss.add(when: ['CONFIG_VHOST_USER', 'CONFIG_VIRTIO'],
if_true: files('vho
softmmu_ss.add(when: 'CONFIG_VIRTIO_CRYPTO', if_true:
files('cryptodev-vhost.c'))
softmmu_ss.add(when: ['CONFIG_VIRTIO_CRYPTO', 'CONFIG_VHOST_CRYPTO'], if_true:
files('cryptodev-vhost-user.c'))
softmmu_ss.add(when: 'CONFIG_GIO', if_true: [files('dbus-vmstate.c'), gio])
+softmmu_ss.add(when: 'CONFIG_SGX', if_true: files('hostmem-epc.c'))
subdir('tpm')
diff --git a/include/hw/i386/hostmem-epc.h b/include/hw/i386/hostmem-epc.h
new file mode 100644
index 00..846c726085
--- /dev/null
+++ b/include/hw/i386/hostmem-epc.h
@@ -0,0 +1,28 @@
+/*
+ * SGX EPC backend
+ *
+ * Copyright (C) 2019 Intel Corporation
+ *
+ * Authors:
+ * Sean Christopherson
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#ifndef QEMU_HOSTMEM_EPC_H
+#define QEMU_HOSTMEM_EPC_H
+
+#include "sysemu/hostmem.h"
+
+#define TYPE_MEMORY_BACKEND_EPC "memory-backend-epc"
+
+#define MEMORY_BACKEND_EPC(obj)\
+OBJECT_CHECK(HostMemoryBackendEpc, (obj), TYPE_ME
[PULL 06/33] vl: Add sgx compound properties to expose SGX EPC sections to guest
From: Sean Christopherson
Because SGX EPC is enumerated through CPUID, EPC "devices" need to be
realized prior to realizing the vCPUs themselves, i.e. long before
generic devices are parsed and realized. From a virtualization
perspective, the CPUID aspect also means that EPC sections cannot be
hotplugged without paravirtualizing the guest kernel (hardware does
not support hotplugging as EPC sections must be locked down during
pre-boot to provide EPC's security properties).
So even though EPC sections could be realized through the generic
-devices command, they need to be created much earlier for them to
actually be usable by the guest. Place all EPC sections in a
contiguous block, somewhat arbitrarily starting after RAM above 4g.
Ensuring EPC is in a contiguous region simplifies calculations, e.g.
device memory base, PCI hole, etc..., allows dynamic calculation of the
total EPC size, e.g. exposing EPC to guests does not require -maxmem,
and last but not least allows all of EPC to be enumerated in a single
ACPI entry, which is expected by some kernels, e.g. Windows 7 and 8.
The new compound properties command for sgx like below:
..
-object memory-backend-epc,id=mem1,size=28M,prealloc=on \
-object memory-backend-epc,id=mem2,size=10M \
-M sgx-epc.0.memdev=mem1,sgx-epc.1.memdev=mem2
Signed-off-by: Sean Christopherson
Signed-off-by: Yang Zhong
Message-Id: <20210719112136.57018-6-yang.zh...@intel.com>
Signed-off-by: Paolo Bonzini
---
hw/i386/sgx-epc.c | 20 ++--
hw/i386/x86.c | 29 +
include/hw/i386/pc.h | 3 +++
include/hw/i386/sgx-epc.h | 14 ++
include/hw/i386/x86.h | 1 +
qapi/machine.json | 26 +-
qemu-options.hx | 10 --
7 files changed, 94 insertions(+), 9 deletions(-)
diff --git a/hw/i386/sgx-epc.c b/hw/i386/sgx-epc.c
index c584acc17b..6677dc74b5 100644
--- a/hw/i386/sgx-epc.c
+++ b/hw/i386/sgx-epc.c
@@ -14,13 +14,8 @@
#include "hw/i386/sgx-epc.h"
#include "hw/mem/memory-device.h"
#include "hw/qdev-properties.h"
-#include "monitor/qdev.h"
#include "qapi/error.h"
#include "qapi/visitor.h"
-#include "qemu/config-file.h"
-#include "qemu/error-report.h"
-#include "qemu/option.h"
-#include "qemu/units.h"
#include "target/i386/cpu.h"
#include "exec/address-spaces.h"
@@ -56,6 +51,8 @@ static void sgx_epc_realize(DeviceState *dev, Error **errp)
{
PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
X86MachineState *x86ms = X86_MACHINE(pcms);
+MemoryDeviceState *md = MEMORY_DEVICE(dev);
+SGXEPCState *sgx_epc = &pcms->sgx_epc;
SGXEPCDevice *epc = SGX_EPC(dev);
HostMemoryBackend *hostmem;
const char *path;
@@ -77,7 +74,18 @@ static void sgx_epc_realize(DeviceState *dev, Error **errp)
return;
}
-error_setg(errp, "'" TYPE_SGX_EPC "' not supported");
+epc->addr = sgx_epc->base + sgx_epc->size;
+
+memory_region_add_subregion(&sgx_epc->mr, epc->addr - sgx_epc->base,
+host_memory_backend_get_memory(hostmem));
+
+host_memory_backend_set_mapped(hostmem, true);
+
+sgx_epc->sections = g_renew(SGXEPCDevice *, sgx_epc->sections,
+sgx_epc->nr_sections + 1);
+sgx_epc->sections[sgx_epc->nr_sections++] = epc;
+
+sgx_epc->size += memory_device_get_region_size(md, errp);
}
static void sgx_epc_unrealize(DeviceState *dev)
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 00448ed55a..41ef9a84a9 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -30,6 +30,8 @@
#include "qapi/error.h"
#include "qapi/qmp/qerror.h"
#include "qapi/qapi-visit-common.h"
+#include "qapi/clone-visitor.h"
+#include "qapi/qapi-visit-machine.h"
#include "qapi/visitor.h"
#include "sysemu/qtest.h"
#include "sysemu/whpx.h"
@@ -1263,6 +1265,27 @@ static void x86_machine_set_bus_lock_ratelimit(Object
*obj, Visitor *v,
visit_type_uint64(v, name, &x86ms->bus_lock_ratelimit, errp);
}
+static void machine_get_sgx_epc(Object *obj, Visitor *v, const char *name,
+void *opaque, Error **errp)
+{
+X86MachineState *x86ms = X86_MACHINE(obj);
+SgxEPCList *list = x86ms->sgx_epc_list;
+
+visit_type_SgxEPCList(v, name, &list, errp);
+}
+
+static void machine_set_sgx_epc(Object *obj, Visitor *v, const char *name,
+void *opaque, Error **errp)
+{
+X86MachineState *x86ms = X86_MACHINE(obj);
+SgxEPCList *list;
+
+list = x86ms->sgx_epc_list;
+visit_type_SgxEPCList(v, name, &x86ms->sgx_epc_list, errp);
+
+qapi_free_SgxEPCList(list);
+}
+
static void x86_machine_initfn(Object *obj)
{
X86MachineState *x86ms = X86_MACHINE(obj);
@@ -1322,6 +1345,12 @@ static void x86_machine_class_init(ObjectClass *oc, void
*data)
x86_machine_set_bus_lock_ratelimit, NULL,
NULL);
object_class_property_set_description(oc, X86_MACHINE_BUS_LOCK_RATELIMIT,
[PULL 05/33] i386: Add 'sgx-epc' device to expose EPC sections to guest
From: Sean Christopherson
SGX EPC is enumerated through CPUID, i.e. EPC "devices" need to be
realized prior to realizing the vCPUs themselves, which occurs long
before generic devices are parsed and realized. Because of this,
do not allow 'sgx-epc' devices to be instantiated after vCPUS have
been created.
The 'sgx-epc' device is essentially a placholder at this time, it will
be fully implemented in a future patch along with a dedicated command
to create 'sgx-epc' devices.
Signed-off-by: Sean Christopherson
Signed-off-by: Yang Zhong
Message-Id: <20210719112136.57018-5-yang.zh...@intel.com>
Signed-off-by: Paolo Bonzini
---
hw/i386/meson.build | 1 +
hw/i386/sgx-epc.c | 167 ++
include/hw/i386/sgx-epc.h | 44 ++
3 files changed, 212 insertions(+)
create mode 100644 hw/i386/sgx-epc.c
create mode 100644 include/hw/i386/sgx-epc.h
diff --git a/hw/i386/meson.build b/hw/i386/meson.build
index 80dad29f2b..b1862c83d4 100644
--- a/hw/i386/meson.build
+++ b/hw/i386/meson.build
@@ -16,6 +16,7 @@ i386_ss.add(when: 'CONFIG_Q35', if_true: files('pc_q35.c'))
i386_ss.add(when: 'CONFIG_VMMOUSE', if_true: files('vmmouse.c'))
i386_ss.add(when: 'CONFIG_VMPORT', if_true: files('vmport.c'))
i386_ss.add(when: 'CONFIG_VTD', if_true: files('intel_iommu.c'))
+i386_ss.add(when: 'CONFIG_SGX', if_true: files('sgx-epc.c'))
i386_ss.add(when: 'CONFIG_ACPI', if_true: files('acpi-common.c'))
i386_ss.add(when: 'CONFIG_ACPI_HW_REDUCED', if_true:
files('generic_event_device_x86.c'))
diff --git a/hw/i386/sgx-epc.c b/hw/i386/sgx-epc.c
new file mode 100644
index 00..c584acc17b
--- /dev/null
+++ b/hw/i386/sgx-epc.c
@@ -0,0 +1,167 @@
+/*
+ * SGX EPC device
+ *
+ * Copyright (C) 2019 Intel Corporation
+ *
+ * Authors:
+ * Sean Christopherson
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#include "qemu/osdep.h"
+#include "hw/i386/pc.h"
+#include "hw/i386/sgx-epc.h"
+#include "hw/mem/memory-device.h"
+#include "hw/qdev-properties.h"
+#include "monitor/qdev.h"
+#include "qapi/error.h"
+#include "qapi/visitor.h"
+#include "qemu/config-file.h"
+#include "qemu/error-report.h"
+#include "qemu/option.h"
+#include "qemu/units.h"
+#include "target/i386/cpu.h"
+#include "exec/address-spaces.h"
+
+static Property sgx_epc_properties[] = {
+DEFINE_PROP_UINT64(SGX_EPC_ADDR_PROP, SGXEPCDevice, addr, 0),
+DEFINE_PROP_LINK(SGX_EPC_MEMDEV_PROP, SGXEPCDevice, hostmem,
+ TYPE_MEMORY_BACKEND_EPC, HostMemoryBackendEpc *),
+DEFINE_PROP_END_OF_LIST(),
+};
+
+static void sgx_epc_get_size(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+Error *local_err = NULL;
+uint64_t value;
+
+value = memory_device_get_region_size(MEMORY_DEVICE(obj), &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+
+visit_type_uint64(v, name, &value, errp);
+}
+
+static void sgx_epc_init(Object *obj)
+{
+object_property_add(obj, SGX_EPC_SIZE_PROP, "uint64", sgx_epc_get_size,
+NULL, NULL, NULL);
+}
+
+static void sgx_epc_realize(DeviceState *dev, Error **errp)
+{
+PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
+X86MachineState *x86ms = X86_MACHINE(pcms);
+SGXEPCDevice *epc = SGX_EPC(dev);
+HostMemoryBackend *hostmem;
+const char *path;
+
+if (x86ms->boot_cpus != 0) {
+error_setg(errp, "'" TYPE_SGX_EPC "' can't be created after vCPUs,"
+ "e.g. via -device");
+return;
+}
+
+if (!epc->hostmem) {
+error_setg(errp, "'" SGX_EPC_MEMDEV_PROP "' property is not set");
+return;
+}
+hostmem = MEMORY_BACKEND(epc->hostmem);
+if (host_memory_backend_is_mapped(hostmem)) {
+path = object_get_canonical_path_component(OBJECT(hostmem));
+error_setg(errp, "can't use already busy memdev: %s", path);
+return;
+}
+
+error_setg(errp, "'" TYPE_SGX_EPC "' not supported");
+}
+
+static void sgx_epc_unrealize(DeviceState *dev)
+{
+SGXEPCDevice *epc = SGX_EPC(dev);
+HostMemoryBackend *hostmem = MEMORY_BACKEND(epc->hostmem);
+
+host_memory_backend_set_mapped(hostmem, false);
+}
+
+static uint64_t sgx_epc_md_get_addr(const MemoryDeviceState *md)
+{
+const SGXEPCDevice *epc = SGX_EPC(md);
+
+return epc->addr;
+}
+
+static void sgx_epc_md_set_addr(MemoryDeviceState *md, uint64_t addr,
+Error **errp)
+{
+object_property_set_uint(OBJECT(md), SGX_EPC_ADDR_PROP, addr, errp);
+}
+
+static uint64_t sgx_epc_md_get_plugged_size(const MemoryDeviceState *md,
+Error **errp)
+{
+return 0;
+}
+
+static MemoryRegion *sgx_epc_md_get_memory_region(MemoryDeviceState *md,
+ Error **errp)
+{
+SGXEPCDe
[PULL 20/33] i386: acpi: Add SGX EPC entry to ACPI tables
From: Sean Christopherson
The ACPI Device entry for SGX EPC is essentially a hack whose primary
purpose is to provide software with a way to autoprobe SGX support,
e.g. to allow software to implement SGX support as a driver. Details
on the individual EPC sections are not enumerated through ACPI tables,
i.e. software must enumerate the EPC sections via CPUID. Furthermore,
software expects to see only a single EPC Device in the ACPI tables
regardless of the number of EPC sections in the system.
However, several versions of Windows do rely on the ACPI tables to
enumerate the address and size of the EPC. So, regardless of the number
of EPC sections exposed to the guest, create exactly *one* EPC device
with a _CRS entry that spans the entirety of all EPC sections (which are
guaranteed to be contiguous in Qemu).
Note, NUMA support for EPC memory is intentionally not considered as
enumerating EPC NUMA information is not yet defined for bare metal.
Signed-off-by: Sean Christopherson
Signed-off-by: Yang Zhong
Message-Id: <20210719112136.57018-20-yang.zh...@intel.com>
Signed-off-by: Paolo Bonzini
---
hw/i386/acpi-build.c | 22 ++
1 file changed, 22 insertions(+)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index dfaa47cdc2..f4d6ae3d02 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1841,6 +1841,28 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
}
#endif
+if (pcms->sgx_epc.size != 0) {
+uint64_t epc_base = pcms->sgx_epc.base;
+uint64_t epc_size = pcms->sgx_epc.size;
+
+dev = aml_device("EPC");
+aml_append(dev, aml_name_decl("_HID", aml_eisaid("INT0E0C")));
+aml_append(dev, aml_name_decl("_STR",
+ aml_unicode("Enclave Page Cache 1.0")));
+crs = aml_resource_template();
+aml_append(crs,
+ aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED,
+AML_MAX_FIXED, AML_NON_CACHEABLE,
+AML_READ_WRITE, 0, epc_base,
+epc_base + epc_size - 1, 0, epc_size));
+aml_append(dev, aml_name_decl("_CRS", crs));
+
+method = aml_method("_STA", 0, AML_NOTSERIALIZED);
+aml_append(method, aml_return(aml_int(0x0f)));
+aml_append(dev, method);
+
+aml_append(sb_scope, dev);
+}
aml_append(dsdt, sb_scope);
/* copy AML table into ACPI tables blob and patch header there */
--
2.31.1
[PULL 00/33] x86 and misc changes for 2021-09-28
The following changes since commit 14f02d8a9ec1746823c106933a4c8f062f9e0f95: Merge remote-tracking branch 'remotes/philmd/tags/integration-testing-20210927' into staging (2021-09-27 19:52:43 +0100) are available in the Git repository at: https://gitlab.com/bonzini/qemu.git tags/for-upstream for you to fetch changes up to 824ba1e99c8bc12048636ea43dec923385ff042f: meson_options.txt: Switch the default value for the vnc option to 'auto' (2021-09-28 14:50:14 +0200) * SGX implementation for x86 * Miscellaneous bugfixes * Fix dependencies from ROMs to qtests Marc-André Lureau (1): build-sys: add HAVE_IPPROTO_MPTCP Paolo Bonzini (2): meson: unpack edk2 firmware even if --disable-blobs tests: qtest: bios-tables-test depends on the unpacked edk2 ROMs Peter Maydell (1): target/i386: Fix memory leak in sev_read_file_base64() Peter Xu (2): memory: Name all the memory listeners memory: Add tracepoint for dirty sync Sean Christopherson (21): memory: Add RAM_PROTECTED flag to skip IOMMU mappings hostmem: Add hostmem-epc as a backend for SGX EPC i386: Add 'sgx-epc' device to expose EPC sections to guest vl: Add sgx compound properties to expose SGX EPC sections to guest i386: Add primary SGX CPUID and MSR defines i386: Add SGX CPUID leaf FEAT_SGX_12_0_EAX i386: Add SGX CPUID leaf FEAT_SGX_12_0_EBX i386: Add SGX CPUID leaf FEAT_SGX_12_1_EAX i386: Add get/set/migrate support for SGX_LEPUBKEYHASH MSRs i386: Add feature control MSR dependency when SGX is enabled i386: Update SGX CPUID info according to hardware/KVM/user input i386: kvm: Add support for exposing PROVISIONKEY to guest i386: Propagate SGX CPUID sub-leafs to KVM Adjust min CPUID level to 0x12 when SGX is enabled hw/i386/fw_cfg: Set SGX bits in feature control fw_cfg accordingly hw/i386/pc: Account for SGX EPC sections when calculating device memory i386/pc: Add e820 entry for SGX EPC section(s) i386: acpi: Add SGX EPC entry to ACPI tables q35: Add support for SGX EPC i440fx: Add support for SGX EPC docs/system: Add SGX documentation to the system manual Thomas Huth (1): meson_options.txt: Switch the default value for the vnc option to 'auto' Yang Zhong (5): Kconfig: Add CONFIG_SGX support qom: Add memory-backend-epc ObjectOptions support sgx-epc: Add the fill_device_info() callback support target/i386: Add HMP and QMP interfaces for SGX target/i386: Add the query-sgx-capabilities QMP command accel/hvf/hvf-accel-ops.c| 1 + accel/kvm/kvm-all.c | 7 +- backends/hostmem-epc.c | 82 ++ backends/meson.build | 1 + configs/devices/i386-softmmu/default.mak | 1 + docs/system/i386/sgx.rst | 165 +++ docs/system/target-i386.rst | 1 + hmp-commands-info.hx | 15 +++ hw/i386/Kconfig | 5 + hw/i386/acpi-build.c | 22 hw/i386/fw_cfg.c | 10 +- hw/i386/meson.build | 2 + hw/i386/pc.c | 15 ++- hw/i386/pc_piix.c| 1 + hw/i386/pc_q35.c | 1 + hw/i386/sgx-epc.c| 184 +++ hw/i386/sgx-stub.c | 26 + hw/i386/sgx.c| 170 hw/i386/x86.c| 29 + hw/i386/xen/xen-hvm.c| 2 + hw/intc/openpic_kvm.c| 1 + hw/remote/proxy-memory-listener.c| 1 + hw/vfio/common.c | 2 + hw/vfio/spapr.c | 1 + hw/virtio/vhost-vdpa.c | 1 + hw/virtio/vhost.c| 2 + hw/virtio/virtio.c | 1 + hw/xen/xen_pt.c | 2 + include/exec/memory.h| 23 +++- include/hw/i386/hostmem-epc.h| 28 + include/hw/i386/pc.h | 6 + include/hw/i386/sgx-epc.h| 67 +++ include/hw/i386/sgx.h| 12 ++ include/hw/i386/x86.h| 1 + include/monitor/hmp-target.h | 1 + include/sysemu/kvm_int.h | 2 +- io/dns-resolver.c| 2 +- meson.build | 18 +-- meson_options.txt| 2 +- monitor/hmp-cmds.c | 10 ++ pc-bios/descriptors/meson.build | 4 +- pc-bios/meson.build | 5 +- qapi/machine.json
[PULL 14/33] i386: kvm: Add support for exposing PROVISIONKEY to guest
From: Sean Christopherson
If the guest want to fully use SGX, the guest needs to be able to
access provisioning key. Add a new KVM_CAP_SGX_ATTRIBUTE to KVM to
support provisioning key to KVM guests.
Signed-off-by: Sean Christopherson
Signed-off-by: Yang Zhong
Message-Id: <20210719112136.57018-14-yang.zh...@intel.com>
Signed-off-by: Paolo Bonzini
---
target/i386/cpu.c | 5 -
target/i386/kvm/kvm.c | 29 +
target/i386/kvm/kvm_i386.h | 2 ++
3 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 8a62986819..de58599a3d 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5542,7 +5542,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
uint32_t count,
*ecx |= XSTATE_FP_MASK | XSTATE_SSE_MASK;
/* Access to PROVISIONKEY requires additional credentials. */
-*eax &= ~(1U << 4);
+if ((*eax & (1U << 4)) &&
+!kvm_enable_sgx_provisioning(cs->kvm_state)) {
+*eax &= ~(1U << 4);
+}
}
#endif
break;
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 6dc40161e0..488926a95f 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -4644,6 +4644,35 @@ void kvm_arch_update_guest_debug(CPUState *cpu, struct
kvm_guest_debug *dbg)
}
}
+static bool has_sgx_provisioning;
+
+static bool __kvm_enable_sgx_provisioning(KVMState *s)
+{
+int fd, ret;
+
+if (!kvm_vm_check_extension(s, KVM_CAP_SGX_ATTRIBUTE)) {
+return false;
+}
+
+fd = qemu_open_old("/dev/sgx_provision", O_RDONLY);
+if (fd < 0) {
+return false;
+}
+
+ret = kvm_vm_enable_cap(s, KVM_CAP_SGX_ATTRIBUTE, 0, fd);
+if (ret) {
+error_report("Could not enable SGX PROVISIONKEY: %s", strerror(-ret));
+exit(1);
+}
+close(fd);
+return true;
+}
+
+bool kvm_enable_sgx_provisioning(KVMState *s)
+{
+return MEMORIZE(__kvm_enable_sgx_provisioning(s), has_sgx_provisioning);
+}
+
static bool host_supports_vmx(void)
{
uint32_t ecx, unused;
diff --git a/target/i386/kvm/kvm_i386.h b/target/i386/kvm/kvm_i386.h
index 54667b35f0..a978509d50 100644
--- a/target/i386/kvm/kvm_i386.h
+++ b/target/i386/kvm/kvm_i386.h
@@ -51,4 +51,6 @@ bool kvm_hyperv_expand_features(X86CPU *cpu, Error **errp);
uint64_t kvm_swizzle_msi_ext_dest_id(uint64_t address);
+bool kvm_enable_sgx_provisioning(KVMState *s);
+
#endif
--
2.31.1
[PULL 08/33] i386: Add SGX CPUID leaf FEAT_SGX_12_0_EAX
From: Sean Christopherson
CPUID leaf 12_0_EAX is an Intel-defined feature bits leaf enumerating
the CPU's SGX capabilities, e.g. supported SGX instruction sets.
Currently there are four enumerated capabilities:
- SGX1 instruction set, i.e. "base" SGX
- SGX2 instruction set for dynamic EPC management
- ENCLV instruction set for VMM oversubscription of EPC
- ENCLS-C instruction set for thread safe variants of ENCLS
Signed-off-by: Sean Christopherson
Signed-off-by: Yang Zhong
Message-Id: <20210719112136.57018-8-yang.zh...@intel.com>
Signed-off-by: Paolo Bonzini
---
target/i386/cpu.c | 20
target/i386/cpu.h | 1 +
2 files changed, 21 insertions(+)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 21d2a325ea..2cd1487bae 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -654,6 +654,7 @@ void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1,
/* missing:
CPUID_XSAVE_XSAVEC, CPUID_XSAVE_XSAVES */
#define TCG_14_0_ECX_FEATURES 0
+#define TCG_SGX_12_0_EAX_FEATURES 0
FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
[FEAT_1_EDX] = {
@@ -1182,6 +1183,25 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
.tcg_features = TCG_14_0_ECX_FEATURES,
},
+[FEAT_SGX_12_0_EAX] = {
+.type = CPUID_FEATURE_WORD,
+.feat_names = {
+"sgx1", "sgx2", NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+},
+.cpuid = {
+.eax = 0x12,
+.needs_ecx = true, .ecx = 0,
+.reg = R_EAX,
+},
+.tcg_features = TCG_SGX_12_0_EAX_FEATURES,
+},
};
typedef struct FeatureMask {
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index b6491df0f5..2b199102ef 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -578,6 +578,7 @@ typedef enum FeatureWord {
FEAT_VMX_BASIC,
FEAT_VMX_VMFUNC,
FEAT_14_0_ECX,
+FEAT_SGX_12_0_EAX, /* CPUID[EAX=0x12,ECX=0].EAX (SGX) */
FEATURE_WORDS,
} FeatureWord;
--
2.31.1
[PULL 23/33] sgx-epc: Add the fill_device_info() callback support
From: Yang Zhong
Since there is no fill_device_info() callback support, and when we
execute "info memory-devices" command in the monitor, the segfault
will be found.
This patch will add this callback support and "info memory-devices"
will show sgx epc memory exposed to guest. The result as below:
qemu) info memory-devices
Memory device [sgx-epc]: ""
memaddr: 0x18000
size: 29360128
memdev: /objects/mem1
Memory device [sgx-epc]: ""
memaddr: 0x181c0
size: 10485760
memdev: /objects/mem2
Signed-off-by: Yang Zhong
Message-Id: <20210719112136.57018-33-yang.zh...@intel.com>
Signed-off-by: Paolo Bonzini
---
hw/i386/sgx-epc.c | 11 ++-
monitor/hmp-cmds.c | 10 ++
qapi/machine.json | 39 ---
3 files changed, 56 insertions(+), 4 deletions(-)
diff --git a/hw/i386/sgx-epc.c b/hw/i386/sgx-epc.c
index 6677dc74b5..55e2217eae 100644
--- a/hw/i386/sgx-epc.c
+++ b/hw/i386/sgx-epc.c
@@ -133,7 +133,16 @@ static MemoryRegion
*sgx_epc_md_get_memory_region(MemoryDeviceState *md,
static void sgx_epc_md_fill_device_info(const MemoryDeviceState *md,
MemoryDeviceInfo *info)
{
-/* TODO */
+SgxEPCDeviceInfo *se = g_new0(SgxEPCDeviceInfo, 1);
+SGXEPCDevice *epc = SGX_EPC(md);
+
+se->memaddr = epc->addr;
+se->size = object_property_get_uint(OBJECT(epc), SGX_EPC_SIZE_PROP,
+NULL);
+se->memdev = object_get_canonical_path(OBJECT(epc->hostmem));
+
+info->u.sgx_epc.data = se;
+info->type = MEMORY_DEVICE_INFO_KIND_SGX_EPC;
}
static void sgx_epc_class_init(ObjectClass *oc, void *data)
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index b5e71d9e6f..bcaa41350e 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1823,6 +1823,7 @@ void hmp_info_memory_devices(Monitor *mon, const QDict
*qdict)
VirtioMEMDeviceInfo *vmi;
MemoryDeviceInfo *value;
PCDIMMDeviceInfo *di;
+SgxEPCDeviceInfo *se;
for (info = info_list; info; info = info->next) {
value = info->value;
@@ -1870,6 +1871,15 @@ void hmp_info_memory_devices(Monitor *mon, const QDict
*qdict)
vmi->block_size);
monitor_printf(mon, " memdev: %s\n", vmi->memdev);
break;
+case MEMORY_DEVICE_INFO_KIND_SGX_EPC:
+se = value->u.sgx_epc.data;
+monitor_printf(mon, "Memory device [%s]: \"%s\"\n",
+ MemoryDeviceInfoKind_str(value->type),
+ se->id ? se->id : "");
+monitor_printf(mon, " memaddr: 0x%" PRIx64 "\n", se->memaddr);
+monitor_printf(mon, " size: %" PRIu64 "\n", se->size);
+monitor_printf(mon, " memdev: %s\n", se->memdev);
+break;
default:
g_assert_not_reached();
}
diff --git a/qapi/machine.json b/qapi/machine.json
index 26c539fe2c..e2f01e9c15 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1194,13 +1194,36 @@
}
}
+##
+# @SgxEPCDeviceInfo:
+#
+# Sgx EPC state information
+#
+# @id: device's ID
+#
+# @memaddr: physical address in memory, where device is mapped
+#
+# @size: size of memory that the device provides
+#
+# @memdev: memory backend linked with device
+#
+# Since: 6.2
+##
+{ 'struct': 'SgxEPCDeviceInfo',
+ 'data': { '*id': 'str',
+'memaddr': 'size',
+'size': 'size',
+'memdev': 'str'
+ }
+}
+
##
# @MemoryDeviceInfoKind:
#
# Since: 2.1
##
{ 'enum': 'MemoryDeviceInfoKind',
- 'data': [ 'dimm', 'nvdimm', 'virtio-pmem', 'virtio-mem' ] }
+ 'data': [ 'dimm', 'nvdimm', 'virtio-pmem', 'virtio-mem', 'sgx-epc' ] }
##
# @PCDIMMDeviceInfoWrapper:
@@ -1225,13 +1248,22 @@
##
{ 'struct': 'VirtioMEMDeviceInfoWrapper',
'data': { 'data': 'VirtioMEMDeviceInfo' } }
+
+##
+# @SgxEPCDeviceInfoWrapper:
+#
+# Since: 6.2
+##
+{ 'struct': 'SgxEPCDeviceInfoWrapper',
+ 'data': { 'data': 'SgxEPCDeviceInfo' } }
+
##
# @MemoryDeviceInfo:
#
# Union containing information about a memory device
#
# nvdimm is included since 2.12. virtio-pmem is included since 4.1.
-# virtio-mem is included since 5.1.
+# virtio-mem is included since 5.1. sgx-epc is included since 6.2.
#
# Since: 2.1
##
@@ -1241,7 +1273,8 @@
'data': { 'dimm': 'PCDIMMDeviceInfoWrapper',
'nvdimm': 'PCDIMMDeviceInfoWrapper',
'virtio-pmem': 'VirtioPMEMDeviceInfoWrapper',
-'virtio-mem': 'VirtioMEMDeviceInfoWrapper'
+'virtio-mem': 'VirtioMEMDeviceInfoWrapper',
+'sgx-epc': 'SgxEPCDeviceInfoWrapper'
}
}
--
2.31.1
[PULL 10/33] i386: Add SGX CPUID leaf FEAT_SGX_12_1_EAX
From: Sean Christopherson
CPUID leaf 12_1_EAX is an Intel-defined feature bits leaf enumerating
the platform's SGX capabilities that may be utilized by an enclave, e.g.
whether or not an enclave can gain access to the provision key.
Currently there are six capabilities:
- INIT: set when the enclave has has been initialized by EINIT. Cannot
be set by software, i.e. forced to zero in CPUID.
- DEBUG: permits a debugger to read/write into the enclave.
- MODE64BIT: the enclave runs in 64-bit mode
- PROVISIONKEY: grants has access to the provision key
- EINITTOKENKEY: grants access to the EINIT token key, i.e. the
enclave can generate EINIT tokens
- KSS: Key Separation and Sharing enabled for the enclave.
Note that the entirety of CPUID.0x12.0x1, i.e. all registers, enumerates
the allowed ATTRIBUTES (128 bits), but only bits 31:0 are directly
exposed to the user (via FEAT_12_1_EAX). Bits 63:32 are currently all
reserved and bits 127:64 correspond to the allowed XSAVE Feature Request
Mask, which is calculated based on other CPU features, e.g. XSAVE, MPX,
AVX, etc... and is not exposed to the user.
Signed-off-by: Sean Christopherson
Signed-off-by: Yang Zhong
Message-Id: <20210719112136.57018-10-yang.zh...@intel.com>
Signed-off-by: Paolo Bonzini
---
target/i386/cpu.c | 21 +
target/i386/cpu.h | 1 +
2 files changed, 22 insertions(+)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index c0d5c3c621..e9ecbf59e5 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -656,6 +656,7 @@ void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1,
#define TCG_14_0_ECX_FEATURES 0
#define TCG_SGX_12_0_EAX_FEATURES 0
#define TCG_SGX_12_0_EBX_FEATURES 0
+#define TCG_SGX_12_1_EAX_FEATURES 0
FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
[FEAT_1_EDX] = {
@@ -1223,6 +1224,26 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
},
.tcg_features = TCG_SGX_12_0_EBX_FEATURES,
},
+
+[FEAT_SGX_12_1_EAX] = {
+.type = CPUID_FEATURE_WORD,
+.feat_names = {
+NULL, "sgx-debug", "sgx-mode64", NULL,
+"sgx-provisionkey", "sgx-tokenkey", NULL, "sgx-kss",
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+},
+.cpuid = {
+.eax = 0x12,
+.needs_ecx = true, .ecx = 1,
+.reg = R_EAX,
+},
+.tcg_features = TCG_SGX_12_1_EAX_FEATURES,
+},
};
typedef struct FeatureMask {
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index e66ec85980..85a9eeeb2b 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -580,6 +580,7 @@ typedef enum FeatureWord {
FEAT_14_0_ECX,
FEAT_SGX_12_0_EAX, /* CPUID[EAX=0x12,ECX=0].EAX (SGX) */
FEAT_SGX_12_0_EBX, /* CPUID[EAX=0x12,ECX=0].EBX (SGX MISCSELECT[31:0]) */
+FEAT_SGX_12_1_EAX, /* CPUID[EAX=0x12,ECX=1].EAX (SGX ATTRIBUTES[31:0]) */
FEATURE_WORDS,
} FeatureWord;
--
2.31.1
[PULL 04/33] qom: Add memory-backend-epc ObjectOptions support
From: Yang Zhong
Add the new 'memory-backend-epc' user creatable QOM object in
the ObjectOptions to support SGX since v6.1, or the sgx backend
object cannot bootup.
Signed-off-by: Yang Zhong
Message-Id: <20210719112136.57018-4-yang.zh...@intel.com>
Signed-off-by: Paolo Bonzini
---
qapi/qom.json | 19 +++
1 file changed, 19 insertions(+)
diff --git a/qapi/qom.json b/qapi/qom.json
index a25616bc7a..0222bb4506 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -647,6 +647,23 @@
'*hugetlbsize': 'size',
'*seal': 'bool' } }
+##
+# @MemoryBackendEpcProperties:
+#
+# Properties for memory-backend-epc objects.
+#
+# The @share boolean option is true by default with epc
+#
+# The @merge boolean option is false by default with epc
+#
+# The @dump boolean option is false by default with epc
+#
+# Since: 6.2
+##
+{ 'struct': 'MemoryBackendEpcProperties',
+ 'base': 'MemoryBackendProperties',
+ 'data': {} }
+
##
# @PrManagerHelperProperties:
#
@@ -797,6 +814,7 @@
{ 'name': 'memory-backend-memfd',
'if': 'CONFIG_LINUX' },
'memory-backend-ram',
+'memory-backend-epc',
'pef-guest',
'pr-manager-helper',
'qtest',
@@ -855,6 +873,7 @@
'memory-backend-memfd': { 'type': 'MemoryBackendMemfdProperties',
'if': 'CONFIG_LINUX' },
'memory-backend-ram': 'MemoryBackendProperties',
+ 'memory-backend-epc': 'MemoryBackendEpcProperties',
'pr-manager-helper': 'PrManagerHelperProperties',
'qtest': 'QtestProperties',
'rng-builtin':'RngProperties',
--
2.31.1
[PULL 07/33] i386: Add primary SGX CPUID and MSR defines
From: Sean Christopherson
Add CPUID defines for SGX and SGX Launch Control (LC), as well as
defines for their associated FEATURE_CONTROL MSR bits. Define the
Launch Enclave Public Key Hash MSRs (LE Hash MSRs), which exist
when SGX LC is present (in CPUID), and are writable when SGX LC is
enabled (in FEATURE_CONTROL).
Signed-off-by: Sean Christopherson
Signed-off-by: Yang Zhong
Message-Id: <20210719112136.57018-7-yang.zh...@intel.com>
Signed-off-by: Paolo Bonzini
---
target/i386/cpu.c | 4 ++--
target/i386/cpu.h | 12
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 6b029f1bdf..21d2a325ea 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -795,7 +795,7 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
[FEAT_7_0_EBX] = {
.type = CPUID_FEATURE_WORD,
.feat_names = {
-"fsgsbase", "tsc-adjust", NULL, "bmi1",
+"fsgsbase", "tsc-adjust", "sgx", "bmi1",
"hle", "avx2", NULL, "smep",
"bmi2", "erms", "invpcid", "rtm",
NULL, NULL, "mpx", NULL,
@@ -821,7 +821,7 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
"la57", NULL, NULL, NULL,
NULL, NULL, "rdpid", NULL,
"bus-lock-detect", "cldemote", NULL, "movdiri",
-"movdir64b", NULL, NULL, "pks",
+"movdir64b", NULL, "sgxlc", "pks",
},
.cpuid = {
.eax = 7,
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index c2954c71ea..b6491df0f5 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -389,9 +389,17 @@ typedef enum X86Seg {
#define MSR_IA32_PKRS 0x6e1
#define FEATURE_CONTROL_LOCKED(1<<0)
+#define FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX (1ULL << 1)
#define FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX (1<<2)
+#define FEATURE_CONTROL_SGX_LC(1ULL << 17)
+#define FEATURE_CONTROL_SGX (1ULL << 18)
#define FEATURE_CONTROL_LMCE (1<<20)
+#define MSR_IA32_SGXLEPUBKEYHASH0 0x8c
+#define MSR_IA32_SGXLEPUBKEYHASH1 0x8d
+#define MSR_IA32_SGXLEPUBKEYHASH2 0x8e
+#define MSR_IA32_SGXLEPUBKEYHASH3 0x8f
+
#define MSR_P6_PERFCTR0 0xc1
#define MSR_IA32_SMBASE 0x9e
@@ -718,6 +726,8 @@ typedef uint64_t FeatureWordArray[FEATURE_WORDS];
/* Support RDFSBASE/RDGSBASE/WRFSBASE/WRGSBASE */
#define CPUID_7_0_EBX_FSGSBASE (1U << 0)
+/* Support SGX */
+#define CPUID_7_0_EBX_SGX (1U << 2)
/* 1st Group of Advanced Bit Manipulation Extensions */
#define CPUID_7_0_EBX_BMI1 (1U << 3)
/* Hardware Lock Elision */
@@ -805,6 +815,8 @@ typedef uint64_t FeatureWordArray[FEATURE_WORDS];
#define CPUID_7_0_ECX_MOVDIRI (1U << 27)
/* Move 64 Bytes as Direct Store Instruction */
#define CPUID_7_0_ECX_MOVDIR64B (1U << 28)
+/* Support SGX Launch Control */
+#define CPUID_7_0_ECX_SGX_LC(1U << 30)
/* Protection Keys for Supervisor-mode Pages */
#define CPUID_7_0_ECX_PKS (1U << 31)
--
2.31.1
[PULL 15/33] i386: Propagate SGX CPUID sub-leafs to KVM
From: Sean Christopherson
The SGX sub-leafs are enumerated at CPUID 0x12. Indices 0 and 1 are
always present when SGX is supported, and enumerate SGX features and
capabilities. Indices >=2 are directly correlated with the platform's
EPC sections. Because the number of EPC sections is dynamic and user
defined, the number of SGX sub-leafs is "NULL" terminated.
Signed-off-by: Sean Christopherson
Signed-off-by: Yang Zhong
Message-Id: <20210719112136.57018-15-yang.zh...@intel.com>
Signed-off-by: Paolo Bonzini
---
target/i386/kvm/kvm.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 488926a95f..f6bbf33bc1 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -1703,6 +1703,25 @@ int kvm_arch_init_vcpu(CPUState *cs)
}
break;
case 0x7:
+case 0x12:
+for (j = 0; ; j++) {
+c->function = i;
+c->flags = KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
+c->index = j;
+cpu_x86_cpuid(env, i, j, &c->eax, &c->ebx, &c->ecx, &c->edx);
+
+if (j > 1 && (c->eax & 0xf) != 1) {
+break;
+}
+
+if (cpuid_i == KVM_MAX_CPUID_ENTRIES) {
+fprintf(stderr, "cpuid_data is full, no space for "
+"cpuid(eax:0x12,ecx:0x%x)\n", j);
+abort();
+}
+c = &cpuid_data.entries[cpuid_i++];
+}
+break;
case 0x14: {
uint32_t times;
--
2.31.1
[PULL 09/33] i386: Add SGX CPUID leaf FEAT_SGX_12_0_EBX
From: Sean Christopherson
CPUID leaf 12_0_EBX is an Intel-defined feature bits leaf enumerating
the platform's SGX extended capabilities. Currently there is a single
capabilitiy:
- EXINFO: record information about #PFs and #GPs in the enclave's SSA
Signed-off-by: Sean Christopherson
Signed-off-by: Yang Zhong
Message-Id: <20210719112136.57018-9-yang.zh...@intel.com>
Signed-off-by: Paolo Bonzini
---
target/i386/cpu.c | 21 +
target/i386/cpu.h | 1 +
2 files changed, 22 insertions(+)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 2cd1487bae..c0d5c3c621 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -655,6 +655,7 @@ void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1,
CPUID_XSAVE_XSAVEC, CPUID_XSAVE_XSAVES */
#define TCG_14_0_ECX_FEATURES 0
#define TCG_SGX_12_0_EAX_FEATURES 0
+#define TCG_SGX_12_0_EBX_FEATURES 0
FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
[FEAT_1_EDX] = {
@@ -1202,6 +1203,26 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
},
.tcg_features = TCG_SGX_12_0_EAX_FEATURES,
},
+
+[FEAT_SGX_12_0_EBX] = {
+.type = CPUID_FEATURE_WORD,
+.feat_names = {
+"sgx-exinfo" , NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+},
+.cpuid = {
+.eax = 0x12,
+.needs_ecx = true, .ecx = 0,
+.reg = R_EBX,
+},
+.tcg_features = TCG_SGX_12_0_EBX_FEATURES,
+},
};
typedef struct FeatureMask {
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 2b199102ef..e66ec85980 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -579,6 +579,7 @@ typedef enum FeatureWord {
FEAT_VMX_VMFUNC,
FEAT_14_0_ECX,
FEAT_SGX_12_0_EAX, /* CPUID[EAX=0x12,ECX=0].EAX (SGX) */
+FEAT_SGX_12_0_EBX, /* CPUID[EAX=0x12,ECX=0].EBX (SGX MISCSELECT[31:0]) */
FEATURE_WORDS,
} FeatureWord;
--
2.31.1
[PULL 30/33] memory: Name all the memory listeners
From: Peter Xu
Provide a name field for all the memory listeners. It can be used to identify
which memory listener is which.
Signed-off-by: Peter Xu
Reviewed-by: David Hildenbrand
Message-Id: <20210817013553.30584-2-pet...@redhat.com>
Signed-off-by: Paolo Bonzini
---
accel/hvf/hvf-accel-ops.c | 1 +
accel/kvm/kvm-all.c | 7 +--
hw/i386/xen/xen-hvm.c | 2 ++
hw/intc/openpic_kvm.c | 1 +
hw/remote/proxy-memory-listener.c | 1 +
hw/vfio/common.c | 1 +
hw/vfio/spapr.c | 1 +
hw/virtio/vhost-vdpa.c| 1 +
hw/virtio/vhost.c | 2 ++
hw/virtio/virtio.c| 1 +
hw/xen/xen_pt.c | 2 ++
include/exec/memory.h | 8
include/sysemu/kvm_int.h | 2 +-
softmmu/physmem.c | 1 +
target/arm/kvm.c | 1 +
target/i386/hax/hax-mem.c | 1 +
target/i386/kvm/kvm.c | 2 +-
target/i386/nvmm/nvmm-all.c | 1 +
target/i386/whpx/whpx-all.c | 1 +
19 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/accel/hvf/hvf-accel-ops.c b/accel/hvf/hvf-accel-ops.c
index 93976f4ece..6cbd2c3f97 100644
--- a/accel/hvf/hvf-accel-ops.c
+++ b/accel/hvf/hvf-accel-ops.c
@@ -295,6 +295,7 @@ static void hvf_region_del(MemoryListener *listener,
}
static MemoryListener hvf_memory_listener = {
+.name = "hvf",
.priority = 10,
.region_add = hvf_region_add,
.region_del = hvf_region_del,
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index cace5ffe64..db8d83b137 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -1129,6 +1129,7 @@ static void kvm_coalesce_pio_del(MemoryListener *listener,
}
static MemoryListener kvm_coalesced_pio_listener = {
+.name = "kvm-coalesced-pio",
.coalesced_io_add = kvm_coalesce_pio_add,
.coalesced_io_del = kvm_coalesce_pio_del,
};
@@ -1633,7 +1634,7 @@ static void kvm_io_ioeventfd_del(MemoryListener *listener,
}
void kvm_memory_listener_register(KVMState *s, KVMMemoryListener *kml,
- AddressSpace *as, int as_id)
+ AddressSpace *as, int as_id, const char
*name)
{
int i;
@@ -1649,6 +1650,7 @@ void kvm_memory_listener_register(KVMState *s,
KVMMemoryListener *kml,
kml->listener.log_start = kvm_log_start;
kml->listener.log_stop = kvm_log_stop;
kml->listener.priority = 10;
+kml->listener.name = name;
if (s->kvm_dirty_ring_size) {
kml->listener.log_sync_global = kvm_log_sync_global;
@@ -1669,6 +1671,7 @@ void kvm_memory_listener_register(KVMState *s,
KVMMemoryListener *kml,
}
static MemoryListener kvm_io_listener = {
+.name = "kvm-io",
.eventfd_add = kvm_io_ioeventfd_add,
.eventfd_del = kvm_io_ioeventfd_del,
.priority = 10,
@@ -2579,7 +2582,7 @@ static int kvm_init(MachineState *ms)
s->memory_listener.listener.coalesced_io_del = kvm_uncoalesce_mmio_region;
kvm_memory_listener_register(s, &s->memory_listener,
- &address_space_memory, 0);
+ &address_space_memory, 0, "kvm-memory");
if (kvm_eventfds_allowed) {
memory_listener_register(&kvm_io_listener,
&address_space_io);
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 9b432773f0..e3d3d5cf89 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -721,6 +721,7 @@ static void xen_log_global_stop(MemoryListener *listener)
}
static MemoryListener xen_memory_listener = {
+.name = "xen-memory",
.region_add = xen_region_add,
.region_del = xen_region_del,
.log_start = xen_log_start,
@@ -732,6 +733,7 @@ static MemoryListener xen_memory_listener = {
};
static MemoryListener xen_io_listener = {
+.name = "xen-io",
.region_add = xen_io_add,
.region_del = xen_io_del,
.priority = 10,
diff --git a/hw/intc/openpic_kvm.c b/hw/intc/openpic_kvm.c
index 21da680389..557dd0c2bf 100644
--- a/hw/intc/openpic_kvm.c
+++ b/hw/intc/openpic_kvm.c
@@ -234,6 +234,7 @@ static void kvm_openpic_realize(DeviceState *dev, Error
**errp)
opp->mem_listener.region_add = kvm_openpic_region_add;
opp->mem_listener.region_del = kvm_openpic_region_del;
+opp->mem_listener.name = "openpic-kvm";
memory_listener_register(&opp->mem_listener, &address_space_memory);
/* indicate pic capabilities */
diff --git a/hw/remote/proxy-memory-listener.c
b/hw/remote/proxy-memory-listener.c
index 901dbf1357..882c9b4854 100644
--- a/hw/remote/proxy-memory-listener.c
+++ b/hw/remote/proxy-memory-listener.c
@@ -219,6 +219,7 @@ void proxy_memory_listener_configure(ProxyMemoryListener
*proxy_listener,
proxy_listener->listener.region_add = proxy_memory_listener_region_addnop;
proxy_listener->listener.region_nop = proxy_memory_listener_region_addnop;
proxy_listener->listener.priorit
[PULL 18/33] hw/i386/pc: Account for SGX EPC sections when calculating device memory
From: Sean Christopherson
Add helpers to detect if SGX EPC exists above 4g, and if so, where SGX
EPC above 4g ends. Use the helpers to adjust the device memory range
if SGX EPC exists above 4g.
For multiple virtual EPC sections, we just put them together physically
contiguous for the simplicity because we don't support EPC NUMA affinity
now. Once the SGX EPC NUMA support in the kernel SGX driver, we will
support this in the future.
Note that SGX EPC is currently hardcoded to reside above 4g.
Signed-off-by: Sean Christopherson
Signed-off-by: Yang Zhong
Message-Id: <20210719112136.57018-18-yang.zh...@intel.com>
Signed-off-by: Paolo Bonzini
---
hw/i386/pc.c | 11 ++-
include/hw/i386/sgx-epc.h | 7 +++
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 557d49c9f8..e41c002539 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -919,8 +919,15 @@ void pc_memory_init(PCMachineState *pcms,
exit(EXIT_FAILURE);
}
+if (pcms->sgx_epc.size != 0) {
+machine->device_memory->base =
sgx_epc_above_4g_end(&pcms->sgx_epc);
+} else {
+machine->device_memory->base =
+0x1ULL + x86ms->above_4g_mem_size;
+}
+
machine->device_memory->base =
-ROUND_UP(0x1ULL + x86ms->above_4g_mem_size, 1 * GiB);
+ROUND_UP(machine->device_memory->base, 1 * GiB);
if (pcmc->enforce_aligned_dimm) {
/* size device region assuming 1G page max alignment per slot */
@@ -1005,6 +1012,8 @@ uint64_t pc_pci_hole64_start(void)
if (!pcmc->broken_reserved_end) {
hole64_start += memory_region_size(&ms->device_memory->mr);
}
+} else if (pcms->sgx_epc.size != 0) {
+hole64_start = sgx_epc_above_4g_end(&pcms->sgx_epc);
} else {
hole64_start = 0x1ULL + x86ms->above_4g_mem_size;
}
diff --git a/include/hw/i386/sgx-epc.h b/include/hw/i386/sgx-epc.h
index 75b19f464c..65a68ca753 100644
--- a/include/hw/i386/sgx-epc.h
+++ b/include/hw/i386/sgx-epc.h
@@ -57,4 +57,11 @@ typedef struct SGXEPCState {
int sgx_epc_get_section(int section_nr, uint64_t *addr, uint64_t *size);
+static inline uint64_t sgx_epc_above_4g_end(SGXEPCState *sgx_epc)
+{
+assert(sgx_epc != NULL && sgx_epc->base >= 0x1ULL);
+
+return sgx_epc->base + sgx_epc->size;
+}
+
#endif
--
2.31.1
[PULL 17/33] hw/i386/fw_cfg: Set SGX bits in feature control fw_cfg accordingly
From: Sean Christopherson
Request SGX an SGX Launch Control to be enabled in FEATURE_CONTROL
when the features are exposed to the guest. Our design is the SGX
Launch Control bit will be unconditionally set in FEATURE_CONTROL,
which is unlike host bios.
Signed-off-by: Sean Christopherson
Signed-off-by: Yang Zhong
Message-Id: <20210719112136.57018-17-yang.zh...@intel.com>
Signed-off-by: Paolo Bonzini
---
hw/i386/fw_cfg.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
index 4e68d5dea4..f6d036dfbe 100644
--- a/hw/i386/fw_cfg.c
+++ b/hw/i386/fw_cfg.c
@@ -159,7 +159,7 @@ void fw_cfg_build_feature_control(MachineState *ms,
FWCfgState *fw_cfg)
{
X86CPU *cpu = X86_CPU(ms->possible_cpus->cpus[0].cpu);
CPUX86State *env = &cpu->env;
-uint32_t unused, ecx, edx;
+uint32_t unused, ebx, ecx, edx;
uint64_t feature_control_bits = 0;
uint64_t *val;
@@ -174,6 +174,14 @@ void fw_cfg_build_feature_control(MachineState *ms,
FWCfgState *fw_cfg)
feature_control_bits |= FEATURE_CONTROL_LMCE;
}
+cpu_x86_cpuid(env, 0x7, 0, &unused, &ebx, &ecx, &unused);
+if (ebx & CPUID_7_0_EBX_SGX) {
+feature_control_bits |= FEATURE_CONTROL_SGX;
+}
+if (ecx & CPUID_7_0_ECX_SGX_LC) {
+feature_control_bits |= FEATURE_CONTROL_SGX_LC;
+}
+
if (!feature_control_bits) {
return;
}
--
2.31.1
[PULL 28/33] tests: qtest: bios-tables-test depends on the unpacked edk2 ROMs
Skip the test if bzip2 is not available, and run it after they are
uncompressed.
Signed-off-by: Paolo Bonzini
Message-Id: <20210923105529.3845741-2-pbonz...@redhat.com>
Signed-off-by: Paolo Bonzini
---
pc-bios/meson.build | 3 ++-
tests/qtest/meson.build | 6 +++---
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/pc-bios/meson.build b/pc-bios/meson.build
index a3b3d87891..a44c9bc127 100644
--- a/pc-bios/meson.build
+++ b/pc-bios/meson.build
@@ -1,3 +1,4 @@
+roms = []
if unpack_edk2_blobs
fds = [
'edk2-aarch64-code.fd',
@@ -11,7 +12,7 @@ if unpack_edk2_blobs
]
foreach f : fds
-custom_target(f,
+roms += custom_target(f,
build_by_default: have_system,
output: f,
input: '@0@.bz2'.format(f),
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 757bb8499a..19444d4752 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -68,12 +68,12 @@ qtests_i386 = \
(config_all_devices.has_key('CONFIG_RTL8139_PCI') ? ['rtl8139-test'] : []) +
\
(config_all_devices.has_key('CONFIG_E1000E_PCI_EXPRESS') ?
['fuzz-e1000e-test'] : []) + \
(config_all_devices.has_key('CONFIG_ESP_PCI') ? ['am53c974-test'] : []) +
\
+ (unpack_edk2_blobs ? ['bios-tables-test'] : []) +
\
qtests_pci +
\
['fdc-test',
'ide-test',
'hd-geo-test',
'boot-order-test',
- 'bios-tables-test',
'rtc-test',
'i440fx-test',
'fw_cfg-test',
@@ -180,7 +180,7 @@ qtests_arm = \
# TODO: once aarch64 TCG is fixed on ARM 32 bit host, make bios-tables-test
unconditional
qtests_aarch64 = \
- (cpu != 'arm' ? ['bios-tables-test'] : []) +
\
+ (cpu != 'arm' and unpack_edk2_blobs ? ['bios-tables-test'] : []) +
\
(config_all_devices.has_key('CONFIG_TPM_TIS_SYSBUS') ?
['tpm-tis-device-test'] : []) +\
(config_all_devices.has_key('CONFIG_TPM_TIS_SYSBUS') ?
['tpm-tis-device-swtpm-test'] : []) + \
['arm-cpu-features',
@@ -269,7 +269,7 @@ foreach dir : target_dirs
qtest_emulator = emulators['qemu-system-' + target_base]
target_qtests = get_variable('qtests_' + target_base, []) + qtests_generic
- test_deps = []
+ test_deps = roms
qtest_env = environment()
if have_tools
qtest_env.set('QTEST_QEMU_IMG', './qemu-img')
--
2.31.1
[PULL 13/33] i386: Update SGX CPUID info according to hardware/KVM/user input
From: Sean Christopherson
Expose SGX to the guest if and only if KVM is enabled and supports
virtualization of SGX. While the majority of ENCLS can be emulated to
some degree, because SGX uses a hardware-based root of trust, the
attestation aspects of SGX cannot be emulated in software, i.e.
ultimately emulation will fail as software cannot generate a valid
quote/report. The complexity of partially emulating SGX in Qemu far
outweighs the value added, e.g. an SGX specific simulator for userspace
applications can emulate SGX for development and testing purposes.
Note, access to the PROVISIONKEY is not yet advertised to the guest as
KVM blocks access to the PROVISIONKEY by default and requires userspace
to provide additional credentials (via ioctl()) to expose PROVISIONKEY.
Signed-off-by: Sean Christopherson
Signed-off-by: Yang Zhong
Message-Id: <20210719112136.57018-13-yang.zh...@intel.com>
Signed-off-by: Paolo Bonzini
---
hw/i386/meson.build | 3 +-
hw/i386/sgx-stub.c| 13 +++
hw/i386/sgx.c | 73 +
include/hw/i386/pc.h | 3 ++
include/hw/i386/sgx-epc.h | 2 +
target/i386/cpu.c | 77 +++
6 files changed, 170 insertions(+), 1 deletion(-)
create mode 100644 hw/i386/sgx-stub.c
create mode 100644 hw/i386/sgx.c
diff --git a/hw/i386/meson.build b/hw/i386/meson.build
index b1862c83d4..c502965219 100644
--- a/hw/i386/meson.build
+++ b/hw/i386/meson.build
@@ -16,7 +16,8 @@ i386_ss.add(when: 'CONFIG_Q35', if_true: files('pc_q35.c'))
i386_ss.add(when: 'CONFIG_VMMOUSE', if_true: files('vmmouse.c'))
i386_ss.add(when: 'CONFIG_VMPORT', if_true: files('vmport.c'))
i386_ss.add(when: 'CONFIG_VTD', if_true: files('intel_iommu.c'))
-i386_ss.add(when: 'CONFIG_SGX', if_true: files('sgx-epc.c'))
+i386_ss.add(when: 'CONFIG_SGX', if_true: files('sgx-epc.c','sgx.c'),
+if_false: files('sgx-stub.c'))
i386_ss.add(when: 'CONFIG_ACPI', if_true: files('acpi-common.c'))
i386_ss.add(when: 'CONFIG_ACPI_HW_REDUCED', if_true:
files('generic_event_device_x86.c'))
diff --git a/hw/i386/sgx-stub.c b/hw/i386/sgx-stub.c
new file mode 100644
index 00..483c72bba6
--- /dev/null
+++ b/hw/i386/sgx-stub.c
@@ -0,0 +1,13 @@
+#include "qemu/osdep.h"
+#include "hw/i386/pc.h"
+#include "hw/i386/sgx-epc.h"
+
+void pc_machine_init_sgx_epc(PCMachineState *pcms)
+{
+memset(&pcms->sgx_epc, 0, sizeof(SGXEPCState));
+}
+
+int sgx_epc_get_section(int section_nr, uint64_t *addr, uint64_t *size)
+{
+g_assert_not_reached();
+}
diff --git a/hw/i386/sgx.c b/hw/i386/sgx.c
new file mode 100644
index 00..8a18cddc3f
--- /dev/null
+++ b/hw/i386/sgx.c
@@ -0,0 +1,73 @@
+/*
+ * SGX common code
+ *
+ * Copyright (C) 2021 Intel Corporation
+ *
+ * Authors:
+ * Yang Zhong
+ * Sean Christopherson
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#include "qemu/osdep.h"
+#include "hw/i386/pc.h"
+#include "hw/i386/sgx-epc.h"
+#include "hw/mem/memory-device.h"
+#include "monitor/qdev.h"
+#include "qapi/error.h"
+#include "exec/address-spaces.h"
+
+int sgx_epc_get_section(int section_nr, uint64_t *addr, uint64_t *size)
+{
+PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
+SGXEPCDevice *epc;
+
+if (pcms->sgx_epc.size == 0 || pcms->sgx_epc.nr_sections <= section_nr) {
+return 1;
+}
+
+epc = pcms->sgx_epc.sections[section_nr];
+
+*addr = epc->addr;
+*size = memory_device_get_region_size(MEMORY_DEVICE(epc), &error_fatal);
+
+return 0;
+}
+
+void pc_machine_init_sgx_epc(PCMachineState *pcms)
+{
+SGXEPCState *sgx_epc = &pcms->sgx_epc;
+X86MachineState *x86ms = X86_MACHINE(pcms);
+SgxEPCList *list = NULL;
+Object *obj;
+
+memset(sgx_epc, 0, sizeof(SGXEPCState));
+if (!x86ms->sgx_epc_list) {
+return;
+}
+
+sgx_epc->base = 0x1ULL + x86ms->above_4g_mem_size;
+
+memory_region_init(&sgx_epc->mr, OBJECT(pcms), "sgx-epc", UINT64_MAX);
+memory_region_add_subregion(get_system_memory(), sgx_epc->base,
+&sgx_epc->mr);
+
+for (list = x86ms->sgx_epc_list; list; list = list->next) {
+obj = object_new("sgx-epc");
+
+/* set the memdev link with memory backend */
+object_property_parse(obj, SGX_EPC_MEMDEV_PROP, list->value->memdev,
+ &error_fatal);
+object_property_set_bool(obj, "realized", true, &error_fatal);
+object_unref(obj);
+}
+
+if ((sgx_epc->base + sgx_epc->size) < sgx_epc->base) {
+error_report("Size of all 'sgx-epc' =0x%"PRIu64" causes EPC to wrap",
+ sgx_epc->size);
+exit(EXIT_FAILURE);
+}
+
+memory_region_set_size(&sgx_epc->mr, sgx_epc->size);
+}
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 668e48be8a..5748d7c55f 100644
--- a/include/hw/i386/p
[PULL 12/33] i386: Add feature control MSR dependency when SGX is enabled
From: Sean Christopherson
SGX adds multiple flags to FEATURE_CONTROL to enable SGX and Flexible
Launch Control.
Signed-off-by: Sean Christopherson
Signed-off-by: Yang Zhong
Message-Id: <20210719112136.57018-12-yang.zh...@intel.com>
Signed-off-by: Paolo Bonzini
---
target/i386/kvm/kvm.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 11551648f9..6dc40161e0 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -1877,6 +1877,11 @@ int kvm_arch_init_vcpu(CPUState *cs)
!!(c->ecx & CPUID_EXT_SMX);
}
+c = cpuid_find_entry(&cpuid_data.cpuid, 7, 0);
+if (c && (c->ebx & CPUID_7_0_EBX_SGX)) {
+has_msr_feature_control = true;
+}
+
if (env->mcg_cap & MCG_LMCE_P) {
has_msr_mcg_ext_ctl = has_msr_feature_control = true;
}
--
2.31.1
[PULL 31/33] memory: Add tracepoint for dirty sync
From: Peter Xu
Trace at memory_region_sync_dirty_bitmap() for log_sync() or global_log_sync()
on memory regions. One trace line should suffice when it finishes, so as to
estimate the time used for each log sync process.
Signed-off-by: Peter Xu
Message-Id: <20210817013706.30986-1-pet...@redhat.com>
Signed-off-by: Paolo Bonzini
---
softmmu/memory.c | 2 ++
softmmu/trace-events | 1 +
2 files changed, 3 insertions(+)
diff --git a/softmmu/memory.c b/softmmu/memory.c
index 54cd0e9824..db182e5d3d 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -2154,6 +2154,7 @@ static void memory_region_sync_dirty_bitmap(MemoryRegion
*mr)
}
}
flatview_unref(view);
+trace_memory_region_sync_dirty(mr ? mr->name : "(all)",
listener->name, 0);
} else if (listener->log_sync_global) {
/*
* No matter whether MR is specified, what we can do here
@@ -2161,6 +2162,7 @@ static void memory_region_sync_dirty_bitmap(MemoryRegion
*mr)
* sync in a finer granularity.
*/
listener->log_sync_global(listener);
+trace_memory_region_sync_dirty(mr ? mr->name : "(all)",
listener->name, 1);
}
}
}
diff --git a/softmmu/trace-events b/softmmu/trace-events
index 7b278590a0..bf1469990e 100644
--- a/softmmu/trace-events
+++ b/softmmu/trace-events
@@ -15,6 +15,7 @@ memory_region_subpage_read(int cpu_index, void *mr, uint64_t
offset, uint64_t va
memory_region_subpage_write(int cpu_index, void *mr, uint64_t offset, uint64_t
value, unsigned size) "cpu %d mr %p offset 0x%"PRIx64" value 0x%"PRIx64" size
%u"
memory_region_ram_device_read(int cpu_index, void *mr, uint64_t addr, uint64_t
value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u"
memory_region_ram_device_write(int cpu_index, void *mr, uint64_t addr,
uint64_t value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64"
size %u"
+memory_region_sync_dirty(const char *mr, const char *listener, int global) "mr
'%s' listener '%s' synced (global=%d)"
flatview_new(void *view, void *root) "%p (root %p)"
flatview_destroy(void *view, void *root) "%p (root %p)"
flatview_destroy_rcu(void *view, void *root) "%p (root %p)"
--
2.31.1
[PULL 21/33] q35: Add support for SGX EPC
From: Sean Christopherson Enable SGX EPC virtualization, which is currently only support by KVM. Signed-off-by: Sean Christopherson Signed-off-by: Yang Zhong Message-Id: <20210719112136.57018-21-yang.zh...@intel.com> Signed-off-by: Paolo Bonzini --- hw/i386/pc_q35.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index 46cd542d17..5481d5c965 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -177,6 +177,7 @@ static void pc_q35_init(MachineState *machine) x86ms->below_4g_mem_size = machine->ram_size; } +pc_machine_init_sgx_epc(pcms); x86_cpus_init(x86ms, pcmc->default_cpu_version); kvmclock_create(pcmc->kvmclock_create_always); -- 2.31.1
[PULL 27/33] meson: unpack edk2 firmware even if --disable-blobs
The edk2 firmware blobs are needed to run bios-tables-test. Unpack
them if any UEFI-enabled target is selected, so that the test can run.
This is a bit more than is actually necessary, since bios-tables-test
does not run for all UEFI-enabled targets, but it is the easiest
way to write this logic.
Signed-off-by: Paolo Bonzini
Message-Id: <20210923105529.3845741-1-pbonz...@redhat.com>
Signed-off-by: Paolo Bonzini
---
meson.build | 16
pc-bios/descriptors/meson.build | 4 ++--
pc-bios/meson.build | 2 +-
3 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/meson.build b/meson.build
index 15ef4d3c41..978e8329f7 100644
--- a/meson.build
+++ b/meson.build
@@ -106,14 +106,14 @@ if targetos != 'darwin'
endif
edk2_targets = [ 'arm-softmmu', 'aarch64-softmmu', 'i386-softmmu',
'x86_64-softmmu' ]
-install_edk2_blobs = false
-if get_option('install_blobs')
- foreach target : target_dirs
-install_edk2_blobs = install_edk2_blobs or target in edk2_targets
- endforeach
-endif
-
-bzip2 = find_program('bzip2', required: install_edk2_blobs)
+unpack_edk2_blobs = false
+foreach target : edk2_targets
+ if target in target_dirs
+bzip2 = find_program('bzip2', required: get_option('install_blobs'))
+unpack_edk2_blobs = bzip2.found()
+break
+ endif
+endforeach
##
# Compiler flags #
diff --git a/pc-bios/descriptors/meson.build b/pc-bios/descriptors/meson.build
index 29efa16d99..66f85d01c4 100644
--- a/pc-bios/descriptors/meson.build
+++ b/pc-bios/descriptors/meson.build
@@ -1,4 +1,4 @@
-if install_edk2_blobs
+if unpack_edk2_blobs and get_option('install_blobs')
foreach f: [
'50-edk2-i386-secure.json',
'50-edk2-x86_64-secure.json',
@@ -10,7 +10,7 @@ if install_edk2_blobs
configure_file(input: files(f),
output: f,
configuration: {'DATADIR': get_option('prefix') /
qemu_datadir},
- install: get_option('install_blobs'),
+ install: true,
install_dir: qemu_datadir / 'firmware')
endforeach
endif
diff --git a/pc-bios/meson.build b/pc-bios/meson.build
index f2b32598af..a3b3d87891 100644
--- a/pc-bios/meson.build
+++ b/pc-bios/meson.build
@@ -1,4 +1,4 @@
-if install_edk2_blobs
+if unpack_edk2_blobs
fds = [
'edk2-aarch64-code.fd',
'edk2-arm-code.fd',
--
2.31.1
[PULL 11/33] i386: Add get/set/migrate support for SGX_LEPUBKEYHASH MSRs
From: Sean Christopherson
On real hardware, on systems that supports SGX Launch Control, those
MSRs are initialized to digest of Intel's signing key; on systems that
don't support SGX Launch Control, those MSRs are not available but
hardware always uses digest of Intel's signing key in EINIT.
KVM advertises SGX LC via CPUID if and only if the MSRs are writable.
Unconditionally initialize those MSRs to digest of Intel's signing key
when CPU is realized and reset to reflect the fact. This avoids
potential bug in case kvm_arch_put_registers() is called before
kvm_arch_get_registers() is called, in which case guest's virtual
SGX_LEPUBKEYHASH MSRs will be set to 0, although KVM initializes those
to digest of Intel's signing key by default, since KVM allows those MSRs
to be updated by Qemu to support live migration.
Save/restore the SGX Launch Enclave Public Key Hash MSRs if SGX Launch
Control (LC) is exposed to the guest. Likewise, migrate the MSRs if they
are writable by the guest.
Signed-off-by: Sean Christopherson
Signed-off-by: Kai Huang
Signed-off-by: Yang Zhong
Message-Id: <20210719112136.57018-11-yang.zh...@intel.com>
Signed-off-by: Paolo Bonzini
---
target/i386/cpu.c | 16 +++-
target/i386/cpu.h | 1 +
target/i386/kvm/kvm.c | 22 ++
target/i386/machine.c | 20
4 files changed, 58 insertions(+), 1 deletion(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index e9ecbf59e5..af6cd73eed 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5700,6 +5700,17 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
uint32_t count,
}
}
+static void x86_cpu_set_sgxlepubkeyhash(CPUX86State *env)
+{
+#ifndef CONFIG_USER_ONLY
+/* Those default values are defined in Skylake HW */
+env->msr_ia32_sgxlepubkeyhash[0] = 0xa6053e051270b7acULL;
+env->msr_ia32_sgxlepubkeyhash[1] = 0x6cfbe8ba8b3b413dULL;
+env->msr_ia32_sgxlepubkeyhash[2] = 0xc4916d99f2b3735dULL;
+env->msr_ia32_sgxlepubkeyhash[3] = 0xd4f8c05909f9bb3bULL;
+#endif
+}
+
static void x86_cpu_reset(DeviceState *dev)
{
CPUState *s = CPU(dev);
@@ -5832,6 +5843,8 @@ static void x86_cpu_reset(DeviceState *dev)
if (kvm_enabled()) {
kvm_arch_reset_vcpu(cpu);
}
+
+x86_cpu_set_sgxlepubkeyhash(env);
#endif
}
@@ -6214,6 +6227,8 @@ static void x86_cpu_realizefn(DeviceState *dev, Error
**errp)
& CPUID_EXT2_AMD_ALIASES);
}
+x86_cpu_set_sgxlepubkeyhash(env);
+
/*
* note: the call to the framework needs to happen after feature expansion,
* but before the checks/modifications to ucode_rev, mwait, phys_bits.
@@ -6901,7 +6916,6 @@ static const TypeInfo x86_cpu_type_info = {
.class_init = x86_cpu_common_class_init,
};
-
/* "base" CPU model, used by query-cpu-model-expansion */
static void x86_cpu_base_class_init(ObjectClass *oc, void *data)
{
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 85a9eeeb2b..29552dc2a7 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1516,6 +1516,7 @@ typedef struct CPUX86State {
uint64_t mcg_status;
uint64_t msr_ia32_misc_enable;
uint64_t msr_ia32_feature_control;
+uint64_t msr_ia32_sgxlepubkeyhash[4];
uint64_t msr_fixed_ctr_ctrl;
uint64_t msr_global_ctrl;
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 500d2e0e68..11551648f9 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -3107,6 +3107,17 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
}
}
+if (env->features[FEAT_7_0_ECX] & CPUID_7_0_ECX_SGX_LC) {
+kvm_msr_entry_add(cpu, MSR_IA32_SGXLEPUBKEYHASH0,
+ env->msr_ia32_sgxlepubkeyhash[0]);
+kvm_msr_entry_add(cpu, MSR_IA32_SGXLEPUBKEYHASH1,
+ env->msr_ia32_sgxlepubkeyhash[1]);
+kvm_msr_entry_add(cpu, MSR_IA32_SGXLEPUBKEYHASH2,
+ env->msr_ia32_sgxlepubkeyhash[2]);
+kvm_msr_entry_add(cpu, MSR_IA32_SGXLEPUBKEYHASH3,
+ env->msr_ia32_sgxlepubkeyhash[3]);
+}
+
/* Note: MSR_IA32_FEATURE_CONTROL is written separately, see
* kvm_put_msr_feature_control. */
}
@@ -3446,6 +3457,13 @@ static int kvm_get_msrs(X86CPU *cpu)
}
}
+if (env->features[FEAT_7_0_ECX] & CPUID_7_0_ECX_SGX_LC) {
+kvm_msr_entry_add(cpu, MSR_IA32_SGXLEPUBKEYHASH0, 0);
+kvm_msr_entry_add(cpu, MSR_IA32_SGXLEPUBKEYHASH1, 0);
+kvm_msr_entry_add(cpu, MSR_IA32_SGXLEPUBKEYHASH2, 0);
+kvm_msr_entry_add(cpu, MSR_IA32_SGXLEPUBKEYHASH3, 0);
+}
+
ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MSRS, cpu->kvm_msr_buf);
if (ret < 0) {
return ret;
@@ -3735,6 +3753,10 @@ static int kvm_get_msrs(X86CPU *cpu)
case MSR_IA32_RTIT_ADDR0_A ... MSR_IA32_RTIT_ADDR3_B:
env->msr_rtit_addrs[index - MSR_IA32_RTIT_ADDR0_A] = msrs[i].data;
