Re: [RESEND PATCH v2] target/i386: Switch back XFRM value
On 3/27/2023 3:33 PM, Christian Ehrhardt wrote:
On Thu, Oct 27, 2022 at 2:36 AM Yang, Weijiang wrote:
On 10/26/2022 7:57 PM, Zhong, Yang wrote:
The previous patch wrongly replaced FEAT_XSAVE_XCR0_{LO|HI} with
FEAT_XSAVE_XSS_{LO|HI} in CPUID(EAX=12,ECX=1):{ECX,EDX}, which made
SGX enclave only supported SSE and x87 feature(xfrm=0x3).
Fixes: 301e90675c3f ("target/i386: Enable support for XSAVES based features")
Signed-off-by: Yang Zhong
---
target/i386/cpu.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index ad623d91e4..19aaed877b 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5584,8 +5584,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
uint32_t count,
} else {
*eax &= env->features[FEAT_SGX_12_1_EAX];
*ebx &= 0; /* ebx reserve */
-*ecx &= env->features[FEAT_XSAVE_XSS_LO];
-*edx &= env->features[FEAT_XSAVE_XSS_HI];
+*ecx &= env->features[FEAT_XSAVE_XCR0_LO];
+*edx &= env->features[FEAT_XSAVE_XCR0_HI];
Oops, that's my fault to replace with wrong definitions, thanks for the fix!
Reviewed-by: Yang Weijiang
Hi,
I do not have any background on this but stumbled over this and wondered,
is there any particular reason why this wasn't applied yet?
It seemed to fix a former mistake, was acked and then ... silence
Chris, thanks for the catch!
I double checked this patch isn't in the latest 8.0.0-rc1 tree.
Hi, Paolo,
Could you help merge this fixup patch? Thanks!
/* FP and SSE are always allowed regardless of XSAVE/XCR0. */
*ecx |= XSTATE_FP_MASK | XSTATE_SSE_MASK;
[PATCH v2 04/10] target/riscv: Remove check on RVH for riscv_cpu_set_virt_enabled
In current implementation, riscv_cpu_set_virt_enabled is only called when
RVH is enabled.
Signed-off-by: Weiwei Li
Signed-off-by: Junqiang Wang
Reviewed-by: Richard Henderson
Reviewed-by: LIU Zhiwei
---
target/riscv/cpu_helper.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 62fd2c90f1..b286118a6b 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -563,12 +563,9 @@ bool riscv_cpu_virt_enabled(CPURISCVState *env)
return get_field(env->virt, VIRT_ONOFF);
}
+/* This function can only be called to set virt when RVH is enabled */
void riscv_cpu_set_virt_enabled(CPURISCVState *env, bool enable)
{
-if (!riscv_has_ext(env, RVH)) {
-return;
-}
-
/* Flush the TLB on all virt mode changes. */
if (get_field(env->virt, VIRT_ONOFF) != enable) {
tlb_flush(env_cpu(env));
--
2.25.1
[PATCH v2 03/10] target/riscv: Remove check on RVH for riscv_cpu_virt_enabled
Since env->virt.VIRT_ONOFF is initialized as false, and will not be set
to true when RVH is disabled, so we can just return this bit(false) when
RVH is not disabled.
Signed-off-by: Weiwei Li
Signed-off-by: Junqiang Wang
Reviewed-by: Richard Henderson
Reviewed-by: LIU Zhiwei
---
target/riscv/cpu_helper.c | 4
1 file changed, 4 deletions(-)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index e140d6a8d0..62fd2c90f1 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -560,10 +560,6 @@ void riscv_cpu_set_geilen(CPURISCVState *env, target_ulong
geilen)
bool riscv_cpu_virt_enabled(CPURISCVState *env)
{
-if (!riscv_has_ext(env, RVH)) {
-return false;
-}
-
return get_field(env->virt, VIRT_ONOFF);
}
--
2.25.1
Re: [PATCH] riscv: Add support for the Zfa extension
On 2023/3/27 16:00, Christoph Muellner wrote:
From: Christoph Müllner
This patch introduces the RISC-V Zfa extension, which introduces
additional floating-point extensions:
* fli (load-immediate) with pre-defined immediates
* fminm/fmaxm (like fmin/fmax but with different NaN behaviour)
* fround/froundmx (round to integer)
* fcvtmod.w.d (Modular Convert-to-Integer)
* fmv* to access high bits of float register bigger than XLEN
* Quiet comparison instructions (fleq/fltq)
Zfa defines its instructions in combination with the following extensions:
* single-precision floating-point (F)
* double-precision floating-point (D)
* quad-precision floating-point (Q)
* half-precision floating-point (Zfh)
Since QEMU does not support the RISC-V quad-precision floating-point
ISA extension (Q), this patch does not include the instructions that
depend on this extension. All other instructions are included in this
patch.
The Zfa specification is not frozen at the moment (which is why this
patch is RFC) and can be found here:
https://github.com/riscv/riscv-isa-manual/blob/master/src/zfa.tex
Signed-off-by: Christoph Müllner
---
target/riscv/cpu.c| 8 +
target/riscv/cpu.h| 1 +
target/riscv/fpu_helper.c | 324 +
target/riscv/helper.h | 22 ++
target/riscv/insn32.decode| 67
target/riscv/insn_trans/trans_rvzfa.c.inc | 410 ++
target/riscv/translate.c | 1 +
7 files changed, 833 insertions(+)
create mode 100644 target/riscv/insn_trans/trans_rvzfa.c.inc
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 1e97473af2..bac9ced4a2 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -83,6 +83,7 @@ static const struct isa_ext_data isa_edata_arr[] = {
ISA_EXT_DATA_ENTRY(zifencei, true, PRIV_VERSION_1_10_0, ext_ifencei),
ISA_EXT_DATA_ENTRY(zihintpause, true, PRIV_VERSION_1_10_0,
ext_zihintpause),
ISA_EXT_DATA_ENTRY(zawrs, true, PRIV_VERSION_1_12_0, ext_zawrs),
+ISA_EXT_DATA_ENTRY(zfa, true, PRIV_VERSION_1_12_0, ext_zfa),
ISA_EXT_DATA_ENTRY(zfh, true, PRIV_VERSION_1_11_0, ext_zfh),
ISA_EXT_DATA_ENTRY(zfhmin, true, PRIV_VERSION_1_12_0, ext_zfhmin),
ISA_EXT_DATA_ENTRY(zfinx, true, PRIV_VERSION_1_12_0, ext_zfinx),
@@ -404,6 +405,7 @@ static void rv64_thead_c906_cpu_init(Object *obj)
cpu->cfg.ext_u = true;
cpu->cfg.ext_s = true;
cpu->cfg.ext_icsr = true;
+cpu->cfg.ext_zfa = true;
cpu->cfg.ext_zfh = true;
cpu->cfg.mmu = true;
cpu->cfg.ext_xtheadba = true;
@@ -865,6 +867,11 @@ static void riscv_cpu_validate_set_extensions(RISCVCPU
*cpu, Error **errp)
return;
}
+if (cpu->cfg.ext_zfa && !cpu->cfg.ext_f) {
+error_setg(errp, "Zfa extension requires F extension");
+return;
+}
+
if (cpu->cfg.ext_zfh) {
cpu->cfg.ext_zfhmin = true;
}
@@ -1381,6 +1388,7 @@ static Property riscv_cpu_extensions[] = {
DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
DEFINE_PROP_BOOL("Zihintpause", RISCVCPU, cfg.ext_zihintpause, true),
DEFINE_PROP_BOOL("Zawrs", RISCVCPU, cfg.ext_zawrs, true),
+DEFINE_PROP_BOOL("Zfa", RISCVCPU, cfg.ext_zfa, false),
DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false),
DEFINE_PROP_BOOL("Zfhmin", RISCVCPU, cfg.ext_zfhmin, false),
DEFINE_PROP_BOOL("Zve32f", RISCVCPU, cfg.ext_zve32f, false),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 638e47c75a..deae410fc2 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -462,6 +462,7 @@ struct RISCVCPUConfig {
bool ext_svpbmt;
bool ext_zdinx;
bool ext_zawrs;
+bool ext_zfa;
bool ext_zfh;
bool ext_zfhmin;
bool ext_zfinx;
diff --git a/target/riscv/fpu_helper.c b/target/riscv/fpu_helper.c
index 449d236df6..55c75bf063 100644
--- a/target/riscv/fpu_helper.c
+++ b/target/riscv/fpu_helper.c
@@ -252,6 +252,18 @@ uint64_t helper_fmin_s(CPURISCVState *env, uint64_t rs1,
uint64_t rs2)
float32_minimum_number(frs1, frs2, >fp_status));
}
+uint64_t helper_fminm_s(CPURISCVState *env, uint64_t rs1, uint64_t rs2)
+{
+float32 frs1 = check_nanbox_s(env, rs1);
+float32 frs2 = check_nanbox_s(env, rs2);
+
+if (float32_is_any_nan(frs1) || float32_is_any_nan(frs2)) {
+return float32_default_nan(>fp_status);
I think we should also add nanbox_s for it.
+}
+
+return nanbox_s(env, float32_minimum_number(frs1, frs2, >fp_status));
+}
+
uint64_t helper_fmax_s(CPURISCVState *env, uint64_t rs1, uint64_t rs2)
{
float32 frs1 = check_nanbox_s(env, rs1);
@@ -261,6 +273,18 @@ uint64_t helper_fmax_s(CPURISCVState *env, uint64_t rs1,
uint64_t rs2)
float32_maximum_number(frs1, frs2, >fp_status));
}
+uint64_t helper_fmaxm_s(CPURISCVState *env, uint64_t rs1, uint64_t rs2)
+{
+float32 frs1 =
Re: [PATCH 2/2] qtest: Add a test case for TPM TIS I2C connected to Aspeed I2C controller
On 3/27/23 03:49, Cédric Le Goater wrote:
On 3/27/23 02:37, Stefan Berger wrote:
Add a test case for the TPM TIS I2C device exercising most of its
functionality, including localities.
Add library functions for being able to read from and write to registers
of the TPM TIS I2C device connected to the Aspeed i2c controller.
Signed-off-by: Stefan Berger
Thanks for doing the I2C qtest driver. This gives the opportunity to write
more unit tests.
---
tests/qtest/meson.build | 3 +
tests/qtest/qtest_aspeed.c | 117 ++
tests/qtest/qtest_aspeed.h | 27 ++
tests/qtest/tpm-tis-i2c-test.c | 628 +
4 files changed, 775 insertions(+)
create mode 100644 tests/qtest/qtest_aspeed.c
create mode 100644 tests/qtest/qtest_aspeed.h
create mode 100644 tests/qtest/tpm-tis-i2c-test.c
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 29a4efb4c2..065a00d34d 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -200,6 +200,7 @@ qtests_arm = \
(config_all_devices.has_key('CONFIG_ASPEED_SOC') ? qtests_aspeed : []) + \
(config_all_devices.has_key('CONFIG_NPCM7XX') ? qtests_npcm7xx : []) + \
(config_all_devices.has_key('CONFIG_GENERIC_LOADER') ? ['hexloader-test'] :
[]) + \
+ (config_all_devices.has_key('CONFIG_TPM_TIS_I2C') ? ['tpm-tis-i2c-test'] :
[]) + \
['arm-cpu-features',
'microbit-test',
'test-arm-mptimer',
@@ -212,6 +213,7 @@ qtests_aarch64 = \
['tpm-tis-device-test', 'tpm-tis-device-swtpm-test'] : []) +
\
(config_all_devices.has_key('CONFIG_XLNX_ZYNQMP_ARM') ? ['xlnx-can-test',
'fuzz-xlnx-dp-test'] : []) + \
(config_all_devices.has_key('CONFIG_RASPI') ? ['bcm2835-dma-test'] : []) +
\
+ (config_all_devices.has_key('CONFIG_TPM_TIS_I2C') ? ['tpm-tis-i2c-test'] :
[]) + \
['arm-cpu-features',
'numa-test',
'boot-serial-test',
@@ -303,6 +305,7 @@ qtests = {
'tpm-crb-test': [io, tpmemu_files],
'tpm-tis-swtpm-test': [io, tpmemu_files, 'tpm-tis-util.c'],
'tpm-tis-test': [io, tpmemu_files, 'tpm-tis-util.c'],
+ 'tpm-tis-i2c-test': [io, tpmemu_files, 'qtest_aspeed.c'],
'tpm-tis-device-swtpm-test': [io, tpmemu_files, 'tpm-tis-util.c'],
'tpm-tis-device-test': [io, tpmemu_files, 'tpm-tis-util.c'],
'vmgenid-test': files('boot-sector.c', 'acpi-utils.c'),
diff --git a/tests/qtest/qtest_aspeed.c b/tests/qtest/qtest_aspeed.c
new file mode 100644
index 00..2b316178e4
--- /dev/null
+++ b/tests/qtest/qtest_aspeed.c
@@ -0,0 +1,117 @@
+/*
+ * Aspeed i2c bus interface to reading and writing to i2c device registers
+ *
+ * Copyright (c) 2023 IBM Corporation
+ *
+ * Authors:
+ * Stefan Berger
+ *
+ * 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 "qtest_aspeed.h"
+
+#include "hw/i2c/aspeed_i2c.h"
+#include "libqtest-single.h"
+
+#define A_I2CD_M_STOP_CMD BIT(5)
+#define A_I2CD_M_RX_CMD BIT(3)
+#define A_I2CD_M_TX_CMD BIT(1)
+#define A_I2CD_M_START_CMD BIT(0)
+
+#define A_I2CD_MASTER_EN BIT(0)
Why do you need to include the aspeed_i2c.h file and add some more
definitions ? Couldn't we gather all of them under the same file ?
I moved them now.
+
+#define I2C_SLAVE_ADDR 0x2e
+#define I2C_DEV_BUS_NUM 10
+
+static const uint8_t TPM_CMD[12] =
+ "\x80\x01\x00\x00\x00\x0c\x00\x00\x01\x44\x00\x00";
+
+uint32_t aspeed_dev_addr = 0X1e78a000 + 0x80 + I2C_DEV_BUS_NUM * 0x80;
0X1e78a000 could be a define
Is it suitable for a public header file or limited to the board we are using it
with?
Where should the define go? Into the qtest_aspeed.h file under this name?
#define AST2600_ASPEED_I2C_BASE_ADDR 0x1e78a
> The resulting address should be calculated with an helper defined in
qtest_aspeed.h, with an ast2600_ prefix in the name since the calculation
is SoC dependent. See aspeed_i2c_realize()
static inline uint32_t ast2600_aspeed_i2c_calc_dev_addr(uint8_t bus_num)
{
return AST2600_ASPEED_I2C_BASE_ADDR + 0x80 + bus_num * 0x80;
}
Like this?
My knowledge on TPM is too limited to comment. Could you please extract
the I2C driver in its own patch ?
Will do.
Stefan
Re: [PATCH v2] virtio: refresh vring region cache after updating a virtqueue size
On Wed, Mar 22 2023, Halil Pasic wrote:
> On Wed, 22 Mar 2023 10:52:31 +0100
> Cornelia Huck wrote:
> [..]
>> >
>> > diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
>> > index e33e5207ab..f44de1a8c1 100644
>> > --- a/hw/s390x/virtio-ccw.c
>> > +++ b/hw/s390x/virtio-ccw.c
>> > @@ -237,6 +237,7 @@ static int virtio_ccw_set_vqs(SubchDev *sch,
>> > VqInfoBlock *info,
>> > return -EINVAL;
>> > }
>> > virtio_queue_set_num(vdev, index, num);
>> > +virtio_init_region_cache(vdev, index);
>>
>> Hmm... this is not wrong, but looking at it again, I see that the guest
>> has no way to change num after our last call to
>> virtio_init_region_cache() (while setting up the queue addresses.) IOW,
>> this introduces an extra round trip that is not really needed.
>>
>
> I don't quite understand. AFAIU the virtio_init_region_cache() would see
> the (new) queue addresses but not the new size (num). Yes virtio-ccw
> already knows the new num but it is yet to call
> to put it into vdev->vq[n].vring.num from where
> virtio_init_region_cache() picks it up.
>
> If we were to first virtio_queue_set_num() and only then the address
> I would understand. But with the code as is, I don't. Am I missing
> something?
Hrm, virtio_queue_set_rings() doesn't pass num, I thought it did... I
wonder whether ordering virtio_queue_set_num() before it would be better
anyway (if the guest gave us an invalid num, we don't need to setup any
addresses and init any caches).
Smth like
if (info) {
if (desc) {
if (virtio_queue_get_max_num(...) < num) {
return -EINVAL;
}
virtio_queue_set_num(...);
}
virtio_queue_set_rings(...);
} else { /* legacy */
if (desc && virtio_queue_get_max_num(...) > num) {
return -EINVAL;
}
virtio_queue_set_addr(...);
}
virtio_queue_set_vector(vdev, index, desc ? index : VIRTIO_NO_VECTOR);
might be easier to follow than the current code.
Or we could just go with this patch, which has the advantage of already
existing :)
Re: [PATCH v7 0/3] Add support for TPM devices over I2C bus
On 3/26/23 21:05, Joel Stanley wrote: Hi Ninad, On Sun, 26 Mar 2023 at 22:44, Ninad Palsule wrote: Hello, I have incorporated review comments from Stefan. Please review. This drop adds support for the TPM devices attached to the I2C bus. It only supports the TPM2 protocol. You need to run it with the external TPM emulator like swtpm. I have tested it with swtpm. Nice work. I tested these stop cedric's aspeed-8.0 qemu tree, using the rainier machine and the openbmc dev-6.1 kernel. We get this message when booting from a kernel: [0.582699] tpm_tis_i2c 12-002e: 2.0 TPM (device-id 0x1, rev-id 1) [0.586361] tpm tpm0: A TPM error (256) occurred attempting the self test [0.586623] tpm tpm0: starting up the TPM manually Do we understand why the error appears? The firmware did not initialize the TPM 2. # grep -r . /sys/class/tpm/tpm0/pcr-sha256/ | sort -n -k 7 -t / /sys/class/tpm/tpm0/pcr-sha256/0: /sys/class/tpm/tpm0/pcr-sha256/1: /sys/class/tpm/tpm0/pcr-sha256/2: /sys/class/tpm/tpm0/pcr-sha256/3: /sys/class/tpm/tpm0/pcr-sha256/4: /sys/class/tpm/tpm0/pcr-sha256/5: /sys/class/tpm/tpm0/pcr-sha256/6: /sys/class/tpm/tpm0/pcr-sha256/7: /sys/class/tpm/tpm0/pcr-sha256/8: /sys/class/tpm/tpm0/pcr-sha256/9: /sys/class/tpm/tpm0/pcr-sha256/10: /sys/class/tpm/tpm0/pcr-sha256/11: /sys/class/tpm/tpm0/pcr-sha256/12: /sys/class/tpm/tpm0/pcr-sha256/13: /sys/class/tpm/tpm0/pcr-sha256/14: /sys/class/tpm/tpm0/pcr-sha256/15: /sys/class/tpm/tpm0/pcr-sha256/16: /sys/class/tpm/tpm0/pcr-sha256/17: /sys/class/tpm/tpm0/pcr-sha256/18: /sys/class/tpm/tpm0/pcr-sha256/19: /sys/class/tpm/tpm0/pcr-sha256/20: /sys/class/tpm/tpm0/pcr-sha256/21: /sys/class/tpm/tpm0/pcr-sha256/22: /sys/class/tpm/tpm0/pcr-sha256/23: If I boot through the openbmc u-boot for the p10bmc machine, which measures things into the PCRs: [0.556713] tpm_tis_i2c 12-002e: 2.0 TPM (device-id 0x1, rev-id 1) In this case the firmware started up the TPM 2. Also the PCRs have been touched by the firmware in this case. / # grep -r . /sys/class/tpm/tpm0/pcr-sha256/ | sort -n -k 7 -t / /sys/class/tpm/tpm0/pcr-sha256/0:AFA13691EFC7BC6E189E92347F20676FB4523302CB957DA9A65C3430C45E8BCC /sys/class/tpm/tpm0/pcr-sha256/1:37F0F710A5502FAE6DB7433B36001FEE1CBF15BA2A7D6923207FF56888584714 /sys/class/tpm/tpm0/pcr-sha256/2:E21B703EE69C77476BCCB43EC0336A9A1B2914B378944F7B00A10214CA8FEA93 /sys/class/tpm/tpm0/pcr-sha256/3:E21B703EE69C77476BCCB43EC0336A9A1B2914B378944F7B00A10214CA8FEA93 /sys/class/tpm/tpm0/pcr-sha256/4:E21B703EE69C77476BCCB43EC0336A9A1B2914B378944F7B00A10214CA8FEA93 /sys/class/tpm/tpm0/pcr-sha256/5:E21B703EE69C77476BCCB43EC0336A9A1B2914B378944F7B00A10214CA8FEA93 /sys/class/tpm/tpm0/pcr-sha256/6:E21B703EE69C77476BCCB43EC0336A9A1B2914B378944F7B00A10214CA8FEA93 /sys/class/tpm/tpm0/pcr-sha256/7:E21B703EE69C77476BCCB43EC0336A9A1B2914B378944F7B00A10214CA8FEA93 /sys/class/tpm/tpm0/pcr-sha256/8:AE67485BD01E8D6FE0208C46C473940173F66E9C6F43C75ABB404375787E9705 /sys/class/tpm/tpm0/pcr-sha256/9:DB99D92EADBB446894CB0C062AEB673F60DDAFBC62BC2A9CA561A13B31E5357C /sys/class/tpm/tpm0/pcr-sha256/10: /sys/class/tpm/tpm0/pcr-sha256/11: /sys/class/tpm/tpm0/pcr-sha256/12: /sys/class/tpm/tpm0/pcr-sha256/13:
Re: [PATCH 2/2] qtest: Add a test case for TPM TIS I2C connected to Aspeed I2C controller
On 3/27/23 02:37, Stefan Berger wrote:
Add a test case for the TPM TIS I2C device exercising most of its
functionality, including localities.
Add library functions for being able to read from and write to registers
of the TPM TIS I2C device connected to the Aspeed i2c controller.
Signed-off-by: Stefan Berger
Thanks for doing the I2C qtest driver. This gives the opportunity to write
more unit tests.
---
tests/qtest/meson.build| 3 +
tests/qtest/qtest_aspeed.c | 117 ++
tests/qtest/qtest_aspeed.h | 27 ++
tests/qtest/tpm-tis-i2c-test.c | 628 +
4 files changed, 775 insertions(+)
create mode 100644 tests/qtest/qtest_aspeed.c
create mode 100644 tests/qtest/qtest_aspeed.h
create mode 100644 tests/qtest/tpm-tis-i2c-test.c
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 29a4efb4c2..065a00d34d 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -200,6 +200,7 @@ qtests_arm = \
(config_all_devices.has_key('CONFIG_ASPEED_SOC') ? qtests_aspeed : []) + \
(config_all_devices.has_key('CONFIG_NPCM7XX') ? qtests_npcm7xx : []) + \
(config_all_devices.has_key('CONFIG_GENERIC_LOADER') ? ['hexloader-test'] :
[]) + \
+ (config_all_devices.has_key('CONFIG_TPM_TIS_I2C') ? ['tpm-tis-i2c-test'] :
[]) + \
['arm-cpu-features',
'microbit-test',
'test-arm-mptimer',
@@ -212,6 +213,7 @@ qtests_aarch64 = \
['tpm-tis-device-test', 'tpm-tis-device-swtpm-test'] : []) +
\
(config_all_devices.has_key('CONFIG_XLNX_ZYNQMP_ARM') ? ['xlnx-can-test',
'fuzz-xlnx-dp-test'] : []) + \
(config_all_devices.has_key('CONFIG_RASPI') ? ['bcm2835-dma-test'] : []) +
\
+ (config_all_devices.has_key('CONFIG_TPM_TIS_I2C') ? ['tpm-tis-i2c-test'] :
[]) + \
['arm-cpu-features',
'numa-test',
'boot-serial-test',
@@ -303,6 +305,7 @@ qtests = {
'tpm-crb-test': [io, tpmemu_files],
'tpm-tis-swtpm-test': [io, tpmemu_files, 'tpm-tis-util.c'],
'tpm-tis-test': [io, tpmemu_files, 'tpm-tis-util.c'],
+ 'tpm-tis-i2c-test': [io, tpmemu_files, 'qtest_aspeed.c'],
'tpm-tis-device-swtpm-test': [io, tpmemu_files, 'tpm-tis-util.c'],
'tpm-tis-device-test': [io, tpmemu_files, 'tpm-tis-util.c'],
'vmgenid-test': files('boot-sector.c', 'acpi-utils.c'),
diff --git a/tests/qtest/qtest_aspeed.c b/tests/qtest/qtest_aspeed.c
new file mode 100644
index 00..2b316178e4
--- /dev/null
+++ b/tests/qtest/qtest_aspeed.c
@@ -0,0 +1,117 @@
+/*
+ * Aspeed i2c bus interface to reading and writing to i2c device registers
+ *
+ * Copyright (c) 2023 IBM Corporation
+ *
+ * Authors:
+ * Stefan Berger
+ *
+ * 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 "qtest_aspeed.h"
+
+#include "hw/i2c/aspeed_i2c.h"
+#include "libqtest-single.h"
+
+#define A_I2CD_M_STOP_CMD BIT(5)
+#define A_I2CD_M_RX_CMD BIT(3)
+#define A_I2CD_M_TX_CMD BIT(1)
+#define A_I2CD_M_START_CMD BIT(0)
+
+#define A_I2CD_MASTER_ENBIT(0)
Why do you need to include the aspeed_i2c.h file and add some more
definitions ? Couldn't we gather all of them under the same file ?
+
+static void aspeed_i2c_startup(uint32_t baseaddr, uint8_t slave_addr,
+ uint8_t reg)
+{
+uint32_t v;
+static int once;
+
+if (!once) {
+/* one time: enable master */
+ writel(baseaddr + A_I2CC_FUN_CTRL, 0);
+ v = readl(baseaddr + A_I2CC_FUN_CTRL) | A_I2CD_MASTER_EN;
+ writel(baseaddr + A_I2CC_FUN_CTRL, v);
+ once = 1;
+}
+
+/* select device */
+writel(baseaddr + A_I2CD_BYTE_BUF, slave_addr << 1);
+writel(baseaddr + A_I2CD_CMD, A_I2CD_M_START_CMD | A_I2CD_M_RX_CMD);
+
+/* select the register to write to */
+writel(baseaddr + A_I2CD_BYTE_BUF, reg);
+writel(baseaddr + A_I2CD_CMD, A_I2CD_M_TX_CMD);
+}
+
+static uint32_t aspeed_i2c_read_n(uint32_t baseaddr, uint8_t slave_addr,
+ uint8_t reg, size_t nbytes)
+{
+uint32_t res = 0;
+uint32_t v;
+size_t i;
+
+aspeed_i2c_startup(baseaddr, slave_addr, reg);
+
+for (i = 0; i < nbytes; i++) {
+writel(baseaddr + A_I2CD_CMD, A_I2CD_M_RX_CMD);
+v = readl(baseaddr + A_I2CD_BYTE_BUF) >> 8;
+res |= (v & 0xff) << (i * 8);
+}
+
+writel(baseaddr + A_I2CD_CMD, A_I2CD_M_STOP_CMD);
+
+return res;
+}
+
+uint32_t aspeed_i2c_readl(uint32_t baseaddr, uint8_t slave_addr, uint8_t reg)
+{
+return aspeed_i2c_read_n(baseaddr, slave_addr, reg, sizeof(uint32_t));
+}
+
+uint16_t aspeed_i2c_readw(uint32_t baseaddr, uint8_t slave_addr, uint8_t reg)
+{
+return aspeed_i2c_read_n(baseaddr, slave_addr, reg, sizeof(uint16_t));
+}
+
+uint8_t aspeed_i2c_readb(uint32_t baseaddr, uint8_t slave_addr, uint8_t reg)
+{
+return aspeed_i2c_read_n(baseaddr, slave_addr, reg,
[PATCH v2 09/10] target/riscv: Fix format for comments
Fix formats for multi-lines comments.
Add spaces around single line comments(after "/*" and before "*/").
Signed-off-by: Weiwei Li
Signed-off-by: Junqiang Wang
Acked-by: Richard Henderson
Reviewed-by: LIU Zhiwei
---
target/riscv/arch_dump.c| 3 +-
target/riscv/cpu.c | 2 +-
target/riscv/cpu.h | 26
target/riscv/cpu_bits.h | 2 +-
target/riscv/cpu_helper.c | 57 +++--
target/riscv/csr.c | 6 +-
target/riscv/insn_trans/trans_rvv.c.inc | 8 ++-
target/riscv/pmp.c | 23 ---
target/riscv/sbi_ecall_interface.h | 8 +--
target/riscv/translate.c| 8 ++-
target/riscv/vector_helper.c| 82 +++--
11 files changed, 135 insertions(+), 90 deletions(-)
diff --git a/target/riscv/arch_dump.c b/target/riscv/arch_dump.c
index 573587810e..434c8a3dbb 100644
--- a/target/riscv/arch_dump.c
+++ b/target/riscv/arch_dump.c
@@ -1,4 +1,5 @@
-/* Support for writing ELF notes for RISC-V architectures
+/*
+ * Support for writing ELF notes for RISC-V architectures
*
* Copyright (C) 2021 Huawei Technologies Co., Ltd
*
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index b0cbacc5f4..7f6184346e 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -56,7 +56,7 @@ struct isa_ext_data {
#define ISA_EXT_DATA_ENTRY(_name, _m_letter, _min_ver, _prop) \
{#_name, _m_letter, _min_ver, offsetof(struct RISCVCPUConfig, _prop)}
-/**
+/*
* Here are the ordering rules of extension naming defined by RISC-V
* specification :
* 1. All extensions should be separated from other multi-letter extensions
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index dc9817b40d..2fcdacf216 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -124,7 +124,7 @@ FIELD(VTYPE, RESERVED, 10, sizeof(target_ulong) * 8 - 11)
typedef struct PMUCTRState {
/* Current value of a counter */
target_ulong mhpmcounter_val;
-/* Current value of a counter in RV32*/
+/* Current value of a counter in RV32 */
target_ulong mhpmcounterh_val;
/* Snapshot values of counter */
target_ulong mhpmcounter_prev;
@@ -278,8 +278,10 @@ struct CPUArchState {
target_ulong satp_hs;
uint64_t mstatus_hs;
-/* Signals whether the current exception occurred with two-stage address
- translation active. */
+/*
+ * Signals whether the current exception occurred with two-stage address
+ * translation active.
+ */
bool two_stage_lookup;
/*
* Signals whether the current exception occurred while doing two-stage
@@ -295,10 +297,10 @@ struct CPUArchState {
/* PMU counter state */
PMUCTRState pmu_ctrs[RV_MAX_MHPMCOUNTERS];
-/* PMU event selector configured values. First three are unused*/
+/* PMU event selector configured values. First three are unused */
target_ulong mhpmevent_val[RV_MAX_MHPMEVENTS];
-/* PMU event selector configured values for RV32*/
+/* PMU event selector configured values for RV32 */
target_ulong mhpmeventh_val[RV_MAX_MHPMEVENTS];
target_ulong sscratch;
@@ -387,7 +389,7 @@ struct CPUArchState {
OBJECT_DECLARE_CPU_TYPE(RISCVCPU, RISCVCPUClass, RISCV_CPU)
-/**
+/*
* RISCVCPUClass:
* @parent_realize: The parent class' realize handler.
* @parent_phases: The parent class' reset phase handlers.
@@ -395,9 +397,9 @@ OBJECT_DECLARE_CPU_TYPE(RISCVCPU, RISCVCPUClass, RISCV_CPU)
* A RISCV CPU model.
*/
struct RISCVCPUClass {
-/*< private >*/
+/* < private > */
CPUClass parent_class;
-/*< public >*/
+/* < public > */
DeviceRealize parent_realize;
ResettablePhases parent_phases;
};
@@ -521,16 +523,16 @@ struct RISCVCPUConfig {
typedef struct RISCVCPUConfig RISCVCPUConfig;
-/**
+/*
* RISCVCPU:
* @env: #CPURISCVState
*
* A RISCV CPU.
*/
struct ArchCPU {
-/*< private >*/
+/* < private > */
CPUState parent_obj;
-/*< public >*/
+/* < public > */
CPUNegativeOffsetState neg;
CPURISCVState env;
@@ -802,7 +804,7 @@ enum {
CSR_TABLE_SIZE = 0x1000
};
-/**
+/*
* The event id are encoded based on the encoding specified in the
* SBI specification v0.3
*/
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 45ddb00aa5..063535b1aa 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -727,7 +727,7 @@ typedef enum RISCVException {
#define MIE_SSIE (1 << IRQ_S_SOFT)
#define MIE_USIE (1 << IRQ_U_SOFT)
-/* General PointerMasking CSR bits*/
+/* General PointerMasking CSR bits */
#define PM_ENABLE 0x0001ULL
#define PM_CURRENT 0x0002ULL
#define PM_INSN 0x0004ULL
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 6f4d0a6030..e46b667239 100644
--- a/target/riscv/cpu_helper.c
+++
Re: [PATCH] Change the default for Mixed declarations.
On Fri, Mar 24, 2023 at 06:39:34PM +0100, Juan Quintela wrote: > Daniel P. Berrangé wrote: > > On Tue, Feb 14, 2023 at 05:07:38PM +0100, Juan Quintela wrote: > >> Hi > >> > >> I want to enter a discussion about changing the default of the style > >> guide. > >> > >> There are several reasons for that: > >> - they exist since C99 (i.e. all supported compilers support them) > >> - they eliminate the posibility of an unitialized variable. > > > > Actually they don't do that reliably. In fact, when combined > > with usage of 'goto', they introduce uninitialized variables, > > despite the declaration having an initialization present, and > > thus actively mislead reviewers into thinking their code is > > safe. > > Wait a minute. > If you use goto, you are already in special rules. > > And don't get confused, I fully agree when using goto for two reasons: > - performance > if you show that the code is x% faster when using goto, it is > justified. It is even better if you send a bug report to gcc/clang, > but I will not opose that use. > - code clearity > Some code (basically error paths) are clearer with goto that without > them. > > But that don't mind that mixed declarations are bad. It means that goto > is complicated. Yes, goto is complicated and we shouldn't make that worse by using a code pattern that encourages mistakes IMHO. > >> - Current documentation already declares that they are allowed in some > >> cases. > >> - Lots of places already use them. > >> > >> We can change the text to whatever you want, just wondering if it is > >> valib to change the standard. > >> > >> Doing a trivial grep through my local qemu messages (around 100k) it > >> shows that some people are complaining that they are not allowed, and > >> other saying that they are used all over the place. > > > > IMHO the status quo is bad because it is actively dangerous when > > combined with goto and we aren't using any compiler warnings to > > help us. > > > > Either we allow it, but use -Wjump-misses-init to prevent mixing > > delayed declarations with gotos, and just avoid this when it triggers > > a false positive. > > > > Or we forbid it, rewrite current cases that use it, and then add > > -Wdeclaration-after-statement to enforce it. > > > > > > IMHO if we are concerned about uninitialized variables then I think > > a better approach is to add -ftrivial-auto-var-init=zero, which will > > make the compiler initialize all variables to 0 if they lack an > > explicit initializer. > > I think this is a bad idea. > If we want to "catch" unitialized variables, using something like: > > -ftrivial-auto-var-init=pattern sounds much saner. It depends on what you are aiming to achieve. In almost all cases where we forgot to initialize something, all-zeros is the value that we would have wanted to be present. IOW, init=zero will (almost) always make the code do what we wanted it to do, and thus is the safe option to make QEMU robust. Using a non-zero value will be potentially dangerous in a number of possible ways. It will lead to loops iterating when they should not, potentially even infinite loops. It will lead to reads/writes off the end of arrays. It will lead to attempts to free() invalid pointers. Essentially all the ways in which an uninitialized value can make the code go wrong wil still potentially happen if we initialized to a non-zero value. The only benefit is that it will go horribly wrong in the same way each time. IOW... * If you want the application to be robust and generally "do the right thing", even in the face of missing initializers, then using -ftrivial-auto-var-init=zero is the right answer. * If you want the application to go horribly wrong, but in a repeatable manner, then -ftrivial-auto-var-init=pattern is the right answer Personally I prefer QEMU to be robust and thus believe we should initialize to zero in real world deployments. Potentially there's a case for CI jobs to use a non-zero pattern though to try to expose edge cases. With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v7 0/3] Add support for TPM devices over I2C bus
On 3/27/23 07:18, Joel Stanley wrote: On Mon, 27 Mar 2023 at 11:11, Stefan Berger wrote: On 3/26/23 21:05, Joel Stanley wrote: Hi Ninad, On Sun, 26 Mar 2023 at 22:44, Ninad Palsule wrote: Hello, I have incorporated review comments from Stefan. Please review. This drop adds support for the TPM devices attached to the I2C bus. It only supports the TPM2 protocol. You need to run it with the external TPM emulator like swtpm. I have tested it with swtpm. Nice work. I tested these stop cedric's aspeed-8.0 qemu tree, using the rainier machine and the openbmc dev-6.1 kernel. We get this message when booting from a kernel: [0.582699] tpm_tis_i2c 12-002e: 2.0 TPM (device-id 0x1, rev-id 1) [0.586361] tpm tpm0: A TPM error (256) occurred attempting the self test [0.586623] tpm tpm0: starting up the TPM manually Do we understand why the error appears? The firmware did not initialize the TPM 2. Which firmware are we talking about here? This happens if either no firmware is used or the firmware doesn't know how to talk to the TPM 2. Linux detects that the TPM 2 wasn't initialized (TPM2_Startup was not sent). Stefan
Re: [PATCH v7 3/3] tpm: Add support for TPM device over I2C bus
Hi Stefan,
On 3/27/23 8:40 AM, Stefan Berger wrote:
On 3/26/23 18:44, Ninad Palsule wrote:
Qemu already supports devices attached to ISA and sysbus. This drop adds
support for the I2C bus attached TPM devices. I2C model only supports
TPM2 protocol.
--- /dev/null
+++ b/hw/tpm/tpm_tis_i2c.c
@@ -0,0 +1,540 @@
+/*
+ * tpm_tis_i2c.c - QEMU's TPM TIS I2C Device
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2
or later.
+ * See the COPYING file in the top-level directory.
+ *
+ * Implementation of the TIS interface according to specs found at
+ * http://www.trustedcomputinggroup.org. This implementation currently
+ * supports version 1.3, 21 March 2013
+ * In the developers menu choose the PC Client section then find the
TIS
+ * specification.
+ *
+ * TPM TIS for TPM 2 implementation following TCG PC Client Platform
+ * TPM Profile (PTP) Specification, Familiy 2.0, Revision 00.43
+ *
+ * TPM I2C implementation follows TCG TPM I2c Interface specification,
+ * Family 2.0, Level 00, Revision 1.00
+ */
+
+#include "qemu/osdep.h"
+#include "hw/i2c/i2c.h"
+#include "hw/sysbus.h"
+#include "hw/acpi/tpm.h"
+#include "migration/vmstate.h"
+#include "tpm_prop.h"
+#include "qemu/log.h"
+#include "trace.h"
+#include "tpm_tis.h"
+
+/* TPM_STS mask for read bits 31:26 must be zero */
+#define TPM_I2C_STS_READ_MASK 0x03ff
+
+/* TPM_I2C_INT_ENABLE mask */
+#define TPM_I2C_INT_ENABLE_MASK (TPM_TIS_INT_ENABLED | \
+ TPM_TIS_INT_DATA_AVAILABLE | \
+ TPM_TIS_INT_STS_VALID | \
+ TPM_TIS_INT_LOCALITY_CHANGED | \
+ TPM_TIS_INT_COMMAND_READY)
Since we cannot test interrupts with Linux since the driver doesn't
support it,
can you set this mask to 0 and move it into the public header file.
Please also
apply the mask to vthe alue returned from reading to the
TPM_INT_CAPABILITY register.
OK, done.
+
+/* Operations */
+#define OP_SEND 1
+#define OP_RECV 2
+
+typedef struct TPMStateI2C {
+ /*< private >*/
+ I2CSlave parent_obj;
+
+ uint8_t offset; /* offset into data[] */
+ uint8_t operation; /* OP_SEND & OP_RECV */
+ uint8_t data[5]; /* Data */
+
+ /* i2c registers */
+ uint8_t loc_sel; /* Current locality */
+ uint8_t csum_enable; /* Is checksum enabled */
+
+ /* Derived from the above */
+ const char *reg_name; /* Register name */
+ uint32_t tis_addr; /* Converted tis address including
locty */
+
+ /*< public >*/
+ TPMState state; /* not a QOM object */
+
+} TPMStateI2C;
+
+DECLARE_INSTANCE_CHECKER(TPMStateI2C, TPM_TIS_I2C,
+ TYPE_TPM_TIS_I2C)
+
+/* Prototype */
+static inline void tpm_tis_i2c_to_tis_reg(TPMStateI2C *i2cst,
uint8_t i2c_reg);
+
+/* Register map */
+typedef struct regMap {
+ uint8_t i2c_reg; /* I2C register */
+ uint16_t tis_reg; /* TIS register */
+ const char *reg_name; /* Register name */
+} I2CRegMap;
+
+/*
+ * The register values in the common code is different than the latest
+ * register numbers as per the spec hence add the conversion map
+ */
+static const I2CRegMap tpm_tis_reg_map[] = {
+ /*
+ * These registers are sent to TIS layer. The register with UNKNOWN
+ * mapping are not sent to TIS layer and handled in I2c layer.
+ * NOTE: Adding frequently used registers at the start
+ */
+ { TPM_I2C_REG_DATA_FIFO, TPM_TIS_REG_DATA_FIFO,
"FIFO", },
+ { TPM_I2C_REG_STS, TPM_TIS_REG_STS, "STS", },
+ { TPM_I2C_REG_DATA_CSUM_GET, TPM_I2C_REG_UNKNOWN,
"CSUM_GET", },
+ { TPM_I2C_REG_LOC_SEL, TPM_I2C_REG_UNKNOWN, "LOC_SEL", },
+ { TPM_I2C_REG_ACCESS, TPM_TIS_REG_ACCESS, "ACCESS", },
+ { TPM_I2C_REG_INT_ENABLE, TPM_TIS_REG_INT_ENABLE,
"INTR_ENABLE",},
+ { TPM_I2C_REG_INT_CAPABILITY, TPM_TIS_REG_INT_ENABLE,
"INTR_CAP", },
This mapping here is wrong. It should map to TPM_TIS_REG_INT_CAPABILITY.
OK, Now I handle this register in the I2C so changed to mapping to UNKNOWN
+ { TPM_I2C_REG_INTF_CAPABILITY, TPM_TIS_REG_INTF_CAPABILITY,
"INTF_CAP", },
+ { TPM_I2C_REG_DID_VID, TPM_TIS_REG_DID_VID, "DID_VID", },
+ { TPM_I2C_REG_RID, TPM_TIS_REG_RID, "RID", },
+ { TPM_I2C_REG_I2C_DEV_ADDRESS, TPM_I2C_REG_UNKNOWN,
"DEV_ADDRESS",},
+ { TPM_I2C_REG_DATA_CSUM_ENABLE, TPM_I2C_REG_UNKNOWN,
"CSUM_ENABLE",},
+};
+
+/*
+ * Send function only remembers data in the buffer and then calls
+ * TPM TIS common code during FINISH event.
+ */
+static int tpm_tis_i2c_send(I2CSlave *i2c, uint8_t data)
+{
+ TPMStateI2C *i2cst = TPM_TIS_I2C(i2c);
+
+ /* Reject non-supported registers. */
+ if (i2cst->offset == 0) {
+ /* Convert I2C register to TIS register */
+ tpm_tis_i2c_to_tis_reg(i2cst, data);
+ if (i2cst->tis_addr
Re: [PATCH 2/2] tests/avocado/boot_linux.py: re-enable test-case for ppc64
Kautuk Consul writes:
> Fixes c0c8687ef0("tests/avocado: disable BootLinuxPPC64 test in CI").
>
> Commit c0c8687ef0fd990db8db1655a8a6c5a5e35dd4bb disabled the test-case
> for PPC64. On investigation, this turns out to be an issue with the
> time taken for downloading the Fedora 31 qcow2 image being included
> within the test-case timeout.
> Re-enable this test-case by setting the timeout to 360 seconds just
> before launching the downloaded VM image.
>
> Signed-off-by: Kautuk Consul
> Reported-by: Alex Bennée
> Tested-by: Hariharan T S hariharan...@linux.vnet.ibm.com
It doesn't really address the principle problem that the
boot_linux.py:BootLinuxPPC64.test_pseries_tcg is super heavyweight for
only 2% extra coverage of the executed lines.
What we really need is a script so we can compare the output between the
two jsons:
gcovr --json --exclude-unreachable-branches --print-summary -o coverage.json
--root ../../ . *.p
because I suspect we could make up that missing few % noodling the
baseline test a bit more.
> ---
> tests/avocado/boot_linux.py | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/tests/avocado/boot_linux.py b/tests/avocado/boot_linux.py
> index be30dcbd58..c3869a987c 100644
> --- a/tests/avocado/boot_linux.py
> +++ b/tests/avocado/boot_linux.py
> @@ -91,9 +91,9 @@ class BootLinuxPPC64(LinuxTest):
> :avocado: tags=arch:ppc64
> """
>
> +# timeout for downloading new VM image.
> timeout = 360
>
> -@skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab')
> def test_pseries_tcg(self):
> """
> :avocado: tags=machine:pseries
> @@ -101,6 +101,10 @@ def test_pseries_tcg(self):
> """
> self.require_accelerator("tcg")
> self.vm.add_args("-accel", "tcg")
> +
> +# timeout for actual Linux PPC boot test
> +self.timeout = 360
> +
> self.launch_and_wait(set_up_ssh_connection=False)
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
[PATCH v8 0/3] Add support for TPM devices over I2C bus
Hello, I have incorporated review comments from Joel & Stefan. Please review. This drop adds support for the TPM devices attached to the I2C bus. It only supports the TPM2 protocol. You need to run it with the external TPM emulator like swtpm. I have tested it with swtpm. I have refered to the work done by zhdan...@meta.com but at the core level out implementation is different. https://github.com/theopolis/qemu/commit/2e2e57cde9e419c36af8071bb85392ad1ed70966 Based-on: $MESSAGE_ID Ninad Palsule (3): docs: Add support for TPM devices over I2C bus tpm: Extend common APIs to support TPM TIS I2C tpm: Add support for TPM device over I2C bus docs/specs/tpm.rst | 21 ++ hw/arm/Kconfig | 1 + hw/tpm/Kconfig | 7 + hw/tpm/meson.build | 1 + hw/tpm/tpm_tis.h| 3 + hw/tpm/tpm_tis_common.c | 36 ++- hw/tpm/tpm_tis_i2c.c| 525 hw/tpm/trace-events | 6 + include/hw/acpi/tpm.h | 37 +++ include/sysemu/tpm.h| 3 + 10 files changed, 632 insertions(+), 8 deletions(-) create mode 100644 hw/tpm/tpm_tis_i2c.c -- 2.37.2
Re: [RFC PATCH v1 1/1] migration: Disable postcopy + multifd migration
On Mon, Mar 27, 2023 at 01:15:18PM -0300, Leonardo Bras wrote: > Since the introduction of multifd, it's possible to perform a multifd > migration and finish it using postcopy. > > A bug introduced by yank (fixed on cfc3bcf373) was previously preventing > a successful use of this migration scenario, and now it should be > working on most cases. > > But since there is not enough testing/support nor any reported users for > this scenario, we should disable this combination before it may cause any > problems for users. > > Suggested-by: Dr. David Alan Gilbert > Signed-off-by: Leonardo Bras Acked-by: Peter Xu -- Peter Xu
Re: [PATCH v6 13/25] target/riscv: Introduce mmuidx_priv
On 3/26/23 19:07, LIU Zhiwei wrote:
+static inline int mmuidx_priv(int mmu_idx)
+{
+ int ret = mmu_idx & 3;
+ if (ret == MMUIdx_S_SUM) {
+ ret = PRV_S;
+ }
+ return ret;
+}
+
Can we remove the PRIV from the tb flags after we have this function?
No, because this is the priv of the memory operation as modified by e.g. MPRV, not the
true cpu priv.
r~
Re: [PATCH v2 2/5] apic: add support for x2APIC mode
On 3/27/23 23:22, David Woodhouse wrote:
On Mon, 2023-03-27 at 22:45 +0700, Bui Quang Minh wrote:
Maybe I'm misreading the patch, but to me it looks that
if (dest == 0xff) apic_get_broadcast_bitmask() bit applies even in
x2apic mode? So delivering to the APIC with physical ID 255 will be
misinterpreted as a broadcast?
In case dest == 0xff the second argument to apic_get_broadcast_bitmask
is set to false which means this is xAPIC broadcast
Yeah, but it *isn't* xAPIC broadcast. It's X2APIC unicast to APIC#255.
I think you want (although you don't have 'dev') something like this:
static void apic_get_delivery_bitmask(uint32_t *deliver_bitmask,
uint32_t dest, uint8_t dest_mode)
{
APICCommonState *apic_iter;
int i;
memset(deliver_bitmask, 0x00, max_apic_words * sizeof(uint32_t));
/* x2APIC broadcast id for both physical and logical (cluster) mode */
if (dest == 0x) {
apic_get_broadcast_bitmask(deliver_bitmask, true);
return;
}
if (dest_mode == 0) {
apic_find_dest(deliver_bitmask, dest);
/* Broadcast to xAPIC mode apics */
-if (dest == 0xff) {
+if (dest == 0xff && is_x2apic_mode(dev)) {
apic_get_broadcast_bitmask(deliver_bitmask, false);
}
} else {
Hmm, the unicast case is handled in apic_find_dest function, the logic
inside the if (dest == 0xff) is for handling the broadcast case only.
This is because when dest == 0xff, it can be both a x2APIC unicast and
xAPIC broadcast in case we have some CPUs that are in xAPIC and others
are in x2APIC. Do you think the code here is tricky and hard to read?
Re: [PATCH v2 2/5] apic: add support for x2APIC mode
On Mon, 2023-03-27 at 23:35 +0700, Bui Quang Minh wrote:
> On 3/27/23 23:22, David Woodhouse wrote:
> > On Mon, 2023-03-27 at 22:45 +0700, Bui Quang Minh wrote:
> > >
> > > > Maybe I'm misreading the patch, but to me it looks that
> > > > if (dest == 0xff) apic_get_broadcast_bitmask() bit applies even in
> > > > x2apic mode? So delivering to the APIC with physical ID 255 will be
> > > > misinterpreted as a broadcast?
> > >
> > > In case dest == 0xff the second argument to apic_get_broadcast_bitmask
> > > is set to false which means this is xAPIC broadcast
> >
> > Yeah, but it *isn't* xAPIC broadcast. It's X2APIC unicast to APIC#255.
> >
> > I think you want (although you don't have 'dev') something like this:
> >
> >
> > static void apic_get_delivery_bitmask(uint32_t *deliver_bitmask,
> > uint32_t dest, uint8_t dest_mode)
> > {
> > APICCommonState *apic_iter;
> > int i;
> >
> > memset(deliver_bitmask, 0x00, max_apic_words * sizeof(uint32_t));
> >
> > /* x2APIC broadcast id for both physical and logical (cluster) mode */
> > if (dest == 0x) {
> > apic_get_broadcast_bitmask(deliver_bitmask, true);
> > return;
> > }
> >
> > if (dest_mode == 0) {
> > apic_find_dest(deliver_bitmask, dest);
> > /* Broadcast to xAPIC mode apics */
> > - if (dest == 0xff) {
> > + if (dest == 0xff && is_x2apic_mode(dev)) {
> > apic_get_broadcast_bitmask(deliver_bitmask, false);
> > }
> > } else {
> >
>
> Hmm, the unicast case is handled in apic_find_dest function, the logic
> inside the if (dest == 0xff) is for handling the broadcast case only.
> This is because when dest == 0xff, it can be both a x2APIC unicast and
> xAPIC broadcast in case we have some CPUs that are in xAPIC and others
> are in x2APIC.
Ah! Yes, I see it now.
Shouldn't apic_get_broadcast_bitmask(… true) add *all* APICs to the
mask, regardless of their mode? An APIC which is still in xAPIC mode
will only look at the low 8 bits and see 0xFF which it also interprets
as broadcast? Or is that not how real hardware behaves?
> Do you think the code here is tricky and hard to read?
Well, I completely failed to read it... :)
I think changing the existing comment something like this might help...
- /* Broadcast to xAPIC mode apics */
+ /* Any APIC in xAPIC mode will interpret 0xFF as broadcast */
Coupled with a comment on apic_get_delivery_bitmask() clarifying that
it depends on the mode of each APIC it considers — which is obvious
enough in retrospect now I read the code and you point it out to me,
but empirically, we have to concede that it wasn't obvious *enough* :)
smime.p7s
Description: S/MIME cryptographic signature
[PULL 0/2] hw/nvme fixes
From: Klaus Jensen Hi Peter, The following changes since commit e3debd5e7d0ce031356024878a0a18b9d109354a: Merge tag 'pull-request-2023-03-24' of https://gitlab.com/thuth/qemu into staging (2023-03-24 16:08:46 +) are available in the Git repository at: https://gitlab.com/birkelund/qemu.git tags/nvme-next-pull-request for you to fetch changes up to ca2a091802872b265bc6007a2d36276d51d8e4b3: hw/nvme: fix missing DNR on compare failure (2023-03-27 19:05:23 +0200) hw/nvme fixes Klaus Jensen (1): hw/nvme: fix missing DNR on compare failure Mateusz Kozlowski (1): hw/nvme: Change alignment in dma functions for nvme_blk_* hw/nvme/ctrl.c | 26 +- 1 file changed, 13 insertions(+), 13 deletions(-) -- 2.39.2
[PULL 1/2] hw/nvme: Change alignment in dma functions for nvme_blk_*
From: Mateusz Kozlowski
Since the nvme_blk_read/write are used by both the data and metadata
portions of the IO, it can't have the 512B alignment requirement.
Without this change any metadata transfer, which length isn't a multiple
of 512B and which is bigger than 512B, will result in only a partial
transfer.
Signed-off-by: Mateusz Kozlowski
Reviewed-by: Klaus Jensen
Signed-off-by: Klaus Jensen
---
hw/nvme/ctrl.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 49c1210fce2b..291009545f03 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -1434,26 +1434,26 @@ uint16_t nvme_bounce_mdata(NvmeCtrl *n, void *ptr,
uint32_t len,
}
static inline void nvme_blk_read(BlockBackend *blk, int64_t offset,
- BlockCompletionFunc *cb, NvmeRequest *req)
+ uint32_t align, BlockCompletionFunc *cb,
+ NvmeRequest *req)
{
assert(req->sg.flags & NVME_SG_ALLOC);
if (req->sg.flags & NVME_SG_DMA) {
-req->aiocb = dma_blk_read(blk, >sg.qsg, offset, BDRV_SECTOR_SIZE,
- cb, req);
+req->aiocb = dma_blk_read(blk, >sg.qsg, offset, align, cb, req);
} else {
req->aiocb = blk_aio_preadv(blk, offset, >sg.iov, 0, cb, req);
}
}
static inline void nvme_blk_write(BlockBackend *blk, int64_t offset,
- BlockCompletionFunc *cb, NvmeRequest *req)
+ uint32_t align, BlockCompletionFunc *cb,
+ NvmeRequest *req)
{
assert(req->sg.flags & NVME_SG_ALLOC);
if (req->sg.flags & NVME_SG_DMA) {
-req->aiocb = dma_blk_write(blk, >sg.qsg, offset, BDRV_SECTOR_SIZE,
- cb, req);
+req->aiocb = dma_blk_write(blk, >sg.qsg, offset, align, cb, req);
} else {
req->aiocb = blk_aio_pwritev(blk, offset, >sg.iov, 0, cb, req);
}
@@ -2207,10 +2207,10 @@ static void nvme_rw_cb(void *opaque, int ret)
}
if (req->cmd.opcode == NVME_CMD_READ) {
-return nvme_blk_read(blk, offset, nvme_rw_complete_cb, req);
+return nvme_blk_read(blk, offset, 1, nvme_rw_complete_cb, req);
}
-return nvme_blk_write(blk, offset, nvme_rw_complete_cb, req);
+return nvme_blk_write(blk, offset, 1, nvme_rw_complete_cb, req);
}
}
@@ -3437,7 +3437,7 @@ static uint16_t nvme_read(NvmeCtrl *n, NvmeRequest *req)
block_acct_start(blk_get_stats(blk), >acct, data_size,
BLOCK_ACCT_READ);
-nvme_blk_read(blk, data_offset, nvme_rw_cb, req);
+nvme_blk_read(blk, data_offset, BDRV_SECTOR_SIZE, nvme_rw_cb, req);
return NVME_NO_COMPLETE;
invalid:
@@ -3607,7 +3607,7 @@ static uint16_t nvme_do_write(NvmeCtrl *n, NvmeRequest
*req, bool append,
block_acct_start(blk_get_stats(blk), >acct, data_size,
BLOCK_ACCT_WRITE);
-nvme_blk_write(blk, data_offset, nvme_rw_cb, req);
+nvme_blk_write(blk, data_offset, BDRV_SECTOR_SIZE, nvme_rw_cb, req);
} else {
req->aiocb = blk_aio_pwrite_zeroes(blk, data_offset, data_size,
BDRV_REQ_MAY_UNMAP, nvme_rw_cb,
--
2.39.2
[PULL 2/2] hw/nvme: fix missing DNR on compare failure
From: Klaus Jensen
Even if the host is somehow using compare to do compare-and-write, the
host should be notified immediately about the compare failure and not
have to wait for the driver to potentially retry the command.
Fixes: 0a384f923f51 ("hw/block/nvme: add compare command")
Reported-by: Jim Harris
Signed-off-by: Klaus Jensen
---
hw/nvme/ctrl.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 291009545f03..8b7be1420912 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -2378,7 +2378,7 @@ static void nvme_compare_mdata_cb(void *opaque, int ret)
for (bufp = buf; mbufp < end; bufp += ns->lbaf.ms, mbufp +=
ns->lbaf.ms) {
if (memcmp(bufp + pil, mbufp + pil, ns->lbaf.ms - pil)) {
-req->status = NVME_CMP_FAILURE;
+req->status = NVME_CMP_FAILURE | NVME_DNR;
goto out;
}
}
@@ -2387,7 +2387,7 @@ static void nvme_compare_mdata_cb(void *opaque, int ret)
}
if (memcmp(buf, ctx->mdata.bounce, ctx->mdata.iov.size)) {
-req->status = NVME_CMP_FAILURE;
+req->status = NVME_CMP_FAILURE | NVME_DNR;
goto out;
}
@@ -2436,7 +2436,7 @@ static void nvme_compare_data_cb(void *opaque, int ret)
}
if (memcmp(buf, ctx->data.bounce, ctx->data.iov.size)) {
-req->status = NVME_CMP_FAILURE;
+req->status = NVME_CMP_FAILURE | NVME_DNR;
goto out;
}
--
2.39.2
Re: [PATCH] riscv: Add support for the Zfa extension
On 3/27/23 01:00, Christoph Muellner wrote:
+uint64_t helper_fminm_s(CPURISCVState *env, uint64_t rs1, uint64_t rs2)
+{
+float32 frs1 = check_nanbox_s(env, rs1);
+float32 frs2 = check_nanbox_s(env, rs2);
+
+if (float32_is_any_nan(frs1) || float32_is_any_nan(frs2)) {
+return float32_default_nan(>fp_status);
+}
+
+return nanbox_s(env, float32_minimum_number(frs1, frs2, >fp_status));
+}
Better to set and clear fp_status->default_nan_mode around the operation.
+uint64_t helper_fround_s(CPURISCVState *env, uint64_t frs1)
+{
+if (float32_is_zero(frs1) ||
+float32_is_infinity(frs1)) {
+return frs1;
+}
+
+if (float32_is_any_nan(frs1)) {
+riscv_cpu_set_fflags(env, FPEXC_NV);
+return frs1;
+}
+
+int32_t tmp = float32_to_int32(frs1, >fp_status);
+return nanbox_s(env, int32_to_float32(tmp, >fp_status));
+}
Very much incorrect, since int32_t does not have the range for the intermediate result.
In any case, the function you want is float32_round_to_int, which eliminates the
zero/inf/nan special cases. It will raise inexact, so perfect for froundnx, but you'll
need to save/restore float_flag_inexact around the function.
+uint64_t helper_fli_s(CPURISCVState *env, uint32_t rs1)
+{
+const uint32_t fli_s_table[] = {
static const. You don't need to use float32_default_nan, use the correct architected
constant. This entire operation should be done at translation time.
+target_ulong helper_fcvtmod_w_d(CPURISCVState *env, uint64_t frs1)
+{
+if (float64_is_any_nan(frs1) ||
+float64_is_infinity(frs1)) {
+return 0;
+}
+
+return float64_to_int32(frs1, >fp_status);
+}
Incorrect, as float64_to_int32 will saturate the result, whereas you need the
modular result.
There is code to do the conversion mod 2**64 in target/alpha/ (do_cvttq). We should move
this to generic code if it is to be used by more than one target.
+bool trans_fmvp_d_x(DisasContext *ctx, arg_fmvp_d_x *a)
+{
+REQUIRE_FPU;
+REQUIRE_ZFA(ctx);
+REQUIRE_EXT(ctx, RVD);
+REQUIRE_32BIT(ctx);
+
+TCGv src1 = get_gpr(ctx, a->rs1, EXT_ZERO);
+TCGv_i64 t1 = tcg_temp_new_i64();
+
+tcg_gen_extu_tl_i64(t1, src1);
+tcg_gen_deposit_i64(cpu_fpr[a->rd], cpu_fpr[a->rd], t1, 32, 32);
+mark_fs_dirty(ctx);
+return true;
+}
This does not match the linked document, which says that this insn has two inputs and sets
the complete fpr.
r~
Re: [PATCH for-8.0 11/11] linux-user/arm: Take more care allocating commpage
On 3/27/23 01:38, Alex Bennée wrote: Richard Henderson writes: User setting of -R reserved_va can lead to an assertion failure in page_set_flags. Sanity check the value of reserved_va and print an error message instead. Do not allocate a commpage at all for m-profile cpus. I see this: TESTconvd on i386 qemu-i386: Unable to reserve 0x1 bytes of virtual address space at 0x8000 (File exists) for use as guest address space (check your virtual memory ulimit setting, min_mmap_addr or reserve less using -R option) on the ubuntu aarch64 static build: https://gitlab.com/stsquad/qemu/-/jobs/4003523064 Odd. Works on aarch64.ci.qemu.org outside of the gitlab environment. r~
Re: [PATCH] hw/loongarch/virt: Fix virt_to_phys_addr function
On 3/27/23 04:23, Tianrui Zhao wrote:
The virt addr should mask TARGET_PHYS_ADDR_SPACE_BITS to
get the phys addr, and this is used by loading kernel elf.
Signed-off-by: Tianrui Zhao
---
hw/loongarch/virt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index b702c3f51e..f4bf14c1c8 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -399,7 +399,7 @@ static struct _loaderparams {
static uint64_t cpu_loongarch_virt_to_phys(void *opaque, uint64_t addr)
{
-return addr & 0x1fffll;
+return addr & MAKE_64BIT_MASK(0, TARGET_PHYS_ADDR_SPACE_BITS);
}
static int64_t load_kernel_info(void)
Looks correct. Any idea where this 29-bit value originated?
Acked-by: Richard Henderson
r~
Re: [PATCH v8 3/3] tpm: Add support for TPM device over I2C bus
Hi Cedric,
On 3/27/23 12:01 PM, Cédric Le Goater wrote:
On 3/27/23 18:16, Ninad Palsule wrote:
Qemu already supports devices attached to ISA and sysbus. This drop adds
support for the I2C bus attached TPM devices. I2C model only supports
TPM2 protocol.
This commit includes changes for the common code.
- Added I2C emulation model. Logic was added in the model to temporarily
cache the data as I2C interface works per byte basis.
- New tpm type "tpm-tis-i2c" added for I2C support. The user has to
provide this string on command line.
Testing:
TPM I2C device module is tested using SWTPM (software based TPM
package). Qemu uses the rainier machine and is connected to swtpm
over
the socket interface.
The command to start swtpm is as follows:
$ swtpm socket --tpmstate dir=/tmp/mytpm1 \
--ctrl type=unixio,path=/tmp/mytpm1/swtpm-sock \
--tpm2 --log level=100
The command to start qemu is as follows:
$ qemu-system-arm -M rainier-bmc -nographic \
-kernel ${IMAGEPATH}/fitImage-linux.bin \
-dtb ${IMAGEPATH}/aspeed-bmc-ibm-rainier.dtb \
-initrd
${IMAGEPATH}/obmc-phosphor-initramfs.rootfs.cpio.xz \
-drive
file=${IMAGEPATH}/obmc-phosphor-image.rootfs.wic.qcow2,if=sd,index=2 \
-net nic -net
user,hostfwd=:127.0.0.1:-:22,hostfwd=:127.0.0.1:2443-:443 \
-chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
-tpmdev emulator,id=tpm0,chardev=chrtpm \
-device
tpm-tis-i2c,tpmdev=tpm0,bus=aspeed.i2c.bus.12,address=0x2e
Signed-off-by: Ninad Palsule
---
V2:
Incorporated Stephen's review comments.
- Handled checksum related register in I2C layer
- Defined I2C interface capabilities and return those instead of
capabilities from TPM TIS. Add required capabilities from TIS.
- Do not cache FIFO data in the I2C layer.
- Make sure that Device address change register is not passed to I2C
layer as capability indicate that it is not supported.
- Added boundary checks.
- Make sure that bits 26-31 are zeroed for the TPM_STS register on read
- Updated Kconfig files for new define.
---
V3:
- Moved processing of register TPM_I2C_LOC_SEL in the I2C. So I2C layer
remembers the locality and pass it to TIS on each read/write.
- The write data is no more cached in the I2C layer so the buffer size
is reduced to 16 bytes.
- Checksum registers are now managed by the I2C layer. Added new
function in TIS layer to return the checksum and used that to process
the request.
- Now 2-4 byte register value will be passed to TIS layer in a single
write call instead of 1 byte at a time. Added functions to convert
between little endian stream of bytes to single 32 bit unsigned
integer. Similarly 32 bit integer to stream of bytes.
- Added restriction on device change register.
- Replace few if-else statement with switch statement for clarity.
- Log warning when unknown register is received.
- Moved all register definations to acpi/tmp.h
---
V4:
Incorporated review comments from Cedric and Stefan.
- Reduced data[] size from 16 byte to 5 bytes.
- Added register name in the mapping table which can be used for
tracing.
- Removed the endian conversion functions instead used simple logic
provided by Stefan.
- Rename I2C registers to reduce the length.
- Added traces for send, recv and event functions. You can turn on trace
on command line by using "-trace "tpm_tis_i2c*" option.
---
V5:
Fixed issues reported by Stefan's test.
- Added mask for the INT_ENABLE register.
- Use correct TIS register for reading interrupt capability.
- Cleanup how register is converted from I2C to TIS and also saved
information like tis_addr and register name in the i2cst so that we
can only convert it once on i2c_send.
- Trace register number for unknown registers.
---
V6:
Fixed review comments from Stefan.
- Fixed some variable size.
- Removed unused variables.
- Added vmstat backin to handle migration.
- Added post load phase to reload tis address and register name.
---
V7:
Incorporated review comments from Stefan.
- Added tpm_tis_i2c_initfn function
- Set the device catagory DEVICE_CATEGORY_MISC.
- Corrected default locality selection.
- Other cleanup. Include file cleanup.
---
V8:
Incorporated review comments from Stefan.
- Removed the irq initialization as linux doesn't support interrupts for
TPM
- Handle INT_CAPABILITY register in I2C only and return 0 to indicate
that it is not supported.
---
hw/arm/Kconfig | 1 +
hw/tpm/Kconfig | 7 +
hw/tpm/meson.build | 1 +
hw/tpm/tpm_tis_i2c.c | 525 +++
hw/tpm/trace-events | 6 +
include/sysemu/tpm.h | 3 +
6 files changed, 543 insertions(+)
create mode 100644 hw/tpm/tpm_tis_i2c.c
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index b5aed4aff5..05d6ef1a31 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -6,6 +6,7 @@ config ARM_VIRT
Re: [PATCH V3] meson: install keyboard maps only if necessary
On Mon, Mar 27, 2023 at 02:21:47PM -0300, casan...@redhat.com wrote: > From: Carlos Santos > > They are required only for system emulation (i.e. have_system is true). > > Signed-off-by: Carlos Santos > --- Reviewed-by: Daniel P. Berrangé With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH V2] tracing: install trace events file only if necessary
On Mon, Mar 27, 2023 at 02:30:58PM -0300, casan...@redhat.com wrote: > From: Carlos Santos > > It is not useful when configuring with --enable-trace-backends=nop. > > Signed-off-by: Carlos Santos > --- Reviewed-by: Daniel P. Berrangé With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v2 06/10] target/riscv: Remove riscv_cpu_virt_enabled()
On 3/27/23 01:08, Weiwei Li wrote: Directly use env->virt_enabled instead. Suggested-by: LIU Zhiwei Signed-off-by: Weiwei Li Signed-off-by: Junqiang Wang --- target/riscv/cpu.c| 2 +- target/riscv/cpu.h| 1 - target/riscv/cpu_helper.c | 51 ++- target/riscv/csr.c| 46 +-- target/riscv/debug.c | 10 target/riscv/op_helper.c | 18 +++--- target/riscv/pmu.c| 4 +-- target/riscv/translate.c | 2 +- 8 files changed, 64 insertions(+), 70 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v9 1/3] docs: Add support for TPM devices over I2C bus
On 3/27/23 14:12, Ninad Palsule wrote: This is a documentation change for I2C TPM device support. Qemu already supports devices attached to ISA and sysbus. This drop adds support for the I2C bus attached TPM devices. Signed-off-by: Ninad Palsule --- V2: Incorporated Stephen's review comments - Added example in the document. --- V4: Incorporate Cedric & Stefan's comments - Added example for ast2600-evb - Corrected statement about arm virtual machine. --- V6: Incorporated review comments from Stefan. --- V8: Incorporate review comments from Joel and Stefan - Removed the rainier example - Added step required to configure on ast2600-evb --- docs/specs/tpm.rst | 21 + 1 file changed, 21 insertions(+) diff --git a/docs/specs/tpm.rst b/docs/specs/tpm.rst index 535912a92b..efe124a148 100644 --- a/docs/specs/tpm.rst +++ b/docs/specs/tpm.rst @@ -21,12 +21,16 @@ QEMU files related to TPM TIS interface: - ``hw/tpm/tpm_tis_common.c`` - ``hw/tpm/tpm_tis_isa.c`` - ``hw/tpm/tpm_tis_sysbus.c`` + - ``hw/tpm/tpm_tis_i2c.c`` - ``hw/tpm/tpm_tis.h`` Both an ISA device and a sysbus device are available. The former is used with pc/q35 machine while the latter can be instantiated in the Arm virt machine. +An I2C device support is also provided which can be instantiated in the Arm +based emulation machines. This device only supports the TPM 2 protocol. + CRB interface - @@ -348,6 +352,23 @@ In case an Arm virt machine is emulated, use the following command line: -drive if=pflash,format=raw,file=flash0.img,readonly=on \ -drive if=pflash,format=raw,file=flash1.img +In case a ast2600-evb bmc machine is emulated and you want to use a TPM device +attached to I2C bus, use the following command line: + +.. code-block:: console + + qemu-system-arm -M ast2600-evb -nographic \ +-kernel arch/arm/boot/zImage \ +-dtb arch/arm/boot/dts/aspeed-ast2600-evb.dtb \ +-initrd rootfs.cpio \ +-chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \ +-tpmdev emulator,id=tpm0,chardev=chrtpm \ +-device tpm-tis-i2c,tpmdev=tpm0,bus=aspeed.i2c.bus.12,address=0x2e + + For testing, use this command to load the driver to the correct address + + echo tpm_tis_i2c 0x2e > /sys/bus/i2c/devices/i2c-12/new_device + In case SeaBIOS is used as firmware, it should show the TPM menu item after entering the menu with 'ESC'. Reviewed-by: Stefan Berger
[PATCH V2] tracing: install trace events file only if necessary
From: Carlos Santos
It is not useful when configuring with --enable-trace-backends=nop.
Signed-off-by: Carlos Santos
---
Changes v1->v2:
Install based on chosen trace backend, not on chosen emulators.
---
trace/meson.build | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/trace/meson.build b/trace/meson.build
index 8e80be895c..3d96b4eea0 100644
--- a/trace/meson.build
+++ b/trace/meson.build
@@ -64,7 +64,7 @@ trace_events_all = custom_target('trace-events-all',
input: trace_events_files,
command: [ 'cat', '@INPUT@' ],
capture: true,
- install: true,
+ install: get_option('trace_backends') != 'nop'
install_dir: qemu_datadir)
if 'ust' in get_option('trace_backends')
--
2.31.1
Re: [PATCH-for-8.0] linux-user/mips: Use P5600 as default CPU to run NaN2008 ELF binaries
On 3/27/23 09:24, Philippe Mathieu-Daudé wrote: Per the release 6.06 revision history: 5.03 August 21, 2013 • ABS2008 and NAN2008 fields of Table 5.7 “FCSR RegisterField Descriptions” were optional in release 3 and could be R/W, but as of release 5 are required, read-only, and preset by hardware. The P5600 core implements the release 5, and has the ABS2008 and NAN2008 bits set in CP1_fcr31. Therefore it is able to run ELF binaries compiled with EF_MIPS_NAN2008, such the CIP United Debian NaN2008 distribution: http://repo.oss.cipunited.com/mipsel-nan2008/README.txt In order to run such compiled binaries, select by default the P5600 core when the ELF 'MIPS_NAN2008' flag is set. Reported-by: Jiaxun Yang Signed-off-by: Philippe Mathieu-Daudé --- linux-user/mips/target_elf.h | 3 +++ 1 file changed, 3 insertions(+) Reviewed-by: Richard Henderson r~
Re: [PATCH v2 05/10] target/riscv: Convert env->virt to a bool env->virt_enabled
On 3/27/23 01:08, Weiwei Li wrote: From: LIU Zhiwei Currently we only use the env->virt to encode the virtual mode enabled status. Let's make it a bool type. Signed-off-by: LIU Zhiwei Reviewed-by: Weiwei Li Message-ID:<20230325145348.1208-1-zhiwei_...@linux.alibaba.com> --- target/riscv/cpu.h| 2 +- target/riscv/cpu_bits.h | 3 --- target/riscv/cpu_helper.c | 6 +++--- target/riscv/machine.c| 6 +++--- target/riscv/translate.c | 4 ++-- 5 files changed, 9 insertions(+), 12 deletions(-) Reviewed-by: Richard Henderson r~
[PATCH v9 2/3] tpm: Extend common APIs to support TPM TIS I2C
Qemu already supports devices attached to ISA and sysbus. This drop adds
support for the I2C bus attached TPM devices.
This commit includes changes for the common code.
- Added support for the new checksum registers which are required for
the I2C support. The checksum calculation is handled in the qemu
common code.
- Added wrapper function for read and write data so that I2C code can
call it without MMIO interface.
The TPM TIS I2C spec describes in the table in section "Interface Locality
Usage per Register" that the TPM_INT_ENABLE and TPM_INT_STATUS registers
must be writable for any locality even if the locality is not the active
locality. Therefore, remove the checks whether the writing locality is the
active locality for these registers.
Signed-off-by: Ninad Palsule
Signed-off-by: Stefan Berger
---
V2:
Incorporated Stephen's comments.
- Removed checksum enable and checksum get registers.
- Added checksum calculation function which can be called from
i2c layer.
---
V3:
Incorporated review comments from Cedric and Stefan.
- Pass locality to the checksum calculation function and cleanup
- Moved I2C related definations in the acpi/tpm.h
---
V4:
Incorporated review comments by Stefan
- Remove the check for locality while calculating checksum
- Use bswap16 instead of cpu_ti_be16.
- Rename TPM_I2C register by dropping _TIS_ from it.
---
V7:
Incorporated review comments from Stefan.
- Removed locality check from INT_ENABLE and INT_STATUS registers write
path.
- Moved TPM_DATA_CSUM_ENABLED define in the tpm.h
---
V8:
Incorporated review comments from Stefan
- Moved the INT_ENABLE mask to tpm.h file.
---
hw/tpm/tpm_tis.h| 3 +++
hw/tpm/tpm_tis_common.c | 36
include/hw/acpi/tpm.h | 37 +
3 files changed, 68 insertions(+), 8 deletions(-)
diff --git a/hw/tpm/tpm_tis.h b/hw/tpm/tpm_tis.h
index f6b5872ba6..6f29a508dd 100644
--- a/hw/tpm/tpm_tis.h
+++ b/hw/tpm/tpm_tis.h
@@ -86,5 +86,8 @@ int tpm_tis_pre_save(TPMState *s);
void tpm_tis_reset(TPMState *s);
enum TPMVersion tpm_tis_get_tpm_version(TPMState *s);
void tpm_tis_request_completed(TPMState *s, int ret);
+uint32_t tpm_tis_read_data(TPMState *s, hwaddr addr, unsigned size);
+void tpm_tis_write_data(TPMState *s, hwaddr addr, uint64_t val, uint32_t size);
+uint16_t tpm_tis_get_checksum(TPMState *s);
#endif /* TPM_TPM_TIS_H */
diff --git a/hw/tpm/tpm_tis_common.c b/hw/tpm/tpm_tis_common.c
index 503be2a541..c07c179dbc 100644
--- a/hw/tpm/tpm_tis_common.c
+++ b/hw/tpm/tpm_tis_common.c
@@ -26,6 +26,8 @@
#include "hw/irq.h"
#include "hw/isa/isa.h"
#include "qapi/error.h"
+#include "qemu/bswap.h"
+#include "qemu/crc-ccitt.h"
#include "qemu/module.h"
#include "hw/acpi/tpm.h"
@@ -447,6 +449,23 @@ static uint64_t tpm_tis_mmio_read(void *opaque, hwaddr
addr,
return val;
}
+/*
+ * A wrapper read function so that it can be directly called without
+ * mmio.
+ */
+uint32_t tpm_tis_read_data(TPMState *s, hwaddr addr, unsigned size)
+{
+return tpm_tis_mmio_read(s, addr, size);
+}
+
+/*
+ * Calculate current data buffer checksum
+ */
+uint16_t tpm_tis_get_checksum(TPMState *s)
+{
+return bswap16(crc_ccitt(0, s->buffer, s->rw_offset));
+}
+
/*
* Write a value to a register of the TIS interface
* See specs pages 33-63 for description of the registers
@@ -588,10 +607,6 @@ static void tpm_tis_mmio_write(void *opaque, hwaddr addr,
break;
case TPM_TIS_REG_INT_ENABLE:
-if (s->active_locty != locty) {
-break;
-}
-
s->loc[locty].inte &= mask;
s->loc[locty].inte |= (val & (TPM_TIS_INT_ENABLED |
TPM_TIS_INT_POLARITY_MASK |
@@ -601,10 +616,6 @@ static void tpm_tis_mmio_write(void *opaque, hwaddr addr,
/* hard wired -- ignore */
break;
case TPM_TIS_REG_INT_STATUS:
-if (s->active_locty != locty) {
-break;
-}
-
/* clearing of interrupt flags */
if (((val & TPM_TIS_INTERRUPTS_SUPPORTED)) &&
(s->loc[locty].ints & TPM_TIS_INTERRUPTS_SUPPORTED)) {
@@ -767,6 +778,15 @@ static void tpm_tis_mmio_write(void *opaque, hwaddr addr,
}
}
+/*
+ * A wrapper write function so that it can be directly called without
+ * mmio.
+ */
+void tpm_tis_write_data(TPMState *s, hwaddr addr, uint64_t val, uint32_t size)
+{
+tpm_tis_mmio_write(s, addr, val, size);
+}
+
const MemoryRegionOps tpm_tis_memory_ops = {
.read = tpm_tis_mmio_read,
.write = tpm_tis_mmio_write,
diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
index 559ba6906c..fb81e1735b 100644
--- a/include/hw/acpi/tpm.h
+++ b/include/hw/acpi/tpm.h
@@ -93,6 +93,7 @@
#define TPM_TIS_CAP_DATA_TRANSFER_64B(3 << 9)
#define TPM_TIS_CAP_DATA_TRANSFER_LEGACY (0 << 9)
#define TPM_TIS_CAP_BURST_COUNT_DYNAMIC (0 << 8)
+#define TPM_TIS_CAP_BURST_COUNT_STATIC (1 << 8)
#define TPM_TIS_CAP_INTERRUPT_LOW_LEVEL
Re: [PATCH v2 2/5] apic: add support for x2APIC mode
On 3/27/23 22:37, David Woodhouse wrote:
On Mon, 2023-03-27 at 22:33 +0700, Bui Quang Minh wrote:
+ memset(deliver_bitmask, 0x00, max_apic_words * sizeof(uint32_t));
+
+ /* x2APIC broadcast id for both physical and logical (cluster) mode */
+ if (dest == 0x) {
+ apic_get_broadcast_bitmask(deliver_bitmask, true);
+ return;
+ }
+
if (dest_mode == 0) {
Might be nice to have a constant for DEST_MODE_PHYS vs.
DEST_MODE_LOGICAL to make this clearer?
I'll fix it in the next version.
+ apic_find_dest(deliver_bitmask, dest);
+ /* Broadcast to xAPIC mode apics */
if (dest == 0xff) {
- memset(deliver_bitmask, 0xff, MAX_APIC_WORDS * sizeof(uint32_t));
- } else {
- int idx = apic_find_dest(dest);
- memset(deliver_bitmask, 0x00, MAX_APIC_WORDS * sizeof(uint32_t));
- if (idx >= 0)
- apic_set_bit(deliver_bitmask, idx);
+ apic_get_broadcast_bitmask(deliver_bitmask, false);
Hrm... aren't you still interpreting destination 0x00FF as
broadcast even for X2APIC mode? Or am I misreading this?
In case the destination is 0xFF, the IPI will be delivered to CPU has
APIC ID 0xFF if it is in x2APIC mode, and it will be delivered to all
CPUs that are in xAPIC mode. In case the destination is 0x, the
IPI is delivered to all CPUs that are in x2APIC mode. I've created
apic_get_broadcast_bitmask function and changed the apic_find_dest to
implement that logic.
Maybe I'm misreading the patch, but to me it looks that
if (dest == 0xff) apic_get_broadcast_bitmask() bit applies even in
x2apic mode? So delivering to the APIC with physical ID 255 will be
misinterpreted as a broadcast?
In case dest == 0xff the second argument to apic_get_broadcast_bitmask
is set to false which means this is xAPIC broadcast
static void apic_get_broadcast_bitmask(uint32_t *deliver_bitmask,
bool is_x2apic_broadcast)
{
int i;
APICCommonState *apic_iter;
for (i = 0; i < max_apics; i++) {
apic_iter = local_apics[i];
if (apic_iter) {
bool apic_in_x2apic = is_x2apic_mode(_iter->parent_obj);
if (is_x2apic_broadcast && apic_in_x2apic) {
apic_set_bit(deliver_bitmask, i);
} else if (!is_x2apic_broadcast && !apic_in_x2apic) {
apic_set_bit(deliver_bitmask, i);
}
}
}
}
In apic_get_broadcast_bitmask, because is_x2apic_broadcast == false, the
delivery bit set only if that apic_in_x2apic == false (that CPU is in
xAPIC mode)
[PATCH-for-8.0] linux-user/mips: Use P5600 as default CPU to run NaN2008 ELF binaries
Per the release 6.06 revision history:
5.03 August 21, 2013
• ABS2008 and NAN2008 fields of Table 5.7 “FCSR RegisterField
Descriptions” were optional in release 3 and could be R/W,
but as of release 5 are required, read-only, and preset by
hardware.
The P5600 core implements the release 5, and has the ABS2008
and NAN2008 bits set in CP1_fcr31. Therefore it is able to run
ELF binaries compiled with EF_MIPS_NAN2008, such the CIP United
Debian NaN2008 distribution:
http://repo.oss.cipunited.com/mipsel-nan2008/README.txt
In order to run such compiled binaries, select by default the
P5600 core when the ELF 'MIPS_NAN2008' flag is set.
Reported-by: Jiaxun Yang
Signed-off-by: Philippe Mathieu-Daudé
---
linux-user/mips/target_elf.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/linux-user/mips/target_elf.h b/linux-user/mips/target_elf.h
index a98c9bd6ad..b965e86b2b 100644
--- a/linux-user/mips/target_elf.h
+++ b/linux-user/mips/target_elf.h
@@ -15,6 +15,9 @@ static inline const char *cpu_get_model(uint32_t eflags)
if ((eflags & EF_MIPS_MACH) == EF_MIPS_MACH_5900) {
return "R5900";
}
+if (eflags & EF_MIPS_NAN2008) {
+return "P5600";
+}
return "24Kf";
}
#endif
--
2.38.1
Re: [PATCH v2 1/5] i386/tcg: implement x2APIC registers MSR access
On Sun, 2023-03-26 at 12:20 +0700, Bui Quang Minh wrote:
>
> +static void apic_mem_write(void *opaque, hwaddr addr, uint64_t val,
> + unsigned size)
> +{
> + int index = (addr >> 4) & 0xff;
> +
> + if (size < 4) {
> + return;
> + }
> +
> + if (addr > 0xfff || !index) {
> + /* MSI and MMIO APIC are at the same memory location,
> + * but actually not on the global bus: MSI is on PCI bus
> + * APIC is connected directly to the CPU.
> + * Mapping them on the global bus happens to work because
> + * MSI registers are reserved in APIC MMIO and vice versa.
> */
> + MSIMessage msi = { .address = addr, .data = val };
> + apic_send_msi();
> + return;
> + }
I know you're just moving this bit around, but note that it means we
*can't* implement the 15-bit MSI trick as things stand, because those
extra 7 bits end up in bits 4-11 of the address, and that means the
'addr > 0xfff' check isn't correct any more.
However, that's only relevant in X2APIC mode... and there's no MMIO
access to registers in X2APIC mode. So the check could perhaps become
something like...
DeviceState *apic = cpu_get_current_apic();
if (!apic || is_x2apic_mode(apic) || addr > 0xfff || !index) {
/* MSI and MMIO APIC are at the same memory location,
* but actually not on the global bus: MSI is on PCI bus
* APIC is connected directly to the CPU.
* Mapping them on the global bus happens to work because
* MSI registers are reserved in xAPIC MMIO and vice versa.
* In X2APIC mode, there is no MMIO and bits 4-11 of the
* address *might* be used to encode the extended dest ID.
*/
MSIMessage msi = ...
smime.p7s
Description: S/MIME cryptographic signature
Re: [PATCH v2 4/7] hw/ipmi: Refactor IPMI interface
Hi, Cedric
The naming scheme is suggested by Corey in a previous review:
https://lists.gnu.org/archive/html/qemu-devel/2021-09/msg02691.html
I originally kept "IpmIBmc" for the host side code talking to BMC but it
might also cause confusion as well. I'm not sure which name is the best
here. Maybe Corey can shed some light on this one? Thank you!
Best Regards,
On Mon, Mar 27, 2023 at 5:34 AM Cédric Le Goater wrote:
> Hello Hao,
>
> On 3/25/23 00:09, Hao Wu wrote:
> > This patch refactors the IPMI interface so that it can be used by both
> > the BMC side and core-side simulation.
> >
> > Detail changes:
> > (1) Split IPMIInterface into IPMIInterfaceHost (for host side
> > simulation) and IPMIInterfaceClient (for BMC side simulation).
> > (2) rename handle_rsp -> handle_msg so the name fits both BMC side and
> > Core side.
> > (3) Add a new class IPMICore. This class represents a simulator/external
> > connection for both BMC and Core side emulation.
> > (4) Change the original IPMIBmc to IPMIBmcHost, representing host side
> > simulation.
> > (5) Add a new type IPMIBmcClient representing BMC side simulation.
> > (6) Appy the changes to the entire IPMI library.
>
> 'IPMIBmcHost' is a BMC object model (internal or external) and
> 'IPMIBmcClient' is a host object model ?
>
> [ ... ]
>
> > @@ -267,15 +267,15 @@ void pnv_bmc_set_pnor(IPMIBmc *bmc, PnvPnor *pnor)
> >* Instantiate the machine BMC. PowerNV uses the QEMU internal
> >* simulator but it could also be external.
> >*/
> > -IPMIBmc *pnv_bmc_create(PnvPnor *pnor)
> > +IPMIBmcHost *pnv_bmc_create(PnvPnor *pnor)
> > {
> > Object *obj;
> >
> > obj = object_new(TYPE_IPMI_BMC_SIMULATOR);
> > qdev_realize(DEVICE(obj), NULL, _fatal);
> > -pnv_bmc_set_pnor(IPMI_BMC(obj), pnor);
> > +pnv_bmc_set_pnor(IPMI_BMC_HOST(obj), pnor);
> >
> > -return IPMI_BMC(obj);
> > +return IPMI_BMC_HOST(obj);
>
> QEMU PowerNV machines model the host side of OpenPOWER systems which
> have an Aspeed SoC based BMC for management. The routine above creates
> an Aspeed *BMC* object model for the PowerNV *host* machine. I find
> 'IPMIBmcHost' confusing. It shouldn't have a 'Host' suffix I think.
>
> 'IPMIBmcClient' sounds ok, or 'IPMIBmcPeer' maybe.
>
> Thanks,
>
> C.
>
>
Re: [PATCH v9 2/3] tpm: Extend common APIs to support TPM TIS I2C
On 3/27/23 14:12, Ninad Palsule wrote:
Qemu already supports devices attached to ISA and sysbus. This drop adds
support for the I2C bus attached TPM devices.
This commit includes changes for the common code.
- Added support for the new checksum registers which are required for
the I2C support. The checksum calculation is handled in the qemu
common code.
- Added wrapper function for read and write data so that I2C code can
call it without MMIO interface.
The TPM TIS I2C spec describes in the table in section "Interface Locality
Usage per Register" that the TPM_INT_ENABLE and TPM_INT_STATUS registers
must be writable for any locality even if the locality is not the active
locality. Therefore, remove the checks whether the writing locality is the
active locality for these registers.
Signed-off-by: Ninad Palsule
Signed-off-by: Stefan Berger
---
V2:
Incorporated Stephen's comments.
- Removed checksum enable and checksum get registers.
- Added checksum calculation function which can be called from
i2c layer.
---
V3:
Incorporated review comments from Cedric and Stefan.
- Pass locality to the checksum calculation function and cleanup
- Moved I2C related definations in the acpi/tpm.h
---
V4:
Incorporated review comments by Stefan
- Remove the check for locality while calculating checksum
- Use bswap16 instead of cpu_ti_be16.
- Rename TPM_I2C register by dropping _TIS_ from it.
---
V7:
Incorporated review comments from Stefan.
- Removed locality check from INT_ENABLE and INT_STATUS registers write
path.
- Moved TPM_DATA_CSUM_ENABLED define in the tpm.h
---
V8:
Incorporated review comments from Stefan
- Moved the INT_ENABLE mask to tpm.h file.
---
hw/tpm/tpm_tis.h| 3 +++
hw/tpm/tpm_tis_common.c | 36
include/hw/acpi/tpm.h | 37 +
3 files changed, 68 insertions(+), 8 deletions(-)
diff --git a/hw/tpm/tpm_tis.h b/hw/tpm/tpm_tis.h
index f6b5872ba6..6f29a508dd 100644
--- a/hw/tpm/tpm_tis.h
+++ b/hw/tpm/tpm_tis.h
@@ -86,5 +86,8 @@ int tpm_tis_pre_save(TPMState *s);
void tpm_tis_reset(TPMState *s);
enum TPMVersion tpm_tis_get_tpm_version(TPMState *s);
void tpm_tis_request_completed(TPMState *s, int ret);
+uint32_t tpm_tis_read_data(TPMState *s, hwaddr addr, unsigned size);
+void tpm_tis_write_data(TPMState *s, hwaddr addr, uint64_t val, uint32_t size);
+uint16_t tpm_tis_get_checksum(TPMState *s);
#endif /* TPM_TPM_TIS_H */
diff --git a/hw/tpm/tpm_tis_common.c b/hw/tpm/tpm_tis_common.c
index 503be2a541..c07c179dbc 100644
--- a/hw/tpm/tpm_tis_common.c
+++ b/hw/tpm/tpm_tis_common.c
@@ -26,6 +26,8 @@
#include "hw/irq.h"
#include "hw/isa/isa.h"
#include "qapi/error.h"
+#include "qemu/bswap.h"
+#include "qemu/crc-ccitt.h"
#include "qemu/module.h"
#include "hw/acpi/tpm.h"
@@ -447,6 +449,23 @@ static uint64_t tpm_tis_mmio_read(void *opaque, hwaddr
addr,
return val;
}
+/*
+ * A wrapper read function so that it can be directly called without
+ * mmio.
+ */
+uint32_t tpm_tis_read_data(TPMState *s, hwaddr addr, unsigned size)
+{
+return tpm_tis_mmio_read(s, addr, size);
+}
+
+/*
+ * Calculate current data buffer checksum
+ */
+uint16_t tpm_tis_get_checksum(TPMState *s)
+{
+return bswap16(crc_ccitt(0, s->buffer, s->rw_offset));
+}
+
/*
* Write a value to a register of the TIS interface
* See specs pages 33-63 for description of the registers
@@ -588,10 +607,6 @@ static void tpm_tis_mmio_write(void *opaque, hwaddr addr,
break;
case TPM_TIS_REG_INT_ENABLE:
-if (s->active_locty != locty) {
-break;
-}
-
s->loc[locty].inte &= mask;
s->loc[locty].inte |= (val & (TPM_TIS_INT_ENABLED |
TPM_TIS_INT_POLARITY_MASK |
@@ -601,10 +616,6 @@ static void tpm_tis_mmio_write(void *opaque, hwaddr addr,
/* hard wired -- ignore */
break;
case TPM_TIS_REG_INT_STATUS:
-if (s->active_locty != locty) {
-break;
-}
-
/* clearing of interrupt flags */
if (((val & TPM_TIS_INTERRUPTS_SUPPORTED)) &&
(s->loc[locty].ints & TPM_TIS_INTERRUPTS_SUPPORTED)) {
@@ -767,6 +778,15 @@ static void tpm_tis_mmio_write(void *opaque, hwaddr addr,
}
}
+/*
+ * A wrapper write function so that it can be directly called without
+ * mmio.
+ */
+void tpm_tis_write_data(TPMState *s, hwaddr addr, uint64_t val, uint32_t size)
+{
+tpm_tis_mmio_write(s, addr, val, size);
+}
+
const MemoryRegionOps tpm_tis_memory_ops = {
.read = tpm_tis_mmio_read,
.write = tpm_tis_mmio_write,
diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
index 559ba6906c..fb81e1735b 100644
--- a/include/hw/acpi/tpm.h
+++ b/include/hw/acpi/tpm.h
@@ -93,6 +93,7 @@
#define TPM_TIS_CAP_DATA_TRANSFER_64B(3 << 9)
#define TPM_TIS_CAP_DATA_TRANSFER_LEGACY (0 << 9)
#define TPM_TIS_CAP_BURST_COUNT_DYNAMIC (0 <<
Re: [PATCH 03/19] target/riscv: introduce riscv_cpu_add_misa_properties()
On 3/27/23 05:42, Daniel Henrique Barboza wrote:
+static void cpu_set_misa_ext_cfg(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+RISCVCPUMisaExtConfig *misa_ext_cfg = opaque;
const
+static void cpu_get_misa_ext_cfg(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+RISCVCPUMisaExtConfig *misa_ext_cfg = opaque;
const
+static RISCVCPUMisaExtConfig misa_ext_cfgs[] = {};
const
r~
Re: [PATCH 03/19] target/riscv: introduce riscv_cpu_add_misa_properties()
On 3/27/23 15:43, Richard Henderson wrote:
On 3/27/23 05:42, Daniel Henrique Barboza wrote:
+ g_autofree char *name = g_strdup_printf("%s", misa_cfg->name);
+ g_autofree char *desc = g_strdup_printf("%s", misa_cfg->description);
What is the point of this? It would seem that you could simply pass the
original string literals to object_property_*.
Yeah, it appears that every other object_property_add() is being called using
the literals without dealing with g_autofree.
I'll change it in v2.
Daniel
r~
Re: [Socratic RFC PATCH] include: attempt to document device_class_set_props
Mark Cave-Ayland writes:
> On 27/03/2023 14:15, Alex Bennée wrote:
>
>> I'm still not sure how I achieve by use case of the parent class
>> defining the following properties:
>>static Property vud_properties[] = {
>>DEFINE_PROP_CHR("chardev", VHostUserDevice, chardev),
>>DEFINE_PROP_UINT16("id", VHostUserDevice, id, 0),
>>DEFINE_PROP_UINT32("num_vqs", VHostUserDevice, num_vqs, 1),
>>DEFINE_PROP_END_OF_LIST(),
>>};
>> But for the specialisation of the class I want the id to default to
>> the actual device id, e.g.:
>>static Property vu_rng_properties[] = {
>>DEFINE_PROP_UINT16("id", VHostUserDevice, id, VIRTIO_ID_RNG),
>>DEFINE_PROP_UINT32("num_vqs", VHostUserDevice, num_vqs, 1),
>>DEFINE_PROP_END_OF_LIST(),
>>};
>> And so far the API for doing that isn't super clear.
>> Signed-off-by: Alex Bennée
>> ---
>> include/hw/qdev-core.h | 9 +
>> 1 file changed, 9 insertions(+)
>> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
>> index bd50ad5ee1..d4bbc30c92 100644
>> --- a/include/hw/qdev-core.h
>> +++ b/include/hw/qdev-core.h
>> @@ -776,6 +776,15 @@ BusState *sysbus_get_default(void);
>> char *qdev_get_fw_dev_path(DeviceState *dev);
>> char *qdev_get_own_fw_dev_path_from_handler(BusState *bus, DeviceState
>> *dev);
>> +/**
>> + * device_class_set_props(): add a set of properties to an device
>> + * @dc: the parent DeviceClass all devices inherit
>> + * @props: an array of properties, terminate by DEFINE_PROP_END_OF_LIST()
>> + *
>> + * This will add a set of properties to the object. It will fault if
>> + * you attempt to add an existing property defined by a parent class.
>> + * To modify an inherited property you need to use
>> + */
>> void device_class_set_props(DeviceClass *dc, Property *props);
>> /**
>
> Hmmm that's an interesting one. Looking at the source in
> hw/core/qdev-properties.c you could possibly get away with something
> like this in vu_rng_class_init():
>
> ObjectProperty *op = object_class_property_find(klass, "id");
> object_property_set_default_uint(op, VIRTIO_ID_RNG);
>
> Of course this is all completely untested :)
Sadly we assert on the existing prop->defval:
static void object_property_set_default(ObjectProperty *prop, QObject *defval)
{
assert(!prop->defval);
assert(!prop->init);
prop->defval = defval;
prop->init = object_property_init_defval;
}
Maybe the assert is too aggressive or we need a different helper, maybe
a:
void object_property_update_default_uint(ObjectProperty *prop, uint64_t value)
?
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [PATCH v2 2/5] apic: add support for x2APIC mode
On Mon, 2023-03-27 at 22:45 +0700, Bui Quang Minh wrote:
>
> > Maybe I'm misreading the patch, but to me it looks that
> > if (dest == 0xff) apic_get_broadcast_bitmask() bit applies even in
> > x2apic mode? So delivering to the APIC with physical ID 255 will be
> > misinterpreted as a broadcast?
>
> In case dest == 0xff the second argument to apic_get_broadcast_bitmask
> is set to false which means this is xAPIC broadcast
Yeah, but it *isn't* xAPIC broadcast. It's X2APIC unicast to APIC#255.
I think you want (although you don't have 'dev') something like this:
static void apic_get_delivery_bitmask(uint32_t *deliver_bitmask,
uint32_t dest, uint8_t dest_mode)
{
APICCommonState *apic_iter;
int i;
memset(deliver_bitmask, 0x00, max_apic_words * sizeof(uint32_t));
/* x2APIC broadcast id for both physical and logical (cluster) mode */
if (dest == 0x) {
apic_get_broadcast_bitmask(deliver_bitmask, true);
return;
}
if (dest_mode == 0) {
apic_find_dest(deliver_bitmask, dest);
/* Broadcast to xAPIC mode apics */
-if (dest == 0xff) {
+if (dest == 0xff && is_x2apic_mode(dev)) {
apic_get_broadcast_bitmask(deliver_bitmask, false);
}
} else {
smime.p7s
Description: S/MIME cryptographic signature
Re: [PATCH v6 04/25] target/riscv: Remove mstatus_hs_{fs, vs} from tb_flags
On 3/26/23 18:34, liweiwei wrote: We seems only need to know whether fs/vs is dirty, so maybe we can just use a bool for them to save more bits from TB_FLAGS. No, we also need disabled. That is checked in REQUIRE_FPU, require_rvv, etc. r~
Re: [PATCH v8 3/3] tpm: Add support for TPM device over I2C bus
On 3/27/23 12:16, Ninad Palsule wrote:
Qemu already supports devices attached to ISA and sysbus. This drop adds
support for the I2C bus attached TPM devices. I2C model only supports
TPM2 protocol.
+ * If data is for FIFO then it is received from tpm_tis_common buffer
+ * otherwise it will be handled using single call to common code and
+ * cached in the local buffer.
+ */
+static uint8_t tpm_tis_i2c_recv(I2CSlave *i2c)
+{
+int ret = 0;
+uint32_t data_read;
+TPMStateI2C *i2cst = TPM_TIS_I2C(i2c);
+TPMState*s = >state;
+uint16_t i2c_reg = i2cst->data[0];
+
+if (i2cst->operation == OP_RECV) {
+
+/* Do not cache FIFO data. */
+if (i2cst->data[0] == TPM_I2C_REG_DATA_FIFO) {
+data_read = tpm_tis_read_data(s, i2cst->tis_addr, 1);
+ret = (data_read & 0xff);
+} else if (i2cst->offset < sizeof(i2cst->data)) {
+ret = i2cst->data[i2cst->offset++];
+}
+
+} else if ((i2cst->operation == OP_SEND) && (i2cst->offset < 2)) {
+/* First receive call after send */
+
+i2cst->operation = OP_RECV;
+
+switch (i2c_reg) {
+case TPM_I2C_REG_LOC_SEL:
+/* Location selection register is managed by i2c */
+i2cst->data[1] = i2cst->loc_sel;
+break;
+case TPM_I2C_REG_DATA_FIFO:
+/* FIFO data is directly read from TPM TIS */
+data_read = tpm_tis_read_data(s, i2cst->tis_addr, 1);
+i2cst->data[1] = (data_read & 0xff);
+break;
+case TPM_I2C_REG_DATA_CSUM_ENABLE:
+i2cst->data[1] = i2cst->csum_enable;
+break;
+case TPM_I2C_REG_INT_CAPABILITY:
+/* Interrupt itpm_tis_read_data(s, i2cst->tis_addr, 1);s not
supported as there is not way to test it. */
We can test that this register returns the right values. What we cannot test is
running this model with interrupts.
+i2cst->data[1] = TPM_I2C_INT_ENABLE_MASK;
+i2cst->data[2] = TPM_I2C_INT_ENABLE_MASK;
+i2cst->data[3] = TPM_I2C_INT_ENABLE_MASK;
+i2cst->data[4] = TPM_I2C_INT_ENABLE_MASK;
If you map the register in the table above you could do:
data_read = tpm_tis_read_data(s, i2cst->tis_addr, 1);
tpm_tis_i2c_set_data(data_read & TPM_I2C_INT_ENABLE_MASK);
Now that it's used in 3 locations the followig funtion would make sense:
static void tpm_tis_i2c_set_data(uint32_t data) {
i2cst->data[1] = data;
i2cst->data[2] = data >> 8;
i2cst->data[3] = data >> 16;
i2cst->data[4] = data >> 24;
}
Otherwise if you don't want to map it just call
tpm_tis_i2c_set_data(0);
+break;
+case TPM_I2C_REG_DATA_CSUM_GET:
+/*
+ * Checksum registers are not supported by common code hence
+ * call a common code to get the checksum.
+ */
+data_read = tpm_tis_get_checksum(s);
+
+/* Save the byte stream in data field */
+i2cst->data[1] = (data_read & 0xff);
+i2cst->data[2] = ((data_read >> 8) & 0xff);
tpm_tis_i2c_set_data(data_read);
+break;
+default:
+data_read = tpm_tis_read_data(s, i2cst->tis_addr, 4);
+
+switch (i2c_reg) {
+case TPM_I2C_REG_INTF_CAPABILITY:
+/* Prepare the capabilities as per I2C interface */
+data_read = tpm_i2c_interface_capability(i2cst,
+ data_read);
+break;
+case TPM_I2C_REG_STS:
+/*
+ * As per specs, STS bit 31:26 are reserved and must
+ * be set to 0
+ */
+data_read &= TPM_I2C_STS_READ_MASK;
+break;
+}
+
+/* Save byte stream in data[] */
+i2cst->data[1] = data_read;
+i2cst->data[2] = data_read >> 8;
+i2cst->data[3] = data_read >> 16;
+i2cst->data[4] = data_read >> 24;
tpm_tis_i2c_set_data(data_read);
+break;
+}
+
The rest looks good.
Re: [PATCH v8 3/3] tpm: Add support for TPM device over I2C bus
On 3/27/23 18:16, Ninad Palsule wrote:
Qemu already supports devices attached to ISA and sysbus. This drop adds
support for the I2C bus attached TPM devices. I2C model only supports
TPM2 protocol.
This commit includes changes for the common code.
- Added I2C emulation model. Logic was added in the model to temporarily
cache the data as I2C interface works per byte basis.
- New tpm type "tpm-tis-i2c" added for I2C support. The user has to
provide this string on command line.
Testing:
TPM I2C device module is tested using SWTPM (software based TPM
package). Qemu uses the rainier machine and is connected to swtpm over
the socket interface.
The command to start swtpm is as follows:
$ swtpm socket --tpmstate dir=/tmp/mytpm1\
--ctrl type=unixio,path=/tmp/mytpm1/swtpm-sock \
--tpm2 --log level=100
The command to start qemu is as follows:
$ qemu-system-arm -M rainier-bmc -nographic \
-kernel ${IMAGEPATH}/fitImage-linux.bin \
-dtb ${IMAGEPATH}/aspeed-bmc-ibm-rainier.dtb \
-initrd ${IMAGEPATH}/obmc-phosphor-initramfs.rootfs.cpio.xz \
-drive
file=${IMAGEPATH}/obmc-phosphor-image.rootfs.wic.qcow2,if=sd,index=2 \
-net nic -net
user,hostfwd=:127.0.0.1:-:22,hostfwd=:127.0.0.1:2443-:443 \
-chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
-tpmdev emulator,id=tpm0,chardev=chrtpm \
-device tpm-tis-i2c,tpmdev=tpm0,bus=aspeed.i2c.bus.12,address=0x2e
Signed-off-by: Ninad Palsule
---
V2:
Incorporated Stephen's review comments.
- Handled checksum related register in I2C layer
- Defined I2C interface capabilities and return those instead of
capabilities from TPM TIS. Add required capabilities from TIS.
- Do not cache FIFO data in the I2C layer.
- Make sure that Device address change register is not passed to I2C
layer as capability indicate that it is not supported.
- Added boundary checks.
- Make sure that bits 26-31 are zeroed for the TPM_STS register on read
- Updated Kconfig files for new define.
---
V3:
- Moved processing of register TPM_I2C_LOC_SEL in the I2C. So I2C layer
remembers the locality and pass it to TIS on each read/write.
- The write data is no more cached in the I2C layer so the buffer size
is reduced to 16 bytes.
- Checksum registers are now managed by the I2C layer. Added new
function in TIS layer to return the checksum and used that to process
the request.
- Now 2-4 byte register value will be passed to TIS layer in a single
write call instead of 1 byte at a time. Added functions to convert
between little endian stream of bytes to single 32 bit unsigned
integer. Similarly 32 bit integer to stream of bytes.
- Added restriction on device change register.
- Replace few if-else statement with switch statement for clarity.
- Log warning when unknown register is received.
- Moved all register definations to acpi/tmp.h
---
V4:
Incorporated review comments from Cedric and Stefan.
- Reduced data[] size from 16 byte to 5 bytes.
- Added register name in the mapping table which can be used for
tracing.
- Removed the endian conversion functions instead used simple logic
provided by Stefan.
- Rename I2C registers to reduce the length.
- Added traces for send, recv and event functions. You can turn on trace
on command line by using "-trace "tpm_tis_i2c*" option.
---
V5:
Fixed issues reported by Stefan's test.
- Added mask for the INT_ENABLE register.
- Use correct TIS register for reading interrupt capability.
- Cleanup how register is converted from I2C to TIS and also saved
information like tis_addr and register name in the i2cst so that we
can only convert it once on i2c_send.
- Trace register number for unknown registers.
---
V6:
Fixed review comments from Stefan.
- Fixed some variable size.
- Removed unused variables.
- Added vmstat backin to handle migration.
- Added post load phase to reload tis address and register name.
---
V7:
Incorporated review comments from Stefan.
- Added tpm_tis_i2c_initfn function
- Set the device catagory DEVICE_CATEGORY_MISC.
- Corrected default locality selection.
- Other cleanup. Include file cleanup.
---
V8:
Incorporated review comments from Stefan.
- Removed the irq initialization as linux doesn't support interrupts for
TPM
- Handle INT_CAPABILITY register in I2C only and return 0 to indicate
that it is not supported.
---
hw/arm/Kconfig | 1 +
hw/tpm/Kconfig | 7 +
hw/tpm/meson.build | 1 +
hw/tpm/tpm_tis_i2c.c | 525 +++
hw/tpm/trace-events | 6 +
include/sysemu/tpm.h | 3 +
6 files changed, 543 insertions(+)
create mode 100644 hw/tpm/tpm_tis_i2c.c
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index b5aed4aff5..05d6ef1a31 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -6,6 +6,7 @@ config ARM_VIRT
imply VFIO_PLATFORM
imply VFIO_XGMAC
imply
Re: [PATCH v2 2/7] docs/specs: IPMI device emulation: main processor
Thank you. I'll include the change in the next patch set when I refactor patch v4 (which might take a while.) On Sat, Mar 25, 2023 at 4:56 PM Corey Minyard wrote: > On Fri, Mar 24, 2023 at 04:08:59PM -0700, Hao Wu wrote: > > From: Havard Skinnemoen > > > > This document is an attempt to briefly document the existing IPMI > > emulation support on the main processor. It provides the necessary > > background for the BMC-side IPMI emulation proposed by the next patch. > > > > Signed-off-by: Havard Skinnemoen > > Signed-off-by: Hao Wu > > --- > > +* An external BMC simulator or emulator, connected over a chardev > > + (``ipmi-bmc-extern``). `ipmi_sim > > + < > https://sourceforge.net/p/openipmi/code/ci//master/tree/lanserv/README.ipmi_sim > >`_ > > Nit, you have a double slash above. > > Other than that, this does a good job of explaining things. I'm good > with these docs. > > -corey >
[PATCH V3] meson: install keyboard maps only if necessary
From: Carlos Santos
They are required only for system emulation (i.e. have_system is true).
Signed-off-by: Carlos Santos
---
Changes v1->v2:
Remove stray --{enable,disable}-install-keymaps addition to
scripts/meson-buildoptions.sh
Changes v2->v3:
Reset submodules (synchronize to origin/master)
---
pc-bios/keymaps/meson.build | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/pc-bios/keymaps/meson.build b/pc-bios/keymaps/meson.build
index 158a3b410c..bff3083313 100644
--- a/pc-bios/keymaps/meson.build
+++ b/pc-bios/keymaps/meson.build
@@ -47,7 +47,7 @@ if native_qemu_keymap.found()
build_by_default: true,
output: km,
command: [native_qemu_keymap, '-f', '@OUTPUT@',
args.split()],
- install: true,
+ install: have_system,
install_dir: qemu_datadir / 'keymaps')
endforeach
@@ -56,4 +56,6 @@ else
install_data(keymaps.keys(), install_dir: qemu_datadir / 'keymaps')
endif
-install_data(['sl', 'sv'], install_dir: qemu_datadir / 'keymaps')
+if have_system
+ install_data(['sl', 'sv'], install_dir: qemu_datadir / 'keymaps')
+endif
--
2.31.1
Re: [PATCH V2] meson: install keyboard maps only if necessary
On Mon, Mar 27, 2023 at 6:25 AM Daniel P. Berrangé wrote: > > On Sun, Mar 26, 2023 at 06:17:00PM -0300, casan...@redhat.com wrote: > > From: Carlos Santos > > > > They are required only for system emulation (i.e. have_system is true). > > > > Signed-off-by: Carlos Santos > > --- > > pc-bios/keymaps/meson.build | 6 -- > > tests/fp/berkeley-testfloat-3 | 2 +- > > ui/keycodemapdb | 2 +- > > This still has the accidental git submodule updates included > > > 3 files changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/pc-bios/keymaps/meson.build b/pc-bios/keymaps/meson.build > > index 158a3b410c..bff3083313 100644 > > --- a/pc-bios/keymaps/meson.build > > +++ b/pc-bios/keymaps/meson.build > > @@ -47,7 +47,7 @@ if native_qemu_keymap.found() > > build_by_default: true, > > output: km, > > command: [native_qemu_keymap, '-f', '@OUTPUT@', > > args.split()], > > - install: true, > > + install: have_system, > > install_dir: qemu_datadir / 'keymaps') > >endforeach > > > > @@ -56,4 +56,6 @@ else > >install_data(keymaps.keys(), install_dir: qemu_datadir / 'keymaps') > > endif > > > > -install_data(['sl', 'sv'], install_dir: qemu_datadir / 'keymaps') > > +if have_system > > + install_data(['sl', 'sv'], install_dir: qemu_datadir / 'keymaps') > > +endif > > diff --git a/tests/fp/berkeley-testfloat-3 b/tests/fp/berkeley-testfloat-3 > > index 40619cbb3b..5a59dcec19 16 > > --- a/tests/fp/berkeley-testfloat-3 > > +++ b/tests/fp/berkeley-testfloat-3 > > @@ -1 +1 @@ > > -Subproject commit 40619cbb3bf32872df8c53cc457039229428a263 > > +Subproject commit 5a59dcec19327396a011a17fd924aed4fec416b3 > > diff --git a/ui/keycodemapdb b/ui/keycodemapdb > > index f5772a62ec..d21009b1c9 16 > > --- a/ui/keycodemapdb > > +++ b/ui/keycodemapdb > > @@ -1 +1 @@ > > -Subproject commit f5772a62ec52591ff6870b7e8ef32482371f22c6 > > +Subproject commit d21009b1c9f94b740ea66be8e48a1d8ad8124023 > > -- > > 2.31.1 I submitted an updated patch. Thanks. -- Carlos Santos Senior Software Maintenance Engineer Red Hat casan...@redhat.comT: +55-11-3534-6186
[PATCH v9 3/3] tpm: Add support for TPM device over I2C bus
Qemu already supports devices attached to ISA and sysbus. This drop adds
support for the I2C bus attached TPM devices. I2C model only supports
TPM2 protocol.
This commit includes changes for the common code.
- Added I2C emulation model. Logic was added in the model to temporarily
cache the data as I2C interface works per byte basis.
- New tpm type "tpm-tis-i2c" added for I2C support. The user has to
provide this string on command line.
Testing:
TPM I2C device module is tested using SWTPM (software based TPM
package). Qemu uses the rainier machine and is connected to swtpm over
the socket interface.
The command to start swtpm is as follows:
$ swtpm socket --tpmstate dir=/tmp/mytpm1\
--ctrl type=unixio,path=/tmp/mytpm1/swtpm-sock \
--tpm2 --log level=100
The command to start qemu is as follows:
$ qemu-system-arm -M rainier-bmc -nographic \
-kernel ${IMAGEPATH}/fitImage-linux.bin \
-dtb ${IMAGEPATH}/aspeed-bmc-ibm-rainier.dtb \
-initrd ${IMAGEPATH}/obmc-phosphor-initramfs.rootfs.cpio.xz \
-drive
file=${IMAGEPATH}/obmc-phosphor-image.rootfs.wic.qcow2,if=sd,index=2 \
-net nic -net
user,hostfwd=:127.0.0.1:-:22,hostfwd=:127.0.0.1:2443-:443 \
-chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
-tpmdev emulator,id=tpm0,chardev=chrtpm \
-device tpm-tis-i2c,tpmdev=tpm0,bus=aspeed.i2c.bus.12,address=0x2e
Signed-off-by: Ninad Palsule
---
V2:
Incorporated Stephen's review comments.
- Handled checksum related register in I2C layer
- Defined I2C interface capabilities and return those instead of
capabilities from TPM TIS. Add required capabilities from TIS.
- Do not cache FIFO data in the I2C layer.
- Make sure that Device address change register is not passed to I2C
layer as capability indicate that it is not supported.
- Added boundary checks.
- Make sure that bits 26-31 are zeroed for the TPM_STS register on read
- Updated Kconfig files for new define.
---
V3:
- Moved processing of register TPM_I2C_LOC_SEL in the I2C. So I2C layer
remembers the locality and pass it to TIS on each read/write.
- The write data is no more cached in the I2C layer so the buffer size
is reduced to 16 bytes.
- Checksum registers are now managed by the I2C layer. Added new
function in TIS layer to return the checksum and used that to process
the request.
- Now 2-4 byte register value will be passed to TIS layer in a single
write call instead of 1 byte at a time. Added functions to convert
between little endian stream of bytes to single 32 bit unsigned
integer. Similarly 32 bit integer to stream of bytes.
- Added restriction on device change register.
- Replace few if-else statement with switch statement for clarity.
- Log warning when unknown register is received.
- Moved all register definations to acpi/tmp.h
---
V4:
Incorporated review comments from Cedric and Stefan.
- Reduced data[] size from 16 byte to 5 bytes.
- Added register name in the mapping table which can be used for
tracing.
- Removed the endian conversion functions instead used simple logic
provided by Stefan.
- Rename I2C registers to reduce the length.
- Added traces for send, recv and event functions. You can turn on trace
on command line by using "-trace "tpm_tis_i2c*" option.
---
V5:
Fixed issues reported by Stefan's test.
- Added mask for the INT_ENABLE register.
- Use correct TIS register for reading interrupt capability.
- Cleanup how register is converted from I2C to TIS and also saved
information like tis_addr and register name in the i2cst so that we
can only convert it once on i2c_send.
- Trace register number for unknown registers.
---
V6:
Fixed review comments from Stefan.
- Fixed some variable size.
- Removed unused variables.
- Added vmstat backin to handle migration.
- Added post load phase to reload tis address and register name.
---
V7:
Incorporated review comments from Stefan.
- Added tpm_tis_i2c_initfn function
- Set the device catagory DEVICE_CATEGORY_MISC.
- Corrected default locality selection.
- Other cleanup. Include file cleanup.
---
V8:
Incorporated review comments from Stefan.
- Removed the irq initialization as linux doesn't support interrupts for
TPM
- Handle INT_CAPABILITY register in I2C only and return 0 to indicate
that it is not supported.
---
V9:
- Added copyright
- Added set data function and called it few places.
- Rename function tpm_i2c_interface_capability
---
hw/arm/Kconfig | 1 +
hw/tpm/Kconfig | 7 +
hw/tpm/meson.build | 1 +
hw/tpm/tpm_tis_i2c.c | 527 +++
hw/tpm/trace-events | 6 +
include/sysemu/tpm.h | 3 +
6 files changed, 545 insertions(+)
create mode 100644 hw/tpm/tpm_tis_i2c.c
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index b5aed4aff5..05d6ef1a31 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -6,6 +6,7 @@ config ARM_VIRT
imply VFIO_PLATFORM
[PATCH v9 1/3] docs: Add support for TPM devices over I2C bus
This is a documentation change for I2C TPM device support. Qemu already supports devices attached to ISA and sysbus. This drop adds support for the I2C bus attached TPM devices. Signed-off-by: Ninad Palsule --- V2: Incorporated Stephen's review comments - Added example in the document. --- V4: Incorporate Cedric & Stefan's comments - Added example for ast2600-evb - Corrected statement about arm virtual machine. --- V6: Incorporated review comments from Stefan. --- V8: Incorporate review comments from Joel and Stefan - Removed the rainier example - Added step required to configure on ast2600-evb --- docs/specs/tpm.rst | 21 + 1 file changed, 21 insertions(+) diff --git a/docs/specs/tpm.rst b/docs/specs/tpm.rst index 535912a92b..efe124a148 100644 --- a/docs/specs/tpm.rst +++ b/docs/specs/tpm.rst @@ -21,12 +21,16 @@ QEMU files related to TPM TIS interface: - ``hw/tpm/tpm_tis_common.c`` - ``hw/tpm/tpm_tis_isa.c`` - ``hw/tpm/tpm_tis_sysbus.c`` + - ``hw/tpm/tpm_tis_i2c.c`` - ``hw/tpm/tpm_tis.h`` Both an ISA device and a sysbus device are available. The former is used with pc/q35 machine while the latter can be instantiated in the Arm virt machine. +An I2C device support is also provided which can be instantiated in the Arm +based emulation machines. This device only supports the TPM 2 protocol. + CRB interface - @@ -348,6 +352,23 @@ In case an Arm virt machine is emulated, use the following command line: -drive if=pflash,format=raw,file=flash0.img,readonly=on \ -drive if=pflash,format=raw,file=flash1.img +In case a ast2600-evb bmc machine is emulated and you want to use a TPM device +attached to I2C bus, use the following command line: + +.. code-block:: console + + qemu-system-arm -M ast2600-evb -nographic \ +-kernel arch/arm/boot/zImage \ +-dtb arch/arm/boot/dts/aspeed-ast2600-evb.dtb \ +-initrd rootfs.cpio \ +-chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \ +-tpmdev emulator,id=tpm0,chardev=chrtpm \ +-device tpm-tis-i2c,tpmdev=tpm0,bus=aspeed.i2c.bus.12,address=0x2e + + For testing, use this command to load the driver to the correct address + + echo tpm_tis_i2c 0x2e > /sys/bus/i2c/devices/i2c-12/new_device + In case SeaBIOS is used as firmware, it should show the TPM menu item after entering the menu with 'ESC'. -- 2.37.2
[PATCH v9 0/3] Add support for TPM devices over I2C bus
Hello, I have incorporated review comments from Cedric & Stefan. Please review. This drop adds support for the TPM devices attached to the I2C bus. It only supports the TPM2 protocol. You need to run it with the external TPM emulator like swtpm. I have tested it with swtpm. I have refered to the work done by zhdan...@meta.com but at the core level out implementation is different. https://github.com/theopolis/qemu/commit/2e2e57cde9e419c36af8071bb85392ad1ed70966 Based-on: $MESSAGE_ID Ninad Palsule (3): docs: Add support for TPM devices over I2C bus tpm: Extend common APIs to support TPM TIS I2C tpm: Add support for TPM device over I2C bus docs/specs/tpm.rst | 21 ++ hw/arm/Kconfig | 1 + hw/tpm/Kconfig | 7 + hw/tpm/meson.build | 1 + hw/tpm/tpm_tis.h| 3 + hw/tpm/tpm_tis_common.c | 36 ++- hw/tpm/tpm_tis_i2c.c| 527 hw/tpm/trace-events | 6 + include/hw/acpi/tpm.h | 37 +++ include/sysemu/tpm.h| 3 + 10 files changed, 634 insertions(+), 8 deletions(-) create mode 100644 hw/tpm/tpm_tis_i2c.c -- 2.37.2
[PATCH v10 1/3] docs: Add support for TPM devices over I2C bus
This is a documentation change for I2C TPM device support. Qemu already supports devices attached to ISA and sysbus. This drop adds support for the I2C bus attached TPM devices. Signed-off-by: Ninad Palsule --- V2: Incorporated Stephen's review comments - Added example in the document. --- V4: Incorporate Cedric & Stefan's comments - Added example for ast2600-evb - Corrected statement about arm virtual machine. --- V6: Incorporated review comments from Stefan. --- V8: Incorporate review comments from Joel and Stefan - Removed the rainier example - Added step required to configure on ast2600-evb --- docs/specs/tpm.rst | 21 + 1 file changed, 21 insertions(+) diff --git a/docs/specs/tpm.rst b/docs/specs/tpm.rst index 535912a92b..efe124a148 100644 --- a/docs/specs/tpm.rst +++ b/docs/specs/tpm.rst @@ -21,12 +21,16 @@ QEMU files related to TPM TIS interface: - ``hw/tpm/tpm_tis_common.c`` - ``hw/tpm/tpm_tis_isa.c`` - ``hw/tpm/tpm_tis_sysbus.c`` + - ``hw/tpm/tpm_tis_i2c.c`` - ``hw/tpm/tpm_tis.h`` Both an ISA device and a sysbus device are available. The former is used with pc/q35 machine while the latter can be instantiated in the Arm virt machine. +An I2C device support is also provided which can be instantiated in the Arm +based emulation machines. This device only supports the TPM 2 protocol. + CRB interface - @@ -348,6 +352,23 @@ In case an Arm virt machine is emulated, use the following command line: -drive if=pflash,format=raw,file=flash0.img,readonly=on \ -drive if=pflash,format=raw,file=flash1.img +In case a ast2600-evb bmc machine is emulated and you want to use a TPM device +attached to I2C bus, use the following command line: + +.. code-block:: console + + qemu-system-arm -M ast2600-evb -nographic \ +-kernel arch/arm/boot/zImage \ +-dtb arch/arm/boot/dts/aspeed-ast2600-evb.dtb \ +-initrd rootfs.cpio \ +-chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \ +-tpmdev emulator,id=tpm0,chardev=chrtpm \ +-device tpm-tis-i2c,tpmdev=tpm0,bus=aspeed.i2c.bus.12,address=0x2e + + For testing, use this command to load the driver to the correct address + + echo tpm_tis_i2c 0x2e > /sys/bus/i2c/devices/i2c-12/new_device + In case SeaBIOS is used as firmware, it should show the TPM menu item after entering the menu with 'ESC'. -- 2.37.2
Re: [PATCH v9 3/3] tpm: Add support for TPM device over I2C bus
Hi Stefan, On 3/27/23 1:26 PM, Stefan Berger wrote: On 3/27/23 14:12, Ninad Palsule wrote: diff --git a/hw/tpm/tpm_tis_i2c.c b/hw/tpm/tpm_tis_i2c.c new file mode 100644 index 00..551b89dac8 --- /dev/null +++ b/hw/tpm/tpm_tis_i2c.c @@ -0,0 +1,527 @@ +/* + * Aspeed i2c bus interface for reading from and writing to i2c device registers This was my suggestion for the format but this is not the correct line. With this line reverted to tpm_tis_i2c.c - QEMU's TPM TIS I2C Device Sorry about that. Fixed. Thanks for the review. Ninad Reviewed-by: Stefan Berger Tested-by: Stefan Berger
[PATCH v10 2/3] tpm: Extend common APIs to support TPM TIS I2C
Qemu already supports devices attached to ISA and sysbus. This drop adds
support for the I2C bus attached TPM devices.
This commit includes changes for the common code.
- Added support for the new checksum registers which are required for
the I2C support. The checksum calculation is handled in the qemu
common code.
- Added wrapper function for read and write data so that I2C code can
call it without MMIO interface.
The TPM TIS I2C spec describes in the table in section "Interface Locality
Usage per Register" that the TPM_INT_ENABLE and TPM_INT_STATUS registers
must be writable for any locality even if the locality is not the active
locality. Therefore, remove the checks whether the writing locality is the
active locality for these registers.
Signed-off-by: Ninad Palsule
Signed-off-by: Stefan Berger
---
V2:
Incorporated Stephen's comments.
- Removed checksum enable and checksum get registers.
- Added checksum calculation function which can be called from
i2c layer.
---
V3:
Incorporated review comments from Cedric and Stefan.
- Pass locality to the checksum calculation function and cleanup
- Moved I2C related definations in the acpi/tpm.h
---
V4:
Incorporated review comments by Stefan
- Remove the check for locality while calculating checksum
- Use bswap16 instead of cpu_ti_be16.
- Rename TPM_I2C register by dropping _TIS_ from it.
---
V7:
Incorporated review comments from Stefan.
- Removed locality check from INT_ENABLE and INT_STATUS registers write
path.
- Moved TPM_DATA_CSUM_ENABLED define in the tpm.h
---
V8:
Incorporated review comments from Stefan
- Moved the INT_ENABLE mask to tpm.h file.
---
hw/tpm/tpm_tis.h| 3 +++
hw/tpm/tpm_tis_common.c | 36
include/hw/acpi/tpm.h | 37 +
3 files changed, 68 insertions(+), 8 deletions(-)
diff --git a/hw/tpm/tpm_tis.h b/hw/tpm/tpm_tis.h
index f6b5872ba6..6f29a508dd 100644
--- a/hw/tpm/tpm_tis.h
+++ b/hw/tpm/tpm_tis.h
@@ -86,5 +86,8 @@ int tpm_tis_pre_save(TPMState *s);
void tpm_tis_reset(TPMState *s);
enum TPMVersion tpm_tis_get_tpm_version(TPMState *s);
void tpm_tis_request_completed(TPMState *s, int ret);
+uint32_t tpm_tis_read_data(TPMState *s, hwaddr addr, unsigned size);
+void tpm_tis_write_data(TPMState *s, hwaddr addr, uint64_t val, uint32_t size);
+uint16_t tpm_tis_get_checksum(TPMState *s);
#endif /* TPM_TPM_TIS_H */
diff --git a/hw/tpm/tpm_tis_common.c b/hw/tpm/tpm_tis_common.c
index 503be2a541..c07c179dbc 100644
--- a/hw/tpm/tpm_tis_common.c
+++ b/hw/tpm/tpm_tis_common.c
@@ -26,6 +26,8 @@
#include "hw/irq.h"
#include "hw/isa/isa.h"
#include "qapi/error.h"
+#include "qemu/bswap.h"
+#include "qemu/crc-ccitt.h"
#include "qemu/module.h"
#include "hw/acpi/tpm.h"
@@ -447,6 +449,23 @@ static uint64_t tpm_tis_mmio_read(void *opaque, hwaddr
addr,
return val;
}
+/*
+ * A wrapper read function so that it can be directly called without
+ * mmio.
+ */
+uint32_t tpm_tis_read_data(TPMState *s, hwaddr addr, unsigned size)
+{
+return tpm_tis_mmio_read(s, addr, size);
+}
+
+/*
+ * Calculate current data buffer checksum
+ */
+uint16_t tpm_tis_get_checksum(TPMState *s)
+{
+return bswap16(crc_ccitt(0, s->buffer, s->rw_offset));
+}
+
/*
* Write a value to a register of the TIS interface
* See specs pages 33-63 for description of the registers
@@ -588,10 +607,6 @@ static void tpm_tis_mmio_write(void *opaque, hwaddr addr,
break;
case TPM_TIS_REG_INT_ENABLE:
-if (s->active_locty != locty) {
-break;
-}
-
s->loc[locty].inte &= mask;
s->loc[locty].inte |= (val & (TPM_TIS_INT_ENABLED |
TPM_TIS_INT_POLARITY_MASK |
@@ -601,10 +616,6 @@ static void tpm_tis_mmio_write(void *opaque, hwaddr addr,
/* hard wired -- ignore */
break;
case TPM_TIS_REG_INT_STATUS:
-if (s->active_locty != locty) {
-break;
-}
-
/* clearing of interrupt flags */
if (((val & TPM_TIS_INTERRUPTS_SUPPORTED)) &&
(s->loc[locty].ints & TPM_TIS_INTERRUPTS_SUPPORTED)) {
@@ -767,6 +778,15 @@ static void tpm_tis_mmio_write(void *opaque, hwaddr addr,
}
}
+/*
+ * A wrapper write function so that it can be directly called without
+ * mmio.
+ */
+void tpm_tis_write_data(TPMState *s, hwaddr addr, uint64_t val, uint32_t size)
+{
+tpm_tis_mmio_write(s, addr, val, size);
+}
+
const MemoryRegionOps tpm_tis_memory_ops = {
.read = tpm_tis_mmio_read,
.write = tpm_tis_mmio_write,
diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
index 559ba6906c..fb81e1735b 100644
--- a/include/hw/acpi/tpm.h
+++ b/include/hw/acpi/tpm.h
@@ -93,6 +93,7 @@
#define TPM_TIS_CAP_DATA_TRANSFER_64B(3 << 9)
#define TPM_TIS_CAP_DATA_TRANSFER_LEGACY (0 << 9)
#define TPM_TIS_CAP_BURST_COUNT_DYNAMIC (0 << 8)
+#define TPM_TIS_CAP_BURST_COUNT_STATIC (1 << 8)
#define TPM_TIS_CAP_INTERRUPT_LOW_LEVEL
[PATCH v10 0/3] Add support for TPM devices over I2C bus
Update to the copyright text. This drop adds support for the TPM devices attached to the I2C bus. It only supports the TPM2 protocol. You need to run it with the external TPM emulator like swtpm. I have tested it with swtpm. I have refered to the work done by zhdan...@meta.com but at the core level out implementation is different. https://github.com/theopolis/qemu/commit/2e2e57cde9e419c36af8071bb85392ad1ed70966 Based-on: $MESSAGE_ID Ninad Palsule (3): docs: Add support for TPM devices over I2C bus tpm: Extend common APIs to support TPM TIS I2C tpm: Add support for TPM device over I2C bus docs/specs/tpm.rst | 21 ++ hw/arm/Kconfig | 1 + hw/tpm/Kconfig | 7 + hw/tpm/meson.build | 1 + hw/tpm/tpm_tis.h| 3 + hw/tpm/tpm_tis_common.c | 36 ++- hw/tpm/tpm_tis_i2c.c| 527 hw/tpm/trace-events | 6 + include/hw/acpi/tpm.h | 37 +++ include/sysemu/tpm.h| 3 + 10 files changed, 634 insertions(+), 8 deletions(-) create mode 100644 hw/tpm/tpm_tis_i2c.c -- 2.37.2
Re: [PATCH 03/19] target/riscv: introduce riscv_cpu_add_misa_properties()
On 3/27/23 05:42, Daniel Henrique Barboza wrote:
+g_autofree char *name = g_strdup_printf("%s", misa_cfg->name);
+g_autofree char *desc = g_strdup_printf("%s", misa_cfg->description);
What is the point of this? It would seem that you could simply pass the original string
literals to object_property_*.
r~
Re: [PATCH-for-8.0] block/dmg: Ignore C99 prototype declaration mismatch from
+Marc-André & Paolo
On 27/3/23 19:08, Stefan Weil wrote:
Am 27.03.23 um 17:13 schrieb Philippe Mathieu-Daudé:
When liblzfe (Apple LZFSE compression library) is present
(for example installed via 'brew') on Darwin, QEMU build
fails as:
Has header "lzfse.h" : YES
Library lzfse found: YES
Dependencies
lzo support : NO
snappy support : NO
bzip2 support : YES
lzfse support : YES
zstd support : YES 1.5.2
User defined options
dmg : enabled
lzfse : enabled
[221/903] Compiling C object libblock.fa.p/block_dmg-lzfse.c.o
FAILED: libblock.fa.p/block_dmg-lzfse.c.o
/opt/homebrew/Cellar/lzfse/1.0/include/lzfse.h:56:43: error: this
function declaration is not a prototype [-Werror,-Wstrict-prototypes]
LZFSE_API size_t lzfse_encode_scratch_size();
^
void
/opt/homebrew/Cellar/lzfse/1.0/include/lzfse.h:94:43: error: this
function declaration is not a prototype [-Werror,-Wstrict-prototypes]
LZFSE_API size_t lzfse_decode_scratch_size();
^
void
2 errors generated.
ninja: build stopped: subcommand failed.
This issue has been reported in the lzfse project in 2016:
https://github.com/lzfse/lzfse/issues/3#issuecomment-226574719
Since the project seems unmaintained, simply ignore the
strict-prototypes warning check for the header,
similarly to how we deal with the GtkItemFactoryCallback
prototype from , indirectly included
by .
Cc: Julio Faracco
Signed-off-by: Philippe Mathieu-Daudé
---
block/dmg-lzfse.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/block/dmg-lzfse.c b/block/dmg-lzfse.c
index 6798cf4fbf..0abc970bf6 100644
--- a/block/dmg-lzfse.c
+++ b/block/dmg-lzfse.c
@@ -23,7 +23,12 @@
*/
#include "qemu/osdep.h"
#include "dmg.h"
+
+/* Work around an -Wstrict-prototypes warning in LZFSE headers */
"Work around a -Wstrict-prototypes" ("a" instead of "an")?
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wstrict-prototypes"
#include
+#pragma GCC diagnostic pop
static int dmg_uncompress_lzfse_do(char *next_in, unsigned int
avail_in,
char *next_out, unsigned int
avail_out)
The warning can also be suppressed if the build uses `-isystem
/opt/homebrew/include` instead of `-I/opt/homebrew/include` as I just
have tested.
IIUC by design meson only allows including *relative* directories,
and manage the system ones:
https://mesonbuild.com/Include-directories.html
If we can find a solution how to implement that I thing it would look
nicer. Technically the patch looks good.
Reviewed-by: Stefan Weil
Thanks!
[PATCH v8 3/3] tpm: Add support for TPM device over I2C bus
Qemu already supports devices attached to ISA and sysbus. This drop adds
support for the I2C bus attached TPM devices. I2C model only supports
TPM2 protocol.
This commit includes changes for the common code.
- Added I2C emulation model. Logic was added in the model to temporarily
cache the data as I2C interface works per byte basis.
- New tpm type "tpm-tis-i2c" added for I2C support. The user has to
provide this string on command line.
Testing:
TPM I2C device module is tested using SWTPM (software based TPM
package). Qemu uses the rainier machine and is connected to swtpm over
the socket interface.
The command to start swtpm is as follows:
$ swtpm socket --tpmstate dir=/tmp/mytpm1\
--ctrl type=unixio,path=/tmp/mytpm1/swtpm-sock \
--tpm2 --log level=100
The command to start qemu is as follows:
$ qemu-system-arm -M rainier-bmc -nographic \
-kernel ${IMAGEPATH}/fitImage-linux.bin \
-dtb ${IMAGEPATH}/aspeed-bmc-ibm-rainier.dtb \
-initrd ${IMAGEPATH}/obmc-phosphor-initramfs.rootfs.cpio.xz \
-drive
file=${IMAGEPATH}/obmc-phosphor-image.rootfs.wic.qcow2,if=sd,index=2 \
-net nic -net
user,hostfwd=:127.0.0.1:-:22,hostfwd=:127.0.0.1:2443-:443 \
-chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
-tpmdev emulator,id=tpm0,chardev=chrtpm \
-device tpm-tis-i2c,tpmdev=tpm0,bus=aspeed.i2c.bus.12,address=0x2e
Signed-off-by: Ninad Palsule
---
V2:
Incorporated Stephen's review comments.
- Handled checksum related register in I2C layer
- Defined I2C interface capabilities and return those instead of
capabilities from TPM TIS. Add required capabilities from TIS.
- Do not cache FIFO data in the I2C layer.
- Make sure that Device address change register is not passed to I2C
layer as capability indicate that it is not supported.
- Added boundary checks.
- Make sure that bits 26-31 are zeroed for the TPM_STS register on read
- Updated Kconfig files for new define.
---
V3:
- Moved processing of register TPM_I2C_LOC_SEL in the I2C. So I2C layer
remembers the locality and pass it to TIS on each read/write.
- The write data is no more cached in the I2C layer so the buffer size
is reduced to 16 bytes.
- Checksum registers are now managed by the I2C layer. Added new
function in TIS layer to return the checksum and used that to process
the request.
- Now 2-4 byte register value will be passed to TIS layer in a single
write call instead of 1 byte at a time. Added functions to convert
between little endian stream of bytes to single 32 bit unsigned
integer. Similarly 32 bit integer to stream of bytes.
- Added restriction on device change register.
- Replace few if-else statement with switch statement for clarity.
- Log warning when unknown register is received.
- Moved all register definations to acpi/tmp.h
---
V4:
Incorporated review comments from Cedric and Stefan.
- Reduced data[] size from 16 byte to 5 bytes.
- Added register name in the mapping table which can be used for
tracing.
- Removed the endian conversion functions instead used simple logic
provided by Stefan.
- Rename I2C registers to reduce the length.
- Added traces for send, recv and event functions. You can turn on trace
on command line by using "-trace "tpm_tis_i2c*" option.
---
V5:
Fixed issues reported by Stefan's test.
- Added mask for the INT_ENABLE register.
- Use correct TIS register for reading interrupt capability.
- Cleanup how register is converted from I2C to TIS and also saved
information like tis_addr and register name in the i2cst so that we
can only convert it once on i2c_send.
- Trace register number for unknown registers.
---
V6:
Fixed review comments from Stefan.
- Fixed some variable size.
- Removed unused variables.
- Added vmstat backin to handle migration.
- Added post load phase to reload tis address and register name.
---
V7:
Incorporated review comments from Stefan.
- Added tpm_tis_i2c_initfn function
- Set the device catagory DEVICE_CATEGORY_MISC.
- Corrected default locality selection.
- Other cleanup. Include file cleanup.
---
V8:
Incorporated review comments from Stefan.
- Removed the irq initialization as linux doesn't support interrupts for
TPM
- Handle INT_CAPABILITY register in I2C only and return 0 to indicate
that it is not supported.
---
hw/arm/Kconfig | 1 +
hw/tpm/Kconfig | 7 +
hw/tpm/meson.build | 1 +
hw/tpm/tpm_tis_i2c.c | 525 +++
hw/tpm/trace-events | 6 +
include/sysemu/tpm.h | 3 +
6 files changed, 543 insertions(+)
create mode 100644 hw/tpm/tpm_tis_i2c.c
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index b5aed4aff5..05d6ef1a31 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -6,6 +6,7 @@ config ARM_VIRT
imply VFIO_PLATFORM
imply VFIO_XGMAC
imply TPM_TIS_SYSBUS
+imply TPM_TIS_I2C
imply NVDIMM
select ARM_GIC
select ACPI
[PATCH v8 1/3] docs: Add support for TPM devices over I2C bus
This is a documentation change for I2C TPM device support. Qemu already supports devices attached to ISA and sysbus. This drop adds support for the I2C bus attached TPM devices. Signed-off-by: Ninad Palsule --- V2: Incorporated Stephen's review comments - Added example in the document. --- V4: Incorporate Cedric & Stefan's comments - Added example for ast2600-evb - Corrected statement about arm virtual machine. --- V6: Incorporated review comments from Stefan. --- V8: Incorporate review comments from Joel and Stefan - Removed the rainier example - Added step required to configure on ast2600-evb --- docs/specs/tpm.rst | 21 + 1 file changed, 21 insertions(+) diff --git a/docs/specs/tpm.rst b/docs/specs/tpm.rst index 535912a92b..efe124a148 100644 --- a/docs/specs/tpm.rst +++ b/docs/specs/tpm.rst @@ -21,12 +21,16 @@ QEMU files related to TPM TIS interface: - ``hw/tpm/tpm_tis_common.c`` - ``hw/tpm/tpm_tis_isa.c`` - ``hw/tpm/tpm_tis_sysbus.c`` + - ``hw/tpm/tpm_tis_i2c.c`` - ``hw/tpm/tpm_tis.h`` Both an ISA device and a sysbus device are available. The former is used with pc/q35 machine while the latter can be instantiated in the Arm virt machine. +An I2C device support is also provided which can be instantiated in the Arm +based emulation machines. This device only supports the TPM 2 protocol. + CRB interface - @@ -348,6 +352,23 @@ In case an Arm virt machine is emulated, use the following command line: -drive if=pflash,format=raw,file=flash0.img,readonly=on \ -drive if=pflash,format=raw,file=flash1.img +In case a ast2600-evb bmc machine is emulated and you want to use a TPM device +attached to I2C bus, use the following command line: + +.. code-block:: console + + qemu-system-arm -M ast2600-evb -nographic \ +-kernel arch/arm/boot/zImage \ +-dtb arch/arm/boot/dts/aspeed-ast2600-evb.dtb \ +-initrd rootfs.cpio \ +-chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \ +-tpmdev emulator,id=tpm0,chardev=chrtpm \ +-device tpm-tis-i2c,tpmdev=tpm0,bus=aspeed.i2c.bus.12,address=0x2e + + For testing, use this command to load the driver to the correct address + + echo tpm_tis_i2c 0x2e > /sys/bus/i2c/devices/i2c-12/new_device + In case SeaBIOS is used as firmware, it should show the TPM menu item after entering the menu with 'ESC'. -- 2.37.2
[PATCH v8 2/3] tpm: Extend common APIs to support TPM TIS I2C
Qemu already supports devices attached to ISA and sysbus. This drop adds
support for the I2C bus attached TPM devices.
This commit includes changes for the common code.
- Added support for the new checksum registers which are required for
the I2C support. The checksum calculation is handled in the qemu
common code.
- Added wrapper function for read and write data so that I2C code can
call it without MMIO interface.
The TPM TIS I2C spec describes in the table in section "Interface Locality
Usage per Register" that the TPM_INT_ENABLE and TPM_INT_STATUS registers
must be writable for any locality even if the locality is not the active
locality. Therefore, remove the checks whether the writing locality is the
active locality for these registers.
Signed-off-by: Ninad Palsule
Signed-off-by: Stefan Berger
---
V2:
Incorporated Stephen's comments.
- Removed checksum enable and checksum get registers.
- Added checksum calculation function which can be called from
i2c layer.
---
V3:
Incorporated review comments from Cedric and Stefan.
- Pass locality to the checksum calculation function and cleanup
- Moved I2C related definations in the acpi/tpm.h
---
V4:
Incorporated review comments by Stefan
- Remove the check for locality while calculating checksum
- Use bswap16 instead of cpu_ti_be16.
- Rename TPM_I2C register by dropping _TIS_ from it.
---
V7:
Incorporated review comments from Stefan.
- Removed locality check from INT_ENABLE and INT_STATUS registers write
path.
- Moved TPM_DATA_CSUM_ENABLED define in the tpm.h
---
V8:
Incorporated review comments from Stefan
- Moved the INT_ENABLE mask to tpm.h file.
---
hw/tpm/tpm_tis.h| 3 +++
hw/tpm/tpm_tis_common.c | 36
include/hw/acpi/tpm.h | 37 +
3 files changed, 68 insertions(+), 8 deletions(-)
diff --git a/hw/tpm/tpm_tis.h b/hw/tpm/tpm_tis.h
index f6b5872ba6..6f29a508dd 100644
--- a/hw/tpm/tpm_tis.h
+++ b/hw/tpm/tpm_tis.h
@@ -86,5 +86,8 @@ int tpm_tis_pre_save(TPMState *s);
void tpm_tis_reset(TPMState *s);
enum TPMVersion tpm_tis_get_tpm_version(TPMState *s);
void tpm_tis_request_completed(TPMState *s, int ret);
+uint32_t tpm_tis_read_data(TPMState *s, hwaddr addr, unsigned size);
+void tpm_tis_write_data(TPMState *s, hwaddr addr, uint64_t val, uint32_t size);
+uint16_t tpm_tis_get_checksum(TPMState *s);
#endif /* TPM_TPM_TIS_H */
diff --git a/hw/tpm/tpm_tis_common.c b/hw/tpm/tpm_tis_common.c
index 503be2a541..c07c179dbc 100644
--- a/hw/tpm/tpm_tis_common.c
+++ b/hw/tpm/tpm_tis_common.c
@@ -26,6 +26,8 @@
#include "hw/irq.h"
#include "hw/isa/isa.h"
#include "qapi/error.h"
+#include "qemu/bswap.h"
+#include "qemu/crc-ccitt.h"
#include "qemu/module.h"
#include "hw/acpi/tpm.h"
@@ -447,6 +449,23 @@ static uint64_t tpm_tis_mmio_read(void *opaque, hwaddr
addr,
return val;
}
+/*
+ * A wrapper read function so that it can be directly called without
+ * mmio.
+ */
+uint32_t tpm_tis_read_data(TPMState *s, hwaddr addr, unsigned size)
+{
+return tpm_tis_mmio_read(s, addr, size);
+}
+
+/*
+ * Calculate current data buffer checksum
+ */
+uint16_t tpm_tis_get_checksum(TPMState *s)
+{
+return bswap16(crc_ccitt(0, s->buffer, s->rw_offset));
+}
+
/*
* Write a value to a register of the TIS interface
* See specs pages 33-63 for description of the registers
@@ -588,10 +607,6 @@ static void tpm_tis_mmio_write(void *opaque, hwaddr addr,
break;
case TPM_TIS_REG_INT_ENABLE:
-if (s->active_locty != locty) {
-break;
-}
-
s->loc[locty].inte &= mask;
s->loc[locty].inte |= (val & (TPM_TIS_INT_ENABLED |
TPM_TIS_INT_POLARITY_MASK |
@@ -601,10 +616,6 @@ static void tpm_tis_mmio_write(void *opaque, hwaddr addr,
/* hard wired -- ignore */
break;
case TPM_TIS_REG_INT_STATUS:
-if (s->active_locty != locty) {
-break;
-}
-
/* clearing of interrupt flags */
if (((val & TPM_TIS_INTERRUPTS_SUPPORTED)) &&
(s->loc[locty].ints & TPM_TIS_INTERRUPTS_SUPPORTED)) {
@@ -767,6 +778,15 @@ static void tpm_tis_mmio_write(void *opaque, hwaddr addr,
}
}
+/*
+ * A wrapper write function so that it can be directly called without
+ * mmio.
+ */
+void tpm_tis_write_data(TPMState *s, hwaddr addr, uint64_t val, uint32_t size)
+{
+tpm_tis_mmio_write(s, addr, val, size);
+}
+
const MemoryRegionOps tpm_tis_memory_ops = {
.read = tpm_tis_mmio_read,
.write = tpm_tis_mmio_write,
diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
index 559ba6906c..fb81e1735b 100644
--- a/include/hw/acpi/tpm.h
+++ b/include/hw/acpi/tpm.h
@@ -93,6 +93,7 @@
#define TPM_TIS_CAP_DATA_TRANSFER_64B(3 << 9)
#define TPM_TIS_CAP_DATA_TRANSFER_LEGACY (0 << 9)
#define TPM_TIS_CAP_BURST_COUNT_DYNAMIC (0 << 8)
+#define TPM_TIS_CAP_BURST_COUNT_STATIC (1 << 8)
#define TPM_TIS_CAP_INTERRUPT_LOW_LEVEL
[RFC PATCH v1 1/1] migration: Disable postcopy + multifd migration
Since the introduction of multifd, it's possible to perform a multifd
migration and finish it using postcopy.
A bug introduced by yank (fixed on cfc3bcf373) was previously preventing
a successful use of this migration scenario, and now it should be
working on most cases.
But since there is not enough testing/support nor any reported users for
this scenario, we should disable this combination before it may cause any
problems for users.
Suggested-by: Dr. David Alan Gilbert
Signed-off-by: Leonardo Bras
---
migration/migration.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/migration/migration.c b/migration/migration.c
index ae2025d9d8..c601964b0e 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1356,6 +1356,11 @@ static bool migrate_caps_check(bool *cap_list,
error_setg(errp, "Postcopy is not compatible with ignore-shared");
return false;
}
+
+if (cap_list[MIGRATION_CAPABILITY_MULTIFD]) {
+error_setg(errp, "Postcopy is not yet compatible with multifd");
+return false;
+}
}
if (cap_list[MIGRATION_CAPABILITY_BACKGROUND_SNAPSHOT]) {
--
2.40.0
Re: [PATCH v6 00/25] target/riscv: MSTATUS_SUM + cleanups
On 3/25/23 07:54, Richard Henderson wrote:
This builds on Fei and Zhiwei's SUM and TB_FLAGS changes.
* Reclaim 5 TB_FLAGS bits, since we nearly ran out.
* Using cpu_mmu_index(env, true) is insufficient to implement
HLVX properly. While that chooses the correct mmu_idx, it
does not perform the read with execute permission.
I add a new tcg interface to perform a read-for-execute with
an arbitrary mmu_idx. This is still not 100% compliant, but
it's closer.
* Handle mstatus.MPV in cpu_mmu_index.
* Use vsstatus.SUM when required for MMUIdx_S_SUM.
* Cleanups for get_physical_address.
While this passes check-avocado, I'm sure that's insufficient.
Please have a close look.
Tested fine in my end with some buildroot tests and 'stress-ng' in a 'virt'
machine with Ubuntu.
Tested-by: Daniel Henrique Barboza
r~
Fei Wu (2):
target/riscv: Separate priv from mmu_idx
target/riscv: Reduce overhead of MSTATUS_SUM change
LIU Zhiwei (4):
target/riscv: Extract virt enabled state from tb flags
target/riscv: Add a general status enum for extensions
target/riscv: Encode the FS and VS on a normal way for tb flags
target/riscv: Add a tb flags field for vstart
Richard Henderson (19):
target/riscv: Remove mstatus_hs_{fs,vs} from tb_flags
accel/tcg: Add cpu_ld*_code_mmu
target/riscv: Use cpu_ld*_code_mmu for HLVX
target/riscv: Handle HLV, HSV via helpers
target/riscv: Rename MMU_HYP_ACCESS_BIT to MMU_2STAGE_BIT
target/riscv: Introduce mmuidx_sum
target/riscv: Introduce mmuidx_priv
target/riscv: Introduce mmuidx_2stage
target/riscv: Move hstatus.spvp check to check_access_hlsv
target/riscv: Set MMU_2STAGE_BIT in riscv_cpu_mmu_index
target/riscv: Check SUM in the correct register
target/riscv: Hoist second stage mode change to callers
target/riscv: Hoist pbmte and hade out of the level loop
target/riscv: Move leaf pte processing out of level loop
target/riscv: Suppress pte update with is_debug
target/riscv: Don't modify SUM with is_debug
target/riscv: Merge checks for reserved pte flags
target/riscv: Reorg access check in get_physical_address
target/riscv: Reorg sum check in get_physical_address
include/exec/cpu_ldst.h | 9 +
target/riscv/cpu.h| 47 ++-
target/riscv/cpu_bits.h | 12 +-
target/riscv/helper.h | 12 +-
target/riscv/internals.h | 35 ++
accel/tcg/cputlb.c| 48 +++
accel/tcg/user-exec.c | 58 +++
target/riscv/cpu.c| 2 +-
target/riscv/cpu_helper.c | 393 +-
target/riscv/csr.c| 21 +-
target/riscv/op_helper.c | 113 -
target/riscv/translate.c | 72 ++--
.../riscv/insn_trans/trans_privileged.c.inc | 2 +-
target/riscv/insn_trans/trans_rvf.c.inc | 2 +-
target/riscv/insn_trans/trans_rvh.c.inc | 135 +++---
target/riscv/insn_trans/trans_rvv.c.inc | 22 +-
target/riscv/insn_trans/trans_xthead.c.inc| 7 +-
17 files changed, 595 insertions(+), 395 deletions(-)
Re: [PATCH-for-8.0] block/dmg: Ignore C99 prototype declaration mismatch from
Am 27.03.23 um 17:13 schrieb Philippe Mathieu-Daudé:
When liblzfe (Apple LZFSE compression library) is present
(for example installed via 'brew') on Darwin, QEMU build
fails as:
Has header "lzfse.h" : YES
Library lzfse found: YES
Dependencies
lzo support : NO
snappy support : NO
bzip2 support: YES
lzfse support: YES
zstd support : YES 1.5.2
User defined options
dmg : enabled
lzfse: enabled
[221/903] Compiling C object libblock.fa.p/block_dmg-lzfse.c.o
FAILED: libblock.fa.p/block_dmg-lzfse.c.o
/opt/homebrew/Cellar/lzfse/1.0/include/lzfse.h:56:43: error: this function
declaration is not a prototype [-Werror,-Wstrict-prototypes]
LZFSE_API size_t lzfse_encode_scratch_size();
^
void
/opt/homebrew/Cellar/lzfse/1.0/include/lzfse.h:94:43: error: this function
declaration is not a prototype [-Werror,-Wstrict-prototypes]
LZFSE_API size_t lzfse_decode_scratch_size();
^
void
2 errors generated.
ninja: build stopped: subcommand failed.
This issue has been reported in the lzfse project in 2016:
https://github.com/lzfse/lzfse/issues/3#issuecomment-226574719
Since the project seems unmaintained, simply ignore the
strict-prototypes warning check for the header,
similarly to how we deal with the GtkItemFactoryCallback
prototype from , indirectly included
by .
Cc: Julio Faracco
Signed-off-by: Philippe Mathieu-Daudé
---
block/dmg-lzfse.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/block/dmg-lzfse.c b/block/dmg-lzfse.c
index 6798cf4fbf..0abc970bf6 100644
--- a/block/dmg-lzfse.c
+++ b/block/dmg-lzfse.c
@@ -23,7 +23,12 @@
*/
#include "qemu/osdep.h"
#include "dmg.h"
+
+/* Work around an -Wstrict-prototypes warning in LZFSE headers */
"Work around a -Wstrict-prototypes" ("a" instead of "an")?
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wstrict-prototypes"
#include
+#pragma GCC diagnostic pop
static int dmg_uncompress_lzfse_do(char *next_in, unsigned int avail_in,
char *next_out, unsigned int avail_out)
The warning can also be suppressed if the build uses `-isystem
/opt/homebrew/include` instead of `-I/opt/homebrew/include` as I just
have tested.
If we can find a solution how to implement that I thing it would look
nicer. Technically the patch looks good.
Reviewed-by: Stefan Weil
Thanks,
Stefan
Re: [PATCH] tracing: install trace events file only if necessary
On Mon, Mar 27, 2023 at 11:44 AM Carlos Santos wrote:
>
> On Mon, Mar 27, 2023 at 11:42 AM Daniel P. Berrangé
> wrote:
> >
> > On Mon, Mar 27, 2023 at 11:28:05AM -0300, Carlos Santos wrote:
> > > On Mon, Mar 27, 2023 at 6:23 AM Daniel P. Berrangé
> > > wrote:
> > > >
> > > > On Sun, Mar 26, 2023 at 06:04:46PM -0300, casan...@redhat.com wrote:
> > > > > From: Carlos Santos
> > > > >
> > > > > It is required only if linux-user, bsd-user or system emulator is
> > > > > built.
> > > > >
> > > > > Signed-off-by: Carlos Santos
> > > > > ---
> > > > > trace/meson.build | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/trace/meson.build b/trace/meson.build
> > > > > index 8e80be895c..3fb41c97a4 100644
> > > > > --- a/trace/meson.build
> > > > > +++ b/trace/meson.build
> > > > > @@ -64,7 +64,7 @@ trace_events_all = custom_target('trace-events-all',
> > > > > input: trace_events_files,
> > > > > command: [ 'cat', '@INPUT@' ],
> > > > > capture: true,
> > > > > - install: true,
> > > > > + install: have_linux_user or
> > > > > have_bsd_user or have_system,
> > > >
> > > > Trace events are used by our command line tools too qemu-img, qemu-io,
> > > > qemu-nbd, qemu-pr-helper, qemu-storage-daemon.
> > > >
> > > > What build scenario are you seeing that does NOT want the trace events
> > > > to be present ? If there is any, then I might even call that situation
> > > > a bug, as we want trace events to be available as a debugging mechanism
> > > > for everything we build.
> > >
> > > I'm aiming for an embedded system or a VM image that only needs
> > > qemu-ga, in which qemu is built with --enable-trace-backends=nop.
> >
> > How about
> >
> > install: get_option('trace_backends') != 'nop'
> >
> > ?
>
> That would be perfect :-)
>
> --
> Carlos Santos
> Senior Software Maintenance Engineer
> Red Hat
> casan...@redhat.comT: +55-11-3534-6186
I submitted an updated patch. Thanks.
--
Carlos Santos
Senior Software Maintenance Engineer
Red Hat
casan...@redhat.comT: +55-11-3534-6186
Re: [PATCH-for-8.0] linux-user/sparc: Don't use 16-bit UIDs on SPARC V9
On 3/27/23 06:19, Philippe Mathieu-Daudé wrote:
The 64-bit SPARC V9 syscall ABI uses 32-bit UIDs. Only enable
the 16-bit UID wrappers for 32-bit SPARC (V7 and V8).
Possibly missed in commit 992f48a036 ("Support for 32 bit
ABI on 64 bit targets (only enabled Sparc64)").
Reported-by: Gregor Riepl
Tested-by: John Paul Adrian Glaubitz
Tested-by: Zach van Rijn
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1394
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
r~
---
linux-user/syscall_defs.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 614a1cbc8e..cc37054cb5 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -61,7 +61,7 @@
#if (defined(TARGET_I386) && defined(TARGET_ABI32)) \
|| (defined(TARGET_ARM) && defined(TARGET_ABI32)) \
-|| defined(TARGET_SPARC) \
+|| (defined(TARGET_SPARC) && defined(TARGET_ABI32)) \
|| defined(TARGET_M68K) || defined(TARGET_SH4) || defined(TARGET_CRIS)
/* 16 bit uid wrappers emulation */
#define USE_UID16
Re: [PATCH 5/5] target/riscv: Add pointer mask support for instruction fetch
On 3/27/23 03:00, Weiwei Li wrote:
@@ -1248,6 +1265,10 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int
size,
qemu_log_mask(CPU_LOG_MMU, "%s ad %" VADDR_PRIx " rw %d mmu_idx %d\n",
__func__, address, access_type, mmu_idx);
+if (access_type == MMU_INST_FETCH) {
+address = adjust_pc_address(env, address);
+}
Why do you want to do this so late, as opposed to earlier in
cpu_get_tb_cpu_state?
r~
Re: [PATCH v9 3/3] tpm: Add support for TPM device over I2C bus
On 3/27/23 14:12, Ninad Palsule wrote: diff --git a/hw/tpm/tpm_tis_i2c.c b/hw/tpm/tpm_tis_i2c.c new file mode 100644 index 00..551b89dac8 --- /dev/null +++ b/hw/tpm/tpm_tis_i2c.c @@ -0,0 +1,527 @@ +/* + * Aspeed i2c bus interface for reading from and writing to i2c device registers This was my suggestion for the format but this is not the correct line. With this line reverted to tpm_tis_i2c.c - QEMU's TPM TIS I2C Device Reviewed-by: Stefan Berger Tested-by: Stefan Berger
[PATCH v10 3/3] tpm: Add support for TPM device over I2C bus
Qemu already supports devices attached to ISA and sysbus. This drop adds
support for the I2C bus attached TPM devices. I2C model only supports
TPM2 protocol.
This commit includes changes for the common code.
- Added I2C emulation model. Logic was added in the model to temporarily
cache the data as I2C interface works per byte basis.
- New tpm type "tpm-tis-i2c" added for I2C support. The user has to
provide this string on command line.
Testing:
TPM I2C device module is tested using SWTPM (software based TPM
package). Qemu uses the rainier machine and is connected to swtpm over
the socket interface.
The command to start swtpm is as follows:
$ swtpm socket --tpmstate dir=/tmp/mytpm1\
--ctrl type=unixio,path=/tmp/mytpm1/swtpm-sock \
--tpm2 --log level=100
The command to start qemu is as follows:
$ qemu-system-arm -M rainier-bmc -nographic \
-kernel ${IMAGEPATH}/fitImage-linux.bin \
-dtb ${IMAGEPATH}/aspeed-bmc-ibm-rainier.dtb \
-initrd ${IMAGEPATH}/obmc-phosphor-initramfs.rootfs.cpio.xz \
-drive
file=${IMAGEPATH}/obmc-phosphor-image.rootfs.wic.qcow2,if=sd,index=2 \
-net nic -net
user,hostfwd=:127.0.0.1:-:22,hostfwd=:127.0.0.1:2443-:443 \
-chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
-tpmdev emulator,id=tpm0,chardev=chrtpm \
-device tpm-tis-i2c,tpmdev=tpm0,bus=aspeed.i2c.bus.12,address=0x2e
Signed-off-by: Ninad Palsule
---
V2:
Incorporated Stephen's review comments.
- Handled checksum related register in I2C layer
- Defined I2C interface capabilities and return those instead of
capabilities from TPM TIS. Add required capabilities from TIS.
- Do not cache FIFO data in the I2C layer.
- Make sure that Device address change register is not passed to I2C
layer as capability indicate that it is not supported.
- Added boundary checks.
- Make sure that bits 26-31 are zeroed for the TPM_STS register on read
- Updated Kconfig files for new define.
---
V3:
- Moved processing of register TPM_I2C_LOC_SEL in the I2C. So I2C layer
remembers the locality and pass it to TIS on each read/write.
- The write data is no more cached in the I2C layer so the buffer size
is reduced to 16 bytes.
- Checksum registers are now managed by the I2C layer. Added new
function in TIS layer to return the checksum and used that to process
the request.
- Now 2-4 byte register value will be passed to TIS layer in a single
write call instead of 1 byte at a time. Added functions to convert
between little endian stream of bytes to single 32 bit unsigned
integer. Similarly 32 bit integer to stream of bytes.
- Added restriction on device change register.
- Replace few if-else statement with switch statement for clarity.
- Log warning when unknown register is received.
- Moved all register definations to acpi/tmp.h
---
V4:
Incorporated review comments from Cedric and Stefan.
- Reduced data[] size from 16 byte to 5 bytes.
- Added register name in the mapping table which can be used for
tracing.
- Removed the endian conversion functions instead used simple logic
provided by Stefan.
- Rename I2C registers to reduce the length.
- Added traces for send, recv and event functions. You can turn on trace
on command line by using "-trace "tpm_tis_i2c*" option.
---
V5:
Fixed issues reported by Stefan's test.
- Added mask for the INT_ENABLE register.
- Use correct TIS register for reading interrupt capability.
- Cleanup how register is converted from I2C to TIS and also saved
information like tis_addr and register name in the i2cst so that we
can only convert it once on i2c_send.
- Trace register number for unknown registers.
---
V6:
Fixed review comments from Stefan.
- Fixed some variable size.
- Removed unused variables.
- Added vmstat backin to handle migration.
- Added post load phase to reload tis address and register name.
---
V7:
Incorporated review comments from Stefan.
- Added tpm_tis_i2c_initfn function
- Set the device catagory DEVICE_CATEGORY_MISC.
- Corrected default locality selection.
- Other cleanup. Include file cleanup.
---
V8:
Incorporated review comments from Stefan.
- Removed the irq initialization as linux doesn't support interrupts for
TPM
- Handle INT_CAPABILITY register in I2C only and return 0 to indicate
that it is not supported.
---
V9:
- Added copyright
- Added set data function and called it few places.
- Rename function tpm_i2c_interface_capability
---
V10:
- Fixed the copyright text.
---
hw/arm/Kconfig | 1 +
hw/tpm/Kconfig | 7 +
hw/tpm/meson.build | 1 +
hw/tpm/tpm_tis_i2c.c | 527 +++
hw/tpm/trace-events | 6 +
include/sysemu/tpm.h | 3 +
6 files changed, 545 insertions(+)
create mode 100644 hw/tpm/tpm_tis_i2c.c
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index b5aed4aff5..05d6ef1a31 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -6,6 +6,7 @@
Re: [RESEND PATCH v2] target/i386: Switch back XFRM value
On Thu, Oct 27, 2022 at 2:36 AM Yang, Weijiang wrote:
>
>
> On 10/26/2022 7:57 PM, Zhong, Yang wrote:
> > The previous patch wrongly replaced FEAT_XSAVE_XCR0_{LO|HI} with
> > FEAT_XSAVE_XSS_{LO|HI} in CPUID(EAX=12,ECX=1):{ECX,EDX}, which made
> > SGX enclave only supported SSE and x87 feature(xfrm=0x3).
> >
> > Fixes: 301e90675c3f ("target/i386: Enable support for XSAVES based
> > features")
> >
> > Signed-off-by: Yang Zhong
> > ---
> > target/i386/cpu.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index ad623d91e4..19aaed877b 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -5584,8 +5584,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
> > uint32_t count,
> > } else {
> > *eax &= env->features[FEAT_SGX_12_1_EAX];
> > *ebx &= 0; /* ebx reserve */
> > -*ecx &= env->features[FEAT_XSAVE_XSS_LO];
> > -*edx &= env->features[FEAT_XSAVE_XSS_HI];
> > +*ecx &= env->features[FEAT_XSAVE_XCR0_LO];
> > +*edx &= env->features[FEAT_XSAVE_XCR0_HI];
>
> Oops, that's my fault to replace with wrong definitions, thanks for the fix!
>
> Reviewed-by: Yang Weijiang
Hi,
I do not have any background on this but stumbled over this and wondered,
is there any particular reason why this wasn't applied yet?
It seemed to fix a former mistake, was acked and then ... silence
> >
> > /* FP and SSE are always allowed regardless of XSAVE/XCR0. */
> > *ecx |= XSTATE_FP_MASK | XSTATE_SSE_MASK;
>
--
Christian Ehrhardt
Senior Staff Engineer, Ubuntu Server
Canonical Ltd
Re: [PATCH v7 0/3] Add support for TPM devices over I2C bus
On Mon, 27 Mar 2023 at 03:52, Ninad Palsule wrote: > > Hi Joel, > > On 3/26/23 8:05 PM, Joel Stanley wrote: > > Hi Ninad, > > > > On Sun, 26 Mar 2023 at 22:44, Ninad Palsule wrote: > >> Hello, > >> > >> I have incorporated review comments from Stefan. Please review. > >> > >> This drop adds support for the TPM devices attached to the I2C bus. It > >> only supports the TPM2 protocol. You need to run it with the external > >> TPM emulator like swtpm. I have tested it with swtpm. > > Nice work. I tested these stop cedric's aspeed-8.0 qemu tree, using > > the rainier machine and the openbmc dev-6.1 kernel. > > > > We get this message when booting from a kernel: > > > > [0.582699] tpm_tis_i2c 12-002e: 2.0 TPM (device-id 0x1, rev-id 1) > > [0.586361] tpm tpm0: A TPM error (256) occurred attempting the self test > > [0.586623] tpm tpm0: starting up the TPM manually > > > > Do we understand why the error appears? > > > Yes, As per kernel code this is an expected error for some emulators. > > On swtpm emulator, It returns TPM2_RC_INITIALIZE if emulator is not > initialized. I searched it in swtpm and it indicated that selftest > requested before it is initialized. I meant to ask Stefan but busy with > the review comments. The swtpm man page mentions some flags we can set. Perhaps they would help? --flags [not-need-init] [,startup-clear|startup-state|startup-deactivated|startup-none] > > This function comment in the driver mentioned below indicate that this > case possible with emulators. > > /** > * tpm2_startup - turn on the TPM > * @chip: TPM chip to use > * > * Normally the firmware should start the TPM. This function is > provided as a > * workaround if this does not happen. A legal case for this could be for > * example when a TPM emulator is used. > * > * Return: same as tpm_transmit_cmd() > */ > > static int tpm2_startup(struct tpm_chip *chip) > > > However on a clean boot into the TPM, the u-boot tpm commands fail: > > > > ast# tpm info > > tpm@2e v2.0: VendorID 0x1014, DeviceID 0x0001, RevisionID 0x01 [closed] > > ast# tpINTERRUPT> > > ast# tpm init > > ast# tpm info > > tpm@2e v2.0: VendorID 0x1014, DeviceID 0x0001, RevisionID 0x01 [open] > > ast# tpm pcr_read 0 0x8100 > > Error: 256 > > ast# md.l 0x8100 16 > > 8100: > > 8110: > > 8120: > > 8130: > > 8140: > > 8150: > > > > This doesn't need to block merging into qemu, as the model works fine > > for pcr measurement and accessing under Linux. However it would be > > good to work though these issues in case there's a modelling > > discrepancy. > > > Yes, Please provide me details on how to reproduce it. I will take a look. This is the buildroot tree I've been using for testing: https://github.com/shenki/buildroot/commits/ast2600-tpm git clone https://github.com/shenki/buildroot -b ast2600-tpm cd buildroot make O=ast2600evb aspeed_ast2600evb_defconfig I launch it with this qemu commandline: swtpm socket --tpmstate dir=$XDG_RUNTIME_DIR --ctrl type=unixio,path=$XDG_RUNTIME_DIR/swtpm-socket --tpm2 qemu-system-arm -M ast2600-evb -nographic -drive file=ast2600evb/images/flash.img,if=mtd,format=raw -chardev socket,id=chrtpm,path=$XDG_RUNTIME_DIR/swtpm-socket -tpmdev emulator,id=tpm0,chardev=chrtpm -device tpm-tis-i2c,tpmdev=tpm0,bus=aspeed.i2c.bus.12,address=0x2e If you want to reproduce the u-boot behaviour, press any key to interrupt the boot. Booting this way, you can also test the u-boot behaviour. Once you're in userspace: # echo tpm_tis_i2c 0x2e > /sys/bus/i2c/devices/i2c-12/new_device [ 13.637081] tpm_tis_i2c 12-002e: 2.0 TPM (device-id 0x1, rev-id 1) [ 13.665239] i2c i2c-12: new_device: Instantiated device tpm_tis_i2c at 0x2e # cat /sys/class/tpm/tpm0/pcr-sha256/0 FE9A732EAA7842D77DEECFC1DC610EBEA9414BFC39BEEBC8D2F071CF030FA592
Re: [PATCH v2 2/5] apic: add support for x2APIC mode
On Sun, 2023-03-26 at 12:20 +0700, Bui Quang Minh wrote:
> This commit extends the APIC ID to 32-bit long and remove the 255 max APIC
> ID limit in userspace APIC. The array that manages local APICs is now
> dynamically allocated based on the max APIC ID of created x86 machine.
> Also, new x2APIC IPI destination determination scheme, self IPI and x2APIC
> mode register access are supported.
>
> Signed-off-by: Bui Quang Minh
> ---
> hw/i386/x86.c | 8 +-
> hw/intc/apic.c | 229 +++-
> hw/intc/apic_common.c | 8 +-
> include/hw/i386/apic.h | 3 +-
> include/hw/i386/apic_internal.h | 2 +-
> 5 files changed, 184 insertions(+), 66 deletions(-)
>
> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> index a88a126123..fa9b15190d 100644
> --- a/hw/i386/x86.c
> +++ b/hw/i386/x86.c
> @@ -132,11 +132,11 @@ void x86_cpus_init(X86MachineState *x86ms, int
> default_cpu_version)
> * Can we support APIC ID 255 or higher?
> *
> * Under Xen: yes.
> - * With userspace emulated lapic: no
> + * With userspace emulated lapic: yes.
Are you making this unconditional? It shall not be possible to emulate
a CPU *without* X2APIC?
> * With KVM's in-kernel lapic: only if X2APIC API is enabled.
> */
> if (x86ms->apic_id_limit > 255 && !xen_enabled() &&
> - (!kvm_irqchip_in_kernel() || !kvm_enable_x2apic())) {
> + kvm_irqchip_in_kernel() && !kvm_enable_x2apic()) {
> error_report("current -smp configuration requires kernel "
> "irqchip and X2APIC API support.");
> exit(EXIT_FAILURE);
...
> @@ -276,16 +288,17 @@ static void apic_bus_deliver(const uint32_t
> *deliver_bitmask,
> apic_set_irq(apic_iter, vector_num, trigger_mode) );
> }
>
> -void apic_deliver_irq(uint8_t dest, uint8_t dest_mode, uint8_t delivery_mode,
> +void apic_deliver_irq(uint32_t dest, uint8_t dest_mode, uint8_t
> delivery_mode,
> uint8_t vector_num, uint8_t trigger_mode)
We can make this 'static' while we're here. It isn't actually called
from anywhere else, is it?
> {
> - uint32_t deliver_bitmask[MAX_APIC_WORDS];
> + uint32_t *deliver_bitmask = g_malloc(max_apic_words * sizeof(uint32_t));
>
> trace_apic_deliver_irq(dest, dest_mode, delivery_mode, vector_num,
> trigger_mode);
>
> apic_get_delivery_bitmask(deliver_bitmask, dest, dest_mode);
> apic_bus_deliver(deliver_bitmask, delivery_mode, vector_num,
> trigger_mode);
> + g_free(deliver_bitmask);
> }
>
> bool is_x2apic_mode(DeviceState *dev)
...
>
> static void apic_get_delivery_bitmask(uint32_t *deliver_bitmask,
> - uint8_t dest, uint8_t dest_mode)
> + uint32_t dest, uint8_t dest_mode)
> {
> APICCommonState *apic_iter;
> int i;
>
> + memset(deliver_bitmask, 0x00, max_apic_words * sizeof(uint32_t));
> +
> + /* x2APIC broadcast id for both physical and logical (cluster) mode */
> + if (dest == 0x) {
> + apic_get_broadcast_bitmask(deliver_bitmask, true);
> + return;
> + }
> +
> if (dest_mode == 0) {
Might be nice to have a constant for DEST_MODE_PHYS vs.
DEST_MODE_LOGICAL to make this clearer?
> + apic_find_dest(deliver_bitmask, dest);
> + /* Broadcast to xAPIC mode apics */
> if (dest == 0xff) {
> - memset(deliver_bitmask, 0xff, MAX_APIC_WORDS * sizeof(uint32_t));
> - } else {
> - int idx = apic_find_dest(dest);
> - memset(deliver_bitmask, 0x00, MAX_APIC_WORDS * sizeof(uint32_t));
> - if (idx >= 0)
> - apic_set_bit(deliver_bitmask, idx);
> + apic_get_broadcast_bitmask(deliver_bitmask, false);
Hrm... aren't you still interpreting destination 0x00FF as
broadcast even for X2APIC mode? Or am I misreading this?
> }
> } else {
> /* XXX: cluster mode */
>
...
> @@ -366,7 +370,7 @@ static const VMStateDescription vmstate_apic_common = {
> VMSTATE_UINT8(arb_id, APICCommonState),
> VMSTATE_UINT8(tpr, APICCommonState),
> VMSTATE_UINT32(spurious_vec, APICCommonState),
> - VMSTATE_UINT8(log_dest, APICCommonState),
> + VMSTATE_UINT32(log_dest, APICCommonState),
> VMSTATE_UINT8(dest_mode, APICCommonState),
> VMSTATE_UINT32_ARRAY(isr, APICCommonState, 8),
> VMSTATE_UINT32_ARRAY(tmr, APICCommonState, 8),
Hm, doesn't this need to be added in a separate subsection, much as
ide_drive/pio_state in the example in docs/devel/migration.rst? Or did
I *not* need to do that in commit ecb0e98b4 (unrelated to x2apic, but
similar addition of state)?
Can you confirm that you've tested the behaviour when migrating back
from this to an older QEMU, both for a guest *with* X2APIC enabled
(which should fail gracefully), and a guest
Re: [PATCH] block/export: only acquire AioContext once for vhost_user_server_stop()
Am 23.03.2023 um 15:58 hat Stefan Hajnoczi geschrieben: > vhost_user_server_stop() uses AIO_WAIT_WHILE(). AIO_WAIT_WHILE() > requires that AioContext is only acquired once. > > Since blk_exp_request_shutdown() already acquires the AioContext it > shouldn't be acquired again in vhost_user_server_stop(). > > Signed-off-by: Stefan Hajnoczi Thanks, applied to the block branch. Kevin
Re: [PATCH v4 06/10] migration: Introduce dirty-limit capability
Hyman Huang writes:
> 在 2023/3/24 22:32, Markus Armbruster 写道:
>> Hyman Huang writes:
>>
>>> 在 2023/3/24 20:11, Markus Armbruster 写道:
huang...@chinatelecom.cn writes:
> From: Hyman Huang(黄勇)
>
> Introduce migration dirty-limit capability, which can
> be turned on before live migration and limit dirty
> page rate durty live migration.
>
> Introduce migrate_dirty_limit function to help check
> if dirty-limit capability enabled during live migration.
>
> Meanwhile, refactor vcpu_dirty_rate_stat_collect
> so that period can be configured instead of hardcoded.
>
> dirty-limit capability is kind of like auto-converge
> but using dirty limit instead of traditional cpu-throttle
> to throttle guest down. To enable this feature, turn on
> the dirty-limit capability before live migration using
> migrate-set-capabilities, and set the parameters
> "x-vcpu-dirty-limit-period", "vcpu-dirty-limit" suitably
> to speed up convergence.
>
> Signed-off-by: Hyman Huang(黄勇)
> Acked-by: Peter Xu
[...]
> diff --git a/qapi/migration.json b/qapi/migration.json
> index d33cc2d582..b7a92be055 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -477,6 +477,8 @@
>#will be handled faster. This is a performance
> feature and
>#should not affect the correctness of postcopy
> migration.
>#(since 7.1)
> +# @dirty-limit: Use dirty-limit to throttle down guest if enabled.
> +# (since 8.0)
Feels too terse. What exactly is used and how? It's not the capability
itself (although the text sure sounds like it). I guess it's the thing
you set with command set-vcpu-dirty-limit.
Is that used only when the capability is set?
>>>
>>> Yes, live migration set "dirty-limit" value when that capability is set,
>>> the comment changes to "Apply the algorithm of dirty page rate limit to
>>> throttle down guest if capability is set, rather than auto-converge".
>>>
>>> Please continue to polish the doc if needed. Thanks.
>>
>> Let's see whether I understand.
>>
>> Throttling happens only during migration.
>>
>> There are two throttling algorithms: "auto-converge" (default) and
>> "dirty page rate limit".
>>
>> The latter can be tuned with set-vcpu-dirty-limit.
>> Correct?
>
> Yes
>
>> What happens when migration capability dirty-limit is enabled, but the
>> user hasn't set a limit with set-vcpu-dirty-limit, or canceled it with
>> cancel-vcpu-dirty-limit?
>
> dirty-limit capability use the default value if user hasn't set.
What is the default value? I can't find it in the doc comments.
> In the path of cancel-vcpu-dirty-limit, canceling should be check and not be
> allowed if migration is in process.
Can you change the dirty limit with set-vcpu-dirty-limit while migration
is in progress? Let's see...
Has the dirty limit any effect while migration is not in progress?
> see the following code in commit:
> [PATCH v4 08/10] migration: Implement dirty-limit convergence algo
>
> --- a/softmmu/dirtylimit.c
> +++ b/softmmu/dirtylimit.c
> @@ -438,6 +438,8 @@ void qmp_cancel_vcpu_dirty_limit(bool has_cpu_index,
> int64_t cpu_index,
> Error **errp)
> {
> +MigrationState *ms = migrate_get_current();
> +
> if (!kvm_enabled() || !kvm_dirty_ring_enabled()) {
> return;
> }
> @@ -451,6 +453,15 @@ void qmp_cancel_vcpu_dirty_limit(bool has_cpu_index,
> return;
> }
>
> +if (migration_is_running(ms->state) &&
> +(!qemu_thread_is_self(>thread)) &&
> +migrate_dirty_limit() &&
> +dirtylimit_in_service()) {
> +error_setg(errp, "can't cancel dirty page limit while"
> + " migration is running");
> +return;
> +}
We can get here even when migration_is_running() is true. Seems to
contradict your claim "no cancel while migration is in progress". Am I
confused?
Please drop the superfluous parenthesis around !qemu_thread_is_self().
> +
> dirtylimit_state_lock();
>
> if (has_cpu_index) {
> @@ -486,6 +497,8 @@ void qmp_set_vcpu_dirty_limit(bool has_cpu_index,
>uint64_t dirty_rate,
>Error **errp)
> {
> +MigrationState *ms = migrate_get_current();
> +
> if (!kvm_enabled() || !kvm_dirty_ring_enabled()) {
> error_setg(errp, "dirty page limit feature requires KVM with"
> " accelerator property 'dirty-ring-size' set'");
> @@ -502,6 +515,15 @@ void qmp_set_vcpu_dirty_limit(bool has_cpu_index,
> return;
> }
>
> +if (migration_is_running(ms->state) &&
> +(!qemu_thread_is_self(>thread)) &&
> +migrate_dirty_limit() &&
> +dirtylimit_in_service()) {
> +error_setg(errp,
Re: [PATCH v3 1/1] util/async-teardown: wire up query-command-line-options
On 24/03/2023 18.45, Claudio Imbrenda wrote:
The recently introduced -async-teardown commandline option was not
wired up properly and did not show up in the output of the QMP command
query-command-line-options. This means that libvirt will have no way to
discover whether the feature is supported.
This patch fixes the issue by correctly wiring up the commandline
option so that it appears in the output of query-command-line-options.
Reported-by: Boris Fiuczynski
Fixes: c891c24b1a ("os-posix: asynchronous teardown for shutdown on Linux")
Signed-off-by: Claudio Imbrenda
---
os-posix.c| 14 ++
qemu-options.hx | 35 ---
util/async-teardown.c | 21 +
3 files changed, 59 insertions(+), 11 deletions(-)
diff --git a/os-posix.c b/os-posix.c
index 5adc69f560..48acd7acf5 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -36,6 +36,8 @@
#include "qemu/log.h"
#include "sysemu/runstate.h"
#include "qemu/cutils.h"
+#include "qemu/config-file.h"
+#include "qemu/option.h"
#ifdef CONFIG_LINUX
#include
@@ -132,6 +134,8 @@ static bool os_parse_runas_uid_gid(const char *optarg)
*/
int os_parse_cmd_args(int index, const char *optarg)
{
+QemuOpts *opts;
+
switch (index) {
case QEMU_OPTION_runas:
user_pwd = getpwnam(optarg);
@@ -155,6 +159,16 @@ int os_parse_cmd_args(int index, const char *optarg)
case QEMU_OPTION_asyncteardown:
init_async_teardown();
break;
+case QEMU_OPTION_teardown:
+opts = qemu_opts_parse_noisily(qemu_find_opts("teardown"),
+ optarg, false);
+if (!opts) {
+return -1;
Maybe it's better to use exit(1) here (like it is done in the -runas part),
otherwise you get a somewhat weird second error message:
$ ./qemu-system-s390x -teardown aysnc=on
qemu-system-s390x: -teardown aysnc=on: Invalid parameter 'aysnc'
qemu-system-s390x: -teardown aysnc=on: Option not supported in this build
+}
+if (qemu_opt_get_bool(opts, "async", false)) {
+init_async_teardown();
+}
+break;
#endif
default:
return -1;
diff --git a/qemu-options.hx b/qemu-options.hx
index d42f60fb91..8582980b12 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4766,20 +4766,33 @@ DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log, "",
QEMU_ARCH_ALL)
DEF("async-teardown", 0, QEMU_OPTION_asyncteardown,
"-async-teardown enable asynchronous teardown\n",
QEMU_ARCH_ALL)
-#endif
SRST
``-async-teardown``
-Enable asynchronous teardown. A new process called "cleanup/"
-will be created at startup sharing the address space with the main qemu
-process, using clone. It will wait for the main qemu process to
-terminate completely, and then exit.
-This allows qemu to terminate very quickly even if the guest was
-huge, leaving the teardown of the address space to the cleanup
-process. Since the cleanup process shares the same cgroups as the
-main qemu process, accounting is performed correctly. This only
-works if the cleanup process is not forcefully killed with SIGKILL
-before the main qemu process has terminated completely.
+Equivalent to -teardown async=on
+ERST
+
+DEF("teardown", HAS_ARG, QEMU_OPTION_teardown,
+"-teardown async[=on|off]\n"
+"process teardown options\n"
+"async=on enables asynchronous teardown\n"
+ ,
+QEMU_ARCH_ALL)
+SRST
+``-teardown``
+Set process teardown options.
+
+``async=on`` enables asynchronous teardown. A new process called
+"cleanup/" will be created at startup sharing the address
+space with the main qemu process, using clone. It will wait for the
While you're at it, we officially spell QEMU with capital letters, so I'd
maybe do a s/qemu/QEMU/g here.
+main qemu process to terminate completely, and then exit. This allows
+qemu to terminate very quickly even if the guest was huge, leaving the
+teardown of the address space to the cleanup process. Since the cleanup
+process shares the same cgroups as the main qemu process, accounting is
+performed correctly. This only works if the cleanup process is not
+forcefully killed with SIGKILL before the main qemu process has
+terminated completely.
ERST
+#endif
DEF("msg", HAS_ARG, QEMU_OPTION_msg,
"-msg [timestamp[=on|off]][,guest-name=[on|off]]\n"
diff --git a/util/async-teardown.c b/util/async-teardown.c
index 62cdeb0f20..4a5dbce958 100644
--- a/util/async-teardown.c
+++ b/util/async-teardown.c
@@ -12,6 +12,9 @@
*/
#include "qemu/osdep.h"
+#include "qemu/config-file.h"
+#include "qemu/option.h"
+#include "qemu/module.h"
#include
#include
#include
@@ -144,3 +147,21 @@ void init_async_teardown(void)
clone(async_teardown_fn, new_stack_for_clone(), CLONE_VM, NULL);
sigprocmask(SIG_SETMASK, _signals,
Re: [PATCH v2] virtio: refresh vring region cache after updating a virtqueue size
On Fri, Mar 24 2023, Halil Pasic wrote: > On Wed, 22 Mar 2023 18:24:33 +0100 > Halil Pasic wrote: > >> > > --- a/hw/s390x/virtio-ccw.c >> > > +++ b/hw/s390x/virtio-ccw.c >> > > @@ -237,6 +237,7 @@ static int virtio_ccw_set_vqs(SubchDev *sch, >> > > VqInfoBlock *info, >> > > return -EINVAL; >> > > } >> > > virtio_queue_set_num(vdev, index, num); >> > > +virtio_init_region_cache(vdev, index); >> > >> > Hmm... this is not wrong, but looking at it again, I see that the guest >> > has no way to change num after our last call to >> > virtio_init_region_cache() (while setting up the queue addresses.) IOW, >> > this introduces an extra round trip that is not really needed. >> > >> >> I don't quite understand. AFAIU the virtio_init_region_cache() would see >> the (new) queue addresses but not the new size (num). Yes virtio-ccw >> already knows the new num but it is yet to call >> to put it into vdev->vq[n].vring.num from where >> virtio_init_region_cache() picks it up. >> >> If we were to first virtio_queue_set_num() and only then the address >> I would understand. But with the code as is, I don't. Am I missing >> something? > > Connie: have you had a chance to have yet another look at this? I > would like to understand the reason for seeing this differently. I'm just back from being sick, please give me some time to work through my backlog.
Re: [PATCH v7 1/3] docs: Add support for TPM devices over I2C bus
On 3/27/23 09:47, Joel Stanley wrote:
On Sun, 26 Mar 2023 at 22:44, Ninad Palsule wrote:
This is a documentation change for I2C TPM device support.
Qemu already supports devices attached to ISA and sysbus.
This drop adds support for the I2C bus attached TPM devices.
Signed-off-by: Ninad Palsule
---
V2:
Incorporated Stephen's review comments
- Added example in the document.
---
V4:
Incorporate Cedric & Stefan's comments
- Added example for ast2600-evb
- Corrected statement about arm virtual machine.
---
V6:
Incorporated review comments from Stefan.
---
docs/specs/tpm.rst | 32
1 file changed, 32 insertions(+)
diff --git a/docs/specs/tpm.rst b/docs/specs/tpm.rst
index 535912a92b..590e670a9a 100644
--- a/docs/specs/tpm.rst
+++ b/docs/specs/tpm.rst
@@ -21,12 +21,16 @@ QEMU files related to TPM TIS interface:
- ``hw/tpm/tpm_tis_common.c``
- ``hw/tpm/tpm_tis_isa.c``
- ``hw/tpm/tpm_tis_sysbus.c``
+ - ``hw/tpm/tpm_tis_i2c.c``
- ``hw/tpm/tpm_tis.h``
Both an ISA device and a sysbus device are available. The former is
used with pc/q35 machine while the latter can be instantiated in the
Arm virt machine.
+An I2C device support is also provided which can be instantiated in the Arm
+based emulation machines. This device only supports the TPM 2 protocol.
+
CRB interface
-
@@ -348,6 +352,34 @@ In case an Arm virt machine is emulated, use the following
command line:
-drive if=pflash,format=raw,file=flash0.img,readonly=on \
-drive if=pflash,format=raw,file=flash1.img
+In case a ast2600-evb bmc machine is emulated and want to use TPM device
+attached to I2C bus, use the following command line:
+
+.. code-block:: console
+
+ qemu-system-arm -M ast2600-evb -nographic \
+-kernel arch/arm/boot/zImage \
+-dtb arch/arm/boot/dts/aspeed-ast2600-evb.dtb \
+-initrd rootfs.cpio \
+-chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
+-tpmdev emulator,id=tpm0,chardev=chrtpm \
+-device tpm-tis-i2c,tpmdev=tpm0,bus=aspeed.i2c.bus.12,address=0x2e
For testing, use this command to load the driver to the correct address:
echo tpm_tis_i2c 0x2e > /sys/bus/i2c/devices/i2c-12/new_device
(I don't know how specific we want to make the instructions, but
adding a line like above would help others from having to re-discover
the command).
or/and add an avocado test for it. See tests/avocado/machine_aspeed.py.
The avocado framework is a bit fragile when reading from the console but
we hope to fix that.
Thanks
C.
+
+In case a Rainier bmc machine is emulated and want to use TPM device
+attached to I2C bus, use the following command line:
+
+.. code-block:: console
+
+ qemu-system-arm -M rainier-bmc -nographic \
+-kernel ${IMAGEPATH}/fitImage-linux.bin \
+-dtb ${IMAGEPATH}/aspeed-bmc-ibm-rainier.dtb \
+-initrd ${IMAGEPATH}/obmc-phosphor-initramfs.rootfs.cpio.xz \
+-drive
file=${IMAGEPATH}/obmc-phosphor-image.rootfs.wic.qcow2,if=sd,index=2\
+-net nic -net
user,hostfwd=:127.0.0.1:-:22,hostfwd=:127.0.0.1:2443-:443\
+-chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
+-tpmdev emulator,id=tpm0,chardev=chrtpm \
+-device tpm-tis-i2c,tpmdev=tpm0,bus=aspeed.i2c.bus.12,address=0x2e
+
I'd drop this example, the above one is enough.
In case SeaBIOS is used as firmware, it should show the TPM menu item
after entering the menu with 'ESC'.
--
2.37.2
[PATCH v2 05/10] target/riscv: Convert env->virt to a bool env->virt_enabled
From: LIU Zhiwei
Currently we only use the env->virt to encode the virtual mode enabled
status. Let's make it a bool type.
Signed-off-by: LIU Zhiwei
Reviewed-by: Weiwei Li
Message-ID: <20230325145348.1208-1-zhiwei_...@linux.alibaba.com>
---
target/riscv/cpu.h| 2 +-
target/riscv/cpu_bits.h | 3 ---
target/riscv/cpu_helper.c | 6 +++---
target/riscv/machine.c| 6 +++---
target/riscv/translate.c | 4 ++--
5 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 5adefe4ab5..22dc5ddb95 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -183,7 +183,7 @@ struct CPUArchState {
#ifndef CONFIG_USER_ONLY
target_ulong priv;
/* This contains QEMU specific information about the virt state. */
-target_ulong virt;
+bool virt_enabled;
target_ulong geilen;
uint64_t resetvec;
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index fca7ef0cef..45ddb00aa5 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -607,9 +607,6 @@ typedef enum {
#define PRV_H 2 /* Reserved */
#define PRV_M 3
-/* Virtulisation Register Fields */
-#define VIRT_ONOFF 1
-
/* RV32 satp CSR field masks */
#define SATP32_MODE 0x8000
#define SATP32_ASID 0x7fc0
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index b286118a6b..c7bc3fc553 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -560,18 +560,18 @@ void riscv_cpu_set_geilen(CPURISCVState *env,
target_ulong geilen)
bool riscv_cpu_virt_enabled(CPURISCVState *env)
{
-return get_field(env->virt, VIRT_ONOFF);
+return env->virt_enabled;
}
/* This function can only be called to set virt when RVH is enabled */
void riscv_cpu_set_virt_enabled(CPURISCVState *env, bool enable)
{
/* Flush the TLB on all virt mode changes. */
-if (get_field(env->virt, VIRT_ONOFF) != enable) {
+if (env->virt_enabled != enable) {
tlb_flush(env_cpu(env));
}
-env->virt = set_field(env->virt, VIRT_ONOFF, enable);
+env->virt_enabled = enable;
if (enable) {
/*
diff --git a/target/riscv/machine.c b/target/riscv/machine.c
index 9c455931d8..0fb3ddda06 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -331,8 +331,8 @@ static const VMStateDescription vmstate_pmu_ctr_state = {
const VMStateDescription vmstate_riscv_cpu = {
.name = "cpu",
-.version_id = 7,
-.minimum_version_id = 7,
+.version_id = 8,
+.minimum_version_id = 8,
.post_load = riscv_cpu_post_load,
.fields = (VMStateField[]) {
VMSTATE_UINTTL_ARRAY(env.gpr, RISCVCPU, 32),
@@ -352,7 +352,7 @@ const VMStateDescription vmstate_riscv_cpu = {
VMSTATE_UINT32(env.misa_mxl_max, RISCVCPU),
VMSTATE_UINT32(env.misa_ext_mask, RISCVCPU),
VMSTATE_UINTTL(env.priv, RISCVCPU),
-VMSTATE_UINTTL(env.virt, RISCVCPU),
+VMSTATE_BOOL(env.virt_enabled, RISCVCPU),
VMSTATE_UINT64(env.resetvec, RISCVCPU),
VMSTATE_UINTTL(env.mhartid, RISCVCPU),
VMSTATE_UINT64(env.mstatus, RISCVCPU),
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 0ee8ee147d..c3adf30b54 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -1255,8 +1255,8 @@ static void riscv_tr_disas_log(const DisasContextBase
*dcbase,
fprintf(logfile, "IN: %s\n", lookup_symbol(dcbase->pc_first));
#ifndef CONFIG_USER_ONLY
-fprintf(logfile, "Priv: "TARGET_FMT_ld"; Virt: "TARGET_FMT_ld"\n",
-env->priv, env->virt);
+fprintf(logfile, "Priv: "TARGET_FMT_ld"; Virt: %d\n",
+env->priv, env->virt_enabled);
#endif
target_disas(logfile, cpu, dcbase->pc_first, dcbase->tb->size);
}
--
2.25.1
[PATCH v2 02/10] target/riscv: Remove redundant check on RVH
Check on riscv_cpu_virt_enabled contains the check on RVH.
Signed-off-by: Weiwei Li
Signed-off-by: Junqiang Wang
Reviewed-by: Richard Henderson
Reviewed-by: LIU Zhiwei
---
target/riscv/op_helper.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 84ee018f7d..1eecae9547 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -278,8 +278,7 @@ target_ulong helper_sret(CPURISCVState *env)
riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
}
-if (riscv_has_ext(env, RVH) && riscv_cpu_virt_enabled(env) &&
-get_field(env->hstatus, HSTATUS_VTSR)) {
+if (riscv_cpu_virt_enabled(env) && get_field(env->hstatus, HSTATUS_VTSR)) {
riscv_raise_exception(env, RISCV_EXCP_VIRT_INSTRUCTION_FAULT, GETPC());
}
--
2.25.1
[PATCH v2 07/10] target/riscv: Remove redundant parentheses
Remove redundant parentheses in get_physical_address.
Signed-off-by: Weiwei Li
Signed-off-by: Junqiang Wang
Reviewed-by: Richard Henderson
Reviewed-by: LIU Zhiwei
---
target/riscv/cpu_helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 1ad39e7157..9145ca0ddb 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1046,7 +1046,7 @@ restart:
if ((pte & PTE_R) || ((pte & PTE_X) && mxr)) {
*prot |= PAGE_READ;
}
-if ((pte & PTE_X)) {
+if (pte & PTE_X) {
*prot |= PAGE_EXEC;
}
/* add write permission on stores or if the page is already dirty,
--
2.25.1
Re: [PATCH v7 0/3] Add support for TPM devices over I2C bus
However on a clean boot into the TPM, the u-boot tpm commands fail: ast# tpm info tpm@2e v2.0: VendorID 0x1014, DeviceID 0x0001, RevisionID 0x01 [closed] ast# tpINTERRUPT> ast# tpm init ast# tpm info tpm@2e v2.0: VendorID 0x1014, DeviceID 0x0001, RevisionID 0x01 [open] ast# tpm pcr_read 0 0x8100 Error: 256 ast# md.l 0x8100 16 8100: 8110: 8120: 8130: 8140: 8150: This doesn't need to block merging into qemu, as the model works fine for pcr measurement and accessing under Linux. However it would be good to work though these issues in case there's a modelling discrepancy. Yes, Please provide me details on how to reproduce it. I will take a look. This is the buildroot tree I've been using for testing: https://github.com/shenki/buildroot/commits/ast2600-tpm git clone https://github.com/shenki/buildroot -b ast2600-tpm cd buildroot make O=ast2600evb aspeed_ast2600evb_defconfig I have pushed binaries here also : https://github.com/legoater/qemu-aspeed-boot/tree/master/images/ast2600-evb/buildroot-2023.02-tpm Cheers, C.
Re: [PATCH for-8.0 11/11] linux-user/arm: Take more care allocating commpage
Richard Henderson writes:
> User setting of -R reserved_va can lead to an assertion
> failure in page_set_flags. Sanity check the value of
> reserved_va and print an error message instead. Do not
> allocate a commpage at all for m-profile cpus.
I see this:
TESTconvd on i386
qemu-i386: Unable to reserve 0x1 bytes of virtual address space
at 0x8000 (File exists) for use as guest address space (check your
virtual memory ulimit setting, min_mmap_addr or reserve less using -R
option)
on the ubuntu aarch64 static build:
https://gitlab.com/stsquad/qemu/-/jobs/4003523064
>
> Signed-off-by: Richard Henderson
> ---
> linux-user/elfload.c | 37 +++--
> 1 file changed, 27 insertions(+), 10 deletions(-)
>
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index b068676340..0529430b1d 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -422,12 +422,32 @@ enum {
>
> static bool init_guest_commpage(void)
> {
> -abi_ptr commpage = HI_COMMPAGE & -qemu_host_page_size;
> -void *want = g2h_untagged(commpage);
> -void *addr = mmap(want, qemu_host_page_size, PROT_READ | PROT_WRITE,
> - MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, -1, 0);
> +ARMCPU *cpu = ARM_CPU(thread_cpu);
> +abi_ptr want = HI_COMMPAGE & TARGET_PAGE_MASK;
> +abi_ptr addr;
>
> -if (addr == MAP_FAILED) {
> +/*
> + * M-profile allocates maximum of 2GB address space, so can never
> + * allocate the commpage. Skip it.
> + */
> +if (arm_feature(>env, ARM_FEATURE_M)) {
> +return true;
> +}
> +
> +/*
> + * If reserved_va does not cover the commpage, we get an assert
> + * in page_set_flags. Produce an intelligent error instead.
> + */
> +if (reserved_va != 0 && want + TARGET_PAGE_SIZE - 1 > reserved_va) {
> +error_report("Allocating guest commpage: -R 0x%" PRIx64 " too small",
> + (uint64_t)reserved_va + 1);
> +exit(EXIT_FAILURE);
> +}
> +
> +addr = target_mmap(want, TARGET_PAGE_SIZE, PROT_READ | PROT_WRITE,
> + MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, -1, 0);
> +
> +if (addr == -1) {
> perror("Allocating guest commpage");
> exit(EXIT_FAILURE);
> }
> @@ -436,15 +456,12 @@ static bool init_guest_commpage(void)
> }
>
> /* Set kernel helper versions; rest of page is 0. */
> -__put_user(5, (uint32_t *)g2h_untagged(0x0ffcu));
> +put_user_u32(5, 0x0ffcu);
>
> -if (mprotect(addr, qemu_host_page_size, PROT_READ)) {
> +if (target_mprotect(addr, qemu_host_page_size, PROT_READ | PROT_EXEC)) {
> perror("Protecting guest commpage");
> exit(EXIT_FAILURE);
> }
> -
> -page_set_flags(commpage, commpage | ~qemu_host_page_mask,
> - PAGE_READ | PAGE_EXEC | PAGE_VALID);
> return true;
> }
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [RFC PATCH] MAINTAINERS: add a section for policy documents
On Fri, Mar 24, 2023 at 05:38:36PM +, Alex Bennée wrote: > We don't update these often but if your the sort of person who enjoys s/your/you are/ > debating and tuning project policies you could now add yourself as a > reviewer here so you don't miss the next debate over tabs vs spaces > ;-) > > Who's with me? Sure, you can add me. > > Signed-off-by: Alex Bennée > Cc: Thomas Huth > Cc: Daniel P. Berrangé > Cc: Markus Armbruster > Cc: Kashyap Chamarthy > Cc: Paolo Bonzini > Cc: Peter Maydell > Cc: Philippe Mathieu-Daudé > Cc: Bernhard Beschow > --- > MAINTAINERS | 10 ++ > 1 file changed, 10 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 9b56ccdd92..992deb2667 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -64,6 +64,16 @@ L: qemu-devel@nongnu.org > F: * > F: */ > > +Project policy and developer guides > +R: Alex Bennée > +W: https://www.qemu.org/docs/master/devel/index.html > +S: Odd Fixes > +F: docs/devel/style.rst > +F: docs/devel/code-of-conduct.rst > +F: docs/devel/conflict-resolution.rst > +F: docs/devel/submitting-a-patch.rst > +F: docs/devel/submitting-a-pull-request.rst > + > Responsible Disclosure, Reporting Security Issues > - > W: https://wiki.qemu.org/SecurityProcess > -- > 2.39.2 > With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[PATCH 1/5] target/riscv: Fix effective address for pointer mask
Since pointer mask works on effective address, and the xl works on the
generation of effective address, so xl related calculation should be done
before pointer mask.
Signed-off-by: Weiwei Li
Signed-off-by: Junqiang Wang
---
target/riscv/translate.c | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 0ee8ee147d..bf0e2d318e 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -568,11 +568,15 @@ static TCGv get_address(DisasContext *ctx, int rs1, int
imm)
TCGv src1 = get_gpr(ctx, rs1, EXT_NONE);
tcg_gen_addi_tl(addr, src1, imm);
+
+if (get_xl(ctx) == MXL_RV32) {
+tcg_gen_ext32u_tl(addr, addr);
+}
+
if (ctx->pm_mask_enabled) {
tcg_gen_andc_tl(addr, addr, pm_mask);
-} else if (get_xl(ctx) == MXL_RV32) {
-tcg_gen_ext32u_tl(addr, addr);
}
+
if (ctx->pm_base_enabled) {
tcg_gen_or_tl(addr, addr, pm_base);
}
@@ -586,11 +590,15 @@ static TCGv get_address_indexed(DisasContext *ctx, int
rs1, TCGv offs)
TCGv src1 = get_gpr(ctx, rs1, EXT_NONE);
tcg_gen_add_tl(addr, src1, offs);
+
+if (get_xl(ctx) == MXL_RV32) {
+tcg_gen_ext32u_tl(addr, addr);
+}
+
if (ctx->pm_mask_enabled) {
tcg_gen_andc_tl(addr, addr, pm_mask);
-} else if (get_xl(ctx) == MXL_RV32) {
-tcg_gen_ext32u_tl(addr, addr);
}
+
if (ctx->pm_base_enabled) {
tcg_gen_or_tl(addr, addr, pm_base);
}
--
2.25.1
[PATCH 4/5] target/riscv: take xl into consideration for vector address
Sign-extend the vector address when xl = 32.
Signed-off-by: Weiwei Li
Signed-off-by: Junqiang Wang
---
target/riscv/vector_helper.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index a58d82af8c..07477663eb 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -172,6 +172,9 @@ static inline uint32_t vext_get_total_elems(CPURISCVState
*env, uint32_t desc,
static inline target_ulong adjust_addr(CPURISCVState *env, target_ulong addr)
{
+if (env->xl == MXL_RV32) {
+addr = (int32_t)addr;
+}
return (addr & ~env->cur_pmmask) | env->cur_pmbase;
}
--
2.25.1
[PATCH 2/5] target/riscv: Use sign-extended data address when xl = 32
Currently, the pc use signed-extend(in gen_set_pc*) when xl = 32. And
data address should use the same memory address space with it when
xl = 32. So we should change their address calculation to use sign-extended
address when xl = 32.
Signed-off-by: Weiwei Li
Signed-off-by: Junqiang Wang
---
target/riscv/translate.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index bf0e2d318e..c48cb19389 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -570,7 +570,7 @@ static TCGv get_address(DisasContext *ctx, int rs1, int imm)
tcg_gen_addi_tl(addr, src1, imm);
if (get_xl(ctx) == MXL_RV32) {
-tcg_gen_ext32u_tl(addr, addr);
+tcg_gen_ext32s_tl(addr, addr);
}
if (ctx->pm_mask_enabled) {
@@ -592,7 +592,7 @@ static TCGv get_address_indexed(DisasContext *ctx, int rs1,
TCGv offs)
tcg_gen_add_tl(addr, src1, offs);
if (get_xl(ctx) == MXL_RV32) {
-tcg_gen_ext32u_tl(addr, addr);
+tcg_gen_ext32s_tl(addr, addr);
}
if (ctx->pm_mask_enabled) {
--
2.25.1
[PATCH 5/5] target/riscv: Add pointer mask support for instruction fetch
Transform the fetch address before page walk when pointer mask is
enabled for instruction fetch.
Signed-off-by: Weiwei Li
Signed-off-by: Junqiang Wang
---
target/riscv/cpu.h| 1 +
target/riscv/cpu_helper.c | 25 +++--
target/riscv/csr.c| 2 --
3 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 638e47c75a..57bd9c3279 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -368,6 +368,7 @@ struct CPUArchState {
#endif
target_ulong cur_pmmask;
target_ulong cur_pmbase;
+bool cur_pminsn;
/* Fields from here on are preserved across CPU reset. */
QEMUTimer *stimer; /* Internal timer for S-mode interrupt */
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index f88c503cf4..77132a3e0c 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -124,6 +124,7 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong
*pc,
void riscv_cpu_update_mask(CPURISCVState *env)
{
target_ulong mask = -1, base = 0;
+bool insn = false;
/*
* TODO: Current RVJ spec does not specify
* how the extension interacts with XLEN.
@@ -135,18 +136,21 @@ void riscv_cpu_update_mask(CPURISCVState *env)
if (env->mmte & M_PM_ENABLE) {
mask = env->mpmmask;
base = env->mpmbase;
+insn = env->mmte & MMTE_M_PM_INSN;
}
break;
case PRV_S:
if (env->mmte & S_PM_ENABLE) {
mask = env->spmmask;
base = env->spmbase;
+insn = env->mmte & MMTE_S_PM_INSN;
}
break;
case PRV_U:
if (env->mmte & U_PM_ENABLE) {
mask = env->upmmask;
base = env->upmbase;
+insn = env->mmte & MMTE_U_PM_INSN;
}
break;
default:
@@ -161,6 +165,7 @@ void riscv_cpu_update_mask(CPURISCVState *env)
env->cur_pmmask = mask;
env->cur_pmbase = base;
}
+env->cur_pminsn = insn;
}
#ifndef CONFIG_USER_ONLY
@@ -1225,6 +1230,17 @@ static void pmu_tlb_fill_incr_ctr(RISCVCPU *cpu,
MMUAccessType access_type)
riscv_pmu_incr_ctr(cpu, pmu_event_type);
}
+static target_ulong adjust_pc_address(CPURISCVState *env, target_ulong pc)
+{
+target_ulong adjust_pc = pc;
+
+if (env->cur_pminsn) {
+adjust_pc = (adjust_pc & ~env->cur_pmmask) | env->cur_pmbase;
+}
+
+return adjust_pc;
+}
+
bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
MMUAccessType access_type, int mmu_idx,
bool probe, uintptr_t retaddr)
@@ -1232,6 +1248,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int
size,
RISCVCPU *cpu = RISCV_CPU(cs);
CPURISCVState *env = >env;
vaddr im_address;
+vaddr orig_address = address;
hwaddr pa = 0;
int prot, prot2, prot_pmp;
bool pmp_violation = false;
@@ -1248,6 +1265,10 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int
size,
qemu_log_mask(CPU_LOG_MMU, "%s ad %" VADDR_PRIx " rw %d mmu_idx %d\n",
__func__, address, access_type, mmu_idx);
+if (access_type == MMU_INST_FETCH) {
+address = adjust_pc_address(env, address);
+}
+
/* MPRV does not affect the virtual-machine load/store
instructions, HLV, HLVX, and HSV. */
if (riscv_cpu_two_stage_lookup(mmu_idx)) {
@@ -1351,13 +1372,13 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address,
int size,
}
if (ret == TRANSLATE_SUCCESS) {
-tlb_set_page(cs, address & ~(tlb_size - 1), pa & ~(tlb_size - 1),
+tlb_set_page(cs, orig_address & ~(tlb_size - 1), pa & ~(tlb_size - 1),
prot, mmu_idx, tlb_size);
return true;
} else if (probe) {
return false;
} else {
-raise_mmu_exception(env, address, access_type, pmp_violation,
+raise_mmu_exception(env, orig_address, access_type, pmp_violation,
first_stage_error,
riscv_cpu_virt_enabled(env) ||
riscv_cpu_two_stage_lookup(mmu_idx),
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index d522efc0b6..4544c9d934 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -3511,8 +3511,6 @@ static RISCVException write_mmte(CPURISCVState *env, int
csrno,
/* for machine mode pm.current is hardwired to 1 */
wpri_val |= MMTE_M_PM_CURRENT;
-/* hardwiring pm.instruction bit to 0, since it's not supported yet */
-wpri_val &= ~(MMTE_M_PM_INSN | MMTE_S_PM_INSN | MMTE_U_PM_INSN);
env->mmte = wpri_val | PM_EXT_DIRTY;
riscv_cpu_update_mask(env);
--
2.25.1
[PATCH 3/5] target/riscv: Fix pointer mask transformation for vector address
actual_address = (requested_address & ~mpmmask) | mpmbase.
Signed-off-by: Weiwei Li
Signed-off-by: Junqiang Wang
---
target/riscv/vector_helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 2423affe37..a58d82af8c 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -172,7 +172,7 @@ static inline uint32_t vext_get_total_elems(CPURISCVState
*env, uint32_t desc,
static inline target_ulong adjust_addr(CPURISCVState *env, target_ulong addr)
{
-return (addr & env->cur_pmmask) | env->cur_pmbase;
+return (addr & ~env->cur_pmmask) | env->cur_pmbase;
}
/*
--
2.25.1
[PATCH 0/5] target/riscv: Fix pointer mask related support
This patchset tries to fix some problems in current implementation for pointer mask extension, and add support for pointer mask of instruction fetch. The port is available here: https://github.com/plctlab/plct-qemu/tree/plct-pm-fix Weiwei Li (5): target/riscv: Fix effective address for pointer mask target/riscv: Use sign-extended data address when xl = 32 target/riscv: Fix pointer mask transformation for vector address target/riscv: take xl into consideration for vector address target/riscv: Add pointer mask support for instruction fetch target/riscv/cpu.h | 1 + target/riscv/cpu_helper.c| 25 +++-- target/riscv/csr.c | 2 -- target/riscv/translate.c | 16 target/riscv/vector_helper.c | 5 - 5 files changed, 40 insertions(+), 9 deletions(-) -- 2.25.1
Re: [PATCH] Change the default for Mixed declarations.
Daniel P. Berrangé writes: > On Tue, Feb 14, 2023 at 05:07:38PM +0100, Juan Quintela wrote: >> Hi >> >> I want to enter a discussion about changing the default of the style >> guide. >> >> There are several reasons for that: >> - they exist since C99 (i.e. all supported compilers support them) >> - they eliminate the posibility of an unitialized variable. > > Actually they don't do that reliably. In fact, when combined > with usage of 'goto', they introduce uninitialized variables, > despite the declaration having an initialization present, and > thus actively mislead reviewers into thinking their code is > safe. > > Consider this example: [...] > What happens is that when you 'goto $LABEL' across a variable > declaration, the variable is in scope at your target label, but > its declared initializers never get run :-( > > Luckily you can protect against that with gcc: > > $ gcc -Wjump-misses-init -Wall -o mixed mixed.c > mixed.c: In function ‘foo’: > mixed.c:7:12: warning: jump skips variable initialization [-Wjump-misses-init] > 7 |goto cleanup; > |^~~~ > mixed.c:15:5: note: label ‘cleanup’ defined here >15 | cleanup: > | ^~~ > mixed.c:11:13: note: ‘items’ declared here >11 |int *items = malloc(sizeof(int) *nitems); > | ^ > mixed.c:7:12: warning: jump skips variable initialization [-Wjump-misses-init] > 7 |goto cleanup; > |^~~~ > mixed.c:15:5: note: label ‘cleanup’ defined here >15 | cleanup: > | ^~~ > mixed.c:10:12: note: ‘nitems’ declared here >10 |int nitems = 3; > |^~ > > > however that will warn about *all* cases where we jump over a > declared variable, even if the variable we're jumping over is > not used at the target label location. IOW, it has significant > false positive rates. There are quite a few triggers for this > in the QEMU code already if we turn on this warning. > > It also doesn't alter that the code initialization is misleading > to read. Yup. Strong dislike. >> - (at least for me), declaring the index inside the for make clear >> that index is not used outside the for. > > I'll admit that declaring loop indexes in the for() is a nice > bit, but I'm not a fan in general of mixing the declarations > in the middle of code for projects that use the 'goto cleanup' > pattern. A declaration in a for statement's first operand is effectively at the beginning of a block. Therefore, use of this feature is already sanctioned by the QEMU Coding Style. The proposed patch at most clarifies this. >> - Current documentation already declares that they are allowed in some >> cases. >> - Lots of places already use them. >> >> We can change the text to whatever you want, just wondering if it is >> valib to change the standard. >> >> Doing a trivial grep through my local qemu messages (around 100k) it >> shows that some people are complaining that they are not allowed, and >> other saying that they are used all over the place. > > IMHO the status quo is bad because it is actively dangerous when > combined with goto and we aren't using any compiler warnings to > help us. > > Either we allow it, but use -Wjump-misses-init to prevent mixing > delayed declarations with gotos, and just avoid this when it triggers > a false positive. > > Or we forbid it, rewrite current cases that use it, and then add > -Wdeclaration-after-statement to enforce it. I'm in favour of -Wdeclaration-after-statement. > IMHO if we are concerned about uninitialized variables then I think > a better approach is to add -ftrivial-auto-var-init=zero, which will > make the compiler initialize all variables to 0 if they lack an > explicit initializer. How often do we get bitten by uninitialized variables despite -Wmaybe-uninitialized? Honest question! >> Discuss.
[PATCH v4 1/1] util/async-teardown: wire up query-command-line-options
The recently introduced -async-teardown commandline option was not
wired up properly and did not show up in the output of the QMP command
query-command-line-options. This means that libvirt had no way to
discover whether the feature was supported.
This patch fixes the issue by replacing the -async-teardown option with
a new -teardown option with a new async=on|off parameter.
The new option is correctly wired up so that it appears in the output
of query-command-line-options.
Reported-by: Boris Fiuczynski
Fixes: c891c24b1a ("os-posix: asynchronous teardown for shutdown on Linux")
Signed-off-by: Claudio Imbrenda
---
os-posix.c| 15 +--
qemu-options.hx | 33 +++--
util/async-teardown.c | 21 +
3 files changed, 53 insertions(+), 16 deletions(-)
diff --git a/os-posix.c b/os-posix.c
index 5adc69f560..c1ca7b1cb3 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -36,6 +36,8 @@
#include "qemu/log.h"
#include "sysemu/runstate.h"
#include "qemu/cutils.h"
+#include "qemu/config-file.h"
+#include "qemu/option.h"
#ifdef CONFIG_LINUX
#include
@@ -132,6 +134,8 @@ static bool os_parse_runas_uid_gid(const char *optarg)
*/
int os_parse_cmd_args(int index, const char *optarg)
{
+QemuOpts *opts;
+
switch (index) {
case QEMU_OPTION_runas:
user_pwd = getpwnam(optarg);
@@ -152,8 +156,15 @@ int os_parse_cmd_args(int index, const char *optarg)
daemonize = 1;
break;
#if defined(CONFIG_LINUX)
-case QEMU_OPTION_asyncteardown:
-init_async_teardown();
+case QEMU_OPTION_teardown:
+opts = qemu_opts_parse_noisily(qemu_find_opts("teardown"),
+ optarg, false);
+if (!opts) {
+exit(1);
+}
+if (qemu_opt_get_bool(opts, "async", false)) {
+init_async_teardown();
+}
break;
#endif
default:
diff --git a/qemu-options.hx b/qemu-options.hx
index d42f60fb91..6a69b84f3c 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4763,23 +4763,28 @@ DEF("qtest", HAS_ARG, QEMU_OPTION_qtest, "",
QEMU_ARCH_ALL)
DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log, "", QEMU_ARCH_ALL)
#ifdef __linux__
-DEF("async-teardown", 0, QEMU_OPTION_asyncteardown,
-"-async-teardown enable asynchronous teardown\n",
+DEF("teardown", HAS_ARG, QEMU_OPTION_teardown,
+"-teardown async[=on|off]\n"
+"process teardown options\n"
+"async=on enables asynchronous teardown\n"
+ ,
QEMU_ARCH_ALL)
-#endif
SRST
-``-async-teardown``
-Enable asynchronous teardown. A new process called "cleanup/"
-will be created at startup sharing the address space with the main qemu
-process, using clone. It will wait for the main qemu process to
-terminate completely, and then exit.
-This allows qemu to terminate very quickly even if the guest was
-huge, leaving the teardown of the address space to the cleanup
-process. Since the cleanup process shares the same cgroups as the
-main qemu process, accounting is performed correctly. This only
-works if the cleanup process is not forcefully killed with SIGKILL
-before the main qemu process has terminated completely.
+``-teardown``
+Set process teardown options.
+
+``async=on`` enables asynchronous teardown. A new process called
+"cleanup/" will be created at startup sharing the address
+space with the main QEMU process, using clone. It will wait for the
+main QEMU process to terminate completely, and then exit. This allows
+QEMU to terminate very quickly even if the guest was huge, leaving the
+teardown of the address space to the cleanup process. Since the cleanup
+process shares the same cgroups as the main QEMU process, accounting is
+performed correctly. This only works if the cleanup process is not
+forcefully killed with SIGKILL before the main QEMU process has
+terminated completely.
ERST
+#endif
DEF("msg", HAS_ARG, QEMU_OPTION_msg,
"-msg [timestamp[=on|off]][,guest-name=[on|off]]\n"
diff --git a/util/async-teardown.c b/util/async-teardown.c
index 62cdeb0f20..4a5dbce958 100644
--- a/util/async-teardown.c
+++ b/util/async-teardown.c
@@ -12,6 +12,9 @@
*/
#include "qemu/osdep.h"
+#include "qemu/config-file.h"
+#include "qemu/option.h"
+#include "qemu/module.h"
#include
#include
#include
@@ -144,3 +147,21 @@ void init_async_teardown(void)
clone(async_teardown_fn, new_stack_for_clone(), CLONE_VM, NULL);
sigprocmask(SIG_SETMASK, _signals, NULL);
}
+
+static QemuOptsList qemu_teardown_opts = {
+.name = "teardown",
+.head = QTAILQ_HEAD_INITIALIZER(qemu_teardown_opts.head),
+.desc = {
+{
+.name = "async",
+.type = QEMU_OPT_BOOL,
+},
+{ /* end of list */ }
+},
+};
+
+static void register_teardown(void)
+{
+qemu_add_opts(_teardown_opts);
+}
+opts_init(register_teardown);
[PATCH v4 0/1] util/async-teardown: wire up query-command-line-options
The recently introduced -async-teardown commandline option was not wired up properly and did not show up in the output of the QMP command query-command-line-options. This means that libvirt will have no way to discover whether the feature is supported. This patch fixes the issue by adding a new -teardown commandline option with an async=on|off parameter, correctly wired up so that it appears in the output of query-command-line-options. v3->v4 * completely remove the useless -async-teardown option, since it was not wired up properly and it had no users [thomas] * QEMU should be always uppercase in text and documentation [thomas] * if the new -teardown option fails to parse, exit immediately instead of returning an error [thomas] v2->v3 * add a new teardown option with an async parameter [Markus] * reworded documentation of existing -async-teardown option so that it points to the new teardown option v1->v2 * remove the unneeded .implied_opt_name initializer [Thomas] Claudio Imbrenda (1): util/async-teardown: wire up query-command-line-options os-posix.c| 15 +-- qemu-options.hx | 33 +++-- util/async-teardown.c | 21 + 3 files changed, 53 insertions(+), 16 deletions(-) -- 2.39.2
Re: [PATCH for-8.0] block/export: Fix graph locking in blk_get_geometry() call
Am 27/03/2023 um 13:39 schrieb Kevin Wolf:
> blk_get_geometry() eventually calls bdrv_nb_sectors(), which is a
> co_wrapper_mixed_bdrv_rdlock. This means that when it is called from
> coroutine context, it already assume to have the graph locked.
>
> However, virtio_blk_sect_range_ok() in block/export/virtio-blk-handler.c
> (used by vhost-user-blk and VDUSE exports) runs in a coroutine, but
> doesn't take the graph lock - blk_*() functions are generally expected
> to do that internally. This causes an assertion failure when accessing
> an export for the first time if it runs in an iothread.
>
> This is an example of the crash:
>
> $ ./storage-daemon/qemu-storage-daemon --object iothread,id=th0 --blockdev
> file,filename=/home/kwolf/images/hd.img,node-name=disk --export
> vhost-user-blk,addr.type=unix,addr.path=/tmp/vhost.sock,node-name=disk,id=exp0,iothread=th0
> qemu-storage-daemon: ../block/graph-lock.c:268: void
> assert_bdrv_graph_readable(void): Assertion `qemu_in_main_thread() ||
> reader_count()' failed.
>
> (gdb) bt
>
> Fix this by creating a new blk_co_get_geometry() that takes the lock,
> and changing blk_get_geometry() to be a co_wrapper_mixed around it.
>
> To make the resulting code cleaner, virtio-blk-handler.c can directly
> call the coroutine version now (though that wouldn't be necessary for
> fixing the bug, taking the lock in blk_co_get_geometry() is what fixes
> it).
>
> Fixes: 8ab8140a04cf771d63e9754d6ba6c1e676bfe507
> Reported-by: Lukáš Doktor
> Signed-off-by: Kevin Wolf
Reviewed-by: Emanuele Giuseppe Esposito
> ---
> include/block/block-io.h | 4 +++-
> include/sysemu/block-backend-io.h | 5 -
> block.c | 5 +++--
> block/block-backend.c | 7 +--
> block/export/virtio-blk-handler.c | 7 ---
> 5 files changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/include/block/block-io.h b/include/block/block-io.h
> index 5da99d4d60..dbc034b728 100644
> --- a/include/block/block-io.h
> +++ b/include/block/block-io.h
> @@ -89,7 +89,9 @@ int64_t co_wrapper
> bdrv_get_allocated_file_size(BlockDriverState *bs);
>
> BlockMeasureInfo *bdrv_measure(BlockDriver *drv, QemuOpts *opts,
> BlockDriverState *in_bs, Error **errp);
> -void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr);
> +
> +void coroutine_fn GRAPH_RDLOCK
> +bdrv_co_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr);
>
> int coroutine_fn GRAPH_RDLOCK
> bdrv_co_delete_file(BlockDriverState *bs, Error **errp);
> diff --git a/include/sysemu/block-backend-io.h
> b/include/sysemu/block-backend-io.h
> index 40ab178719..c672b77247 100644
> --- a/include/sysemu/block-backend-io.h
> +++ b/include/sysemu/block-backend-io.h
> @@ -70,7 +70,10 @@ void co_wrapper blk_eject(BlockBackend *blk, bool
> eject_flag);
> int64_t coroutine_fn blk_co_getlength(BlockBackend *blk);
> int64_t co_wrapper_mixed blk_getlength(BlockBackend *blk);
>
> -void blk_get_geometry(BlockBackend *blk, uint64_t *nb_sectors_ptr);
> +void coroutine_fn blk_co_get_geometry(BlockBackend *blk,
> + uint64_t *nb_sectors_ptr);
> +void co_wrapper_mixed blk_get_geometry(BlockBackend *blk,
> + uint64_t *nb_sectors_ptr);
>
> int64_t coroutine_fn blk_co_nb_sectors(BlockBackend *blk);
> int64_t co_wrapper_mixed blk_nb_sectors(BlockBackend *blk);
> diff --git a/block.c b/block.c
> index 0dd604d0f6..e0c6c648b1 100644
> --- a/block.c
> +++ b/block.c
> @@ -5879,9 +5879,10 @@ int64_t coroutine_fn
> bdrv_co_getlength(BlockDriverState *bs)
> }
>
> /* return 0 as number of sectors if no device present or error */
> -void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr)
> +void coroutine_fn bdrv_co_get_geometry(BlockDriverState *bs,
> + uint64_t *nb_sectors_ptr)
> {
> -int64_t nb_sectors = bdrv_nb_sectors(bs);
> +int64_t nb_sectors = bdrv_co_nb_sectors(bs);
> IO_CODE();
>
> *nb_sectors_ptr = nb_sectors < 0 ? 0 : nb_sectors;
> diff --git a/block/block-backend.c b/block/block-backend.c
> index 278b04ce69..2ee39229e4 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -1615,13 +1615,16 @@ int64_t coroutine_fn blk_co_getlength(BlockBackend
> *blk)
> return bdrv_co_getlength(blk_bs(blk));
> }
>
> -void blk_get_geometry(BlockBackend *blk, uint64_t *nb_sectors_ptr)
> +void coroutine_fn blk_co_get_geometry(BlockBackend *blk,
> + uint64_t *nb_sectors_ptr)
> {
> IO_CODE();
> +GRAPH_RDLOCK_GUARD();
> +
> if (!blk_bs(blk)) {
> *nb_sectors_ptr = 0;
> } else {
> -bdrv_get_geometry(blk_bs(blk), nb_sectors_ptr);
> +bdrv_co_get_geometry(blk_bs(blk), nb_sectors_ptr);
> }
> }
>
> diff --git a/block/export/virtio-blk-handler.c
> b/block/export/virtio-blk-handler.c
> index
Re: [PATCH] Change the default for Mixed declarations.
On Fri, Mar 24, 2023 at 05:56:46PM +, Alex Bennée wrote: > > Juan Quintela writes: > > > Daniel P. Berrangé wrote: > >> On Tue, Feb 14, 2023 at 05:07:38PM +0100, Juan Quintela wrote: > >>> Hi > >>> > >>> I want to enter a discussion about changing the default of the style > >>> guide. > >>> > >>> There are several reasons for that: > >>> - they exist since C99 (i.e. all supported compilers support them) > >>> - they eliminate the posibility of an unitialized variable. > >> > >> Actually they don't do that reliably. In fact, when combined > >> with usage of 'goto', they introduce uninitialized variables, > >> despite the declaration having an initialization present, and > >> thus actively mislead reviewers into thinking their code is > >> safe. > > > > Wait a minute. > > If you use goto, you are already in special rules. > > > > And don't get confused, I fully agree when using goto for two reasons: > > - performance > > if you show that the code is x% faster when using goto, it is > > justified. It is even better if you send a bug report to gcc/clang, > > but I will not opose that use. > > I await a clear example in the context of QEMU - there is almost always > a better way to structure things. > > > - code clearity > > Some code (basically error paths) are clearer with goto that without > > them. > > Now we have g_auto* and lock guards we should encourage their use. goto > error_path is a relic of a simpler time ;-) > > > >> IMHO if we are concerned about uninitialized variables then I think > >> a better approach is to add -ftrivial-auto-var-init=zero, which will > >> make the compiler initialize all variables to 0 if they lack an > >> explicit initializer. > > > > I think this is a bad idea. > > If we want to "catch" unitialized variables, using something like: > > > > -ftrivial-auto-var-init=pattern sounds much saner. > > > > Obviously gcc is missing > > > > -ftrivial-auto-var-init=42 > > I think we could at least eat the runtime cost of > -ftrvial-auto-var-init=0xDEADBEEF for our --enable-debug builds. If there is ever a case where an uninitialized var gets used as a loop counter, that's 3,735,928,559 iterations. A small value pattern would avoid such CPU burn. With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH] meson: install keyboard maps only if necessary
On Sun, Mar 26, 2023 at 06:04:27PM -0300, casan...@redhat.com wrote:
> From: Carlos Santos
>
> They are required only for system emulation (i.e. have_system is true).
>
> Signed-off-by: Carlos Santos
> ---
> pc-bios/keymaps/meson.build | 6 --
> scripts/meson-buildoptions.sh | 2 ++
> tests/fp/berkeley-testfloat-3 | 2 +-
> ui/keycodemapdb | 2 +-
You've got some git submodule updates included by accident
here.
> 4 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/pc-bios/keymaps/meson.build b/pc-bios/keymaps/meson.build
> index 158a3b410c..bff3083313 100644
> --- a/pc-bios/keymaps/meson.build
> +++ b/pc-bios/keymaps/meson.build
> @@ -47,7 +47,7 @@ if native_qemu_keymap.found()
> build_by_default: true,
> output: km,
> command: [native_qemu_keymap, '-f', '@OUTPUT@',
> args.split()],
> - install: true,
> + install: have_system,
> install_dir: qemu_datadir / 'keymaps')
>endforeach
>
> @@ -56,4 +56,6 @@ else
>install_data(keymaps.keys(), install_dir: qemu_datadir / 'keymaps')
> endif
>
> -install_data(['sl', 'sv'], install_dir: qemu_datadir / 'keymaps')
> +if have_system
> + install_data(['sl', 'sv'], install_dir: qemu_datadir / 'keymaps')
> +endif
> diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
> index 009fab1515..6eec7bc57f 100644
> --- a/scripts/meson-buildoptions.sh
> +++ b/scripts/meson-buildoptions.sh
> @@ -301,6 +301,8 @@ _meson_option_parse() {
> --includedir=*) quote_sh "-Dincludedir=$2" ;;
> --enable-install-blobs) printf "%s" -Dinstall_blobs=true ;;
> --disable-install-blobs) printf "%s" -Dinstall_blobs=false ;;
> +--enable-install-keymaps) printf "%s" -Dinstall_keymaps=true ;;
> +--disable-install-keymaps) printf "%s" -Dinstall_keymaps=false ;;
> --interp-prefix=*) quote_sh "-Dinterp_prefix=$2" ;;
> --enable-jack) printf "%s" -Djack=enabled ;;
> --disable-jack) printf "%s" -Djack=disabled ;;
> diff --git a/tests/fp/berkeley-testfloat-3 b/tests/fp/berkeley-testfloat-3
> index 40619cbb3b..5a59dcec19 16
> --- a/tests/fp/berkeley-testfloat-3
> +++ b/tests/fp/berkeley-testfloat-3
> @@ -1 +1 @@
> -Subproject commit 40619cbb3bf32872df8c53cc457039229428a263
> +Subproject commit 5a59dcec19327396a011a17fd924aed4fec416b3
> diff --git a/ui/keycodemapdb b/ui/keycodemapdb
> index f5772a62ec..d21009b1c9 16
> --- a/ui/keycodemapdb
> +++ b/ui/keycodemapdb
> @@ -1 +1 @@
> -Subproject commit f5772a62ec52591ff6870b7e8ef32482371f22c6
> +Subproject commit d21009b1c9f94b740ea66be8e48a1d8ad8124023
> --
> 2.31.1
>
>
With regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH V2] meson: install keyboard maps only if necessary
On Sun, Mar 26, 2023 at 06:17:00PM -0300, casan...@redhat.com wrote: > From: Carlos Santos > > They are required only for system emulation (i.e. have_system is true). > > Signed-off-by: Carlos Santos > --- > pc-bios/keymaps/meson.build | 6 -- > tests/fp/berkeley-testfloat-3 | 2 +- > ui/keycodemapdb | 2 +- This still has the accidental git submodule updates included > 3 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/pc-bios/keymaps/meson.build b/pc-bios/keymaps/meson.build > index 158a3b410c..bff3083313 100644 > --- a/pc-bios/keymaps/meson.build > +++ b/pc-bios/keymaps/meson.build > @@ -47,7 +47,7 @@ if native_qemu_keymap.found() > build_by_default: true, > output: km, > command: [native_qemu_keymap, '-f', '@OUTPUT@', > args.split()], > - install: true, > + install: have_system, > install_dir: qemu_datadir / 'keymaps') >endforeach > > @@ -56,4 +56,6 @@ else >install_data(keymaps.keys(), install_dir: qemu_datadir / 'keymaps') > endif > > -install_data(['sl', 'sv'], install_dir: qemu_datadir / 'keymaps') > +if have_system > + install_data(['sl', 'sv'], install_dir: qemu_datadir / 'keymaps') > +endif > diff --git a/tests/fp/berkeley-testfloat-3 b/tests/fp/berkeley-testfloat-3 > index 40619cbb3b..5a59dcec19 16 > --- a/tests/fp/berkeley-testfloat-3 > +++ b/tests/fp/berkeley-testfloat-3 > @@ -1 +1 @@ > -Subproject commit 40619cbb3bf32872df8c53cc457039229428a263 > +Subproject commit 5a59dcec19327396a011a17fd924aed4fec416b3 > diff --git a/ui/keycodemapdb b/ui/keycodemapdb > index f5772a62ec..d21009b1c9 16 > --- a/ui/keycodemapdb > +++ b/ui/keycodemapdb > @@ -1 +1 @@ > -Subproject commit f5772a62ec52591ff6870b7e8ef32482371f22c6 > +Subproject commit d21009b1c9f94b740ea66be8e48a1d8ad8124023 > -- > 2.31.1 > > With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH] tracing: install trace events file only if necessary
On Sun, Mar 26, 2023 at 06:04:46PM -0300, casan...@redhat.com wrote:
> From: Carlos Santos
>
> It is required only if linux-user, bsd-user or system emulator is built.
>
> Signed-off-by: Carlos Santos
> ---
> trace/meson.build | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/trace/meson.build b/trace/meson.build
> index 8e80be895c..3fb41c97a4 100644
> --- a/trace/meson.build
> +++ b/trace/meson.build
> @@ -64,7 +64,7 @@ trace_events_all = custom_target('trace-events-all',
> input: trace_events_files,
> command: [ 'cat', '@INPUT@' ],
> capture: true,
> - install: true,
> + install: have_linux_user or have_bsd_user
> or have_system,
Trace events are used by our command line tools too qemu-img, qemu-io,
qemu-nbd, qemu-pr-helper, qemu-storage-daemon.
What build scenario are you seeing that does NOT want the trace events
to be present ? If there is any, then I might even call that situation
a bug, as we want trace events to be available as a debugging mechanism
for everything we build.
With regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v7 0/3] Add support for TPM devices over I2C bus
On Mon, 27 Mar 2023 at 11:11, Stefan Berger wrote: > > > > On 3/26/23 21:05, Joel Stanley wrote: > > Hi Ninad, > > > > On Sun, 26 Mar 2023 at 22:44, Ninad Palsule wrote: > >> > >> Hello, > >> > >> I have incorporated review comments from Stefan. Please review. > >> > >> This drop adds support for the TPM devices attached to the I2C bus. It > >> only supports the TPM2 protocol. You need to run it with the external > >> TPM emulator like swtpm. I have tested it with swtpm. > > > > Nice work. I tested these stop cedric's aspeed-8.0 qemu tree, using > > the rainier machine and the openbmc dev-6.1 kernel. > > > > We get this message when booting from a kernel: > > > > [0.582699] tpm_tis_i2c 12-002e: 2.0 TPM (device-id 0x1, rev-id 1) > > [0.586361] tpm tpm0: A TPM error (256) occurred attempting the self test > > [0.586623] tpm tpm0: starting up the TPM manually > > > > Do we understand why the error appears? > > The firmware did not initialize the TPM 2. Which firmware are we talking about here? In the case of these systems, we (u-boot+linux) are what would traditionally be referred to as firmware. > > # grep -r . /sys/class/tpm/tpm0/pcr-sha256/ | sort -n -k 7 -t / > > /sys/class/tpm/tpm0/pcr-sha256/0: > > /sys/class/tpm/tpm0/pcr-sha256/1: > > /sys/class/tpm/tpm0/pcr-sha256/2: > > /sys/class/tpm/tpm0/pcr-sha256/3: > > /sys/class/tpm/tpm0/pcr-sha256/4: > > /sys/class/tpm/tpm0/pcr-sha256/5: > > /sys/class/tpm/tpm0/pcr-sha256/6: > > /sys/class/tpm/tpm0/pcr-sha256/7: > > /sys/class/tpm/tpm0/pcr-sha256/8: > > /sys/class/tpm/tpm0/pcr-sha256/9: > > /sys/class/tpm/tpm0/pcr-sha256/10: > > /sys/class/tpm/tpm0/pcr-sha256/11: > > /sys/class/tpm/tpm0/pcr-sha256/12: > > /sys/class/tpm/tpm0/pcr-sha256/13: > > /sys/class/tpm/tpm0/pcr-sha256/14: > > /sys/class/tpm/tpm0/pcr-sha256/15: > > /sys/class/tpm/tpm0/pcr-sha256/16: > > /sys/class/tpm/tpm0/pcr-sha256/17: > > /sys/class/tpm/tpm0/pcr-sha256/18: > > /sys/class/tpm/tpm0/pcr-sha256/19: > > /sys/class/tpm/tpm0/pcr-sha256/20: > > /sys/class/tpm/tpm0/pcr-sha256/21: > > /sys/class/tpm/tpm0/pcr-sha256/22: > > /sys/class/tpm/tpm0/pcr-sha256/23: > > > > If I boot through the openbmc u-boot for the p10bmc machine, which > > measures things into the PCRs: > > > > [0.556713] tpm_tis_i2c 12-002e: 2.0 TPM (device-id 0x1, rev-id 1) > > In this case the firmware started up the TPM 2. Also the PCRs have been > touched by the firmware in this case. > > > > > / # grep -r . /sys/class/tpm/tpm0/pcr-sha256/ | sort -n -k 7 -t / > > /sys/class/tpm/tpm0/pcr-sha256/0:AFA13691EFC7BC6E189E92347F20676FB4523302CB957DA9A65C3430C45E8BCC > > /sys/class/tpm/tpm0/pcr-sha256/1:37F0F710A5502FAE6DB7433B36001FEE1CBF15BA2A7D6923207FF56888584714 > > /sys/class/tpm/tpm0/pcr-sha256/2:E21B703EE69C77476BCCB43EC0336A9A1B2914B378944F7B00A10214CA8FEA93 > > /sys/class/tpm/tpm0/pcr-sha256/3:E21B703EE69C77476BCCB43EC0336A9A1B2914B378944F7B00A10214CA8FEA93 > > /sys/class/tpm/tpm0/pcr-sha256/4:E21B703EE69C77476BCCB43EC0336A9A1B2914B378944F7B00A10214CA8FEA93 > > /sys/class/tpm/tpm0/pcr-sha256/5:E21B703EE69C77476BCCB43EC0336A9A1B2914B378944F7B00A10214CA8FEA93 > > /sys/class/tpm/tpm0/pcr-sha256/6:E21B703EE69C77476BCCB43EC0336A9A1B2914B378944F7B00A10214CA8FEA93 > > /sys/class/tpm/tpm0/pcr-sha256/7:E21B703EE69C77476BCCB43EC0336A9A1B2914B378944F7B00A10214CA8FEA93 > > /sys/class/tpm/tpm0/pcr-sha256/8:AE67485BD01E8D6FE0208C46C473940173F66E9C6F43C75ABB404375787E9705 > >
[PATCH] hw/loongarch/virt: Fix virt_to_phys_addr function
The virt addr should mask TARGET_PHYS_ADDR_SPACE_BITS to
get the phys addr, and this is used by loading kernel elf.
Signed-off-by: Tianrui Zhao
---
hw/loongarch/virt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index b702c3f51e..f4bf14c1c8 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -399,7 +399,7 @@ static struct _loaderparams {
static uint64_t cpu_loongarch_virt_to_phys(void *opaque, uint64_t addr)
{
-return addr & 0x1fffll;
+return addr & MAKE_64BIT_MASK(0, TARGET_PHYS_ADDR_SPACE_BITS);
}
static int64_t load_kernel_info(void)
--
2.31.1
[PATCH for-8.0] block/export: Fix graph locking in blk_get_geometry() call
blk_get_geometry() eventually calls bdrv_nb_sectors(), which is a
co_wrapper_mixed_bdrv_rdlock. This means that when it is called from
coroutine context, it already assume to have the graph locked.
However, virtio_blk_sect_range_ok() in block/export/virtio-blk-handler.c
(used by vhost-user-blk and VDUSE exports) runs in a coroutine, but
doesn't take the graph lock - blk_*() functions are generally expected
to do that internally. This causes an assertion failure when accessing
an export for the first time if it runs in an iothread.
This is an example of the crash:
$ ./storage-daemon/qemu-storage-daemon --object iothread,id=th0 --blockdev
file,filename=/home/kwolf/images/hd.img,node-name=disk --export
vhost-user-blk,addr.type=unix,addr.path=/tmp/vhost.sock,node-name=disk,id=exp0,iothread=th0
qemu-storage-daemon: ../block/graph-lock.c:268: void
assert_bdrv_graph_readable(void): Assertion `qemu_in_main_thread() ||
reader_count()' failed.
(gdb) bt
Fix this by creating a new blk_co_get_geometry() that takes the lock,
and changing blk_get_geometry() to be a co_wrapper_mixed around it.
To make the resulting code cleaner, virtio-blk-handler.c can directly
call the coroutine version now (though that wouldn't be necessary for
fixing the bug, taking the lock in blk_co_get_geometry() is what fixes
it).
Fixes: 8ab8140a04cf771d63e9754d6ba6c1e676bfe507
Reported-by: Lukáš Doktor
Signed-off-by: Kevin Wolf
---
include/block/block-io.h | 4 +++-
include/sysemu/block-backend-io.h | 5 -
block.c | 5 +++--
block/block-backend.c | 7 +--
block/export/virtio-blk-handler.c | 7 ---
5 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/include/block/block-io.h b/include/block/block-io.h
index 5da99d4d60..dbc034b728 100644
--- a/include/block/block-io.h
+++ b/include/block/block-io.h
@@ -89,7 +89,9 @@ int64_t co_wrapper
bdrv_get_allocated_file_size(BlockDriverState *bs);
BlockMeasureInfo *bdrv_measure(BlockDriver *drv, QemuOpts *opts,
BlockDriverState *in_bs, Error **errp);
-void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr);
+
+void coroutine_fn GRAPH_RDLOCK
+bdrv_co_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr);
int coroutine_fn GRAPH_RDLOCK
bdrv_co_delete_file(BlockDriverState *bs, Error **errp);
diff --git a/include/sysemu/block-backend-io.h
b/include/sysemu/block-backend-io.h
index 40ab178719..c672b77247 100644
--- a/include/sysemu/block-backend-io.h
+++ b/include/sysemu/block-backend-io.h
@@ -70,7 +70,10 @@ void co_wrapper blk_eject(BlockBackend *blk, bool
eject_flag);
int64_t coroutine_fn blk_co_getlength(BlockBackend *blk);
int64_t co_wrapper_mixed blk_getlength(BlockBackend *blk);
-void blk_get_geometry(BlockBackend *blk, uint64_t *nb_sectors_ptr);
+void coroutine_fn blk_co_get_geometry(BlockBackend *blk,
+ uint64_t *nb_sectors_ptr);
+void co_wrapper_mixed blk_get_geometry(BlockBackend *blk,
+ uint64_t *nb_sectors_ptr);
int64_t coroutine_fn blk_co_nb_sectors(BlockBackend *blk);
int64_t co_wrapper_mixed blk_nb_sectors(BlockBackend *blk);
diff --git a/block.c b/block.c
index 0dd604d0f6..e0c6c648b1 100644
--- a/block.c
+++ b/block.c
@@ -5879,9 +5879,10 @@ int64_t coroutine_fn bdrv_co_getlength(BlockDriverState
*bs)
}
/* return 0 as number of sectors if no device present or error */
-void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr)
+void coroutine_fn bdrv_co_get_geometry(BlockDriverState *bs,
+ uint64_t *nb_sectors_ptr)
{
-int64_t nb_sectors = bdrv_nb_sectors(bs);
+int64_t nb_sectors = bdrv_co_nb_sectors(bs);
IO_CODE();
*nb_sectors_ptr = nb_sectors < 0 ? 0 : nb_sectors;
diff --git a/block/block-backend.c b/block/block-backend.c
index 278b04ce69..2ee39229e4 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1615,13 +1615,16 @@ int64_t coroutine_fn blk_co_getlength(BlockBackend *blk)
return bdrv_co_getlength(blk_bs(blk));
}
-void blk_get_geometry(BlockBackend *blk, uint64_t *nb_sectors_ptr)
+void coroutine_fn blk_co_get_geometry(BlockBackend *blk,
+ uint64_t *nb_sectors_ptr)
{
IO_CODE();
+GRAPH_RDLOCK_GUARD();
+
if (!blk_bs(blk)) {
*nb_sectors_ptr = 0;
} else {
-bdrv_get_geometry(blk_bs(blk), nb_sectors_ptr);
+bdrv_co_get_geometry(blk_bs(blk), nb_sectors_ptr);
}
}
diff --git a/block/export/virtio-blk-handler.c
b/block/export/virtio-blk-handler.c
index 313666e8ab..bc1cec6757 100644
--- a/block/export/virtio-blk-handler.c
+++ b/block/export/virtio-blk-handler.c
@@ -22,8 +22,9 @@ struct virtio_blk_inhdr {
unsigned char status;
};
-static bool virtio_blk_sect_range_ok(BlockBackend *blk, uint32_t block_size,
- uint64_t sector, size_t size)
+static bool
[PATCH 1/2] tests/requirements.txt: bump up avocado-framework version to 101.0
Avocado version 101.0 has a fix to re-compute the checksum of an asset file if the algorithm used in the *-CHECKSUM file isn't the same as the one being passed to it by the avocado user (i.e. the avocado_qemu python module). In the earlier avocado versions this fix wasn't there due to which if the checksum wouldn't match the earlier checksum (calculated by a different algorithm), the avocado code would start downloading a fresh image from the internet URL thus making the test-cases take longer to execute. Bump up the avocado-framework version to 101.0. Signed-off-by: Kautuk Consul Tested-by: Hariharan T S --- tests/requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/requirements.txt b/tests/requirements.txt index 0ba561b6bd..a6f73da681 100644 --- a/tests/requirements.txt +++ b/tests/requirements.txt @@ -2,5 +2,5 @@ # in the tests/venv Python virtual environment. For more info, # refer to: https://pip.pypa.io/en/stable/user_guide/#id1 # Note that qemu.git/python/ is always implicitly installed. -avocado-framework==88.1 +avocado-framework==101.0 pycdlib==1.11.0 -- 2.39.2
