Re: [SPAM] [PATCH v2 1/4] docs: aspeed: Add new boards
On 11/17/21 07:57, Joel Stanley wrote: Add X11, FP5280G2, G220A, Rainier and Fuji. Mention that Swift will be removed in v7.0. Signed-off-by: Joel Stanley Reviewed-by: Cédric Le Goater Thanks C. --- v2: - Add POWER10 to Rainier description - Include Fuji - Mention Swift to be removed --- docs/system/arm/aspeed.rst | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst index cec87e3743d0..41a9bd5608e8 100644 --- a/docs/system/arm/aspeed.rst +++ b/docs/system/arm/aspeed.rst @@ -14,6 +14,7 @@ AST2400 SoC based machines : - ``palmetto-bmc`` OpenPOWER Palmetto POWER8 BMC - ``quanta-q71l-bmc`` OpenBMC Quanta BMC +- ``supermicrox11-bmc``Supermicro X11 BMC AST2500 SoC based machines : @@ -21,12 +22,16 @@ AST2500 SoC based machines : - ``romulus-bmc`` OpenPOWER Romulus POWER9 BMC - ``witherspoon-bmc`` OpenPOWER Witherspoon POWER9 BMC - ``sonorapass-bmc`` OCP SonoraPass BMC -- ``swift-bmc``OpenPOWER Swift BMC POWER9 +- ``swift-bmc``OpenPOWER Swift BMC POWER9 (to be removed in v7.0) +- ``fp5280g2-bmc`` Inspur FP5280G2 BMC +- ``g220a-bmc``Bytedance G220A BMC AST2600 SoC based machines : - ``ast2600-evb`` Aspeed AST2600 Evaluation board (Cortex-A7) - ``tacoma-bmc`` OpenPOWER Witherspoon POWER9 AST2600 BMC +- ``rainier-bmc`` IBM Rainier POWER10 BMC +- ``fuji-bmc`` Facebook Fuji BMC Supported devices -
Re: [PATCH v3] s390: kvm: adjust diag318 resets to retain data
Am 09.11.21 um 21:56 schrieb Collin Walling:
The CPNC portion of the diag 318 data is erroneously reset during an
initial CPU reset caused by SIGP. Let's go ahead and relocate the
diag318_info field within the CPUS390XState struct such that it is
only zeroed during a clear reset. This way, the CPNC will be retained
for each VCPU in the configuration after the diag 318 instruction
has been invoked.
Signed-off-by: Collin Walling
Fixes: fabdada9357b ("s390: guest support for diagnose 0x318")
Reported-by: Christian Borntraeger
Reviewed-by: Christian Borntraeger
maybe add cc stable just in case there will be one.
Can you resend with the final patch description and add Thomas as TO (not cc)
as this should probably go via Thomas tree.
---
Changelog:
v2
- handler uses run_on_cpu again
- reworded commit message slightly
- added fixes and reported-by tags
v3
- nixed code reduction changes
- added a comment to diag318 handler to briefly describe
when relevent data is zeroed
---
target/s390x/cpu.h | 4 ++--
target/s390x/kvm/kvm.c | 4
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 3153d053e9..88aace36ff 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -63,6 +63,8 @@ struct CPUS390XState {
uint64_t etoken; /* etoken */
uint64_t etoken_extension; /* etoken extension */
+uint64_t diag318_info;
+
/* Fields up to this point are not cleared by initial CPU reset */
struct {} start_initial_reset_fields;
@@ -118,8 +120,6 @@ struct CPUS390XState {
uint16_t external_call_addr;
DECLARE_BITMAP(emergency_signals, S390_MAX_CPUS);
-uint64_t diag318_info;
-
#if !defined(CONFIG_USER_ONLY)
uint64_t tlb_fill_tec; /* translation exception code during tlb_fill */
int tlb_fill_exc;/* exception number seen during tlb_fill */
diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
index 5b1fdb55c4..6acf14d5ec 100644
--- a/target/s390x/kvm/kvm.c
+++ b/target/s390x/kvm/kvm.c
@@ -1585,6 +1585,10 @@ void kvm_s390_set_diag318(CPUState *cs, uint64_t
diag318_info)
env->diag318_info = diag318_info;
cs->kvm_run->s.regs.diag318 = diag318_info;
cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_DIAG318;
+/*
+ * diag 318 info is zeroed during a clear reset and
+ * diag 308 IPL subcodes.
+ */
}
}
Re: [PATCH-for-7.0 v4 08/11] tests/unit/test-smp-parse: Add 'smp-without-dies-valid' machine type
Hi Philippe,
On 2021/11/15 22:58, Philippe Mathieu-Daudé wrote:
Keep the common TYPE_MACHINE class initialization in
machine_base_class_init(), make it abstract, and move
the non-common code to a new class: "smp-without-dies-valid".
Signed-off-by: Philippe Mathieu-Daudé
---
tests/unit/test-smp-parse.c | 19 +++
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index dfe7f1313b0..90a249fe8c8 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -478,13 +478,19 @@ static void machine_base_class_init(ObjectClass *oc, void
*data)
{
MachineClass *mc = MACHINE_CLASS(oc);
+mc->smp_props.prefer_sockets = true;
+
+mc->name = g_strdup(SMP_MACHINE_NAME);
+}
+
+static void machine_without_dies_valid_class_init(ObjectClass *oc, void *data)
+{
+MachineClass *mc = MACHINE_CLASS(oc);
+
mc->min_cpus = MIN_CPUS;
mc->max_cpus = MAX_CPUS;
-mc->smp_props.prefer_sockets = true;
mc->smp_props.dies_supported = false;
-
-mc->name = g_strdup(SMP_MACHINE_NAME);
}
static void machine_without_dies_invalid_class_init(ObjectClass *oc, void *data)
@@ -606,9 +612,14 @@ static const TypeInfo smp_machine_types[] = {
{
.name = TYPE_MACHINE,
.parent = TYPE_OBJECT,
+.abstract = true,
.class_init = machine_base_class_init,
.class_size = sizeof(MachineClass),
.instance_size = sizeof(MachineState),
+}, {
+.name = MACHINE_TYPE_NAME("smp-without-dies-valid"),
+.parent = TYPE_MACHINE,
+.class_init = machine_without_dies_valid_class_init,
}, {
.name = MACHINE_TYPE_NAME("smp-without-dies-invalid"),
.parent = TYPE_MACHINE,
@@ -629,7 +640,7 @@ int main(int argc, char *argv[])
g_test_init(, , NULL);
g_test_add_data_func("/test-smp-parse/generic/valid",
- TYPE_MACHINE,
+ MACHINE_TYPE_NAME("smp-without-dies-valid"),
test_generic_valid);
g_test_add_data_func("/test-smp-parse/generic/invalid",
MACHINE_TYPE_NAME("smp-without-dies-invalid"),
After patch #7 and #8, we will have sub-tests as below. It looks nice,
but it will
probably be better to tweak "smp-without-dies-valid" to "smp-generic-valid",
and "smp-without-dies-invalid" to "smp-generic-invalid", which will be more
consistent with the corresponding sub-test name.
It's Ok now as we only have dies currently besides generic
sockets/cores/threads,
but "smp-without-dies-xxx" will become a bit confusing when new topology
members are introduced and tested here.
int main(int argc, char *argv[])
{
module_call_init(MODULE_INIT_QOM);
g_test_init(, , NULL);
g_test_add_data_func("/test-smp-parse/generic/valid",
MACHINE_TYPE_NAME("smp-without-dies-valid"),
test_generic_valid);
g_test_add_data_func("/test-smp-parse/generic/invalid",
MACHINE_TYPE_NAME("smp-without-dies-invalid"),
test_generic_invalid);
g_test_add_data_func("/test-smp-parse/with_dies",
MACHINE_TYPE_NAME("smp-with-dies"),
test_with_dies);
g_test_run();
return 0;
}
Otherwise, #7 and #8 look nice. Thanks for your work!
Thanks,
Yanan
Re: [RFC PATCH v2 09/30] target/loongarch: Add TLB instruction support
Hi, Richard:
On 11/12/2021 02:14 AM, Richard Henderson wrote:
> On 11/11/21 2:35 AM, Xiaojuan Yang wrote:
>> +static bool trans_tlbwr(DisasContext *ctx, arg_tlbwr *a)
>> +{
>> +gen_helper_check_plv(cpu_env);
>> +gen_helper_tlbwr(cpu_env);
>> +tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next + 4);
>> +ctx->base.is_jmp = DISAS_EXIT;
>> +return true;
>> +}
>
> I think you can skip the EXIT if paging is disabled, which it usually will be
> in the software tlb handler. You'd be able to tell with the mmu_idx being
> the one you use for paging disabled.
The paging disabled only enabled at the bios startup, we can get the phys
address directly, tlbwr instruction will not be used when paging enabled.
>
>> +static void loongarch_invalidate_tlb_entry(CPULoongArchState *env,
>> + loongarch_tlb *tlb)
>> +{
>> +CPUState *cs = env_cpu(env);
>> +target_ulong addr, end, mask;
>> +int tlb_v0, tlb_v1;
>> +uint64_t tlb_vppn;
>> +uint8_t tlb_ps;
>> +
>> +tlb_v0 = FIELD_EX64(tlb->tlb_entry0, ENTRY0, V);
>> +tlb_v1 = FIELD_EX64(tlb->tlb_entry1, ENTRY1, V);
>> +tlb_vppn = FIELD_EX64(tlb->tlb_misc, TLB_MISC, VPPN);
>> +tlb_ps = FIELD_EX64(tlb->tlb_misc, TLB_MISC, PS);
>> +mask = (1 << (1 + tlb_ps)) - 1;
>
> MAKE_64BIT_MASK.
>
>> +
>> +if (tlb_v0) {
>> +addr = tlb_vppn & ~mask;/* xxx...xxx[0]000.. */
>> +end = addr | (mask >> 1); /* xxx...xxx[0]111.. */
>> +while (addr < end) {
>> +tlb_flush_page(cs, addr);
>> +addr += TARGET_PAGE_SIZE;
>
> tlb_flush_range_by_mmuidx.
>
>> +tlb->tlb_misc = FIELD_DP64(tlb->tlb_misc, TLB_MISC, VPPN, csr_vppn);
>> +tlb->tlb_misc = FIELD_DP64(tlb->tlb_misc, TLB_MISC, E, 1);
>> +csr_asid = FIELD_EX64(env->CSR_ASID, CSR_ASID, ASID);
>> +tlb->tlb_misc = FIELD_DP64(tlb->tlb_misc, TLB_MISC, ASID, csr_asid);
>> +
>> +csr_g = FIELD_EX64(env->CSR_TLBELO0, CSR_TLBELO0, G) &
>> + FIELD_EX64(env->CSR_TLBELO1, CSR_TLBELO1, G);
>> +tlb->tlb_misc = FIELD_DP64(tlb->tlb_misc, TLB_MISC, G, csr_g);
>> +
>> +tlb->tlb_entry0 = FIELD_DP64(tlb->tlb_entry0, ENTRY0, V,
>> + FIELD_EX64(lo0, CSR_TLBELO0, V));/* [0] */
>> +tlb->tlb_entry0 = FIELD_DP64(tlb->tlb_entry0, ENTRY0, D,
>> + FIELD_EX64(lo0, CSR_TLBELO0, D));/* [1] */
>> +tlb->tlb_entry0 = FIELD_DP64(tlb->tlb_entry0, ENTRY0, PLV,
>> + FIELD_EX64(lo0, CSR_TLBELO0, PLV));/*
>> [3:2] */
>> +tlb->tlb_entry0 = FIELD_DP64(tlb->tlb_entry0, ENTRY0, MAT,
>> + FIELD_EX64(lo0, CSR_TLBELO0, MAT));/*
>> [5:4] */
>> +tlb->tlb_entry0 = FIELD_DP64(tlb->tlb_entry0, ENTRY0, PPN,
>> + FIELD_EX64(lo0, CSR_TLBELO0, PPN));/*
>> [47:12] */
>> +tlb->tlb_entry0 = FIELD_DP64(tlb->tlb_entry0, ENTRY0, NR,
>> + FIELD_EX64(lo0, CSR_TLBELO0, NR));/* [61]
>> */
>> +tlb->tlb_entry0 = FIELD_DP64(tlb->tlb_entry0, ENTRY0, NX,
>> + FIELD_EX64(lo0, CSR_TLBELO0, NX));/* [62]
>> */
>> +tlb->tlb_entry0 = FIELD_DP64(tlb->tlb_entry0, ENTRY0, RPLV,
>> + FIELD_EX64(lo0, CSR_TLBELO0, RPLV));/*
>> [63] */
>> +
>> +tlb->tlb_entry1 = FIELD_DP64(tlb->tlb_entry1, ENTRY1, V,
>> + FIELD_EX64(lo1, CSR_TLBELO1, V));/* [0] */
>> +tlb->tlb_entry1 = FIELD_DP64(tlb->tlb_entry1, ENTRY1, D,
>> + FIELD_EX64(lo1, CSR_TLBELO1, D));/* [1] */
>> +tlb->tlb_entry1 = FIELD_DP64(tlb->tlb_entry1, ENTRY1, PLV,
>> + FIELD_EX64(lo1, CSR_TLBELO1, PLV));/*
>> [3:2] */
>> +tlb->tlb_entry1 = FIELD_DP64(tlb->tlb_entry1, ENTRY1, MAT,
>> + FIELD_EX64(lo1, CSR_TLBELO1, MAT));/*
>> [5:4] */
>> +tlb->tlb_entry1 = FIELD_DP64(tlb->tlb_entry1, ENTRY1, PPN,
>> + FIELD_EX64(lo1, CSR_TLBELO1, PPN));/*
>> [47:12] */
>> +tlb->tlb_entry1 = FIELD_DP64(tlb->tlb_entry1, ENTRY1, NR,
>> + FIELD_EX64(lo1, CSR_TLBELO1, NR));/* [61]
>> */
>> +tlb->tlb_entry1 = FIELD_DP64(tlb->tlb_entry1, ENTRY1, NX,
>> + FIELD_EX64(lo1, CSR_TLBELO1, NX));/* [62]
>> */
>> +tlb->tlb_entry1 = FIELD_DP64(tlb->tlb_entry1, ENTRY1, RPLV,
>> + FIELD_EX64(lo1, CSR_TLBELO1, RPLV));/*
>> [63] */
>
> The point of making the two values have the same field layout is so that you
> can just assign the whole value across, not extract and re-deposit each field.
Yes, it is much simpler when use the same field layout.
>
>> +void helper_tlbsrch(CPULoongArchState *env)
>> +{
>> +loongarch_tlb *tlb;
>> +uint64_t vpn, tlb_vppn;
>> +uint16_t csr_asid, tlb_asid, tlb_ps, tlb_e, tlb_g;
>> +
>> +int stlb_size =
Re: [PATCH] linux-user/hexagon: Use generic target_stat64 structure
On 11/16/21 10:09 PM, Philippe Mathieu-Daudé wrote: Linux Hexagon port doesn't define a specific 'struct stat' but uses the generic one (see Linux commit 6103ec56c65c [*] "asm-generic: add generic ABI headers" which predates the introduction of the Hexagon port). Remove the target specific target_stat (which in fact is the target_stat64 structure but uses incorrect target_long and ABI unsafe long long types) and use the generic target_stat64 instead. [*]https://github.com/torvalds/linux/commit/6103ec56c65c3#diff-5f59b07b38273b7d6a74193bc81a8cd18928c688276eae20cb10c569de3253ee Signed-off-by: Philippe Mathieu-Daudé --- linux-user/syscall_defs.h | 28 ++-- 1 file changed, 2 insertions(+), 26 deletions(-) Reviewed-by: Richard Henderson r~
Re: [RFC PATCH v2 07/30] target/loongarch: Add MMU support for LoongArch CPU.
On 11/17/21 7:37 AM, yangxiaojuan wrote: +*physical = (tlb_ppn1 << 12) | (address & ((1 << tlb_ps) - 1)); TARGET_PAGE_SIZE. Maybe TARGET_PAGE_SIZE is not fit for a huge page. MAKE_64BIT_MASK(0, tlb_ps) is ok? I meant the first << 12. But, yes, MAKE_64BIT_MASK is a good improvement. Bit of a shame to have a linear search. I guess it's ok for a start, but you'll find that this function is critical to the emulation speed of qemu, so you should think about other ways to organize the data. The stlb search by the set, the mtlb uses the linear search, I have no other idea, do you have some advice? Well, stlb are all the same page size, and duplicate matches are undefined. A hash table of the valid entries might be workable, with the virtual page and asid taken into account. I'd imagine that would get you down from 2048 comparisons to just a few. The mtlb entries may all have different page size, so I don't immediately know how to search them more efficiently. But there are only 64 of them. It might be worth having a couple of bitsets that summarize the valid entries. This could be used to search less than 64 mtlb entries. It could also be used to influence the "random" selection of a new tlb entry: always select an empty tlb entry first, then evict a random entry. r~
[PATCH v2 4/4] docs: aspeed: ADC is now modelled
Move it to the supported list. Signed-off-by: Joel Stanley --- v2: New patch --- docs/system/arm/aspeed.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst index 6aafd611e9a5..d8b102fa0ad0 100644 --- a/docs/system/arm/aspeed.rst +++ b/docs/system/arm/aspeed.rst @@ -56,13 +56,13 @@ Supported devices * Front LEDs (PCA9552 on I2C bus) * LPC Peripheral Controller (a subset of subdevices are supported) * Hash/Crypto Engine (HACE) - Hash support only. TODO: HMAC and RSA + * ADC Missing devices --- * Coprocessor support - * ADC (out of tree implementation) * PWM and Fan Controller * Slave GPIO Controller * Super I/O Controller -- 2.33.0
[PATCH v2 2/4] docs: aspeed: Update OpenBMC image URL
This is the latest URL for the OpenBMC CI. The old URL still works, but redirects. Reviewed-by: Cédric Le Goater Signed-off-by: Joel Stanley --- docs/system/arm/aspeed.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst index 41a9bd5608e8..b87697fcf0b1 100644 --- a/docs/system/arm/aspeed.rst +++ b/docs/system/arm/aspeed.rst @@ -82,7 +82,7 @@ The Aspeed machines can be started using the ``-kernel`` option to load a Linux kernel or from a firmware. Images can be downloaded from the OpenBMC jenkins : - https://jenkins.openbmc.org/job/ci-openbmc/lastSuccessfulBuild/distro=ubuntu,label=docker-builder + https://jenkins.openbmc.org/job/ci-openbmc/lastSuccessfulBuild/ or directly from the OpenBMC GitHub release repository : -- 2.33.0
Re: [PATCH] gitlab-ci/cirrus: Increase timeout to 80 minutes
On 16/11/2021 19.20, Daniel P. Berrangé wrote: On Tue, Nov 16, 2021 at 06:36:50PM +0100, Richard Henderson wrote: On 11/16/21 6:22 PM, Thomas Huth wrote: On 16/11/2021 18.09, Philippe Mathieu-Daudé wrote: On 11/16/21 17:49, Daniel P. Berrangé wrote: On Tue, Nov 16, 2021 at 05:33:09PM +0100, Thomas Huth wrote: The jobs on Cirrus-CI sometimes get delayed quite a bit, waiting to be scheduled, so while the build test itself finishes within 60 minutes, the total run time of the jobs can be longer due to this waiting time. Thus let's increase the timeout on the gitlab side a little bit, so that these jobs are not marked as failing just because of the delay. ...>>> diff --git a/.gitlab-ci.d/cirrus.yml b/.gitlab-ci.d/cirrus.yml index e7b25e7427..22d42585e4 100644 --- a/.gitlab-ci.d/cirrus.yml +++ b/.gitlab-ci.d/cirrus.yml @@ -14,6 +14,7 @@ stage: build image: registry.gitlab.com/libvirt/libvirt-ci/cirrus-run:master needs: [] + timeout: 80m allow_failure: true script: - source .gitlab-ci.d/cirrus/$NAME.vars Whether 80 or 100 minute, consider it Reviewed-by: Daniel P. Berrangé This pipeline took 1h51m09s: https://gitlab.com/qemu-project/qemu/-/pipelines/409666733/builds But Richard restarted unstable jobs, which probably added time to the total. IIRC from a maintainer perspective 1h15 is the upper limit. 80m fits, 100m is over. I think I agree ... I normally don't want to wait more than a little bit more than one hour, so 100 minutes feels too long already. We already have some 70m timeouts in other jobs, and one 80 minute timeout in .gitlab-ci.d/crossbuild-template.yml, so I'd say 80 minutes are really the upper boundary that we should use. We are also talking apples and oranges: Gitlab timeouts are on the amount of time the job runs. Cirrus timeouts appear to be on the amount of time the job is queued. If cirrus would just not start accounting until the thing runs we'd be fine. Unfortunately it isn't that easy. Our cirrus CI jobs are launched using the cirrus-run tool, from a gitlab job. The timeouts we're usually hitting are from the gitlab job which is sitting around waiting for the cirrus job it launched to finish, so it can report back stats. Cirrus CI does itself have a job timeout, but I'm not aware of us hitting that typically, unless i'm misinterpreting something. Right, the problem is the timeout on the gitlab-CI side, not the timeout on the Cirrus-CI side. I've never seen us hitting the timeout on the Cirrus side either. Thomas
[PATCH v2 3/4] docs: aspeed: Give an example of booting a kernel
A common use case for the ASPEED machine is to boot a Linux kernel. Provide a full example command line. Reviewed-by: Cédric Le Goater Signed-off-by: Joel Stanley --- docs/system/arm/aspeed.rst | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst index b87697fcf0b1..6aafd611e9a5 100644 --- a/docs/system/arm/aspeed.rst +++ b/docs/system/arm/aspeed.rst @@ -78,9 +78,9 @@ Missing devices Boot options -The Aspeed machines can be started using the ``-kernel`` option to -load a Linux kernel or from a firmware. Images can be downloaded from -the OpenBMC jenkins : +The Aspeed machines can be started using the ``-kernel`` and ``-dtb`` options +to load a Linux kernel or from a firmware. Images can be downloaded from the +OpenBMC jenkins : https://jenkins.openbmc.org/job/ci-openbmc/lastSuccessfulBuild/ @@ -88,6 +88,15 @@ or directly from the OpenBMC GitHub release repository : https://github.com/openbmc/openbmc/releases +To boot a kernel directly from a Linux build tree: + +.. code-block:: bash + + $ qemu-system-arm -M ast2600-evb -nographic \ +-kernel arch/arm/boot/zImage \ +-dtb arch/arm/boot/dts/aspeed-ast2600-evb.dtb \ +-initrd rootfs.cpio + The image should be attached as an MTD drive. Run : .. code-block:: bash -- 2.33.0
[PATCH v2 1/4] docs: aspeed: Add new boards
Add X11, FP5280G2, G220A, Rainier and Fuji. Mention that Swift will be removed in v7.0. Signed-off-by: Joel Stanley --- v2: - Add POWER10 to Rainier description - Include Fuji - Mention Swift to be removed --- docs/system/arm/aspeed.rst | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst index cec87e3743d0..41a9bd5608e8 100644 --- a/docs/system/arm/aspeed.rst +++ b/docs/system/arm/aspeed.rst @@ -14,6 +14,7 @@ AST2400 SoC based machines : - ``palmetto-bmc`` OpenPOWER Palmetto POWER8 BMC - ``quanta-q71l-bmc`` OpenBMC Quanta BMC +- ``supermicrox11-bmc``Supermicro X11 BMC AST2500 SoC based machines : @@ -21,12 +22,16 @@ AST2500 SoC based machines : - ``romulus-bmc`` OpenPOWER Romulus POWER9 BMC - ``witherspoon-bmc`` OpenPOWER Witherspoon POWER9 BMC - ``sonorapass-bmc`` OCP SonoraPass BMC -- ``swift-bmc``OpenPOWER Swift BMC POWER9 +- ``swift-bmc``OpenPOWER Swift BMC POWER9 (to be removed in v7.0) +- ``fp5280g2-bmc`` Inspur FP5280G2 BMC +- ``g220a-bmc``Bytedance G220A BMC AST2600 SoC based machines : - ``ast2600-evb`` Aspeed AST2600 Evaluation board (Cortex-A7) - ``tacoma-bmc`` OpenPOWER Witherspoon POWER9 AST2600 BMC +- ``rainier-bmc`` IBM Rainier POWER10 BMC +- ``fuji-bmc`` Facebook Fuji BMC Supported devices - -- 2.33.0
[PATCH v2 0/4] docs: aspeed: Minor updates
Here are some small updates to the aspeed docs. v2: Tweak board changes, add patch to move ADC to the supported list Joel Stanley (4): docs: aspeed: Add new boards docs: aspeed: Update OpenBMC image URL docs: aspeed: Give an example of booting a kernel docs: aspeed: ADC is now modelled docs/system/arm/aspeed.rst | 26 -- 1 file changed, 20 insertions(+), 6 deletions(-) -- 2.33.0
Re: [SPAM] [RESEND PATCH 2/3] docs: aspeed: Update OpenBMC image URL
On 11/17/21 02:09, Joel Stanley wrote: This is the latest URL for the OpenBMC CI. The old URL still works, but redirects. Signed-off-by: Joel Stanley Reviewed-by: Cédric Le Goater Thanks C. --- docs/system/arm/aspeed.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst index b091c0c61dec..4bed7b5221b4 100644 --- a/docs/system/arm/aspeed.rst +++ b/docs/system/arm/aspeed.rst @@ -81,7 +81,7 @@ The Aspeed machines can be started using the ``-kernel`` option to load a Linux kernel or from a firmware. Images can be downloaded from the OpenBMC jenkins : - https://jenkins.openbmc.org/job/ci-openbmc/lastSuccessfulBuild/distro=ubuntu,label=docker-builder + https://jenkins.openbmc.org/job/ci-openbmc/lastSuccessfulBuild/ or directly from the OpenBMC GitHub release repository :
Re: [SPAM] [RESEND PATCH 3/3] docs: aspeed: Give an example of booting a kernel
On 11/17/21 02:09, Joel Stanley wrote: A common use case for the ASPEED machine is to boot a Linux kernel. Provide a full example command line. Signed-off-by: Joel Stanley Reviewed-by: Cédric Le Goater Thanks C. --- docs/system/arm/aspeed.rst | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst index 4bed7b5221b4..de408b0364ea 100644 --- a/docs/system/arm/aspeed.rst +++ b/docs/system/arm/aspeed.rst @@ -77,9 +77,9 @@ Missing devices Boot options -The Aspeed machines can be started using the ``-kernel`` option to -load a Linux kernel or from a firmware. Images can be downloaded from -the OpenBMC jenkins : +The Aspeed machines can be started using the ``-kernel`` and ``-dtb`` options +to load a Linux kernel or from a firmware. Images can be downloaded from the +OpenBMC jenkins : https://jenkins.openbmc.org/job/ci-openbmc/lastSuccessfulBuild/ @@ -87,6 +87,15 @@ or directly from the OpenBMC GitHub release repository : https://github.com/openbmc/openbmc/releases +To boot a kernel directly from a Linux build tree: + +.. code-block:: bash + + $ qemu-system-arm -M ast2600-evb -nographic \ +-kernel arch/arm/boot/zImage \ +-dtb arch/arm/boot/dts/aspeed-ast2600-evb.dtb \ +-initrd rootfs.cpio + The image should be attached as an MTD drive. Run : .. code-block:: bash
Re: [SPAM] [RESEND PATCH 1/3] docs: aspeed: Add new boards
On 11/17/21 02:09, Joel Stanley wrote: Signed-off-by: Joel Stanley --- docs/system/arm/aspeed.rst | 4 1 file changed, 4 insertions(+) diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst index cec87e3743d0..b091c0c61dec 100644 --- a/docs/system/arm/aspeed.rst +++ b/docs/system/arm/aspeed.rst @@ -14,6 +14,7 @@ AST2400 SoC based machines : - ``palmetto-bmc`` OpenPOWER Palmetto POWER8 BMC - ``quanta-q71l-bmc`` OpenBMC Quanta BMC +- ``supermicrox11-bmc``Supermicro X11 BMC AST2500 SoC based machines : @@ -22,11 +23,14 @@ AST2500 SoC based machines : - ``witherspoon-bmc`` OpenPOWER Witherspoon POWER9 BMC - ``sonorapass-bmc`` OCP SonoraPass BMC - ``swift-bmc``OpenPOWER Swift BMC POWER9 That reminds me that this board is scheduled for removal in 7.0 +- ``fp5280g2-bmc`` Inspur FP5280G2 BMC +- ``g220a-bmc``Bytedance G220A BMC AST2600 SoC based machines : - ``ast2600-evb`` Aspeed AST2600 Evaluation board (Cortex-A7) - ``tacoma-bmc`` OpenPOWER Witherspoon POWER9 AST2600 BMC +- ``rainier-bmc`` IBM Rainier BMC May be we should add POWER10 in the description ^ and we merged the Fuji BMC also. Thanks, C. Supported devices -
Re: [RFC PATCH v2 07/30] target/loongarch: Add MMU support for LoongArch CPU.
Hi, Richard:
On 11/11/2021 11:53 PM, Richard Henderson wrote:
> On 11/11/21 2:35 AM, Xiaojuan Yang wrote:
>> This patch introduces basic TLB interfaces.
>>
>> Signed-off-by: Xiaojuan Yang
>> Signed-off-by: Song Gao
>> ---
>> target/loongarch/cpu-param.h | 3 +
>> target/loongarch/cpu.c| 36
>> target/loongarch/cpu.h| 57 ++
>> target/loongarch/internals.h | 7 +
>> target/loongarch/machine.c| 56 ++
>> target/loongarch/meson.build | 1 +
>> target/loongarch/tlb_helper.c | 339 ++
>> 7 files changed, 499 insertions(+)
>> create mode 100644 target/loongarch/tlb_helper.c
>>
>> diff --git a/target/loongarch/cpu-param.h b/target/loongarch/cpu-param.h
>> index 9a769b67e0..5a2147fb90 100644
>> --- a/target/loongarch/cpu-param.h
>> +++ b/target/loongarch/cpu-param.h
>> @@ -12,6 +12,9 @@
>> #define TARGET_PHYS_ADDR_SPACE_BITS 48
>> #define TARGET_VIRT_ADDR_SPACE_BITS 48
>> +#define TARGET_PHYS_MASK ((1UL << TARGET_PHYS_ADDR_SPACE_BITS) - 1)
>> +#define TARGET_VIRT_MASK ((1UL << TARGET_VIRT_ADDR_SPACE_BITS) - 1)
>
> As before, unsigned long is wrong; use MAKE_64BIT_MASK.
>
> These do not belong in cpu-param.h anyway; probably only tlb_helper.c needs
> them.
>
>> +#ifndef CONFIG_USER_ONLY
>> +qemu_fprintf(f, "EUEN0x%lx\n", env->CSR_EUEN);
>> +qemu_fprintf(f, "ESTAT 0x%lx\n", env->CSR_ESTAT);
>> +qemu_fprintf(f, "ERA 0x%lx\n", env->CSR_ERA);
>> +qemu_fprintf(f, "CRMD0x%lx\n", env->CSR_CRMD);
>> +qemu_fprintf(f, "PRMD0x%lx\n", env->CSR_PRMD);
>> +qemu_fprintf(f, "BadVAddr0x%lx\n", env->CSR_BADV);
>> +qemu_fprintf(f, "TLB refill ERA 0x%lx\n", env->CSR_TLBRERA);
>> +qemu_fprintf(f, "TLB refill BadV 0x%lx\n", env->CSR_TLBRBADV);
>> +qemu_fprintf(f, "EENTRY 0x%lx\n", env->CSR_EENTRY);
>> +qemu_fprintf(f, "BadInstr0x%lx\n", env->CSR_BADI);
>> +qemu_fprintf(f, "PRCFG10x%lx\nPRCFG2 0x%lx\nPRCFG3 0x%lx\n",
>> + env->CSR_PRCFG1, env->CSR_PRCFG3, env->CSR_PRCFG3);
>
> %lx is wrong; use PRIx64.
>
>> +#define LOONGARCH_TLB_MAX 2112 /* 2048 STLB + 64 MTLB */
>
> Better to write (2048 + 64).
>
>> +FIELD(TLB_MISC, E, 0, 1)
>> +FIELD(TLB_MISC, ASID, 1, 10)
>> +FIELD(TLB_MISC, G, 11, 1)
>> +FIELD(TLB_MISC, PS, 12, 6)
>> +FIELD(TLB_MISC, VPPN, 18, 35)
>> +
>> +/* Corresponding to CSR_TLBELO0/1 */
>> +FIELD(ENTRY0, V, 0, 1)
>> +FIELD(ENTRY0, D, 1, 1)
>> +FIELD(ENTRY0, NR, 2, 1)
>> +FIELD(ENTRY0, NX, 3, 1)
>> +FIELD(ENTRY0, MAT, 4, 2)
>> +FIELD(ENTRY0, PLV, 6, 2)
>> +FIELD(ENTRY0, RPLV, 8, 1)
>> +FIELD(ENTRY0, PPN, 9, 36)
>> +
>> +FIELD(ENTRY1, V, 0, 1)
>> +FIELD(ENTRY1, D, 1, 1)
>> +FIELD(ENTRY1, NR, 2, 1)
>> +FIELD(ENTRY1, NX, 3, 1)
>> +FIELD(ENTRY1, MAT, 4, 2)
>> +FIELD(ENTRY1, PLV, 6, 2)
>> +FIELD(ENTRY1, RPLV, 8, 1)
>> +FIELD(ENTRY1, PPN, 9, 36)
>
> Why are you duplicating the CSR_TLBELO* fields?
>
>> +const VMStateInfo vmstate_info_tlb = {
>> +.name = "tlb_entry",
>> +.get = get_tlb,
>> +.put = put_tlb,
>> +};
>
> Better to use .fields.
>
>> +#define VMSTATE_TLB_ARRAY_V(_f, _s, _n, _v) \
>> +VMSTATE_ARRAY(_f, _s, _n, _v, vmstate_info_tlb, loongarch_tlb)
>> +
>> +#define VMSTATE_TLB_ARRAY(_f, _s, _n) \
>> +VMSTATE_TLB_ARRAY_V(_f, _s, _n, 0)
>
> Don't need these.
>
>> +
>> +const VMStateDescription vmstate_tlb = {
>> +.name = "cpu/tlb",
>> +.version_id = 0,
>> +.minimum_version_id = 0,
>> +.fields = (VMStateField[]) {
>> +VMSTATE_TLB_ARRAY(env.tlb, LoongArchCPU, LOONGARCH_TLB_MAX),
>
> VMSTATE_STRUCT_ARRAY.
>
>> +VMSTATE_END_OF_LIST()
>> +}
>> +};
>> /* LoongArch CPU state */
>> @@ -22,6 +70,10 @@ const VMStateDescription vmstate_loongarch_cpu = {
>> VMSTATE_UINT64_ARRAY(env.fpr, LoongArchCPU, 32),
>> VMSTATE_UINT32(env.fcsr0, LoongArchCPU),
>> +/* TLB */
>> +VMSTATE_UINT32(env.stlb_size, LoongArchCPU),
>> +VMSTATE_UINT32(env.mtlb_size, LoongArchCPU),
>
> Might as well keep these in vmstate_tlb.
All of the vmstate has been modified.
>> +/* TLB address map */
>> +static int loongarch_map_tlb_entry(CPULoongArchState *env, hwaddr *physical,
>> + int *prot, target_ulong address,
>> + int access_type, loongarch_tlb *tlb)
>> +{
>> +uint64_t plv = FIELD_EX64(env->CSR_CRMD, CSR_CRMD, PLV);
>
> Incorrect. PLV associated with mmu_idx, so you need to use that.
>
>> +uint8_t tlb_ps, n, tlb_v0, tlb_v1, tlb_d0, tlb_d1;
>> +uint8_t tlb_nx0, tlb_nx1, tlb_nr0, tlb_nr1;
>> +uint64_t tlb_ppn0, tlb_ppn1;
>> +uint8_t tlb_rplv0, tlb_rplv1, tlb_plv0, tlb_plv1;
>> +
>> +tlb_ps = FIELD_EX64(tlb->tlb_misc, TLB_MISC, PS);
>> +n = (address >> tlb_ps) & 0x1;/* Odd or even */
>
> Surely you need to pass in tlb_ps, since it's not present in
Re: [PATCH v5 0/6] Add vmnet.framework based network backend
On Tue, Nov 16, 2021 at 11:30 PM Vladislav Yaroshchuk wrote: > > > > вт, 16 нояб. 2021 г. в 10:23, Jason Wang : >> >> On Mon, Nov 15, 2021 at 6:45 PM Vladislav Yaroshchuk >> wrote: >> > >> > >> > >> > пн, 15 нояб. 2021 г. в 07:53, Jason Wang : >> >> >> >> On Fri, Nov 12, 2021 at 5:14 PM Vladislav Yaroshchuk >> >> wrote: >> >> > >> >> > macOS provides networking API for VMs called 'vmnet.framework': >> >> > https://developer.apple.com/documentation/vmnet >> >> > >> >> > We can provide its support as the new QEMU network backends which >> >> > represent three different vmnet.framework interface usage modes: >> >> > >> >> > * `vmnet-shared`: >> >> > allows the guest to communicate with other guests in shared mode and >> >> > also with external network (Internet) via NAT. Has (macOS-provided) >> >> > DHCP server; subnet mask and IP range can be configured; >> >> > >> >> > * `vmnet-host`: >> >> > allows the guest to communicate with other guests in host mode. >> >> > By default has enabled DHCP as `vmnet-shared`, but providing >> >> > network unique id (uuid) can make `vmnet-host` interfaces isolated >> >> > from each other and also disables DHCP. >> >> > >> >> > * `vmnet-bridged`: >> >> > bridges the guest with a physical network interface. >> >> > >> >> > This backends cannot work on macOS Catalina 10.15 cause we use >> >> > vmnet.framework API provided only with macOS 11 and newer. Seems >> >> > that it is not a problem, because QEMU guarantees to work on two most >> >> > recent versions of macOS which now are Big Sur (11) and Monterey (12). >> >> > >> >> > Also, we have one inconvenient restriction: vmnet.framework interfaces >> >> > can create only privileged user: >> >> > `$ sudo qemu-system-x86_64 -nic vmnet-shared` >> >> > >> >> > Attempt of `vmnet-*` netdev creation being unprivileged user fails with >> >> > vmnet's 'general failure'. >> >> > >> >> > This happens because vmnet.framework requires `com.apple.vm.networking` >> >> > entitlement which is: "restricted to developers of virtualization >> >> > software. >> >> > To request this entitlement, contact your Apple representative." as >> >> > Apple >> >> > documentation says: >> >> > https://developer.apple.com/documentation/bundleresources/entitlements/com_apple_vm_networking >> >> >> >> Do you know how multipass work? Looks like it uses vmnet without >> >> privileges. >> >> >> > >> > I've just checked this, and they still need root privileges. They have a >> > `multipassd` daemon which is launched as root by launchd by default. >> > >> > ``` >> > bash-5.1$ ps axo ruser,ruid,comm | grep multipass >> > root 0 /Library/Application >> > Support/com.canonical.multipass/bin/multipassd >> > root 0 /Library/Application >> > Support/com.canonical.multipass/bin/hyperkit >> > ``` >> > >> > That's the reason why it's required to 'enter a password' while multipass >> > installation: >> > it creates launchd plist (kinda launch rule) and places it to >> > /Library/LaunchDaemons/, >> > which can be done only with root privileges. >> > >> > ``` >> > bash-5.1$ ll /Library/LaunchDaemons | grep multipass >> > -rw-r--r-- 1 root wheel 1.1K 15 Nov 12:47 >> > com.canonical.multipassd.plist >> > ``` >> > >> > And after this launchd launches this multipassd daemon as root every boot. >> > >> > So an unprivileged user can launch a multipass VM instance, but actually >> > the >> > `hyperkit` process which interacts with vmnet is gonna be launched by >> > multipassd >> > running as root. >> >> I wonder how it passes the vmnet object to qemu? Nothing obvious from >> the qemu command line that multipass launched: >> >> -nic vmnet-macos,mode=shared,model=virtio-net-pci,mac=52:54:00:52:e8:e4 >> >> (But I haven't had time to check their qemu codes). >> > > It seems they just use QEMU with patch by Phillip Tennen: > https://patchew.org/QEMU/20210218134947.1860-1-phillip.en...@gmail.com/ > > In that patch he does quite the same as we in this series, the > difference remains in foreground: he creates one new 'vmnet-macos' > netdev, and uses 'mode=shared' property to choose vmnet > operating mode. I decided to create three different netdevs instead > (vmnet-shared, vmnet-host, vmnet-bridged). Also I've added some > features related to isolation and ipv6. Ok. > > > I wonder how it passes the vmnet object to qemu > I hope I clearly described this. A silly question, how did the 'hyperkit' process pass the vmnet object to qemu? > >> > >> > tl;dr sadly, we can't interact with vmnet.framework without having our >> > binary correctly >> > signed and being an unprivileged user. Root privileges or special >> > signature with >> > entitlement is required. >> >> This is something similar to what happens in other OS. E.g for the tap >> backend, it can't be created without privileges. So qemu allows to: >> >> 1) the TAP to be created by privileged program like libvirt, and its >> fd could be passed to qemu
Re: [PATCH 1/2] block:hdev: support BLKSECDISCARD
On Tue, Nov 16, 2021 at 10:58:30AM +, Stefan Hajnoczi wrote: > Question for Jens and Christoph: > > Is there a way for userspace to detect whether a Linux block device > supports SECDISCARD? I don't know of one. > If not, then maybe a new sysfs attribute can be added: This looks correct, but if we start looking into SECDISCARD seriously I'd like to split the max_sectors settings for it from discard as that is currently a bit of a mess. If we then expose the secure erase max sectors in sysfs that would also be a good indicator. What is the use case for exposing secure erase in qemu? The whole concept for a LBA based secure erase is generally not a very smart idea for flash based media..
RE: [PATCH 1/2] migration/colo: Optimize COLO start code path
> -Original Message-
> From: Juan Quintela
> Sent: Wednesday, November 17, 2021 12:28 AM
> To: Zhang, Chen
> Cc: Hailiang Zhang ; Dr . David Alan
> Gilbert ; qemu-dev
> Subject: Re: [PATCH 1/2] migration/colo: Optimize COLO start code path
>
> Zhang Chen wrote:
> > There is no need to start COLO through MIGRATION_STATUS_ACTIVE.
>
> Hi
>
> I don't understand what you are trying to do. In my reading, at least the
> commit message is wrong:
>
> void migrate_start_colo_process(MigrationState *s) {
> ...
> migrate_set_state(>state, MIGRATION_STATUS_ACTIVE,
> MIGRATION_STATUS_COLO);
> ...
> }
>
> and
>
> void *colo_process_incoming_thread(void *opaque) {
> ...
> migrate_set_state(>state, MIGRATION_STATUS_ACTIVE,
> MIGRATION_STATUS_COLO);
>
> So colo starts with MIGRATION_STATUS_ACTIVE.
Yes, this patch just optimized COLO primary code
path(migrate_start_colo_process()).
We can see this patch removed the
migrate_set_state(>state, MIGRATION_STATUS_ACTIVE,
MIGRATION_STATUS_COLO);
In the migrate_start_colo_process().
Current COLO status path:
MIGRATION_STATUS_XXX ---> MIGRATION_STATUS_ACTIVE --->
MIGRATION_STATUS_COLO ---> MIGRATION_STATUS_COMPLETED
This patch try to remove redundant " MIGRATION_STATUS_ACTIVE " in COLO start.
MIGRATION_STATUS_XXX ---> MIGRATION_STATUS_COLO --->
MIGRATION_STATUS_COMPLETED
Actually COLO primary code did nothing when running on
"MIGRATION_STATUS_ACTIVE".
But for COLO secondary (void *colo_process_incoming_thread()), it shared some
code with normal migration. No need to do this.
So, I will fix commit message to:
Optimize COLO primary start path to:
MIGRATION_STATUS_XXX ---> MIGRATION_STATUS_COLO --->
MIGRATION_STATUS_COMPLETED
No need to start primary COLO through "MIGRATION_STATUS_ACTIVE".
How about it?
>
>
> > Signed-off-by: Zhang Chen
> > ---
> > migration/colo.c | 2 --
> > migration/migration.c | 18 +++---
> > 2 files changed, 11 insertions(+), 9 deletions(-)
> >
> > diff --git a/migration/colo.c b/migration/colo.c index
> > 2415325262..ad1a4426b3 100644
> > --- a/migration/colo.c
> > +++ b/migration/colo.c
> > @@ -667,8 +667,6 @@ void migrate_start_colo_process(MigrationState *s)
> > colo_checkpoint_notify, s);
> >
> > qemu_sem_init(>colo_exit_sem, 0);
> > -migrate_set_state(>state, MIGRATION_STATUS_ACTIVE,
> > - MIGRATION_STATUS_COLO);
> > colo_process_checkpoint(s);
> > qemu_mutex_lock_iothread();
> > }
> > diff --git a/migration/migration.c b/migration/migration.c index
> > abaf6f9e3d..4c8662a839 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -3222,7 +3222,10 @@ static void migration_completion(MigrationState
> *s)
> > goto fail_invalidate;
> > }
> >
> > -if (!migrate_colo_enabled()) {
> > +if (migrate_colo_enabled()) {
> > +migrate_set_state(>state, current_active_state,
> > + MIGRATION_STATUS_COLO);
> > +} else {
> > migrate_set_state(>state, current_active_state,
> >MIGRATION_STATUS_COMPLETED);
> > }
>
> This moves the setup to MIGRATION_STATUS_COLO to completion time
> instead of the beggining of the process. I have no clue why. I guess you can
> put a comment/commit message to say what you ar.e trynig to do.
You are right, no need to setup here.
I will remove this in next version.
>
> > @@ -3607,12 +3610,7 @@ static void
> migration_iteration_finish(MigrationState *s)
> > migration_calculate_complete(s);
> > runstate_set(RUN_STATE_POSTMIGRATE);
> > break;
> > -
> > -case MIGRATION_STATUS_ACTIVE:
> > -/*
> > - * We should really assert here, but since it's during
> > - * migration, let's try to reduce the usage of assertions.
> > - */
> > +case MIGRATION_STATUS_COLO:
> > if (!migrate_colo_enabled()) {
> > error_report("%s: critical error: calling COLO code without "
> > "COLO enabled", __func__); @@ -3622,6
> > +3620,12 @@ static void migration_iteration_finish(MigrationState *s)
> > * Fixme: we will run VM in COLO no matter its old running state.
> > * After exited COLO, we will keep running.
> > */
> > + /* Fallthrough */
> > +case MIGRATION_STATUS_ACTIVE:
> > +/*
> > + * We should really assert here, but since it's during
> > + * migration, let's try to reduce the usage of assertions.
> > + */
> > s->vm_was_running = true;
> > /* Fallthrough */
> > case MIGRATION_STATUS_FAILED:
>
> I guess this change is related to the previous one, but I don't understand
> colo
> enough to review it.
I think this patch is the general code, little background needed.
You can simple understand COLO is two VMs(primary
Re: [PATCH v1 0/9] hw/mos6522: VIA timer emulation fixes and improvements
On Fri, 24 Sep 2021, I wrote:
> This is a patch series for QEMU that I started last year. The aim was to
> try to get a monotonic clocksource for Linux/m68k guests. That hasn't
> been achieved yet (for q800 machines). I'm submitting the patch series
> because,
>
> - it improves 6522 emulation fidelity, although slightly slower, and
>
I did some more benchmarking to examine the performance implications.
I measured a performance improvement with this patch series. For a
Linux/m68k guest, the execution time for a gettimeofday syscall dropped
from 9 us to 5 us. (This is a fairly common syscall.)
The host CPU time consumed by qemu in starting the guest kernel and
executing a benchmark involving 20 million gettimeofday calls was as
follows.
qemu-system-m68k mainline (42f6c9179b):
real 198 s
sys 123 s
user 73 s
sys/user 1.68
qemu-system-m68k patched (0a0bca4711):
real 112 s
sys 63 s
user 47 s
sys/user 1.34
As with any microbenchmark, this workload is not a real-world one. For
comparison, here are measurements of the time to fully startup and
immediately shut down Debian Linux/m68k SID (systemd):
qemu-system-m68k mainline (42f6c9179b)
real 31.5 s
sys 1.59 s
user 27.4 s
sys/user 0.06
qemu-system-m68k patched (0a0bca4711)
real 31.2 s
sys 1.17 s
user 27.6 s
sys/user 0.04
The decrease in sys/user ratio reflects the small cost that has to be paid
for the improvement in 6522 emulation fidelity and timer accuracy. But
note that in both benchmarks wallclock execution time dropped, meaning
that the system is faster overall.
The gettimeofday testing revealed that the Linux kernel does not properly
protect userland from pathological hardware timers, and the gettimeofday
result was seen to jump backwards (that was unexpected, though Mark did
predict it).
This backwards jump was often observed in the mainline build during the
gettimeofday benchmark and is result of bugs in mos6522.c. The patched
build did not exhibit this problem (as yet).
The two benefits described here are offered in addition to all of the
other benefits described in the patches themselves. Please consider
merging this series.
> - it allows Linux/m68k to make use of the additional timer that the
>hardware indeed offers, but which QEMU omits, and which may be of
>benefit to Linux guests [1], and
>
> - I see that Mark has been working on timer emulation issues in his
>github repo [2] and it seems likely that MacOS, NetBSD or A/UX guests
>will also require better 6522 emulation.
>
> To make collaboration easier these patches can also be fetched from
> github [3].
>
> On a real Quadra, accesses to the SY6522 chips are slow because they are
> synchronous with the 783360 Hz "phase 2" clock. In QEMU, they are slow
> because of the division operation in the timer count calculation. This
> patch series improves the fidelity of the emulated chip, but the price
> is more division ops.
>
> The emulated 6522 still deviates from the behaviour of the real thing,
> however. For example, two consecutive accesses to a real 6522 timer
> counter can never yield the same value. This is not true of the emulated
> 6522 in QEMU 6, wherein two consecutive accesses to a timer count
> register have been observed to yield the same value.
>
> Two problems presently affecting a Linux guest are clock drift and
> monotonicity failure in the 'via' clocksource. That is, the clocksource
> counter can jump backwards. This can be observed by patching Linux like
> so,
>
> diff --git a/arch/m68k/mac/via.c b/arch/m68k/mac/via.c
> --- a/arch/m68k/mac/via.c
> +++ b/arch/m68k/mac/via.c
> @@ -606,6 +606,8 @@ void __init via_init_clock(void)
> clocksource_register_hz(_clk, VIA_CLOCK_FREQ);
> }
>
> +static u32 prev_ticks;
> +
> static u64 mac_read_clk(struct clocksource *cs)
> {
> unsigned long flags;
> @@ -631,6 +633,8 @@ static u64 mac_read_clk(struct clocksource *cs)
> count = count_high << 8;
> ticks = VIA_TIMER_CYCLES - count;
> ticks += clk_offset + clk_total;
> + if (ticks < prev_ticks) pr_warn("%s: %u < %u\n", __func__, ticks,
> prev_ticks);
> + prev_ticks = ticks;
> local_irq_restore(flags);
>
> return ticks;
>
>
> Or just enable CONFIG_DEBUG_TIMEKEEPING:
>
> [ 1872.72] INFO: timekeeping: Cycle offset (4294966426) is larger than
> the 'via1' clock's 50% safety margin (2147483647)
> [ 1872.72] timekeeping: Your kernel is still fine, but is feeling a bit
> nervous
> [ 1907.51] INFO: timekeeping: Cycle offset (4294962989) is larger than
> the 'via1' clock's 50% safety margin (2147483647)
> [ 1907.51] timekeeping: Your kernel is still fine, but is feeling a bit
> nervous
> [ 1907.90] INFO: timekeeping: Cycle offset (4294963248) is larger than
> the 'via1' clock's 50% safety margin (2147483647)
> [ 1907.90]
[PATCH v2] vfio: Fix memory leak of hostwin
hostwin is allocated and added to hostwin_list in vfio_host_win_add, but
it is only deleted from hostwin_list in vfio_host_win_del, which causes
a memory leak. Also, freeing all elements in hostwin_list is missing in
vfio_disconnect_container.
Fix: 2e4109de8e58 ("vfio/spapr: Create DMA window dynamically (SPAPR IOMMU v2)")
CC: qemu-sta...@nongnu.org
Signed-off-by: Peng Liang
---
v1 -> v2:
- Don't change to _SAFE variant in vfio_host_win_del. [Alex]
---
hw/vfio/common.c | 8
1 file changed, 8 insertions(+)
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index dd387b0d3959..080046e3f511 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -551,6 +551,7 @@ static int vfio_host_win_del(VFIOContainer *container,
hwaddr min_iova,
QLIST_FOREACH(hostwin, >hostwin_list, hostwin_next) {
if (hostwin->min_iova == min_iova && hostwin->max_iova == max_iova) {
QLIST_REMOVE(hostwin, hostwin_next);
+g_free(hostwin);
return 0;
}
}
@@ -2239,6 +2240,7 @@ static void vfio_disconnect_container(VFIOGroup *group)
if (QLIST_EMPTY(>group_list)) {
VFIOAddressSpace *space = container->space;
VFIOGuestIOMMU *giommu, *tmp;
+VFIOHostDMAWindow *hostwin, *next;
QLIST_REMOVE(container, next);
@@ -2249,6 +2251,12 @@ static void vfio_disconnect_container(VFIOGroup *group)
g_free(giommu);
}
+QLIST_FOREACH_SAFE(hostwin, >hostwin_list, hostwin_next,
+ next) {
+QLIST_REMOVE(hostwin, hostwin_next);
+g_free(hostwin);
+}
+
trace_vfio_disconnect_container(container->fd);
close(container->fd);
g_free(container);
--
2.33.1
Re: [PATCH] vfio: Fix memory leak of hostwin
On 11/17/2021 3:01 AM, Alex Williamson wrote:
> On Tue, 16 Nov 2021 19:56:26 +0800
> Peng Liang wrote:
>
>> hostwin is allocated and added to hostwin_list in vfio_host_win_add, but
>> it is only deleted from hostwin_list in vfio_host_win_del, which causes
>> a memory leak. Also, freeing all elements in hostwin_list is missing in
>> vfio_disconnect_container.
>>
>> Fix: 2e4109de8e58 ("vfio/spapr: Create DMA window dynamically (SPAPR IOMMU
>> v2)")
>> CC: qemu-sta...@nongnu.org
>> Signed-off-by: Peng Liang
>> ---
>> hw/vfio/common.c | 12 ++--
>> 1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>> index dd387b0d3959..2cce60c5fac3 100644
>> --- a/hw/vfio/common.c
>> +++ b/hw/vfio/common.c
>> @@ -546,11 +546,12 @@ static void vfio_host_win_add(VFIOContainer *container,
>> static int vfio_host_win_del(VFIOContainer *container, hwaddr min_iova,
>> hwaddr max_iova)
>> {
>> -VFIOHostDMAWindow *hostwin;
>> +VFIOHostDMAWindow *hostwin, *next;
>>
>> -QLIST_FOREACH(hostwin, >hostwin_list, hostwin_next) {
>> +QLIST_FOREACH_SAFE(hostwin, >hostwin_list, hostwin_next,
>> next) {
>
> Unnecessary conversion to _SAFE variant here, we don't continue to walk
> the list after removing an object.
Ok, I'll remove it in the next version.
Thanks,
Peng
>
>> if (hostwin->min_iova == min_iova && hostwin->max_iova == max_iova)
>> {
>> QLIST_REMOVE(hostwin, hostwin_next);
>> +g_free(hostwin);
>> return 0;
>> }
>> }
>> @@ -2239,6 +2240,7 @@ static void vfio_disconnect_container(VFIOGroup *group)
>> if (QLIST_EMPTY(>group_list)) {
>> VFIOAddressSpace *space = container->space;
>> VFIOGuestIOMMU *giommu, *tmp;
>> +VFIOHostDMAWindow *hostwin, *next;
>>
>> QLIST_REMOVE(container, next);
>>
>> @@ -2249,6 +2251,12 @@ static void vfio_disconnect_container(VFIOGroup
>> *group)
>> g_free(giommu);
>> }
>>
>> +QLIST_FOREACH_SAFE(hostwin, >hostwin_list, hostwin_next,
>> + next) {
>> +QLIST_REMOVE(hostwin, hostwin_next);
>> +g_free(hostwin);
>> +}
>> +
>
> This usage looks good. Thanks,
>
> Alex
>
>> trace_vfio_disconnect_container(container->fd);
>> close(container->fd);
>> g_free(container);
>
> .
>
[RESEND PATCH 2/3] docs: aspeed: Update OpenBMC image URL
This is the latest URL for the OpenBMC CI. The old URL still works, but redirects. Signed-off-by: Joel Stanley --- docs/system/arm/aspeed.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst index b091c0c61dec..4bed7b5221b4 100644 --- a/docs/system/arm/aspeed.rst +++ b/docs/system/arm/aspeed.rst @@ -81,7 +81,7 @@ The Aspeed machines can be started using the ``-kernel`` option to load a Linux kernel or from a firmware. Images can be downloaded from the OpenBMC jenkins : - https://jenkins.openbmc.org/job/ci-openbmc/lastSuccessfulBuild/distro=ubuntu,label=docker-builder + https://jenkins.openbmc.org/job/ci-openbmc/lastSuccessfulBuild/ or directly from the OpenBMC GitHub release repository : -- 2.33.0
[RESEND PATCH 0/3] docs: aspeed: Minor updates
Here are some small updates to the aspeed docs. Joel Stanley (3): docs: aspeed: Add new boards docs: aspeed: Update OpenBMC image URL docs: aspeed: Give an example of booting a kernel docs/system/arm/aspeed.rst | 21 + 1 file changed, 17 insertions(+), 4 deletions(-) -- 2.33.0
[RESEND PATCH 3/3] docs: aspeed: Give an example of booting a kernel
A common use case for the ASPEED machine is to boot a Linux kernel. Provide a full example command line. Signed-off-by: Joel Stanley --- docs/system/arm/aspeed.rst | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst index 4bed7b5221b4..de408b0364ea 100644 --- a/docs/system/arm/aspeed.rst +++ b/docs/system/arm/aspeed.rst @@ -77,9 +77,9 @@ Missing devices Boot options -The Aspeed machines can be started using the ``-kernel`` option to -load a Linux kernel or from a firmware. Images can be downloaded from -the OpenBMC jenkins : +The Aspeed machines can be started using the ``-kernel`` and ``-dtb`` options +to load a Linux kernel or from a firmware. Images can be downloaded from the +OpenBMC jenkins : https://jenkins.openbmc.org/job/ci-openbmc/lastSuccessfulBuild/ @@ -87,6 +87,15 @@ or directly from the OpenBMC GitHub release repository : https://github.com/openbmc/openbmc/releases +To boot a kernel directly from a Linux build tree: + +.. code-block:: bash + + $ qemu-system-arm -M ast2600-evb -nographic \ +-kernel arch/arm/boot/zImage \ +-dtb arch/arm/boot/dts/aspeed-ast2600-evb.dtb \ +-initrd rootfs.cpio + The image should be attached as an MTD drive. Run : .. code-block:: bash -- 2.33.0
[RESEND PATCH 1/3] docs: aspeed: Add new boards
Signed-off-by: Joel Stanley --- docs/system/arm/aspeed.rst | 4 1 file changed, 4 insertions(+) diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst index cec87e3743d0..b091c0c61dec 100644 --- a/docs/system/arm/aspeed.rst +++ b/docs/system/arm/aspeed.rst @@ -14,6 +14,7 @@ AST2400 SoC based machines : - ``palmetto-bmc`` OpenPOWER Palmetto POWER8 BMC - ``quanta-q71l-bmc`` OpenBMC Quanta BMC +- ``supermicrox11-bmc``Supermicro X11 BMC AST2500 SoC based machines : @@ -22,11 +23,14 @@ AST2500 SoC based machines : - ``witherspoon-bmc`` OpenPOWER Witherspoon POWER9 BMC - ``sonorapass-bmc`` OCP SonoraPass BMC - ``swift-bmc``OpenPOWER Swift BMC POWER9 +- ``fp5280g2-bmc`` Inspur FP5280G2 BMC +- ``g220a-bmc``Bytedance G220A BMC AST2600 SoC based machines : - ``ast2600-evb`` Aspeed AST2600 Evaluation board (Cortex-A7) - ``tacoma-bmc`` OpenPOWER Witherspoon POWER9 AST2600 BMC +- ``rainier-bmc`` IBM Rainier BMC Supported devices - -- 2.33.0
Re: [PATCH v2 6/7] target/riscv: cpu: Enable native debug feature on virt and sifive_u CPUs
On Sat, Oct 30, 2021 at 11:56 PM Bin Meng wrote: > > Turn on native debug feature on virt and sifive_u CPUs. Is there a reason why it's only these 2 machines? Could this be enabled by default for all CPUs? Alistair > > Signed-off-by: Bin Meng > > --- > > Changes in v2: > - new patch: enable native debug feature on virt and sifive_u CPUs > > target/riscv/cpu.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index 6f69ef4f50..b4d3c58dea 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -153,6 +153,7 @@ static void rv64_base_cpu_init(Object *obj) > CPURISCVState *env = _CPU(obj)->env; > /* We set this in the realise function */ > set_misa(env, MXL_RV64, 0); > +qdev_prop_set_bit(DEVICE(obj), "debug", true); > } > > static void rv64_sifive_u_cpu_init(Object *obj) > @@ -160,6 +161,7 @@ static void rv64_sifive_u_cpu_init(Object *obj) > CPURISCVState *env = _CPU(obj)->env; > set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU); > set_priv_version(env, PRIV_VERSION_1_10_0); > +qdev_prop_set_bit(DEVICE(obj), "debug", true); > } > > static void rv64_sifive_e_cpu_init(Object *obj) > @@ -175,6 +177,7 @@ static void rv32_base_cpu_init(Object *obj) > CPURISCVState *env = _CPU(obj)->env; > /* We set this in the realise function */ > set_misa(env, MXL_RV32, 0); > +qdev_prop_set_bit(DEVICE(obj), "debug", true); > } > > static void rv32_sifive_u_cpu_init(Object *obj) > @@ -182,6 +185,7 @@ static void rv32_sifive_u_cpu_init(Object *obj) > CPURISCVState *env = _CPU(obj)->env; > set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU); > set_priv_version(env, PRIV_VERSION_1_10_0); > +qdev_prop_set_bit(DEVICE(obj), "debug", true); > } > > static void rv32_sifive_e_cpu_init(Object *obj) > -- > 2.25.1 > >
Re: [PATCH v2 5/7] target/riscv: csr: Hook debug CSR read/write
On Sun, Oct 31, 2021 at 12:03 AM Bin Meng wrote:
>
> This adds debug CSR read/write support to the RISC-V CSR RW table.
>
> Signed-off-by: Bin Meng
> ---
>
> (no changes since v1)
>
> target/riscv/cpu.c | 6 +
> target/riscv/csr.c | 57 ++
> 2 files changed, 63 insertions(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 84116768ce..6f69ef4f50 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -575,6 +575,12 @@ static void riscv_cpu_realize(DeviceState *dev, Error
> **errp)
>
> riscv_cpu_register_gdb_regs_for_features(cs);
>
> +#ifndef CONFIG_USER_ONLY
> +if (riscv_feature(env, RISCV_FEATURE_DEBUG)) {
> +riscv_trigger_init(env);
This function should be added here instead of patch 1
Alistair
> +}
> +#endif
> +
> qemu_init_vcpu(cs);
> cpu_reset(cs);
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 9f41954894..dc47ec8d3b 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -219,6 +219,15 @@ static RISCVException epmp(CPURISCVState *env, int csrno)
>
> return RISCV_EXCP_ILLEGAL_INST;
> }
> +
> +static RISCVException debug(CPURISCVState *env, int csrno)
> +{
> +if (riscv_feature(env, RISCV_FEATURE_DEBUG)) {
> +return RISCV_EXCP_NONE;
> +}
> +
> +return RISCV_EXCP_ILLEGAL_INST;
> +}
> #endif
>
> /* User Floating-Point CSRs */
> @@ -1435,6 +1444,48 @@ static RISCVException write_pmpaddr(CPURISCVState
> *env, int csrno,
> return RISCV_EXCP_NONE;
> }
>
> +static RISCVException read_tselect(CPURISCVState *env, int csrno,
> + target_ulong *val)
> +{
> +*val = tselect_csr_read(env);
> +return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException write_tselect(CPURISCVState *env, int csrno,
> +target_ulong val)
> +{
> +tselect_csr_write(env, val);
> +return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException read_tdata(CPURISCVState *env, int csrno,
> + target_ulong *val)
> +{
> +/* return 0 in tdata1 to end the trigger enumeration */
> +if (env->trigger_cur >= TRIGGER_NUM && csrno == CSR_TDATA1) {
> +*val = 0;
> +return RISCV_EXCP_NONE;
> +}
> +
> +if (!tdata_available(env, csrno - CSR_TDATA1)) {
> +return RISCV_EXCP_ILLEGAL_INST;
> +}
> +
> +*val = tdata_csr_read(env, csrno - CSR_TDATA1);
> +return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException write_tdata(CPURISCVState *env, int csrno,
> + target_ulong val)
> +{
> +if (!tdata_available(env, csrno - CSR_TDATA1)) {
> +return RISCV_EXCP_ILLEGAL_INST;
> +}
> +
> +tdata_csr_write(env, csrno - CSR_TDATA1, val);
> +return RISCV_EXCP_NONE;
> +}
> +
> /*
> * Functions to access Pointer Masking feature registers
> * We have to check if current priv lvl could modify
> @@ -1931,6 +1982,12 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> [CSR_PMPADDR14] = { "pmpaddr14", pmp, read_pmpaddr, write_pmpaddr },
> [CSR_PMPADDR15] = { "pmpaddr15", pmp, read_pmpaddr, write_pmpaddr },
>
> +/* Debug CSRs */
> +[CSR_TSELECT] = { "tselect", debug, read_tselect, write_tselect },
> +[CSR_TDATA1]= { "tdata1", debug, read_tdata, write_tdata },
> +[CSR_TDATA2]= { "tdata2", debug, read_tdata, write_tdata },
> +[CSR_TDATA3]= { "tdata3", debug, read_tdata, write_tdata },
> +
> /* User Pointer Masking */
> [CSR_UMTE]={ "umte",pointer_masking, read_umte,
> write_umte},
> [CSR_UPMMASK] ={ "upmmask", pointer_masking, read_upmmask,
> write_upmmask },
> --
> 2.25.1
>
>
Re: [PATCH v2 3/7] target/riscv: debug: Implement debug related TCGCPUOps
On Sat, Oct 30, 2021 at 11:59 PM Bin Meng wrote:
>
> Implement .debug_excp_handler, .debug_check_{breakpoint, watchpoint}
> TCGCPUOps and hook them into riscv_tcg_ops.
>
> Signed-off-by: Bin Meng
Reviewed-by: Alistair Francis
Alistair
>
> ---
>
> Changes in v2:
> - use 0 instead of GETPC()
>
> target/riscv/debug.h | 4 +++
> target/riscv/cpu.c | 3 ++
> target/riscv/debug.c | 75
> 3 files changed, 82 insertions(+)
>
> diff --git a/target/riscv/debug.h b/target/riscv/debug.h
> index cb8a6e0024..fddc103650 100644
> --- a/target/riscv/debug.h
> +++ b/target/riscv/debug.h
> @@ -107,4 +107,8 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index,
> target_ulong val);
>
> void riscv_trigger_init(CPURISCVState *env);
>
> +void riscv_cpu_debug_excp_handler(CPUState *cs);
> +bool riscv_cpu_debug_check_breakpoint(CPUState *cs);
> +bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp);
> +
> #endif /* RISCV_DEBUG_H */
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 7d53125dbc..7061ae05fb 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -701,6 +701,9 @@ static const struct TCGCPUOps riscv_tcg_ops = {
> .do_interrupt = riscv_cpu_do_interrupt,
> .do_transaction_failed = riscv_cpu_do_transaction_failed,
> .do_unaligned_access = riscv_cpu_do_unaligned_access,
> +.debug_excp_handler = riscv_cpu_debug_excp_handler,
> +.debug_check_breakpoint = riscv_cpu_debug_check_breakpoint,
> +.debug_check_watchpoint = riscv_cpu_debug_check_watchpoint,
> #endif /* !CONFIG_USER_ONLY */
> };
>
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index 9bcca27b72..9cb2a6d7ba 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -364,3 +364,78 @@ void riscv_trigger_init(CPURISCVState *env)
> env->trigger_type2[i].wp = NULL;
> }
> }
> +
> +void riscv_cpu_debug_excp_handler(CPUState *cs)
> +{
> +RISCVCPU *cpu = RISCV_CPU(cs);
> +CPURISCVState *env = >env;
> +
> +if (cs->watchpoint_hit) {
> +if (cs->watchpoint_hit->flags & BP_CPU) {
> +cs->watchpoint_hit = NULL;
> +riscv_raise_exception(env, RISCV_EXCP_BREAKPOINT, 0);
> +}
> +} else {
> +if (cpu_breakpoint_test(cs, env->pc, BP_CPU)) {
> +riscv_raise_exception(env, RISCV_EXCP_BREAKPOINT, 0);
> +}
> +}
> +}
> +
> +bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
> +{
> +RISCVCPU *cpu = RISCV_CPU(cs);
> +CPURISCVState *env = >env;
> +CPUBreakpoint *bp;
> +target_ulong ctrl;
> +target_ulong pc;
> +int i;
> +
> +QTAILQ_FOREACH(bp, >breakpoints, entry) {
> +for (i = 0; i < TRIGGER_TYPE2_NUM; i++) {
> +ctrl = env->trigger_type2[i].mcontrol;
> +pc = env->trigger_type2[i].maddress;
> +
> +if ((ctrl & TYPE2_EXEC) && (bp->pc == pc)) {
> +/* check U/S/M bit against current privilege level */
> +if ((ctrl >> 3) & BIT(env->priv)) {
> +return true;
> +}
> +}
> +}
> +}
> +
> +return false;
> +}
> +
> +bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
> +{
> +RISCVCPU *cpu = RISCV_CPU(cs);
> +CPURISCVState *env = >env;
> +target_ulong ctrl;
> +target_ulong addr;
> +int flags;
> +int i;
> +
> +for (i = 0; i < TRIGGER_TYPE2_NUM; i++) {
> +ctrl = env->trigger_type2[i].mcontrol;
> +addr = env->trigger_type2[i].maddress;
> +flags = 0;
> +
> +if (ctrl & TYPE2_LOAD) {
> +flags |= BP_MEM_READ;
> +}
> +if (ctrl & TYPE2_STORE) {
> +flags |= BP_MEM_WRITE;
> +}
> +
> +if ((wp->flags & flags) && (wp->vaddr == addr)) {
> +/* check U/S/M bit against current privilege level */
> +if ((ctrl >> 3) & BIT(env->priv)) {
> +return true;
> +}
> +}
> +}
> +
> +return false;
> +}
> --
> 2.25.1
>
>
Re: [PATCH v2 2/7] target/riscv: machine: Add debug state description
On Sat, Oct 30, 2021 at 11:56 PM Bin Meng wrote:
>
> Add a subsection to machine.c to migrate debug CSR state.
>
> Signed-off-by: Bin Meng
Reviewed-by: Alistair Francis
Alistair
>
> ---
>
> Changes in v2:
> - new patch: add debug state description
>
> target/riscv/machine.c | 33 +
> 1 file changed, 33 insertions(+)
>
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index ad8248ebfd..25aa3b38f7 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -164,6 +164,38 @@ static const VMStateDescription vmstate_pointermasking =
> {
> }
> };
>
> +static bool debug_needed(void *opaque)
> +{
> +RISCVCPU *cpu = opaque;
> +CPURISCVState *env = >env;
> +
> +return riscv_feature(env, RISCV_FEATURE_DEBUG);
> +}
> +
> +static const VMStateDescription vmstate_debug_type2 = {
> +.name = "cpu/debug/type2",
> +.version_id = 1,
> +.minimum_version_id = 1,
> +.fields = (VMStateField[]) {
> +VMSTATE_UINTTL(mcontrol, trigger_type2_t),
> +VMSTATE_UINTTL(maddress, trigger_type2_t),
> +VMSTATE_END_OF_LIST()
> +}
> +};
> +
> +static const VMStateDescription vmstate_debug = {
> +.name = "cpu/debug",
> +.version_id = 1,
> +.minimum_version_id = 1,
> +.needed = debug_needed,
> +.fields = (VMStateField[]) {
> +VMSTATE_UINTTL(env.trigger_cur, RISCVCPU),
> +VMSTATE_STRUCT_ARRAY(env.trigger_type2, RISCVCPU, TRIGGER_TYPE2_NUM,
> + 0, vmstate_debug_type2, trigger_type2_t),
> +VMSTATE_END_OF_LIST()
> +}
> +};
> +
> const VMStateDescription vmstate_riscv_cpu = {
> .name = "cpu",
> .version_id = 3,
> @@ -218,6 +250,7 @@ const VMStateDescription vmstate_riscv_cpu = {
> _hyper,
> _vector,
> _pointermasking,
> +_debug,
> NULL
> }
> };
> --
> 2.25.1
>
>
Re: [PATCH] target/riscv: Check PMP rules num before propagation
0
On 2021/11/17 上午8:03, Alistair Francis wrote:
On Wed, Nov 17, 2021 at 1:12 AM LIU Zhiwei wrote:
If PMP rules number is zero, it should not influence the TLB entry for
M-mode program.
This doesn't sound right. From what I can tell if we have no rules
pmp_is_range_in_tlb() shouldn't have an effect on the tlb_size. What
error are you seeing?
When address is in [0-4K] and no pmp rule configured, the tlb_size will
be set to 1.
This is caused by pmp_get_tlb_size return a value 1.
if (pmp_sa >= tlb_sa && pmp_ea <= tlb_ea) {
return pmp_ea - pmp_sa + 1;
}
Here pmp_sa == 0 and pmp_ea == 0.
Thanks,
Zhiwei
Alistair
Signed-off-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 9eeed38c7e..48da872d39 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -376,7 +376,7 @@ static int get_physical_address_pmp(CPURISCVState *env, int
*prot,
}
*prot = pmp_priv_to_page_prot(pmp_priv);
-if (tlb_size != NULL) {
+if ((tlb_size != NULL) && pmp_get_num_rules(env)) {
if (pmp_is_range_in_tlb(env, addr & ~(*tlb_size - 1), _size_pmp))
{
*tlb_size = tlb_size_pmp;
}
--
2.25.1
[PULL 5/5] scripts/device-crash-test: hide tracebacks for QMP connect errors
Generally, the traceback for a connection failure is uninteresting and
all we need to know is that the connection attempt failed.
Reduce the verbosity in these cases, except when debugging.
Signed-off-by: John Snow
Reported-by: Thomas Huth
Tested-by: Thomas Huth
Message-id: 2021143719.2162525-6-js...@redhat.com
Signed-off-by: John Snow
---
scripts/device-crash-test | 21 ++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/scripts/device-crash-test b/scripts/device-crash-test
index 49bcd61b4f..3db0ffe5b8 100755
--- a/scripts/device-crash-test
+++ b/scripts/device-crash-test
@@ -36,6 +36,7 @@ from itertools import chain
sys.path.append(os.path.join(os.path.dirname(__file__), '..', 'python'))
from qemu.machine import QEMUMachine
+from qemu.aqmp import ConnectError
logger = logging.getLogger('device-crash-test')
dbg = logger.debug
@@ -355,10 +356,12 @@ def checkOneCase(args, testcase):
dbg("will launch QEMU: %s", cmdline)
vm = QEMUMachine(binary=binary, args=args)
+exc = None
exc_traceback = None
try:
vm.launch()
-except Exception:
+except Exception as this_exc:
+exc = this_exc
exc_traceback = traceback.format_exc()
dbg("Exception while running test case")
finally:
@@ -366,8 +369,9 @@ def checkOneCase(args, testcase):
ec = vm.exitcode()
log = vm.get_log()
-if exc_traceback is not None or ec != 0:
-return {'exc_traceback':exc_traceback,
+if exc is not None or ec != 0:
+return {'exc': exc,
+'exc_traceback':exc_traceback,
'exitcode':ec,
'log':log,
'testcase':testcase,
@@ -455,6 +459,17 @@ def logFailure(f, level):
for l in f['log'].strip().split('\n'):
logger.log(level, "log: %s", l)
logger.log(level, "exit code: %r", f['exitcode'])
+
+# If the Exception is merely a QMP connect error,
+# reduce the logging level for its traceback to
+# improve visual clarity.
+if isinstance(f.get('exc'), ConnectError):
+logger.log(level, "%s.%s: %s",
+ type(f['exc']).__module__,
+ type(f['exc']).__qualname__,
+ str(f['exc']))
+level = logging.DEBUG
+
if f['exc_traceback']:
logger.log(level, "exception:")
for l in f['exc_traceback'].split('\n'):
--
2.31.1
Re: [PATCH for-6.2] meson.build: Merge riscv32 and riscv64 cpu family
On Tue, Nov 16, 2021 at 7:51 PM Richard Henderson wrote: > > In ba0e73336200, we merged riscv32 and riscv64 in configure. > However, meson does not treat them the same. We need to merge > them here as well. > > Fixes: ba0e73336200 > Signed-off-by: Richard Henderson Reviewed-by: Alistair Francis Alistair > --- > > At the moment, configure for riscv64 host fails during meson. > > > r~ > > --- > meson.build | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/meson.build b/meson.build > index 2ece4fe088..ccc6cefc25 100644 > --- a/meson.build > +++ b/meson.build > @@ -59,6 +59,12 @@ supported_cpus = ['ppc', 'ppc64', 's390x', 'riscv', 'x86', > 'x86_64', >'arm', 'aarch64', 'mips', 'mips64', 'sparc', 'sparc64'] > > cpu = host_machine.cpu_family() > + > +# Unify riscv* to a single family. > +if cpu in ['riscv32', 'riscv64'] > + cpu = 'riscv' > +endif > + > targetos = host_machine.system() > > if cpu in ['x86', 'x86_64'] > -- > 2.25.1 > >
[PULL 4/5] scripts/device-crash-test: don't emit AQMP connection errors to stdout
These errors are expected, so they shouldn't clog up terminal output. In
the event that they're *not* expected, we'll be seeing an awful lot more
output concerning the nature of the failure.
Reported-by: Thomas Huth
Signed-off-by: John Snow
Tested-by: Thomas Huth
Message-id: 2021143719.2162525-5-js...@redhat.com
Signed-off-by: John Snow
---
scripts/device-crash-test | 6 ++
1 file changed, 6 insertions(+)
diff --git a/scripts/device-crash-test b/scripts/device-crash-test
index d91e8616ef..49bcd61b4f 100755
--- a/scripts/device-crash-test
+++ b/scripts/device-crash-test
@@ -499,6 +499,12 @@ def main():
lvl = logging.WARN
logging.basicConfig(stream=sys.stdout, level=lvl, format='%(levelname)s:
%(message)s')
+if not args.debug:
+# Async QMP, when in use, is chatty about connection failures.
+# This script knowingly generates a ton of connection errors.
+# Silence this logger.
+logging.getLogger('qemu.aqmp.qmp_client').setLevel(logging.CRITICAL)
+
fatal_failures = []
wl_stats = {}
skipped = 0
--
2.31.1
[PULL 3/5] scripts/device-crash-test: simplify Exception handling
We don't need to handle KeyboardInterruptError specifically; we can
instead tighten the scope of the broad Exception handlers to only catch
"Exception", which has the effect of allowing all BaseException classes
that do not inherit from Exception to be raised through.
KeyboardInterruptError and a few other important ones are
BaseExceptions, so this does the same thing with less code.
Signed-off-by: John Snow
Reported-by: Thomas Huth
Tested-by: Thomas Huth
Message-id: 2021143719.2162525-4-js...@redhat.com
Signed-off-by: John Snow
---
scripts/device-crash-test | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/scripts/device-crash-test b/scripts/device-crash-test
index 8331c057b8..d91e8616ef 100755
--- a/scripts/device-crash-test
+++ b/scripts/device-crash-test
@@ -317,9 +317,7 @@ class QemuBinaryInfo(object):
try:
vm.launch()
mi['runnable'] = True
-except KeyboardInterrupt:
-raise
-except:
+except Exception:
dbg("exception trying to run binary=%s machine=%s", self.binary,
machine, exc_info=sys.exc_info())
dbg("log: %r", vm.get_log())
mi['runnable'] = False
@@ -360,9 +358,7 @@ def checkOneCase(args, testcase):
exc_traceback = None
try:
vm.launch()
-except KeyboardInterrupt:
-raise
-except:
+except Exception:
exc_traceback = traceback.format_exc()
dbg("Exception while running test case")
finally:
--
2.31.1
[PULL 2/5] python/aqmp: fix ConnectError string method
When ConnectError is used to wrap an Exception that was initialized
without an error message, we are treated to a traceback with a rubbish
line like this:
... ConnectError: Failed to establish session:
Correct this to use the name of an exception as a fallback message:
... ConnectError: Failed to establish session: EOFError
Better!
Signed-off-by: John Snow
Reported-by: Thomas Huth
Tested-by: Thomas Huth
Message-id: 2021143719.2162525-3-js...@redhat.com
Signed-off-by: John Snow
---
python/qemu/aqmp/protocol.py | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/python/qemu/aqmp/protocol.py b/python/qemu/aqmp/protocol.py
index 860b79512d..5190b33b13 100644
--- a/python/qemu/aqmp/protocol.py
+++ b/python/qemu/aqmp/protocol.py
@@ -79,7 +79,11 @@ def __init__(self, error_message: str, exc: Exception):
self.exc: Exception = exc
def __str__(self) -> str:
-return f"{self.error_message}: {self.exc!s}"
+cause = str(self.exc)
+if not cause:
+# If there's no error string, use the exception name.
+cause = exception_summary(self.exc)
+return f"{self.error_message}: {cause}"
class StateError(AQMPError):
--
2.31.1
[PULL 1/5] python/aqmp: Fix disconnect during capabilities negotiation
If we receive ConnectionResetError (ECONNRESET) while attempting to
perform capabilities negotiation -- prior to the establishment of the
async reader/writer tasks -- the disconnect function is not aware that
we are in an error pathway.
As a result, when attempting to close the StreamWriter, we'll see the
same ConnectionResetError that caused us to initiate a disconnect in the
first place, which will cause the disconnect task itself to fail, which
emits a CRITICAL logging event.
I still don't know if there's a smarter way to check to see if an
exception received at this point is "the same" exception as the one that
caused the initial disconnect, but for now the problem can be avoided by
improving the error pathway detection in the exit path.
Reported-by: Thomas Huth
Signed-off-by: John Snow
Tested-by: Thomas Huth
Message-id: 2021143719.2162525-2-js...@redhat.com
Signed-off-by: John Snow
---
python/qemu/aqmp/protocol.py | 18 +-
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/python/qemu/aqmp/protocol.py b/python/qemu/aqmp/protocol.py
index ae1df24026..860b79512d 100644
--- a/python/qemu/aqmp/protocol.py
+++ b/python/qemu/aqmp/protocol.py
@@ -623,13 +623,21 @@ async def _bh_disconnect(self) -> None:
def _done(task: Optional['asyncio.Future[Any]']) -> bool:
return task is not None and task.done()
-# NB: We can't rely on _bh_tasks being done() here, it may not
-# yet have had a chance to run and gather itself.
+# Are we already in an error pathway? If either of the tasks are
+# already done, or if we have no tasks but a reader/writer; we
+# must be.
+#
+# NB: We can't use _bh_tasks to check for premature task
+# completion, because it may not yet have had a chance to run
+# and gather itself.
tasks = tuple(filter(None, (self._writer_task, self._reader_task)))
error_pathway = _done(self._reader_task) or _done(self._writer_task)
+if not tasks:
+error_pathway |= bool(self._reader) or bool(self._writer)
try:
-# Try to flush the writer, if possible:
+# Try to flush the writer, if possible.
+# This *may* cause an error and force us over into the error path.
if not error_pathway:
await self._bh_flush_writer()
except BaseException as err:
@@ -639,7 +647,7 @@ def _done(task: Optional['asyncio.Future[Any]']) -> bool:
self.logger.debug("%s:\n%s\n", emsg, pretty_traceback())
raise
finally:
-# Cancel any still-running tasks:
+# Cancel any still-running tasks (Won't raise):
if self._writer_task is not None and not self._writer_task.done():
self.logger.debug("Cancelling writer task.")
self._writer_task.cancel()
@@ -652,7 +660,7 @@ def _done(task: Optional['asyncio.Future[Any]']) -> bool:
self.logger.debug("Waiting for tasks to complete ...")
await asyncio.wait(tasks)
-# Lastly, close the stream itself. (May raise):
+# Lastly, close the stream itself. (*May raise*!):
await self._bh_close_stream(error_pathway)
self.logger.debug("Disconnected.")
--
2.31.1
[PULL 0/5] Python patches
The following changes since commit 2b22e7540d6ab4efe82d442363e3fc900cea6584: Merge tag 'm68k-for-6.2-pull-request' of git://github.com/vivier/qemu-m68k into staging (2021-11-09 13:16:56 +0100) are available in the Git repository at: https://gitlab.com/jsnow/qemu.git tags/python-pull-request for you to fetch changes up to c398a241ec4138e0b995a0215dea84ca93b0384f: scripts/device-crash-test: hide tracebacks for QMP connect errors (2021-11-16 14:26:36 -0500) Pull request John Snow (5): python/aqmp: Fix disconnect during capabilities negotiation python/aqmp: fix ConnectError string method scripts/device-crash-test: simplify Exception handling scripts/device-crash-test: don't emit AQMP connection errors to stdout scripts/device-crash-test: hide tracebacks for QMP connect errors python/qemu/aqmp/protocol.py | 24 ++-- scripts/device-crash-test| 33 + 2 files changed, 43 insertions(+), 14 deletions(-) -- 2.31.1
Re: [PATCH] target/riscv: Check PMP rules num before propagation
On Wed, Nov 17, 2021 at 1:12 AM LIU Zhiwei wrote:
>
> If PMP rules number is zero, it should not influence the TLB entry for
> M-mode program.
This doesn't sound right. From what I can tell if we have no rules
pmp_is_range_in_tlb() shouldn't have an effect on the tlb_size. What
error are you seeing?
Alistair
>
> Signed-off-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 9eeed38c7e..48da872d39 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -376,7 +376,7 @@ static int get_physical_address_pmp(CPURISCVState *env,
> int *prot,
> }
>
> *prot = pmp_priv_to_page_prot(pmp_priv);
> -if (tlb_size != NULL) {
> +if ((tlb_size != NULL) && pmp_get_num_rules(env)) {
> if (pmp_is_range_in_tlb(env, addr & ~(*tlb_size - 1),
> _size_pmp)) {
> *tlb_size = tlb_size_pmp;
> }
> --
> 2.25.1
>
>
Re: [PATCH for-6.2] meson.build: Merge riscv32 and riscv64 cpu family
On Tue, Nov 16, 2021 at 7:51 PM Richard Henderson wrote: > > In ba0e73336200, we merged riscv32 and riscv64 in configure. > However, meson does not treat them the same. We need to merge > them here as well. > > Fixes: ba0e73336200 > Signed-off-by: Richard Henderson Reviewed-by: Alistair Francis Alistair > --- > > At the moment, configure for riscv64 host fails during meson. > > > r~ > > --- > meson.build | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/meson.build b/meson.build > index 2ece4fe088..ccc6cefc25 100644 > --- a/meson.build > +++ b/meson.build > @@ -59,6 +59,12 @@ supported_cpus = ['ppc', 'ppc64', 's390x', 'riscv', 'x86', > 'x86_64', >'arm', 'aarch64', 'mips', 'mips64', 'sparc', 'sparc64'] > > cpu = host_machine.cpu_family() > + > +# Unify riscv* to a single family. > +if cpu in ['riscv32', 'riscv64'] > + cpu = 'riscv' > +endif > + > targetos = host_machine.system() > > if cpu in ['x86', 'x86_64'] > -- > 2.25.1 > >
Re: [PULL SUBSYSTEM qemu-pseries] pseries: Update SLOF firmware image
On 17/11/2021 04:28, Richard Henderson wrote: On 11/16/21 6:12 PM, Philippe Mathieu-Daudé wrote: On 11/16/21 17:46, Cédric Le Goater wrote: On 11/14/21 01:51, Alexey Kardashevskiy wrote: The following changes since commit d139786e1b3d67991e6cb49a8a59bb2182350285: ppc/mmu_helper.c: do not truncate 'ea' in booke206_invalidate_ea_tlb() (2021-11-11 11:35:13 +0100) are available in the Git repository at: g...@github.com:aik/qemu.git tags/qemu-slof-2022 for you to fetch changes up to 73944a4bf4ab259b489af8128b4aec525484d642: pseries: Update SLOF firmware image (2021-11-13 14:47:56 +1100) Alexey Kardashevskiy (1): pseries: Update SLOF firmware image pc-bios/README | 2 +- pc-bios/slof.bin | Bin 991744 -> 991920 bytes roms/SLOF | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) Queued for 7.0. I am not sure this is a good idea, as this will make bisection painful over the release tag. It is my understanding that Cedric will rebase for the mainline PR. At least that's how David was handling subsystem pulls. Yup. I am doing SLOF updates this way for ages after diifs became quite huge to make mailman barfing on the size, and the "subsystem" in the subj was the way to reduce the noise Peter had to respond to :) btw should I be signing those? I am not now. -- Alexey
[PATCH] edid: Added support for 4k@60 Hz monitor
From: Satyeshwar Singh
Previously, the large modes (>1080p) that were generated by Qemu in its EDID
were all 50 Hz. If we provide them to a Guest OS and the user selects
one of these modes, then the OS by default only gets 50 FPS. This is
especially true for Windows OS. With this patch, we are now exposing a
3840x2160@60 Hz which will allow the guest OS to get 60 FPS.
Cc: Gerd Hoffmann
Signed-off-by: Satyeshwar Singh
---
hw/display/edid-generate.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/display/edid-generate.c b/hw/display/edid-generate.c
index f2b874d5e3..397af0bd63 100644
--- a/hw/display/edid-generate.c
+++ b/hw/display/edid-generate.c
@@ -24,6 +24,9 @@ static const struct edid_mode {
{ .xres = 2048, .yres = 1152 },
{ .xres = 1920, .yres = 1080, .dta = 31 },
+/* dea/dta extension timings (all @ 60 Hz) */
+{ .xres = 3840, .yres = 2160, .dta = 97 },
+
/* additional standard timings 3 (all @ 60Hz) */
{ .xres = 1920, .yres = 1200, .xtra3 = 10, .bit = 0 },
{ .xres = 1600, .yres = 1200, .xtra3 = 9, .bit = 2 },
--
2.20.1
Re: [PATCH v4 8/9] common-user: Adjust system call return on FreeBSD
On 11/16/21 9:58 PM, Warner Losh wrote: +#elif defined(__FreeBSD__) + /* FreeBSD kernel returns positive errno and C bit set. */ + jcs 1f I needed to change this to 'jc' and that's all google found for Intel. Yep, that's me jumping between too many arches in one day. It's jc/jnc for intel. r~
[PATCH] sev: check which processor the ASK/ARK chain should match
The AMD ASK/ARK certificate chain differs between AMD SEV
processor generations. SEV capabilities should provide
which ASK/ARK certificate should be used based on the host
processor.
Signed-off-by: Tyler Fanelli
---
qapi/misc-target.json | 28 ++--
target/i386/sev.c | 17 ++---
2 files changed, 40 insertions(+), 5 deletions(-)
diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index 5aa2b95b7d..c64aa3ff57 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -166,6 +166,24 @@
{ 'command': 'query-sev-launch-measure', 'returns': 'SevLaunchMeasureInfo',
'if': 'TARGET_I386' }
+##
+# @SevAskArkCertName:
+#
+# This enum describes which ASK/ARK certificate should be
+# used based on the generation of an AMD Secure Encrypted
+# Virtualization processor.
+#
+# @naples: AMD Naples processor (SEV 1st generation)
+#
+# @rome: AMD Rome processor (SEV 2nd generation)
+#
+# @milan: AMD Milan processor (SEV 3rd generation)
+#
+# Since: 7.0
+##
+{ 'enum': 'SevAskArkCertName',
+ 'data': ['naples', 'rome', 'milan'],
+ 'if': 'TARGET_I386' }
##
# @SevCapability:
@@ -182,13 +200,18 @@
# @reduced-phys-bits: Number of physical Address bit reduction when SEV is
# enabled
#
+# @ask-ark-cert-name: The generation in which the AMD
+# ARK/ASK should be derived from
+# (since 7.0)
+#
# Since: 2.12
##
{ 'struct': 'SevCapability',
'data': { 'pdh': 'str',
'cert-chain': 'str',
'cbitpos': 'int',
-'reduced-phys-bits': 'int'},
+'reduced-phys-bits': 'int',
+'ask-ark-cert-name': 'SevAskArkCertName'},
'if': 'TARGET_I386' }
##
@@ -205,7 +228,8 @@
#
# -> { "execute": "query-sev-capabilities" }
# <- { "return": { "pdh": "8CCDD8DDD", "cert-chain": "888CCCDDDEE",
-# "cbitpos": 47, "reduced-phys-bits": 5}}
+# "cbitpos": 47, "reduced-phys-bits": 5,
+# "ask-ark-cert-name": "naples"}}
#
##
{ 'command': 'query-sev-capabilities', 'returns': 'SevCapability',
diff --git a/target/i386/sev.c b/target/i386/sev.c
index eede07f11d..f30171e5ba 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -506,8 +506,9 @@ static SevCapability *sev_get_capabilities(Error **errp)
guchar *pdh_data = NULL;
guchar *cert_chain_data = NULL;
size_t pdh_len = 0, cert_chain_len = 0;
-uint32_t ebx;
-int fd;
+uint32_t eax, ebx;
+int fd, es, snp;
+
if (!kvm_enabled()) {
error_setg(errp, "KVM not enabled");
@@ -534,9 +535,19 @@ static SevCapability *sev_get_capabilities(Error **errp)
cap->pdh = g_base64_encode(pdh_data, pdh_len);
cap->cert_chain = g_base64_encode(cert_chain_data, cert_chain_len);
-host_cpuid(0x801F, 0, NULL, , NULL, NULL);
+host_cpuid(0x801F, 0, , , NULL, NULL);
cap->cbitpos = ebx & 0x3f;
+es = eax & 0x8;
+snp = eax & 0x10;
+if (!es && !snp) {
+ cap->ask_ark_cert_name = SEV_ASK_ARK_CERT_NAME_NAPLES;
+} else if (es && !snp) {
+ cap->ask_ark_cert_name = SEV_ASK_ARK_CERT_NAME_ROME;
+} else {
+ cap->ask_ark_cert_name = SEV_ASK_ARK_CERT_NAME_MILAN;
+}
+
/*
* When SEV feature is enabled, we loose one bit in guest physical
* addressing.
--
2.31.1
Re: [PATCH v2 08/15] hw/nvme: Implement the Function Level Reset
On Tue, Nov 16, 2021 at 04:34:39PM +0100, Łukasz Gieryk wrote:
> if (!pci_is_vf(>parent_obj) && n->params.sriov_max_vfs) {
> -pcie_sriov_pf_disable_vfs(>parent_obj);
> +if (rst != NVME_RESET_CONTROLLER) {
> +pcie_sriov_pf_disable_vfs(>parent_obj);
Shouldn't this be 'if (rst == NVME_RESET_FUNCTION)'?
Re: [PATCH v4 0/9] linux-user: simplify safe signal handling
On Tue, Nov 16, 2021 at 4:02 AM Richard Henderson <
richard.hender...@linaro.org> wrote:
> Warner's v3:
> https://patchew.org/QEMU/2023045603.60391-1-...@bsdimp.com/
>
> Changes for v4:
> * Move errno handling into the assembly. While returning the
> raw -errno is handy for x86 linux (and a few others), it is
> in fact more complex for other hosts that return a separate
> error indicator. At which point we wind up jumping through
> hoops to return -errno, only to have the caller put it right
> back into +errno with -1 result, just like syscall(3).
>
> Pass in , because the method of calculating this
> varies wildly between glibc, musl, etc. This means that
> the assembly need only store to a provided pointer.
>
> * Add mips and sparc safe-syscall implementations.
> Both of which, btw, have separate error indicators. ;-)
>
> * All hosts now have it, so remove HAVE_SAFE_SYSCALL.
>
> * Add meson.build rules for common-user/safe-syscall.S, so
> that we don't have to have weird includes from *-user.
>
> I'll note that this last patch will at present break bsd-user,
> because TARGET_ERESTARTSYS and the header from whence it comes
> is currently missing there.
>
> In addition, I think that this should be reorganized further
> so that TARGET_ERESTARTSYS is (1) renamed because in *this*
> context it is pretending to be a host errno, and (2) placed
> in a file of its own under include/user/. At which point,
> meson.build could be simplified further so that safe-syscall.S
> is compiled once, not per target.
>
> Anyway, the final patch needs some bsd-user changes sorted first.
>
I've got bsd-user to compile with these changes by creating
bsd-user/target_errno_defs.h that was just #include "errno_defs.h", which
suggests a simple rename and #include adjustment in bsd-user/syscall_defs.h
would also work. On *BSD, the errno namespace is the same for all
architectures since they never followed the rather divergent System V ABIs
that Linux follows (or at least did for the early ports).
I've noted a couple of other tweaks I needed as well, but there seemed to
be no good place to share this.
I'd be happy to change these vague descriptions into actual code I can
push to gitlab that you can pull into the patch series as well (or I can
send them to the list, I'm not sure about this finer point of qemu and want
to fit in).
Thanks for expanding my start at this.
Warner
>
> r~
>
>
> Richard Henderson (4):
> common-user: Move syscall error detection into safe_syscall_base
> common-user/host/mips: Add safe-syscall.inc.S
> common-user/host/sparc64: Add safe-syscall.inc.S
> linux-user: Remove HAVE_SAFE_SYSCALL and hostdep.h
>
> Warner Losh (5):
> linux-user: Add host_signal_set_pc to set pc in mcontext
> linux-user/signal.c: Create a common rewind_if_in_safe_syscall
> linux-user/safe-syscall.inc.S: Move to common-user
> common-user: Adjust system call return on FreeBSD
> common-user: Move safe-syscall.* from *-user
>
> meson.build | 9 +-
> {linux-user => include/user}/safe-syscall.h | 31 ++--
> linux-user/host/aarch64/host-signal.h | 5 +
> linux-user/host/aarch64/hostdep.h | 38 -
> linux-user/host/alpha/host-signal.h | 5 +
> linux-user/host/arm/host-signal.h | 5 +
> linux-user/host/arm/hostdep.h | 38 -
> linux-user/host/i386/host-signal.h| 5 +
> linux-user/host/i386/hostdep.h| 38 -
> linux-user/host/ia64/hostdep.h| 15 --
> linux-user/host/mips/host-signal.h| 5 +
> linux-user/host/mips/hostdep.h| 15 --
> linux-user/host/ppc/host-signal.h | 5 +
> linux-user/host/ppc/hostdep.h | 15 --
> linux-user/host/ppc64/hostdep.h | 38 -
> linux-user/host/riscv/host-signal.h | 5 +
> linux-user/host/riscv/hostdep.h | 34 -
> linux-user/host/s390/host-signal.h| 5 +
> linux-user/host/s390/hostdep.h| 15 --
> linux-user/host/s390x/hostdep.h | 38 -
> linux-user/host/sparc/host-signal.h | 9 ++
> linux-user/host/sparc/hostdep.h | 15 --
> linux-user/host/sparc64/hostdep.h | 15 --
> linux-user/host/x32/hostdep.h | 15 --
> linux-user/host/x86_64/host-signal.h | 5 +
> linux-user/host/x86_64/hostdep.h | 38 -
> linux-user/user-internals.h | 1 -
> linux-user/signal.c | 13 +-
> linux-user/syscall.c | 2 +-
> .../host/aarch64/safe-syscall.inc.S | 65 ++---
> .../host/arm/safe-syscall.inc.S | 69 ++---
> .../host/i386/safe-syscall.inc.S | 61 +---
> common-user/host/mips/safe-syscall.inc.S
[PATCH] linux-user/hexagon: Use generic target_stat64 structure
Linux Hexagon port doesn't define a specific 'struct stat'
but uses the generic one (see Linux commit 6103ec56c65c [*]
"asm-generic: add generic ABI headers" which predates the
introduction of the Hexagon port).
Remove the target specific target_stat (which in fact is the
target_stat64 structure but uses incorrect target_long and
ABI unsafe long long types) and use the generic target_stat64
instead.
[*]
https://github.com/torvalds/linux/commit/6103ec56c65c3#diff-5f59b07b38273b7d6a74193bc81a8cd18928c688276eae20cb10c569de3253ee
Signed-off-by: Philippe Mathieu-Daudé
---
linux-user/syscall_defs.h | 28 ++--
1 file changed, 2 insertions(+), 26 deletions(-)
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index a5ce487dcc3..7ab612d163b 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -2129,7 +2129,8 @@ struct target_stat64 {
abi_ulong __unused5;
};
-#elif defined(TARGET_OPENRISC) || defined(TARGET_NIOS2) ||
defined(TARGET_RISCV)
+#elif defined(TARGET_OPENRISC) || defined(TARGET_NIOS2) \
+|| defined(TARGET_RISCV) || defined(TARGET_HEXAGON)
/* These are the asm-generic versions of the stat and stat64 structures */
@@ -2240,31 +2241,6 @@ struct target_stat64 {
uint64_t st_ino;
};
-#elif defined(TARGET_HEXAGON)
-
-struct target_stat {
-unsigned long long st_dev;
-unsigned long long st_ino;
-unsigned int st_mode;
-unsigned int st_nlink;
-unsigned int st_uid;
-unsigned int st_gid;
-unsigned long long st_rdev;
-target_ulong __pad1;
-long long st_size;
-target_long st_blksize;
-int __pad2;
-long long st_blocks;
-
-target_long target_st_atime;
-target_long target_st_atime_nsec;
-target_long target_st_mtime;
-target_long target_st_mtime_nsec;
-target_long target_st_ctime;
-target_long target_st_ctime_nsec;
-int __unused[2];
-};
-
#else
#error unsupported CPU
#endif
--
2.31.1
Re: [PATCH v4 3/9] linux-user/safe-syscall.inc.S: Move to common-user
On Tue, Nov 16, 2021 at 4:03 AM Richard Henderson <
richard.hender...@linaro.org> wrote:
> From: Warner Losh
>
> Move all the safe_syscall.inc.S files to common-user. They are almost
> identical between linux-user and bsd-user to re-use.
>
> Signed-off-by: Warner Losh
> Reviewed-by: Richard Henderson
> Message-Id: <2023045603.60391-4-...@bsdimp.com>
> Signed-off-by: Richard Henderson
> ---
> meson.build | 1 +
> {linux-user => common-user}/host/aarch64/hostdep.h | 0
> {linux-user => common-user}/host/arm/hostdep.h | 0
> {linux-user => common-user}/host/i386/hostdep.h | 0
> {linux-user => common-user}/host/ppc64/hostdep.h| 0
> {linux-user => common-user}/host/riscv/hostdep.h| 0
> {linux-user => common-user}/host/s390x/hostdep.h| 0
> {linux-user => common-user}/host/x86_64/hostdep.h | 0
> {linux-user => common-user}/host/aarch64/safe-syscall.inc.S | 0
> {linux-user => common-user}/host/arm/safe-syscall.inc.S | 0
> {linux-user => common-user}/host/i386/safe-syscall.inc.S| 0
> {linux-user => common-user}/host/ppc64/safe-syscall.inc.S | 0
> {linux-user => common-user}/host/riscv/safe-syscall.inc.S | 0
> {linux-user => common-user}/host/s390x/safe-syscall.inc.S | 0
> {linux-user => common-user}/host/x86_64/safe-syscall.inc.S | 0
> 15 files changed, 1 insertion(+)
> rename {linux-user => common-user}/host/aarch64/hostdep.h (100%)
> rename {linux-user => common-user}/host/arm/hostdep.h (100%)
> rename {linux-user => common-user}/host/i386/hostdep.h (100%)
> rename {linux-user => common-user}/host/ppc64/hostdep.h (100%)
> rename {linux-user => common-user}/host/riscv/hostdep.h (100%)
> rename {linux-user => common-user}/host/s390x/hostdep.h (100%)
> rename {linux-user => common-user}/host/x86_64/hostdep.h (100%)
> rename {linux-user => common-user}/host/aarch64/safe-syscall.inc.S (100%)
> rename {linux-user => common-user}/host/arm/safe-syscall.inc.S (100%)
> rename {linux-user => common-user}/host/i386/safe-syscall.inc.S (100%)
> rename {linux-user => common-user}/host/ppc64/safe-syscall.inc.S (100%)
> rename {linux-user => common-user}/host/riscv/safe-syscall.inc.S (100%)
> rename {linux-user => common-user}/host/s390x/safe-syscall.inc.S (100%)
> rename {linux-user => common-user}/host/x86_64/safe-syscall.inc.S (100%)
>
> diff --git a/meson.build b/meson.build
> index ccc6cefc25..ec22cf05c1 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2878,6 +2878,7 @@ foreach target : target_dirs
> if 'CONFIG_LINUX_USER' in config_target
>base_dir = 'linux-user'
>target_inc += include_directories('linux-user/host/' /
> config_host['ARCH'])
> + target_inc += include_directories('common-user/host/' /
> config_host['ARCH'])
> endif
> if 'CONFIG_BSD_USER' in config_target
>base_dir = 'bsd-user'
>
I had to add this:
diff --git a/meson.build b/meson.build
index 0a88bff8d2..349e7a988f 100644
--- a/meson.build
+++ b/meson.build
@@ -2880,6 +2880,8 @@ foreach target : target_dirs
endif
if 'CONFIG_BSD_USER' in config_target
base_dir = 'bsd-user'
+ target_inc += include_directories('bsd-user/host/' /
config_host['ARCH'])
+ target_inc += include_directories('common-user/host/' /
config_host['ARCH'])
target_inc += include_directories('bsd-user/' / targetos)
dir = base_dir / abi
arch_srcs += files(dir / 'target_arch_cpu.c')
to get bsd-user compiling.
> diff --git a/linux-user/host/aarch64/hostdep.h
> b/common-user/host/aarch64/hostdep.h
> similarity index 100%
> rename from linux-user/host/aarch64/hostdep.h
> rename to common-user/host/aarch64/hostdep.h
> diff --git a/linux-user/host/arm/hostdep.h b/common-user/host/arm/hostdep.h
> similarity index 100%
> rename from linux-user/host/arm/hostdep.h
> rename to common-user/host/arm/hostdep.h
> diff --git a/linux-user/host/i386/hostdep.h
> b/common-user/host/i386/hostdep.h
> similarity index 100%
> rename from linux-user/host/i386/hostdep.h
> rename to common-user/host/i386/hostdep.h
> diff --git a/linux-user/host/ppc64/hostdep.h
> b/common-user/host/ppc64/hostdep.h
> similarity index 100%
> rename from linux-user/host/ppc64/hostdep.h
> rename to common-user/host/ppc64/hostdep.h
> diff --git a/linux-user/host/riscv/hostdep.h
> b/common-user/host/riscv/hostdep.h
> similarity index 100%
> rename from linux-user/host/riscv/hostdep.h
> rename to common-user/host/riscv/hostdep.h
> diff --git a/linux-user/host/s390x/hostdep.h
> b/common-user/host/s390x/hostdep.h
> similarity index 100%
> rename from linux-user/host/s390x/hostdep.h
> rename to common-user/host/s390x/hostdep.h
> diff --git a/linux-user/host/x86_64/hostdep.h
> b/common-user/host/x86_64/hostdep.h
> similarity index 100%
> rename from linux-user/host/x86_64/hostdep.h
> rename to common-user/host/x86_64/hostdep.h
> diff --git
Re: [PATCH v4 8/9] common-user: Adjust system call return on FreeBSD
On Tue, Nov 16, 2021 at 4:03 AM Richard Henderson <
richard.hender...@linaro.org> wrote:
> From: Warner Losh
>
> FreeBSD system calls return positive errno. On the 4 hosts for
> which we have support, error is indicated by the C bit set or clear.
>
> Signed-off-by: Warner Losh
> [rth: Rebase on new safe_syscall_base api; add #error check.]
> Signed-off-by: Richard Henderson
> ---
> common-user/host/aarch64/safe-syscall.inc.S | 12 +++-
> common-user/host/arm/safe-syscall.inc.S | 11 +++
> common-user/host/i386/safe-syscall.inc.S| 10 ++
> common-user/host/x86_64/safe-syscall.inc.S | 10 ++
> 4 files changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/common-user/host/aarch64/safe-syscall.inc.S
> b/common-user/host/aarch64/safe-syscall.inc.S
> index 95c60d8609..d3f065cdef 100644
> --- a/common-user/host/aarch64/safe-syscall.inc.S
> +++ b/common-user/host/aarch64/safe-syscall.inc.S
> @@ -65,12 +65,22 @@ safe_syscall_start:
> safe_syscall_end:
>
> /* code path for having successfully executed the syscall */
> - cmn x0, #4095
> +#if defined(__linux__)
> + /* Linux kernel returns (small) negative errno. */
> + cmn x0, #4096
> + b.hi0f
> +#elif defined(__FreeBSD__)
> + /* FreeBSD kernel returns positive errno and C bit set. */
> b.cs1f
> +#else
> +#error "unsupported os"
> +#endif
> ret
>
> /* code path setting errno */
> +#ifdef __linux__
> 0: neg w0, w0 /* create positive errno */
> +#endif
> 1: str w0, [x11] /* store errno */
> mov x0, #-1
> ret
> diff --git a/common-user/host/arm/safe-syscall.inc.S
> b/common-user/host/arm/safe-syscall.inc.S
> index 17839c6486..328299021d 100644
> --- a/common-user/host/arm/safe-syscall.inc.S
> +++ b/common-user/host/arm/safe-syscall.inc.S
> @@ -82,12 +82,23 @@ safe_syscall_start:
> safe_syscall_end:
>
> /* code path for having successfully executed the syscall */
> +#if defined(__linux__)
> + /* Linux kernel returns (small) negative errno. */
> cmp r0, #-4096
> bhi 0f
> +#elif defined(__FreeBSD__)
> + /* FreeBSD kernel returns positive errno and C bit set. */
> + bcs 1f
> +#else
> +#error "unsupported os"
> +#endif
> +
> 9: pop { r4, r5, r6, r7, r8, r9, r10, pc }
>
> /* code path setting errno */
> +#ifdef __linux__
> 0: neg r0, r0 /* create positive errno */
> +#endif
> 1: str r0, [r9]/* store errno */
> mov r0, #-1
> b 9b
> diff --git a/common-user/host/i386/safe-syscall.inc.S
> b/common-user/host/i386/safe-syscall.inc.S
> index ad89521783..a9382f777e 100644
> --- a/common-user/host/i386/safe-syscall.inc.S
> +++ b/common-user/host/i386/safe-syscall.inc.S
> @@ -76,8 +76,16 @@ safe_syscall_start:
> safe_syscall_end:
>
> /* code path for having successfully executed the syscall */
> +#if defined(__linux__)
> + /* Linux kernel returns (small) negative errno. */
> cmp $-4095, %eax
> jae 0f
> +#elif defined(__FreeBSD__)
> + /* FreeBSD kernel returns positive errno and C bit set. */
> + jcs 1f
>
I needed to change this to 'jc' and that's all google found for Intel.
+#else
> +#error "unsupported os"
> +#endif
>
> 9: pop %ebx
> .cfi_remember_state
> @@ -97,7 +105,9 @@ safe_syscall_end:
> .cfi_restore_state
>
> /* code path setting errno */
> +#ifdef __linux__
> 0: neg %eax/* create positive errno */
> +#endif
> 1: mov 8+16(%esp), %ebx/* load errno pointer */
> mov %eax, (%ebx)/* store errno */
> mov $-1, %eax
> diff --git a/common-user/host/x86_64/safe-syscall.inc.S
> b/common-user/host/x86_64/safe-syscall.inc.S
> index 9a0c4c93b4..36b7efe2ca 100644
> --- a/common-user/host/x86_64/safe-syscall.inc.S
> +++ b/common-user/host/x86_64/safe-syscall.inc.S
> @@ -75,8 +75,16 @@ safe_syscall_start:
> safe_syscall_end:
>
> /* code path for having successfully executed the syscall */
> +#if defined(__linux__)
> + /* Linux kernel returns (small) negative errno. */
> cmp$-4095, %rax
> jae0f
> +#elif defined(__FreeBSD__)
> + /* FreeBSD kernel returns positive errno and C bit set. */
> + jcs 1f
>
Likewise.
> +#else
> +#error "unsupported os"
> +#endif
>
> 9: pop %rbp
> .cfi_remember_state
> @@ -86,7 +94,9 @@ safe_syscall_end:
> .cfi_restore_state
>
> /* code path setting errno */
> +#ifdef __linux__
> 0: neg%eax/* create positive errno */
> +#endif
> 1: mov%eax, (%rbp)/* store errno */
> mov$-1, %rax
> jmp9b
>
I've not yet tested this on my arm/aarch64 systems, but I think it will
work there.
Warner
RE: Fwd: New Defects reported by Coverity Scan for QEMU
From: Matheus K. Ferst
> Hi Cédric,
>
> The only change was the helper name that is now uppercase, so nothing new
> here. The underlying cause is that dfp_finalize_decimal64 only sets
> dfp->vt.VsrD(1) and set_dfp64 receives a pointer to the complete struct.
>
> But since set_dfp64 also only access VsrD(1), it shouldn't be a real
> problem AFAICT. The same applies to CID 1465776~1465786 and
> 1465788~1465790.
Right. Coverity is probably reporting these as new just because the helper
macros were re-written as part of the move to decodetree.
I believe these should be marked as false positives.
We *could* also wrap set_dfp{64,128} in new macros that would then reference
only the appropriate parts of dfp, but, in this case, I don't think it's worth
the trouble.
Thanks,
--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer
Re: [PATCH-for-7.0] target/i386/kvm: Replace use of __u32 type
On 11/16/21 8:39 PM, Philippe Mathieu-Daudé wrote: QEMU coding style mandates to not use Linux kernel internal types for scalars types. Replace __u32 by uint32_t. Signed-off-by: Philippe Mathieu-Daudé --- target/i386/kvm/kvm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Richard Henderson r~
Re: [PATCH] tests/9pfs: use g_autofree where possible
On Tue, 16 Nov 2021 20:59:32 +0100
Christian Schoenebeck wrote:
> On Dienstag, 16. November 2021 19:12:21 CET Greg Kurz wrote:
> > On Tue, 16 Nov 2021 17:40:08 +0100
> >
> > Christian Schoenebeck wrote:
> > > Signed-off-by: Christian Schoenebeck
> > > ---
> >
> > Since g_autofree is scope based, I guess you could also convert this
> > snippet, that appears twice in the file BTW :
> >
> > for (int i = 0; i < QTEST_V9FS_SYNTH_READDIR_NFILES; ++i) {
> > char *name = g_strdup_printf(QTEST_V9FS_SYNTH_READDIR_FILE, i);
> > g_assert_cmpint(fs_dirents_contain_name(entries, name), ==, true);
> > g_free(name);
> > }
> >
> > No big deal.
>
> Ah, that slipped through as I was misreading it as an array free.
>
> If you don't mind I just add this on my end without sending a v2, it is
> trivial enough I think.
>
Sure ! And keep the R-b of course :-)
Cheers,
--
Greg
> > Reviewed-by: Greg Kurz
>
> Thanks!
>
> > > tests/qtest/virtio-9p-test.c | 86 +++-
> > > 1 file changed, 25 insertions(+), 61 deletions(-)
> > >
> > > diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
> > > index 41fed41de1..11861aaf7d 100644
> > > --- a/tests/qtest/virtio-9p-test.c
> > > +++ b/tests/qtest/virtio-9p-test.c
> > > @@ -84,7 +84,7 @@ static void pci_config(void *obj, void *data,
> > > QGuestAllocator *t_alloc)>
> > > QVirtio9P *v9p = obj;
> > > alloc = t_alloc;
> > > size_t tag_len = qvirtio_config_readw(v9p->vdev, 0);
> > >
> > > -char *tag;
> > > +g_autofree char *tag = NULL;
> > >
> > > int i;
> > >
> > > g_assert_cmpint(tag_len, ==, strlen(MOUNT_TAG));
> > >
> > > @@ -94,7 +94,6 @@ static void pci_config(void *obj, void *data,
> > > QGuestAllocator *t_alloc)>
> > > tag[i] = qvirtio_config_readb(v9p->vdev, i + 2);
> > >
> > > }
> > > g_assert_cmpmem(tag, tag_len, MOUNT_TAG, tag_len);
> > >
> > > -g_free(tag);
> > >
> > > }
> > >
> > > #define P9_MAX_SIZE 4096 /* Max size of a T-message or R-message */
> > >
> > > @@ -580,7 +579,7 @@ static void do_version(QVirtio9P *v9p)
> > >
> > > {
> > >
> > > const char *version = "9P2000.L";
> > > uint16_t server_len;
> > >
> > > -char *server_version;
> > > +g_autofree char *server_version = NULL;
> > >
> > > P9Req *req;
> > >
> > > req = v9fs_tversion(v9p, P9_MAX_SIZE, version, P9_NOTAG);
> > >
> > > @@ -588,8 +587,6 @@ static void do_version(QVirtio9P *v9p)
> > >
> > > v9fs_rversion(req, _len, _version);
> > >
> > > g_assert_cmpmem(server_version, server_len, version,
> > > strlen(version));
> > >
> > > -
> > > -g_free(server_version);
> > >
> > > }
> > >
> > > /* utility function: walk to requested dir and return fid for that dir */
> > >
> > > @@ -637,7 +634,7 @@ static void fs_walk(void *obj, void *data,
> > > QGuestAllocator *t_alloc)>
> > > alloc = t_alloc;
> > > char *wnames[P9_MAXWELEM];
> > > uint16_t nwqid;
> > >
> > > -v9fs_qid *wqid;
> > > +g_autofree v9fs_qid *wqid = NULL;
> > >
> > > int i;
> > > P9Req *req;
> > >
> > > @@ -655,8 +652,6 @@ static void fs_walk(void *obj, void *data,
> > > QGuestAllocator *t_alloc)>
> > > for (i = 0; i < P9_MAXWELEM; i++) {
> > >
> > > g_free(wnames[i]);
> > >
> > > }
> > >
> > > -
> > > -g_free(wqid);
> > >
> > > }
> > >
> > > static bool fs_dirents_contain_name(struct V9fsDirent *e, const char*
> > > name)
> > >
> > > @@ -984,7 +979,8 @@ static void fs_walk_dotdot(void *obj, void *data,
> > > QGuestAllocator *t_alloc)>
> > > QVirtio9P *v9p = obj;
> > > alloc = t_alloc;
> > > char *const wnames[] = { g_strdup("..") };
> > >
> > > -v9fs_qid root_qid, *wqid;
> > > +v9fs_qid root_qid;
> > > +g_autofree v9fs_qid *wqid = NULL;
> > >
> > > P9Req *req;
> > >
> > > do_version(v9p);
> > >
> > > @@ -998,7 +994,6 @@ static void fs_walk_dotdot(void *obj, void *data,
> > > QGuestAllocator *t_alloc)>
> > > g_assert_cmpmem(_qid, 13, wqid[0], 13);
> > >
> > > -g_free(wqid);
> > >
> > > g_free(wnames[0]);
> > >
> > > }
> > >
> > > @@ -1027,7 +1022,7 @@ static void fs_write(void *obj, void *data,
> > > QGuestAllocator *t_alloc)>
> > > alloc = t_alloc;
> > > static const uint32_t write_count = P9_MAX_SIZE / 2;
> > > char *const wnames[] = { g_strdup(QTEST_V9FS_SYNTH_WRITE_FILE) };
> > >
> > > -char *buf = g_malloc0(write_count);
> > > +g_autofree char *buf = g_malloc0(write_count);
> > >
> > > uint32_t count;
> > > P9Req *req;
> > >
> > > @@ -1045,7 +1040,6 @@ static void fs_write(void *obj, void *data,
> > > QGuestAllocator *t_alloc)>
> > > v9fs_rwrite(req, );
> > > g_assert_cmpint(count, ==, write_count);
> > >
> > > -g_free(buf);
> > >
> > > g_free(wnames[0]);
> > >
> > > }
> > >
> > > @@ -1125,7 +1119,7
Re: [PULL 0/2] NBD patches for 6.2-rc1, 2021-11-16
On 11/16/21 5:54 PM, Eric Blake wrote: The following changes since commit 9f0f846465d4c52ce9857787e947dffb64367fae: Merge tag 'machine-core-2025' of https://github.com/philmd/qemu into staging (2021-11-16 12:50:27 +0100) are available in the Git repository at: https://repo.or.cz/qemu/ericb.git tags/pull-nbd-2021-11-16 for you to fetch changes up to 3d212b41e9ccb3f37d04f22c59a960bac099c1d4: nbd/server: Add --selinux-label option (2021-11-16 10:16:38 -0600) nbd patches for 2021-11-16 - Rich Jones: Add 'qemu-nbd --selinux-label' option for running Unix socket with appropriate SELinux labeling - Eric Blake: Address clang sanitizer warning Eric Blake (1): nbd/server: Silence clang sanitizer warning Richard W.M. Jones (1): nbd/server: Add --selinux-label option meson.build | 10 - nbd/server.c | 13 +-- qemu-nbd.c| 46 +++ meson_options.txt | 3 ++ scripts/meson-buildoptions.sh | 3 ++ tests/docker/dockerfiles/centos8.docker | 1 + tests/docker/dockerfiles/fedora-i386-cross.docker | 1 + tests/docker/dockerfiles/fedora.docker| 1 + tests/docker/dockerfiles/opensuse-leap.docker | 1 + tests/docker/dockerfiles/ubuntu1804.docker| 1 + tests/docker/dockerfiles/ubuntu2004.docker| 1 + 11 files changed, 76 insertions(+), 5 deletions(-) Applied, thanks. r~
Re: Issue with acpi hotplug on pcie root ports in case of q35 + ovmf + machine type 6.1
On Tue, 02 Nov 2021 16:03:11 +0800 lma wrote: > Hi list, > > Have you experienced any acpi hotplugging issue while using q35 + ovmf + > machine type 6.1? > According to the error message in guest kernel, It seems qemu with ovmf > doesn't prepare > enough resource in PCI space for acpi hotplugging on multifunction > bridges. > > Please refer to issue#705 for details: > https://gitlab.com/qemu-project/qemu/-/issues/705 > > Thanks, > Lin > it should be fixed in current master with the latest machine type (6.2).
Re: [PATCH] tests/9pfs: use g_autofree where possible
On Dienstag, 16. November 2021 19:12:21 CET Greg Kurz wrote:
> On Tue, 16 Nov 2021 17:40:08 +0100
>
> Christian Schoenebeck wrote:
> > Signed-off-by: Christian Schoenebeck
> > ---
>
> Since g_autofree is scope based, I guess you could also convert this
> snippet, that appears twice in the file BTW :
>
> for (int i = 0; i < QTEST_V9FS_SYNTH_READDIR_NFILES; ++i) {
> char *name = g_strdup_printf(QTEST_V9FS_SYNTH_READDIR_FILE, i);
> g_assert_cmpint(fs_dirents_contain_name(entries, name), ==, true);
> g_free(name);
> }
>
> No big deal.
Ah, that slipped through as I was misreading it as an array free.
If you don't mind I just add this on my end without sending a v2, it is
trivial enough I think.
> Reviewed-by: Greg Kurz
Thanks!
> > tests/qtest/virtio-9p-test.c | 86 +++-
> > 1 file changed, 25 insertions(+), 61 deletions(-)
> >
> > diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
> > index 41fed41de1..11861aaf7d 100644
> > --- a/tests/qtest/virtio-9p-test.c
> > +++ b/tests/qtest/virtio-9p-test.c
> > @@ -84,7 +84,7 @@ static void pci_config(void *obj, void *data,
> > QGuestAllocator *t_alloc)>
> > QVirtio9P *v9p = obj;
> > alloc = t_alloc;
> > size_t tag_len = qvirtio_config_readw(v9p->vdev, 0);
> >
> > -char *tag;
> > +g_autofree char *tag = NULL;
> >
> > int i;
> >
> > g_assert_cmpint(tag_len, ==, strlen(MOUNT_TAG));
> >
> > @@ -94,7 +94,6 @@ static void pci_config(void *obj, void *data,
> > QGuestAllocator *t_alloc)>
> > tag[i] = qvirtio_config_readb(v9p->vdev, i + 2);
> >
> > }
> > g_assert_cmpmem(tag, tag_len, MOUNT_TAG, tag_len);
> >
> > -g_free(tag);
> >
> > }
> >
> > #define P9_MAX_SIZE 4096 /* Max size of a T-message or R-message */
> >
> > @@ -580,7 +579,7 @@ static void do_version(QVirtio9P *v9p)
> >
> > {
> >
> > const char *version = "9P2000.L";
> > uint16_t server_len;
> >
> > -char *server_version;
> > +g_autofree char *server_version = NULL;
> >
> > P9Req *req;
> >
> > req = v9fs_tversion(v9p, P9_MAX_SIZE, version, P9_NOTAG);
> >
> > @@ -588,8 +587,6 @@ static void do_version(QVirtio9P *v9p)
> >
> > v9fs_rversion(req, _len, _version);
> >
> > g_assert_cmpmem(server_version, server_len, version,
> > strlen(version));
> >
> > -
> > -g_free(server_version);
> >
> > }
> >
> > /* utility function: walk to requested dir and return fid for that dir */
> >
> > @@ -637,7 +634,7 @@ static void fs_walk(void *obj, void *data,
> > QGuestAllocator *t_alloc)>
> > alloc = t_alloc;
> > char *wnames[P9_MAXWELEM];
> > uint16_t nwqid;
> >
> > -v9fs_qid *wqid;
> > +g_autofree v9fs_qid *wqid = NULL;
> >
> > int i;
> > P9Req *req;
> >
> > @@ -655,8 +652,6 @@ static void fs_walk(void *obj, void *data,
> > QGuestAllocator *t_alloc)>
> > for (i = 0; i < P9_MAXWELEM; i++) {
> >
> > g_free(wnames[i]);
> >
> > }
> >
> > -
> > -g_free(wqid);
> >
> > }
> >
> > static bool fs_dirents_contain_name(struct V9fsDirent *e, const char*
> > name)
> >
> > @@ -984,7 +979,8 @@ static void fs_walk_dotdot(void *obj, void *data,
> > QGuestAllocator *t_alloc)>
> > QVirtio9P *v9p = obj;
> > alloc = t_alloc;
> > char *const wnames[] = { g_strdup("..") };
> >
> > -v9fs_qid root_qid, *wqid;
> > +v9fs_qid root_qid;
> > +g_autofree v9fs_qid *wqid = NULL;
> >
> > P9Req *req;
> >
> > do_version(v9p);
> >
> > @@ -998,7 +994,6 @@ static void fs_walk_dotdot(void *obj, void *data,
> > QGuestAllocator *t_alloc)>
> > g_assert_cmpmem(_qid, 13, wqid[0], 13);
> >
> > -g_free(wqid);
> >
> > g_free(wnames[0]);
> >
> > }
> >
> > @@ -1027,7 +1022,7 @@ static void fs_write(void *obj, void *data,
> > QGuestAllocator *t_alloc)>
> > alloc = t_alloc;
> > static const uint32_t write_count = P9_MAX_SIZE / 2;
> > char *const wnames[] = { g_strdup(QTEST_V9FS_SYNTH_WRITE_FILE) };
> >
> > -char *buf = g_malloc0(write_count);
> > +g_autofree char *buf = g_malloc0(write_count);
> >
> > uint32_t count;
> > P9Req *req;
> >
> > @@ -1045,7 +1040,6 @@ static void fs_write(void *obj, void *data,
> > QGuestAllocator *t_alloc)>
> > v9fs_rwrite(req, );
> > g_assert_cmpint(count, ==, write_count);
> >
> > -g_free(buf);
> >
> > g_free(wnames[0]);
> >
> > }
> >
> > @@ -1125,7 +1119,7 @@ static void fs_flush_ignored(void *obj, void *data,
> > QGuestAllocator *t_alloc)>
> > static void do_mkdir(QVirtio9P *v9p, const char *path, const char *cname)
> > {
> >
> > -char *const name = g_strdup(cname);
> > +g_autofree char *name = g_strdup(cname);
> >
> > uint32_t fid;
> > P9Req *req;
> >
> > @@ -1134,15 +1128,13 @@ static void do_mkdir(QVirtio9P *v9p, const char
> > *path, const char *cname)>
> > req =
Re: [PATCH] pmu: fix pmu vmstate subsection list
On 16/11/2021 15:08, Laurent Vivier wrote:
The subsection is not closed by a NULL marker so this can trigger
a segfault when the pmu vmstate is saved.
This can be easily shown with:
$ ./qemu-system-ppc64 -dump-vmstate vmstate.json
Segmentation fault (core dumped)
Fixes: d811d61fbc6c ("mac_newworld: add PMU device")
Cc: mark.cave-ayl...@ilande.co.uk
Signed-off-by: Laurent Vivier
---
hw/misc/macio/pmu.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/misc/macio/pmu.c b/hw/misc/macio/pmu.c
index 4ad4f50e08c3..eb39c64694aa 100644
--- a/hw/misc/macio/pmu.c
+++ b/hw/misc/macio/pmu.c
@@ -718,6 +718,7 @@ static const VMStateDescription vmstate_pmu = {
},
.subsections = (const VMStateDescription * []) {
_pmu_adb,
+NULL
}
};
Eeek. Good spot, looks like this bug has been around for some time:
Reviewed-by: Mark Cave-Ayland
ATB,
Mark.
Re: Guests wont start with 15 pcie-root-port devices
On Mon, 15 Nov 2021 11:57:43 -0500 Brian Rak wrote: > Will this fix make it into 6.2? yes, it was just merged 2aa1842d6d79..7e6055c99f2f1f PS: Native PCIe hotplug fixes from Gerd were merged as well, so if you'd like to use native hotplug, use --global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off to turn off ACPI hotplug. > On 11/12/2021 3:51 PM, Igor Mammedov wrote: > > On Fri, 12 Nov 2021 17:53:42 + > > Daniel P. Berrangé wrote: > > > >> On Fri, Nov 12, 2021 at 12:35:07PM -0500, Brian Rak wrote: > >>> In 6.1, a guest with 15 empty pcie-root-port devices will not boot > >>> properly > >>> - it just hangs on "Guest has not initialized the display (yet).". As > >>> soon > >>> as I remove the last pcie-root-port, the guest begins starting up > >>> normally. > >> Yes, QEMU 6.1 has a regression > >> > >>https://gitlab.com/qemu-project/qemu/-/issues/641 > >> > >> > >>> commit e2a6290aab578b2170c1f5909fa556385dc0d820 > >>> Author: Marcel Apfelbaum > >>> Date: Mon Aug 2 12:00:57 2021 +0300 > >>> > >>> hw/pcie-root-port: Fix hotplug for PCI devices requiring IO > >>> > >>> Although I can't say I really understand why that commit triggered it. > >> It caused the firmware to always allocate I/O space for every port > >> and there's limited total I/O space, so it runs out at 15 devices. > > alternatively instead of reverting to native PCIe hotplug as in the issue > > Daniel's mentioned, you can apply following fix > > https://patchew.org/QEMU/2022110857.3116853-1-imamm...@redhat.com/ > > > >> Regards, > >> Daniel >
[PATCH-for-7.0] target/i386/kvm: Replace use of __u32 type
QEMU coding style mandates to not use Linux kernel internal
types for scalars types. Replace __u32 by uint32_t.
Signed-off-by: Philippe Mathieu-Daudé
---
target/i386/kvm/kvm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 5a698bde19a..13f8e30c2a5 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -1406,7 +1406,7 @@ static int hyperv_fill_cpuids(CPUState *cs,
c->edx = cpu->hyperv_limits[2];
if (hyperv_feat_enabled(cpu, HYPERV_FEAT_EVMCS)) {
-__u32 function;
+uint32_t function;
/* Create zeroed 0x4006..0x4009 leaves */
for (function = HV_CPUID_IMPLEMENT_LIMITS + 1;
--
2.31.1
[PATCH-for-7.0] hw/net/rocker: Remove unused definitions
Signed-off-by: Philippe Mathieu-Daudé
---
hw/net/rocker/rocker.h | 5 -
1 file changed, 5 deletions(-)
diff --git a/hw/net/rocker/rocker.h b/hw/net/rocker/rocker.h
index 412fa44d017..d22bbd2bf80 100644
--- a/hw/net/rocker/rocker.h
+++ b/hw/net/rocker/rocker.h
@@ -36,13 +36,8 @@ static inline GCC_FMT_ATTR(1, 2) int DPRINTF(const char
*fmt, ...)
}
#endif
-#define __le16 uint16_t
-#define __le32 uint32_t
-#define __le64 uint64_t
-
#define __be16 uint16_t
#define __be32 uint32_t
-#define __be64 uint64_t
static inline bool ipv4_addr_is_multicast(__be32 addr)
{
--
2.31.1
Re: [PULL 20/20] pcie: expire pending delete
On Mon, 15 Nov 2021 11:39:09 -0500
"Michael S. Tsirkin" wrote:
> From: Gerd Hoffmann
>
> Add an expire time for pending delete, once the time is over allow
> pressing the attention button again.
>
> This makes pcie hotplug behave more like acpi hotplug, where one can
> try sending an 'device_del' monitor command again in case the guest
> didn't respond to the first attempt.
>
> Signed-off-by: Gerd Hoffmann
> Message-Id: <2021130859.1171890-7-kra...@redhat.com>
> Reviewed-by: Michael S. Tsirkin
> Signed-off-by: Michael S. Tsirkin
> ---
> include/hw/qdev-core.h | 1 +
> hw/pci/pcie.c | 2 ++
> softmmu/qdev-monitor.c | 4 +++-
> 3 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 72622bd337..20d3066595 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -181,6 +181,7 @@ struct DeviceState {
> char *canonical_path;
> bool realized;
> bool pending_deleted_event;
> +int64_t pending_deleted_expires_ms;
> QDict *opts;
> int hotplugged;
> bool allow_unplug_during_migration;
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index a930ac738a..c5ed266337 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -548,6 +548,8 @@ void pcie_cap_slot_unplug_request_cb(HotplugHandler
> *hotplug_dev,
> }
>
> dev->pending_deleted_event = true;
> +dev->pending_deleted_expires_ms =
> +qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 5000; /* 5 secs */
do we block migration if unplug was requested?
(if not we might loose this state on destionatio, do we care about it?)
>
> /* In case user cancel the operation of multi-function hot-add,
> * remove the function that is unexposed to guest individually,
> diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
> index 588a62b88d..5925f1ae5f 100644
> --- a/softmmu/qdev-monitor.c
> +++ b/softmmu/qdev-monitor.c
> @@ -943,7 +943,9 @@ void qmp_device_del(const char *id, Error **errp)
> {
> DeviceState *dev = find_device_state(id, errp);
> if (dev != NULL) {
> -if (dev->pending_deleted_event) {
> +if (dev->pending_deleted_event &&
> +(dev->pending_deleted_expires_ms == 0 ||
> + dev->pending_deleted_expires_ms >
> qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL))) {
> error_setg(errp, "Device %s is already in the "
> "process of unplug", id);
> return;
Re: [PATCH] q35: turn off power_controller_present when acpi hotplug is enabled
On Tue, 16 Nov 2021 10:04:33 +0100
Gerd Hoffmann wrote:
> Disable power control for pcie slots in case acpi hotplug is enabled
> (6.2+ only for compatibility reasons). This makes sure we don't get
> unpleasant surprises with pci devices not being functional due to slot
> power being turned off.
>
> Signed-off-by: Gerd Hoffmann
> ---
> hw/i386/pc_q35.c | 13 ++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index e1e100316d93..869ca4c130f0 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -247,9 +247,16 @@ static void pc_q35_init(MachineState *machine)
> "x-keep-pci-slot-hpc",
> NULL);
>
> -if (!keep_pci_slot_hpc && acpi_pcihp) {
> -object_register_sugar_prop(TYPE_PCIE_SLOT, "x-native-hotplug",
> - "false", true);
> +if (acpi_pcihp) {
> +if (keep_pci_slot_hpc) {
> +/* 6.2+ default: acpi-hotplug=on native-hotplug=on
> power-ctrl=off */
> +object_register_sugar_prop(TYPE_PCIE_SLOT, COMPAT_PROP_PCP,
> + "false", true);
that will also turn off COMPAT_PROP_PCP on ports attached to PXBs,
where ACPI hotplug is not used and native one is active.
So question is if it's expected behavior?
> +} else {
> +/* 6.1 default: acpi-hotplug=on native-hotplug=off */
> +object_register_sugar_prop(TYPE_PCIE_SLOT, "x-native-hotplug",
> + "false", true);
> +}
> }
>
> /* irq lines */
Re: [PATCH] vfio: Fix memory leak of hostwin
On Tue, 16 Nov 2021 19:56:26 +0800
Peng Liang wrote:
> hostwin is allocated and added to hostwin_list in vfio_host_win_add, but
> it is only deleted from hostwin_list in vfio_host_win_del, which causes
> a memory leak. Also, freeing all elements in hostwin_list is missing in
> vfio_disconnect_container.
>
> Fix: 2e4109de8e58 ("vfio/spapr: Create DMA window dynamically (SPAPR IOMMU
> v2)")
> CC: qemu-sta...@nongnu.org
> Signed-off-by: Peng Liang
> ---
> hw/vfio/common.c | 12 ++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index dd387b0d3959..2cce60c5fac3 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -546,11 +546,12 @@ static void vfio_host_win_add(VFIOContainer *container,
> static int vfio_host_win_del(VFIOContainer *container, hwaddr min_iova,
> hwaddr max_iova)
> {
> -VFIOHostDMAWindow *hostwin;
> +VFIOHostDMAWindow *hostwin, *next;
>
> -QLIST_FOREACH(hostwin, >hostwin_list, hostwin_next) {
> +QLIST_FOREACH_SAFE(hostwin, >hostwin_list, hostwin_next,
> next) {
Unnecessary conversion to _SAFE variant here, we don't continue to walk
the list after removing an object.
> if (hostwin->min_iova == min_iova && hostwin->max_iova == max_iova) {
> QLIST_REMOVE(hostwin, hostwin_next);
> +g_free(hostwin);
> return 0;
> }
> }
> @@ -2239,6 +2240,7 @@ static void vfio_disconnect_container(VFIOGroup *group)
> if (QLIST_EMPTY(>group_list)) {
> VFIOAddressSpace *space = container->space;
> VFIOGuestIOMMU *giommu, *tmp;
> +VFIOHostDMAWindow *hostwin, *next;
>
> QLIST_REMOVE(container, next);
>
> @@ -2249,6 +2251,12 @@ static void vfio_disconnect_container(VFIOGroup *group)
> g_free(giommu);
> }
>
> +QLIST_FOREACH_SAFE(hostwin, >hostwin_list, hostwin_next,
> + next) {
> +QLIST_REMOVE(hostwin, hostwin_next);
> +g_free(hostwin);
> +}
> +
This usage looks good. Thanks,
Alex
> trace_vfio_disconnect_container(container->fd);
> close(container->fd);
> g_free(container);
Re: [PATCH] gitlab-ci/cirrus: Increase timeout to 80 minutes
On Tue, Nov 16, 2021 at 06:36:50PM +0100, Richard Henderson wrote: > On 11/16/21 6:22 PM, Thomas Huth wrote: > > On 16/11/2021 18.09, Philippe Mathieu-Daudé wrote: > > > On 11/16/21 17:49, Daniel P. Berrangé wrote: > > > > On Tue, Nov 16, 2021 at 05:33:09PM +0100, Thomas Huth wrote: > > > > > The jobs on Cirrus-CI sometimes get delayed quite a bit, waiting to > > > > > be scheduled, so while the build test itself finishes within 60 > > > > > minutes, > > > > > the total run time of the jobs can be longer due to this waiting time. > > > > > Thus let's increase the timeout on the gitlab side a little bit, so > > > > > that these jobs are not marked as failing just because of the delay. > > ...>>> diff --git a/.gitlab-ci.d/cirrus.yml b/.gitlab-ci.d/cirrus.yml > > > > > index e7b25e7427..22d42585e4 100644 > > > > > --- a/.gitlab-ci.d/cirrus.yml > > > > > +++ b/.gitlab-ci.d/cirrus.yml > > > > > @@ -14,6 +14,7 @@ > > > > > stage: build > > > > > image: registry.gitlab.com/libvirt/libvirt-ci/cirrus-run:master > > > > > needs: [] > > > > > + timeout: 80m > > > > > allow_failure: true > > > > > script: > > > > > - source .gitlab-ci.d/cirrus/$NAME.vars > > > > > > > > Whether 80 or 100 minute, consider it > > > > > > > > Reviewed-by: Daniel P. Berrangé > > > > > > This pipeline took 1h51m09s: > > > https://gitlab.com/qemu-project/qemu/-/pipelines/409666733/builds > > > But Richard restarted unstable jobs, which probably added time > > > to the total. > > > > > > IIRC from a maintainer perspective 1h15 is the upper limit. > > > 80m fits, 100m is over. > > > > I think I agree ... I normally don't want to wait more than a little bit > > more than one hour, so 100 minutes feels too long already. We already > > have some 70m timeouts in other jobs, and one 80 minute timeout in > > .gitlab-ci.d/crossbuild-template.yml, so I'd say 80 minutes are really > > the upper boundary that we should use. > > We are also talking apples and oranges: > Gitlab timeouts are on the amount of time the job runs. > Cirrus timeouts appear to be on the amount of time the job is queued. > > If cirrus would just not start accounting until the thing runs we'd be fine. Unfortunately it isn't that easy. Our cirrus CI jobs are launched using the cirrus-run tool, from a gitlab job. The timeouts we're usually hitting are from the gitlab job which is sitting around waiting for the cirrus job it launched to finish, so it can report back stats. Cirrus CI does itself have a job timeout, but I'm not aware of us hitting that typically, unless i'm misinterpreting something. 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] tests/9pfs: use g_autofree where possible
On Tue, 16 Nov 2021 17:40:08 +0100
Christian Schoenebeck wrote:
> Signed-off-by: Christian Schoenebeck
> ---
Since g_autofree is scope based, I guess you could also convert this
snippet, that appears twice in the file BTW :
for (int i = 0; i < QTEST_V9FS_SYNTH_READDIR_NFILES; ++i) {
char *name = g_strdup_printf(QTEST_V9FS_SYNTH_READDIR_FILE, i);
g_assert_cmpint(fs_dirents_contain_name(entries, name), ==, true);
g_free(name);
}
No big deal.
Reviewed-by: Greg Kurz
> tests/qtest/virtio-9p-test.c | 86 +++-
> 1 file changed, 25 insertions(+), 61 deletions(-)
>
> diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
> index 41fed41de1..11861aaf7d 100644
> --- a/tests/qtest/virtio-9p-test.c
> +++ b/tests/qtest/virtio-9p-test.c
> @@ -84,7 +84,7 @@ static void pci_config(void *obj, void *data,
> QGuestAllocator *t_alloc)
> QVirtio9P *v9p = obj;
> alloc = t_alloc;
> size_t tag_len = qvirtio_config_readw(v9p->vdev, 0);
> -char *tag;
> +g_autofree char *tag = NULL;
> int i;
>
> g_assert_cmpint(tag_len, ==, strlen(MOUNT_TAG));
> @@ -94,7 +94,6 @@ static void pci_config(void *obj, void *data,
> QGuestAllocator *t_alloc)
> tag[i] = qvirtio_config_readb(v9p->vdev, i + 2);
> }
> g_assert_cmpmem(tag, tag_len, MOUNT_TAG, tag_len);
> -g_free(tag);
> }
>
> #define P9_MAX_SIZE 4096 /* Max size of a T-message or R-message */
> @@ -580,7 +579,7 @@ static void do_version(QVirtio9P *v9p)
> {
> const char *version = "9P2000.L";
> uint16_t server_len;
> -char *server_version;
> +g_autofree char *server_version = NULL;
> P9Req *req;
>
> req = v9fs_tversion(v9p, P9_MAX_SIZE, version, P9_NOTAG);
> @@ -588,8 +587,6 @@ static void do_version(QVirtio9P *v9p)
> v9fs_rversion(req, _len, _version);
>
> g_assert_cmpmem(server_version, server_len, version, strlen(version));
> -
> -g_free(server_version);
> }
>
> /* utility function: walk to requested dir and return fid for that dir */
> @@ -637,7 +634,7 @@ static void fs_walk(void *obj, void *data,
> QGuestAllocator *t_alloc)
> alloc = t_alloc;
> char *wnames[P9_MAXWELEM];
> uint16_t nwqid;
> -v9fs_qid *wqid;
> +g_autofree v9fs_qid *wqid = NULL;
> int i;
> P9Req *req;
>
> @@ -655,8 +652,6 @@ static void fs_walk(void *obj, void *data,
> QGuestAllocator *t_alloc)
> for (i = 0; i < P9_MAXWELEM; i++) {
> g_free(wnames[i]);
> }
> -
> -g_free(wqid);
> }
>
> static bool fs_dirents_contain_name(struct V9fsDirent *e, const char* name)
> @@ -984,7 +979,8 @@ static void fs_walk_dotdot(void *obj, void *data,
> QGuestAllocator *t_alloc)
> QVirtio9P *v9p = obj;
> alloc = t_alloc;
> char *const wnames[] = { g_strdup("..") };
> -v9fs_qid root_qid, *wqid;
> +v9fs_qid root_qid;
> +g_autofree v9fs_qid *wqid = NULL;
> P9Req *req;
>
> do_version(v9p);
> @@ -998,7 +994,6 @@ static void fs_walk_dotdot(void *obj, void *data,
> QGuestAllocator *t_alloc)
>
> g_assert_cmpmem(_qid, 13, wqid[0], 13);
>
> -g_free(wqid);
> g_free(wnames[0]);
> }
>
> @@ -1027,7 +1022,7 @@ static void fs_write(void *obj, void *data,
> QGuestAllocator *t_alloc)
> alloc = t_alloc;
> static const uint32_t write_count = P9_MAX_SIZE / 2;
> char *const wnames[] = { g_strdup(QTEST_V9FS_SYNTH_WRITE_FILE) };
> -char *buf = g_malloc0(write_count);
> +g_autofree char *buf = g_malloc0(write_count);
> uint32_t count;
> P9Req *req;
>
> @@ -1045,7 +1040,6 @@ static void fs_write(void *obj, void *data,
> QGuestAllocator *t_alloc)
> v9fs_rwrite(req, );
> g_assert_cmpint(count, ==, write_count);
>
> -g_free(buf);
> g_free(wnames[0]);
> }
>
> @@ -1125,7 +1119,7 @@ static void fs_flush_ignored(void *obj, void *data,
> QGuestAllocator *t_alloc)
>
> static void do_mkdir(QVirtio9P *v9p, const char *path, const char *cname)
> {
> -char *const name = g_strdup(cname);
> +g_autofree char *name = g_strdup(cname);
> uint32_t fid;
> P9Req *req;
>
> @@ -1134,15 +1128,13 @@ static void do_mkdir(QVirtio9P *v9p, const char
> *path, const char *cname)
> req = v9fs_tmkdir(v9p, fid, name, 0750, 0, 0);
> v9fs_req_wait_for_reply(req, NULL);
> v9fs_rmkdir(req, NULL);
> -
> -g_free(name);
> }
>
> /* create a regular file with Tlcreate and return file's fid */
> static uint32_t do_lcreate(QVirtio9P *v9p, const char *path,
> const char *cname)
> {
> -char *const name = g_strdup(cname);
> +g_autofree char *name = g_strdup(cname);
> uint32_t fid;
> P9Req *req;
>
> @@ -1152,7 +1144,6 @@ static uint32_t do_lcreate(QVirtio9P *v9p, const char
> *path,
> v9fs_req_wait_for_reply(req, NULL);
> v9fs_rlcreate(req, NULL, NULL);
>
> -g_free(name);
> return fid;
> }
>
> @@ -1160,8
Re: [PULL SUBSYSTEM qemu-pseries] pseries: Update SLOF firmware image
Queued for 7.0. I am not sure this is a good idea, as this will make bisection painful over the release tag. It is my understanding that Cedric will rebase for the mainline PR. At least that's how David was handling subsystem pulls. Yes. I don't plan to send a PR on tree without rebasing. I am just collecting now. Oh this is not a signed subsystem pull request, so you can rebase, OK. I didn't know that. I understand your concern now. Thanks, C.
Re: [PULL SUBSYSTEM qemu-pseries] pseries: Update SLOF firmware image
On 11/16/21 18:30, Cédric Le Goater wrote: > On 11/16/21 18:28, Richard Henderson wrote: >> On 11/16/21 6:12 PM, Philippe Mathieu-Daudé wrote: >>> On 11/16/21 17:46, Cédric Le Goater wrote: On 11/14/21 01:51, Alexey Kardashevskiy wrote: > The following changes since commit > d139786e1b3d67991e6cb49a8a59bb2182350285: > > ppc/mmu_helper.c: do not truncate 'ea' in > booke206_invalidate_ea_tlb() (2021-11-11 11:35:13 +0100) > > are available in the Git repository at: > > g...@github.com:aik/qemu.git tags/qemu-slof-2022 > > for you to fetch changes up to > 73944a4bf4ab259b489af8128b4aec525484d642: > > pseries: Update SLOF firmware image (2021-11-13 14:47:56 +1100) > > > Alexey Kardashevskiy (1): > pseries: Update SLOF firmware image > > pc-bios/README | 2 +- > pc-bios/slof.bin | Bin 991744 -> 991920 bytes > roms/SLOF | 2 +- > 3 files changed, 2 insertions(+), 2 deletions(-) Queued for 7.0. >>> >>> I am not sure this is a good idea, as this will make bisection >>> painful over the release tag. >> >> It is my understanding that Cedric will rebase for the mainline PR. >> At least that's how David was handling subsystem pulls. > > Yes. I don't plan to send a PR on tree without rebasing. I am just > collecting now. Oh this is not a signed subsystem pull request, so you can rebase, OK.
Re: [PULL for 6.2 0/7] misc build and test fixes
On 11/16/21 5:25 PM, Alex Bennée wrote: The following changes since commit 871c71b1bad2d2647641500603a2236869135c7f: Merge tag 'pull-block-2021-11-16' of https://gitlab.com/hreitz/qemu into staging (2021-11-16 14:20:39 +0100) are available in the Git repository at: https://github.com/stsquad/qemu.git tags/pull-for-6.2-161121-1 for you to fetch changes up to 9968de0a4a5470bd7b98dcd2fae5d5269908f16b: gitlab: skip cirrus jobs on master and stable branches (2021-11-16 16:19:53 +) Misc build and test fixes: - force NOUSER for base docker images - don't run TCG VM tests by default - remove useless meson test - add Centos 8 custom runner - split up custom-runners to individual files - skip cirrus checks on master/stable branches Alex Bennée (3): tests/docker: force NOUSER=1 for base images tests/vm: sort the special variable list tests/vm: don't build using TCG by default Cleber Rosa (1): Jobs based on custom runners: add CentOS Stream 8 Daniel P. Berrangé (1): gitlab: skip cirrus jobs on master and stable branches Paolo Bonzini (1): meson: remove useless libdl test Philippe Mathieu-Daudé (1): gitlab-ci: Split custom-runners.yml in one file per runner docs/devel/ci-jobs.rst.inc | 7 + meson.build| 8 +- .gitlab-ci.d/cirrus.yml| 3 + .gitlab-ci.d/custom-runners.yml| 239 + .../custom-runners/centos-stream-8-x86_64.yml | 28 +++ .gitlab-ci.d/custom-runners/ubuntu-18.04-s390x.yml | 118 ++ .../custom-runners/ubuntu-20.04-aarch64.yml| 118 ++ accel/tcg/meson.build | 2 +- .../ci/org.centos/stream/8/build-environment.yml | 51 + scripts/ci/org.centos/stream/8/x86_64/configure| 208 ++ scripts/ci/org.centos/stream/8/x86_64/test-avocado | 70 ++ scripts/ci/org.centos/stream/README| 17 ++ scripts/ci/setup/build-environment.yml | 38 tests/docker/Makefile.include | 3 + tests/vm/Makefile.include | 29 ++- 15 files changed, 686 insertions(+), 253 deletions(-) create mode 100644 .gitlab-ci.d/custom-runners/centos-stream-8-x86_64.yml create mode 100644 .gitlab-ci.d/custom-runners/ubuntu-18.04-s390x.yml create mode 100644 .gitlab-ci.d/custom-runners/ubuntu-20.04-aarch64.yml create mode 100644 scripts/ci/org.centos/stream/8/build-environment.yml create mode 100755 scripts/ci/org.centos/stream/8/x86_64/configure create mode 100755 scripts/ci/org.centos/stream/8/x86_64/test-avocado create mode 100644 scripts/ci/org.centos/stream/README Applied, thanks. r~
Re: [PATCH] gitlab-ci/cirrus: Increase timeout to 80 minutes
On 11/16/21 6:22 PM, Thomas Huth wrote: On 16/11/2021 18.09, Philippe Mathieu-Daudé wrote: On 11/16/21 17:49, Daniel P. Berrangé wrote: On Tue, Nov 16, 2021 at 05:33:09PM +0100, Thomas Huth wrote: The jobs on Cirrus-CI sometimes get delayed quite a bit, waiting to be scheduled, so while the build test itself finishes within 60 minutes, the total run time of the jobs can be longer due to this waiting time. Thus let's increase the timeout on the gitlab side a little bit, so that these jobs are not marked as failing just because of the delay. ...>>> diff --git a/.gitlab-ci.d/cirrus.yml b/.gitlab-ci.d/cirrus.yml index e7b25e7427..22d42585e4 100644 --- a/.gitlab-ci.d/cirrus.yml +++ b/.gitlab-ci.d/cirrus.yml @@ -14,6 +14,7 @@ stage: build image: registry.gitlab.com/libvirt/libvirt-ci/cirrus-run:master needs: [] + timeout: 80m allow_failure: true script: - source .gitlab-ci.d/cirrus/$NAME.vars Whether 80 or 100 minute, consider it Reviewed-by: Daniel P. Berrangé This pipeline took 1h51m09s: https://gitlab.com/qemu-project/qemu/-/pipelines/409666733/builds But Richard restarted unstable jobs, which probably added time to the total. IIRC from a maintainer perspective 1h15 is the upper limit. 80m fits, 100m is over. I think I agree ... I normally don't want to wait more than a little bit more than one hour, so 100 minutes feels too long already. We already have some 70m timeouts in other jobs, and one 80 minute timeout in .gitlab-ci.d/crossbuild-template.yml, so I'd say 80 minutes are really the upper boundary that we should use. We are also talking apples and oranges: Gitlab timeouts are on the amount of time the job runs. Cirrus timeouts appear to be on the amount of time the job is queued. If cirrus would just not start accounting until the thing runs we'd be fine. r~
Re: [PATCH] sev: allow capabilities to check for SEV-ES support
On 11/16/21 12:23 PM, Daniel P. Berrangé wrote:
On Tue, Nov 16, 2021 at 11:58:12AM -0500, Tyler Fanelli wrote:
On 11/16/21 10:53 AM, Daniel P. Berrangé wrote:
On Tue, Nov 16, 2021 at 10:29:35AM -0500, Tyler Fanelli wrote:
On 11/16/21 4:17 AM, Daniel P. Berrangé wrote:
On Mon, Nov 15, 2021 at 02:38:04PM -0500, Tyler Fanelli wrote:
Probe for SEV-ES and SEV-SNP capabilities to distinguish between Rome,
Naples, and Milan processors. Use the CPUID function to probe if a
processor is capable of running SEV-ES or SEV-SNP, rather than if it
actually is running SEV-ES or SEV-SNP.
Signed-off-by: Tyler Fanelli
---
qapi/misc-target.json | 11 +--
target/i386/sev.c | 6 --
2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index 5aa2b95b7d..c3e9bce12b 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -182,13 +182,19 @@
# @reduced-phys-bits: Number of physical Address bit reduction when SEV is
# enabled
#
+# @es: SEV-ES capability of the machine.
+#
+# @snp: SEV-SNP capability of the machine.
+#
# Since: 2.12
##
{ 'struct': 'SevCapability',
'data': { 'pdh': 'str',
'cert-chain': 'str',
'cbitpos': 'int',
-'reduced-phys-bits': 'int'},
+'reduced-phys-bits': 'int',
+'es': 'bool',
+'snp': 'bool'},
'if': 'TARGET_I386' }
##
@@ -205,7 +211,8 @@
#
# -> { "execute": "query-sev-capabilities" }
# <- { "return": { "pdh": "8CCDD8DDD", "cert-chain": "888CCCDDDEE",
-# "cbitpos": 47, "reduced-phys-bits": 5}}
+# "cbitpos": 47, "reduced-phys-bits": 5
+# "es": false, "snp": false}}
We've previously had patches posted to support SNP in QEMU
https://lists.gnu.org/archive/html/qemu-devel/2021-08/msg04761.html
and this included an update to query-sev for reporting info
about the VM instance.
Your patch is updating query-sev-capabilities, which is a
counterpart for detecting host capabilities separate from
a guest instance.
Yes, that's because with this patch, I'm more interested in determining
which AMD processor is running on a host, and less if ES or SNP is actually
running on a guest instance or not.
None the less I wonder if the same design questions from
query-sev apply. ie do we need to have the ability to
report any SNP specific information fields, if so we need
to use a discriminated union of structs, not just bool
flags.
More generally I'm some what wary of adding this to
query-sev-capabilities at all, unless it is part of the
main SEV-SNP series.
Also what's the intended usage for the mgmt app from just
having these boolean fields ? Are they other more explicit
feature flags we should be reporting, instead of what are
essentially SEV generation codenames.
If by "mgmt app" you're referring to sevctl, in order to determine which
certificate chain to use (Naples vs Rome vs Milan ARK/ASK) we must query
which processor we are running on. Although sevctl has a feature which can
do this already, we cannot guarantee that sevctl is running on the same host
that a VM is running on, so we must query this capability from QEMU. My
logic was determining the processor would have been the following:
I'm not really talking about a specific, rather any tool which wants
to deal with SEV and QEMU, whether libvirt or an app using libvirt,
or something else using QEMU directly.
Ah, my mistake.
Where does the actual cert chain payload come from ? Is that something
the app has to acquire out of band, or can the full cert chain be
acquired from the hardware itself ?
The cert chain (or the ARK/ASK specifically) comes from AMD's KDS, yet
sevctl is able to cache the values, and has them on-hand when needed. This
patch would tell sevctl *which* of the cert chains to use (Naples vs Rome vs
Milan chain). If need be, I could just focus on Naples and Rome processors
for now and bring support for SNP (Milan processors) later on when it is
more mature.
!es && !snp --> Naples
es && !snp --> Rome
es && snp --> Milan
This approach isn't future proof if subsequent generations introduce
new certs. It feels like we should be explicitly reporting something
about the certs rather than relying on every app to re-implement tihs
logic.
Alright, like an encoding of which processor generation the host is running
on?
IIUC (from looking at sev-tool), the certificates can be acquired
from
https://developer.amd.com/wp-content/resources/ask_ark_{gen}.cert
where {gen} is one of "milan", "naples", "rome".
With this in mind, I'd think that query-sev-capabilities could just
report the required certificate name. e.g.
{ 'enum': 'SevAskArkCertName',
'data': ['milan', 'naples', 'rome'] }
and then report it in SevCapability struct with
"ask-ark-cert-name": "SevAskArkCertName"
That seems reasonable to me, I'll give it a try and submit a
Re: [PULL SUBSYSTEM qemu-pseries] pseries: Update SLOF firmware image
On 11/16/21 18:28, Richard Henderson wrote: On 11/16/21 6:12 PM, Philippe Mathieu-Daudé wrote: On 11/16/21 17:46, Cédric Le Goater wrote: On 11/14/21 01:51, Alexey Kardashevskiy wrote: The following changes since commit d139786e1b3d67991e6cb49a8a59bb2182350285: ppc/mmu_helper.c: do not truncate 'ea' in booke206_invalidate_ea_tlb() (2021-11-11 11:35:13 +0100) are available in the Git repository at: g...@github.com:aik/qemu.git tags/qemu-slof-2022 for you to fetch changes up to 73944a4bf4ab259b489af8128b4aec525484d642: pseries: Update SLOF firmware image (2021-11-13 14:47:56 +1100) Alexey Kardashevskiy (1): pseries: Update SLOF firmware image pc-bios/README | 2 +- pc-bios/slof.bin | Bin 991744 -> 991920 bytes roms/SLOF | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) Queued for 7.0. I am not sure this is a good idea, as this will make bisection painful over the release tag. It is my understanding that Cedric will rebase for the mainline PR. At least that's how David was handling subsystem pulls. Yes. I don't plan to send a PR on tree without rebasing. I am just collecting now. Thanks, C.
Re: [PULL SUBSYSTEM qemu-pseries] pseries: Update SLOF firmware image
On 11/16/21 6:12 PM, Philippe Mathieu-Daudé wrote: On 11/16/21 17:46, Cédric Le Goater wrote: On 11/14/21 01:51, Alexey Kardashevskiy wrote: The following changes since commit d139786e1b3d67991e6cb49a8a59bb2182350285: ppc/mmu_helper.c: do not truncate 'ea' in booke206_invalidate_ea_tlb() (2021-11-11 11:35:13 +0100) are available in the Git repository at: g...@github.com:aik/qemu.git tags/qemu-slof-2022 for you to fetch changes up to 73944a4bf4ab259b489af8128b4aec525484d642: pseries: Update SLOF firmware image (2021-11-13 14:47:56 +1100) Alexey Kardashevskiy (1): pseries: Update SLOF firmware image pc-bios/README | 2 +- pc-bios/slof.bin | Bin 991744 -> 991920 bytes roms/SLOF | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) Queued for 7.0. I am not sure this is a good idea, as this will make bisection painful over the release tag. It is my understanding that Cedric will rebase for the mainline PR. At least that's how David was handling subsystem pulls. r~
[PATCH-for-7.0] hw/pci: Don't open-code pci_intx()
Use the pci_intx() helper instead of open-coding it.
Signed-off-by: Philippe Mathieu-Daudé
---
Based-on: <2026170133.724751-3-fbar...@linux.ibm.com>
---
hw/net/vmxnet3.c | 2 +-
hw/remote/iohub.c | 6 ++
hw/remote/proxy.c | 3 +--
3 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 41f796a247d..c7fc5f44d8f 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -1350,7 +1350,7 @@ static void vmxnet3_update_features(VMXNET3State *s)
static bool vmxnet3_verify_intx(VMXNET3State *s, int intx)
{
return s->msix_used || msi_enabled(PCI_DEVICE(s))
-|| intx == pci_get_byte(s->parent_obj.config + PCI_INTERRUPT_PIN) - 1;
+|| intx == pci_intx(PCI_DEVICE(s));
}
static void vmxnet3_validate_interrupt_idx(bool is_msix, int idx)
diff --git a/hw/remote/iohub.c b/hw/remote/iohub.c
index 547d597f0fe..0e0bb651d1a 100644
--- a/hw/remote/iohub.c
+++ b/hw/remote/iohub.c
@@ -93,11 +93,9 @@ void process_set_irqfd_msg(PCIDevice *pci_dev, MPQemuMsg
*msg)
{
RemoteMachineState *machine = REMOTE_MACHINE(current_machine);
RemoteIOHubState *iohub = >iohub;
-int pirq, intx;
+int pirq;
-intx = pci_get_byte(pci_dev->config + PCI_INTERRUPT_PIN) - 1;
-
-pirq = remote_iohub_map_irq(pci_dev, intx);
+pirq = remote_iohub_map_irq(pci_dev, pci_intx(pci_dev));
if (event_notifier_get_fd(>irqfds[pirq]) != -1) {
qemu_set_fd_handler(event_notifier_get_fd(>resamplefds[pirq]),
diff --git a/hw/remote/proxy.c b/hw/remote/proxy.c
index bad164299dd..22f32a5930b 100644
--- a/hw/remote/proxy.c
+++ b/hw/remote/proxy.c
@@ -32,14 +32,13 @@ static void proxy_intx_update(PCIDevice *pci_dev)
{
PCIProxyDev *dev = PCI_PROXY_DEV(pci_dev);
PCIINTxRoute route;
-int pin = pci_get_byte(pci_dev->config + PCI_INTERRUPT_PIN) - 1;
if (dev->virq != -1) {
kvm_irqchip_remove_irqfd_notifier_gsi(kvm_state, >intr,
dev->virq);
dev->virq = -1;
}
-route = pci_device_route_intx_to_irq(pci_dev, pin);
+route = pci_device_route_intx_to_irq(pci_dev, pci_intx(pci_dev));
dev->virq = route.irq;
--
2.31.1
Re: [PATCH] sev: allow capabilities to check for SEV-ES support
On Tue, Nov 16, 2021 at 11:58:12AM -0500, Tyler Fanelli wrote:
> On 11/16/21 10:53 AM, Daniel P. Berrangé wrote:
> > On Tue, Nov 16, 2021 at 10:29:35AM -0500, Tyler Fanelli wrote:
> > > On 11/16/21 4:17 AM, Daniel P. Berrangé wrote:
> > > > On Mon, Nov 15, 2021 at 02:38:04PM -0500, Tyler Fanelli wrote:
> > > > > Probe for SEV-ES and SEV-SNP capabilities to distinguish between Rome,
> > > > > Naples, and Milan processors. Use the CPUID function to probe if a
> > > > > processor is capable of running SEV-ES or SEV-SNP, rather than if it
> > > > > actually is running SEV-ES or SEV-SNP.
> > > > >
> > > > > Signed-off-by: Tyler Fanelli
> > > > > ---
> > > > >qapi/misc-target.json | 11 +--
> > > > >target/i386/sev.c | 6 --
> > > > >2 files changed, 13 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> > > > > index 5aa2b95b7d..c3e9bce12b 100644
> > > > > --- a/qapi/misc-target.json
> > > > > +++ b/qapi/misc-target.json
> > > > > @@ -182,13 +182,19 @@
> > > > ># @reduced-phys-bits: Number of physical Address bit reduction
> > > > > when SEV is
> > > > ># enabled
> > > > >#
> > > > > +# @es: SEV-ES capability of the machine.
> > > > > +#
> > > > > +# @snp: SEV-SNP capability of the machine.
> > > > > +#
> > > > ># Since: 2.12
> > > > >##
> > > > >{ 'struct': 'SevCapability',
> > > > > 'data': { 'pdh': 'str',
> > > > >'cert-chain': 'str',
> > > > >'cbitpos': 'int',
> > > > > -'reduced-phys-bits': 'int'},
> > > > > +'reduced-phys-bits': 'int',
> > > > > +'es': 'bool',
> > > > > +'snp': 'bool'},
> > > > > 'if': 'TARGET_I386' }
> > > > >##
> > > > > @@ -205,7 +211,8 @@
> > > > >#
> > > > ># -> { "execute": "query-sev-capabilities" }
> > > > ># <- { "return": { "pdh": "8CCDD8DDD", "cert-chain": "888CCCDDDEE",
> > > > > -# "cbitpos": 47, "reduced-phys-bits": 5}}
> > > > > +# "cbitpos": 47, "reduced-phys-bits": 5
> > > > > +# "es": false, "snp": false}}
> > > > We've previously had patches posted to support SNP in QEMU
> > > >
> > > > https://lists.gnu.org/archive/html/qemu-devel/2021-08/msg04761.html
> > > >
> > > > and this included an update to query-sev for reporting info
> > > > about the VM instance.
> > > >
> > > > Your patch is updating query-sev-capabilities, which is a
> > > > counterpart for detecting host capabilities separate from
> > > > a guest instance.
> > > Yes, that's because with this patch, I'm more interested in determining
> > > which AMD processor is running on a host, and less if ES or SNP is
> > > actually
> > > running on a guest instance or not.
> > > > None the less I wonder if the same design questions from
> > > > query-sev apply. ie do we need to have the ability to
> > > > report any SNP specific information fields, if so we need
> > > > to use a discriminated union of structs, not just bool
> > > > flags.
> > > >
> > > > More generally I'm some what wary of adding this to
> > > > query-sev-capabilities at all, unless it is part of the
> > > > main SEV-SNP series.
> > > >
> > > > Also what's the intended usage for the mgmt app from just
> > > > having these boolean fields ? Are they other more explicit
> > > > feature flags we should be reporting, instead of what are
> > > > essentially SEV generation codenames.
> > > If by "mgmt app" you're referring to sevctl, in order to determine which
> > > certificate chain to use (Naples vs Rome vs Milan ARK/ASK) we must query
> > > which processor we are running on. Although sevctl has a feature which can
> > > do this already, we cannot guarantee that sevctl is running on the same
> > > host
> > > that a VM is running on, so we must query this capability from QEMU. My
> > > logic was determining the processor would have been the following:
> > I'm not really talking about a specific, rather any tool which wants
> > to deal with SEV and QEMU, whether libvirt or an app using libvirt,
> > or something else using QEMU directly.
>
> Ah, my mistake.
>
> > Where does the actual cert chain payload come from ? Is that something
> > the app has to acquire out of band, or can the full cert chain be
> > acquired from the hardware itself ?
>
> The cert chain (or the ARK/ASK specifically) comes from AMD's KDS, yet
> sevctl is able to cache the values, and has them on-hand when needed. This
> patch would tell sevctl *which* of the cert chains to use (Naples vs Rome vs
> Milan chain). If need be, I could just focus on Naples and Rome processors
> for now and bring support for SNP (Milan processors) later on when it is
> more mature.
>
> > > !es && !snp --> Naples
> > >
> > > es && !snp --> Rome
> > >
> > > es && snp --> Milan
> > This approach isn't future proof if subsequent generations introduce
> > new certs. It feels like we should be explicitly
Re: [PATCH] gitlab-ci/cirrus: Increase timeout to 80 minutes
On 16/11/2021 18.09, Philippe Mathieu-Daudé wrote: On 11/16/21 17:49, Daniel P. Berrangé wrote: On Tue, Nov 16, 2021 at 05:33:09PM +0100, Thomas Huth wrote: The jobs on Cirrus-CI sometimes get delayed quite a bit, waiting to be scheduled, so while the build test itself finishes within 60 minutes, the total run time of the jobs can be longer due to this waiting time. Thus let's increase the timeout on the gitlab side a little bit, so that these jobs are not marked as failing just because of the delay. ...>>> diff --git a/.gitlab-ci.d/cirrus.yml b/.gitlab-ci.d/cirrus.yml index e7b25e7427..22d42585e4 100644 --- a/.gitlab-ci.d/cirrus.yml +++ b/.gitlab-ci.d/cirrus.yml @@ -14,6 +14,7 @@ stage: build image: registry.gitlab.com/libvirt/libvirt-ci/cirrus-run:master needs: [] + timeout: 80m allow_failure: true script: - source .gitlab-ci.d/cirrus/$NAME.vars Whether 80 or 100 minute, consider it Reviewed-by: Daniel P. Berrangé This pipeline took 1h51m09s: https://gitlab.com/qemu-project/qemu/-/pipelines/409666733/builds But Richard restarted unstable jobs, which probably added time to the total. IIRC from a maintainer perspective 1h15 is the upper limit. 80m fits, 100m is over. I think I agree ... I normally don't want to wait more than a little bit more than one hour, so 100 minutes feels too long already. We already have some 70m timeouts in other jobs, and one 80 minute timeout in .gitlab-ci.d/crossbuild-template.yml, so I'd say 80 minutes are really the upper boundary that we should use. Reviewed-by: Philippe Mathieu-Daudé Thank to all for your reviews! Thomas
Re: [PATCH] gitlab-ci/cirrus: Increase timeout to 80 minutes
On Tue, Nov 16, 2021 at 1:33 PM Thomas Huth wrote: > > The jobs on Cirrus-CI sometimes get delayed quite a bit, waiting to > be scheduled, so while the build test itself finishes within 60 minutes, > the total run time of the jobs can be longer due to this waiting time. > Thus let's increase the timeout on the gitlab side a little bit, so > that these jobs are not marked as failing just because of the delay. > > Signed-off-by: Thomas Huth > --- > .gitlab-ci.d/cirrus.yml | 1 + > 1 file changed, 1 insertion(+) > Reviewed-by: Willian Rampazzo
Re: [PULL SUBSYSTEM qemu-pseries] pseries: Update SLOF firmware image
On 11/16/21 17:46, Cédric Le Goater wrote: > On 11/14/21 01:51, Alexey Kardashevskiy wrote: >> The following changes since commit >> d139786e1b3d67991e6cb49a8a59bb2182350285: >> >> ppc/mmu_helper.c: do not truncate 'ea' in >> booke206_invalidate_ea_tlb() (2021-11-11 11:35:13 +0100) >> >> are available in the Git repository at: >> >> g...@github.com:aik/qemu.git tags/qemu-slof-2022 >> >> for you to fetch changes up to 73944a4bf4ab259b489af8128b4aec525484d642: >> >> pseries: Update SLOF firmware image (2021-11-13 14:47:56 +1100) >> >> >> Alexey Kardashevskiy (1): >> pseries: Update SLOF firmware image >> >> pc-bios/README | 2 +- >> pc-bios/slof.bin | Bin 991744 -> 991920 bytes >> roms/SLOF | 2 +- >> 3 files changed, 2 insertions(+), 2 deletions(-) > > Queued for 7.0. I am not sure this is a good idea, as this will make bisection painful over the release tag. Alexey, do you mind reposting after v7.0 is released?
Re: [PATCH] gitlab-ci/cirrus: Increase timeout to 80 minutes
On 11/16/21 17:49, Daniel P. Berrangé wrote: > On Tue, Nov 16, 2021 at 05:33:09PM +0100, Thomas Huth wrote: >> The jobs on Cirrus-CI sometimes get delayed quite a bit, waiting to >> be scheduled, so while the build test itself finishes within 60 minutes, >> the total run time of the jobs can be longer due to this waiting time. >> Thus let's increase the timeout on the gitlab side a little bit, so >> that these jobs are not marked as failing just because of the delay. > > On a successful pipeline I see > > freebsd-11 - 28 minutes > freebsd-12 - 57 minutes > macos - 30 minutes > > We know cirrus allows 2 concurrent jobs, so from that I infer > that the freebsd-12 job was queued for ~30 minutes waiting for > either the freebsd-11 or macos job to finish, and then it > ran in 30 minutes, giving the ~60 minute total. > > That's too close to the 60 minute gitlab default job timeout > for comfort - it can easily slip over 60 minutes by just a > small amount. > > 80 minutes will certainly help in the case where we > randomly take a little longer than 30 minutes to build, > and have 1 of the 3 jobs queued. > > When we're running jobs on both master + staging, we can > have 2 jobs running and 4 more queued - 2 of those queued > might just finish in time, but 2 will definitely fail. > My patch will cut these extra jobs on master, so in common > case we only ever get 1 queued, which should work well in > combo with your patch here. That should be good enough > for the qemu-project namespace, unless someone is triggering > pipelines for stable branch staging at the same time as > the master branch staging. > > If we do want to worry about more than 2 queued jobs > again for that reason, we might consider putting > it upto 100 minutes. That would give us enough slack to > have 4 queued jobs behind two running jobs and have > them all succeed > >> Signed-off-by: Thomas Huth >> --- >> .gitlab-ci.d/cirrus.yml | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/.gitlab-ci.d/cirrus.yml b/.gitlab-ci.d/cirrus.yml >> index e7b25e7427..22d42585e4 100644 >> --- a/.gitlab-ci.d/cirrus.yml >> +++ b/.gitlab-ci.d/cirrus.yml >> @@ -14,6 +14,7 @@ >>stage: build >>image: registry.gitlab.com/libvirt/libvirt-ci/cirrus-run:master >>needs: [] >> + timeout: 80m >>allow_failure: true >>script: >> - source .gitlab-ci.d/cirrus/$NAME.vars > > Whether 80 or 100 minute, consider it > > Reviewed-by: Daniel P. Berrangé This pipeline took 1h51m09s: https://gitlab.com/qemu-project/qemu/-/pipelines/409666733/builds But Richard restarted unstable jobs, which probably added time to the total. IIRC from a maintainer perspective 1h15 is the upper limit. 80m fits, 100m is over. Up to the project maintainers (personally I don't have any objection, in particular if this reduces the failures rate). Reviewed-by: Philippe Mathieu-Daudé
[PATCH 0/3] Fix irq allocation of PCI host bridge on powernv
This series removes a bogus allocation of a LSI interrupt for the PCI Host Bridge found in the powernv model (phb4). The real hardware doesn't declare any LSI, so the model should match. It was causing some inconsistencies in the interrupt controller data. However, removing that LSI shows that the PCI AER code assumes an interrupt is defined (LSI or MSI or MSI-X), which is not the case with the root bridge device on powernv. So the last patch adds a check to make sure a LSI is defined before entering pci_set_irq() as it asserts if it's called with no LSI defined. Frederic Barrat (3): ppc/pnv: Tune the POWER9 PCIe Host bridge model pci: Export the pci_intx() function pcie_aer: Don't trigger a LSI if none are defined hw/pci-host/pnv_phb4.c | 5 - hw/pci/pci.c | 5 - hw/pci/pcie_aer.c | 4 +++- include/hw/pci/pci.h | 5 + 4 files changed, 12 insertions(+), 7 deletions(-) -- 2.33.1
[PATCH 1/3] ppc/pnv: Tune the POWER9 PCIe Host bridge model
The PHB v4 found on POWER9 doesn't request any LSI, so let's clear the Interrupt Pin register in the config space so that the model matches the hardware. If we don't, then we inherit from the default pcie root bridge, which requests a LSI. And because we don't map it correctly in the device tree, all PHBs allocate the same bogus hw interrupt. We end up with inconsistent interrupt controller (xive) data. The problem goes away if we don't allocate the LSI in the first place. Signed-off-by: Frederic Barrat --- hw/pci-host/pnv_phb4.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c index 5c375a9f28..1659d55b4f 100644 --- a/hw/pci-host/pnv_phb4.c +++ b/hw/pci-host/pnv_phb4.c @@ -1234,10 +1234,13 @@ static void pnv_phb4_reset(DeviceState *dev) PCIDevice *root_dev = PCI_DEVICE(>root); /* - * Configure PCI device id at reset using a property. + * Configure the PCI device at reset: + * - set the Vendor and Device ID to for the root bridge + * - no LSI */ pci_config_set_vendor_id(root_dev->config, PCI_VENDOR_ID_IBM); pci_config_set_device_id(root_dev->config, phb->device_id); +pci_config_set_interrupt_pin(root_dev->config, 0); } static const char *pnv_phb4_root_bus_path(PCIHostState *host_bridge, -- 2.33.1
[PATCH 2/3] pci: Export the pci_intx() function
Move the pci_intx() definition to the PCI header file, so that it can
be called from other PCI files. It is used by the next patch.
Signed-off-by: Frederic Barrat
---
hw/pci/pci.c | 5 -
include/hw/pci/pci.h | 5 +
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index e5993c1ef5..249d7e4cf6 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1497,11 +1497,6 @@ static void pci_irq_handler(void *opaque, int irq_num,
int level)
pci_change_irq_level(pci_dev, irq_num, change);
}
-static inline int pci_intx(PCIDevice *pci_dev)
-{
-return pci_get_byte(pci_dev->config + PCI_INTERRUPT_PIN) - 1;
-}
-
qemu_irq pci_allocate_irq(PCIDevice *pci_dev)
{
int intx = pci_intx(pci_dev);
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index e7cdf2d5ec..35f8eb67bd 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -735,6 +735,11 @@ void lsi53c8xx_handle_legacy_cmdline(DeviceState *lsi_dev);
qemu_irq pci_allocate_irq(PCIDevice *pci_dev);
void pci_set_irq(PCIDevice *pci_dev, int level);
+static inline int pci_intx(PCIDevice *pci_dev)
+{
+return pci_get_byte(pci_dev->config + PCI_INTERRUPT_PIN) - 1;
+}
+
static inline void pci_irq_assert(PCIDevice *pci_dev)
{
pci_set_irq(pci_dev, 1);
--
2.33.1
[PATCH 3/3] pcie_aer: Don't trigger a LSI if none are defined
Skip triggering an LSI when the AER root error status is updated if no
LSI is defined for the device. We can have a root bridge with no LSI,
MSI and MSI-X defined, for example on POWER systems.
Signed-off-by: Frederic Barrat
---
hw/pci/pcie_aer.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
index 27f9cc56af..e1a8a88c8c 100644
--- a/hw/pci/pcie_aer.c
+++ b/hw/pci/pcie_aer.c
@@ -774,7 +774,9 @@ void pcie_aer_root_write_config(PCIDevice *dev,
uint32_t root_cmd = pci_get_long(aer_cap + PCI_ERR_ROOT_COMMAND);
/* 6.2.4.1.2 Interrupt Generation */
if (!msix_enabled(dev) && !msi_enabled(dev)) {
-pci_set_irq(dev, !!(root_cmd & enabled_cmd));
+if (pci_intx(dev) != -1) {
+pci_set_irq(dev, !!(root_cmd & enabled_cmd));
+}
return;
}
--
2.33.1
Re: [PATCH] sev: allow capabilities to check for SEV-ES support
On 11/16/21 10:53 AM, Daniel P. Berrangé wrote:
On Tue, Nov 16, 2021 at 10:29:35AM -0500, Tyler Fanelli wrote:
On 11/16/21 4:17 AM, Daniel P. Berrangé wrote:
On Mon, Nov 15, 2021 at 02:38:04PM -0500, Tyler Fanelli wrote:
Probe for SEV-ES and SEV-SNP capabilities to distinguish between Rome,
Naples, and Milan processors. Use the CPUID function to probe if a
processor is capable of running SEV-ES or SEV-SNP, rather than if it
actually is running SEV-ES or SEV-SNP.
Signed-off-by: Tyler Fanelli
---
qapi/misc-target.json | 11 +--
target/i386/sev.c | 6 --
2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index 5aa2b95b7d..c3e9bce12b 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -182,13 +182,19 @@
# @reduced-phys-bits: Number of physical Address bit reduction when SEV is
# enabled
#
+# @es: SEV-ES capability of the machine.
+#
+# @snp: SEV-SNP capability of the machine.
+#
# Since: 2.12
##
{ 'struct': 'SevCapability',
'data': { 'pdh': 'str',
'cert-chain': 'str',
'cbitpos': 'int',
-'reduced-phys-bits': 'int'},
+'reduced-phys-bits': 'int',
+'es': 'bool',
+'snp': 'bool'},
'if': 'TARGET_I386' }
##
@@ -205,7 +211,8 @@
#
# -> { "execute": "query-sev-capabilities" }
# <- { "return": { "pdh": "8CCDD8DDD", "cert-chain": "888CCCDDDEE",
-# "cbitpos": 47, "reduced-phys-bits": 5}}
+# "cbitpos": 47, "reduced-phys-bits": 5
+# "es": false, "snp": false}}
We've previously had patches posted to support SNP in QEMU
https://lists.gnu.org/archive/html/qemu-devel/2021-08/msg04761.html
and this included an update to query-sev for reporting info
about the VM instance.
Your patch is updating query-sev-capabilities, which is a
counterpart for detecting host capabilities separate from
a guest instance.
Yes, that's because with this patch, I'm more interested in determining
which AMD processor is running on a host, and less if ES or SNP is actually
running on a guest instance or not.
None the less I wonder if the same design questions from
query-sev apply. ie do we need to have the ability to
report any SNP specific information fields, if so we need
to use a discriminated union of structs, not just bool
flags.
More generally I'm some what wary of adding this to
query-sev-capabilities at all, unless it is part of the
main SEV-SNP series.
Also what's the intended usage for the mgmt app from just
having these boolean fields ? Are they other more explicit
feature flags we should be reporting, instead of what are
essentially SEV generation codenames.
If by "mgmt app" you're referring to sevctl, in order to determine which
certificate chain to use (Naples vs Rome vs Milan ARK/ASK) we must query
which processor we are running on. Although sevctl has a feature which can
do this already, we cannot guarantee that sevctl is running on the same host
that a VM is running on, so we must query this capability from QEMU. My
logic was determining the processor would have been the following:
I'm not really talking about a specific, rather any tool which wants
to deal with SEV and QEMU, whether libvirt or an app using libvirt,
or something else using QEMU directly.
Ah, my mistake.
Where does the actual cert chain payload come from ? Is that something
the app has to acquire out of band, or can the full cert chain be
acquired from the hardware itself ?
The cert chain (or the ARK/ASK specifically) comes from AMD's KDS, yet
sevctl is able to cache the values, and has them on-hand when needed.
This patch would tell sevctl *which* of the cert chains to use (Naples
vs Rome vs Milan chain). If need be, I could just focus on Naples and
Rome processors for now and bring support for SNP (Milan processors)
later on when it is more mature.
!es && !snp --> Naples
es && !snp --> Rome
es && snp --> Milan
This approach isn't future proof if subsequent generations introduce
new certs. It feels like we should be explicitly reporting something
about the certs rather than relying on every app to re-implement tihs
logic.
Alright, like an encoding of which processor generation the host is
running on?
Regards,
Daniel
Tyler.
--
Tyler Fanelli (tfanelli)
[PULL 2/2] nbd/server: Add --selinux-label option
From: "Richard W.M. Jones"
Under SELinux, Unix domain sockets have two labels. One is on the
disk and can be set with commands such as chcon(1). There is a
different label stored in memory (called the process label). This can
only be set by the process creating the socket. When using SELinux +
SVirt and wanting qemu to be able to connect to a qemu-nbd instance,
you must set both labels correctly first.
For qemu-nbd the options to set the second label are awkward. You can
create the socket in a wrapper program and then exec into qemu-nbd.
Or you could try something with LD_PRELOAD.
This commit adds the ability to set the label straightforwardly on the
command line, via the new --selinux-label flag. (The name of the flag
is the same as the equivalent nbdkit option.)
A worked example showing how to use the new option can be found in
this bug: https://bugzilla.redhat.com/show_bug.cgi?id=1984938
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1984938
Signed-off-by: Richard W.M. Jones
Reviewed-by: Daniel P. Berrangé
[eblake: rebase to configure changes, reject --selinux-label if it is
not compiled in or not used on a Unix socket]
Note that we may relax some of these restrictions at a later date,
such as making it possible to label a TCP socket, although it may be
smarter to do so as a generic QMP action rather than more one-off
command lines in qemu-nbd.
Signed-off-by: Eric Blake
Message-Id: <2025202944.615966-1-ebl...@redhat.com>
Reviewed-by: Thomas Huth
[eblake: adjust meson output as suggested by thuth]
Signed-off-by: Eric Blake
---
meson.build | 10 +++-
qemu-nbd.c| 46 +++
meson_options.txt | 3 ++
scripts/meson-buildoptions.sh | 3 ++
tests/docker/dockerfiles/centos8.docker | 1 +
.../dockerfiles/fedora-i386-cross.docker | 1 +
tests/docker/dockerfiles/fedora.docker| 1 +
tests/docker/dockerfiles/opensuse-leap.docker | 1 +
tests/docker/dockerfiles/ubuntu1804.docker| 1 +
tests/docker/dockerfiles/ubuntu2004.docker| 1 +
10 files changed, 67 insertions(+), 1 deletion(-)
diff --git a/meson.build b/meson.build
index 2ece4fe0889a..084806a9415c 100644
--- a/meson.build
+++ b/meson.build
@@ -1201,6 +1201,11 @@ keyutils = dependency('libkeyutils', required: false,
has_gettid = cc.has_function('gettid')
+# libselinux
+selinux = dependency('libselinux',
+ required: get_option('selinux'),
+ method: 'pkg-config', kwargs: static_kwargs)
+
# Malloc tests
malloc = []
@@ -1479,6 +1484,7 @@ config_host_data.set('CONFIG_SPICE_PROTOCOL',
spice_protocol.found())
config_host_data.set('CONFIG_SPICE', spice.found())
config_host_data.set('CONFIG_X11', x11.found())
config_host_data.set('CONFIG_CFI', get_option('cfi'))
+config_host_data.set('CONFIG_SELINUX', selinux.found())
config_host_data.set('QEMU_VERSION', '"@0@"'.format(meson.project_version()))
config_host_data.set('QEMU_VERSION_MAJOR',
meson.project_version().split('.')[0])
config_host_data.set('QEMU_VERSION_MINOR',
meson.project_version().split('.')[1])
@@ -3054,7 +3060,8 @@ if have_tools
qemu_io = executable('qemu-io', files('qemu-io.c'),
dependencies: [block, qemuutil], install: true)
qemu_nbd = executable('qemu-nbd', files('qemu-nbd.c'),
- dependencies: [blockdev, qemuutil, gnutls], install: true)
+ dependencies: [blockdev, qemuutil, gnutls, selinux],
+ install: true)
subdir('storage-daemon')
subdir('contrib/rdmacm-mux')
@@ -3430,6 +3437,7 @@ summary_info += {'libdaxctl support': libdaxctl}
summary_info += {'libudev': libudev}
# Dummy dependency, keep .found()
summary_info += {'FUSE lseek':fuse_lseek.found()}
+summary_info += {'selinux': selinux}
summary(summary_info, bool_yn: true, section: 'Dependencies')
if not supported_cpus.contains(cpu)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 9d895ba24b1e..c6c20df68a4d 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -47,6 +47,10 @@
#include "trace/control.h"
#include "qemu-version.h"
+#ifdef CONFIG_SELINUX
+#include
+#endif
+
#ifdef __linux__
#define HAVE_NBD_DEVICE 1
#else
@@ -64,6 +68,7 @@
#define QEMU_NBD_OPT_FORK 263
#define QEMU_NBD_OPT_TLSAUTHZ 264
#define QEMU_NBD_OPT_PID_FILE 265
+#define QEMU_NBD_OPT_SELINUX_LABEL 266
#define MBR_SIZE 512
@@ -116,6 +121,9 @@ static void usage(const char *name)
" --forkfork off the server process and exit the parent\n"
"once the server is running\n"
" --pid-file=PATH store the server's process ID in the given file\n"
+#ifdef CONFIG_SELINUX
+" --selinux-label=LABEL set SELinux process label on listening socket\n"
+#endif
#if HAVE_NBD_DEVICE
"\n"
"Kernel NBD client support:\n"
@@ -454,6 +462,7 @@ static const char
[PULL 1/2] nbd/server: Silence clang sanitizer warning
clang's sanitizer is picky: memset(NULL, x, 0) is technically
undefined behavior, even though no sane implementation of memset()
deferences the NULL. Caught by the nbd-qemu-allocation iotest.
The alternative to checking before each memset is to instead force an
allocation of 1 element instead of g_new0(type, 0)'s behavior of
returning NULL for a 0-length array.
Reported-by: Peter Maydell
Fixes: 3b1f244c59 (nbd: Allow export of multiple bitmaps for one device)
Signed-off-by: Eric Blake
Message-Id: <2025223943.626416-1-ebl...@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé
---
nbd/server.c | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index 6d03e8a4b436..d9164ee6d0da 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2016-2020 Red Hat, Inc.
+ * Copyright (C) 2016-2021 Red Hat, Inc.
* Copyright (C) 2005 Anthony Liguori
*
* Network Block Device Server Side
@@ -879,7 +879,9 @@ static bool nbd_meta_qemu_query(NBDClient *client,
NBDExportMetaContexts *meta,
if (!*query) {
if (client->opt == NBD_OPT_LIST_META_CONTEXT) {
meta->allocation_depth = meta->exp->allocation_depth;
-memset(meta->bitmaps, 1, meta->exp->nr_export_bitmaps);
+if (meta->exp->nr_export_bitmaps) {
+memset(meta->bitmaps, 1, meta->exp->nr_export_bitmaps);
+}
}
trace_nbd_negotiate_meta_query_parse("empty");
return true;
@@ -894,7 +896,8 @@ static bool nbd_meta_qemu_query(NBDClient *client,
NBDExportMetaContexts *meta,
if (nbd_strshift(, "dirty-bitmap:")) {
trace_nbd_negotiate_meta_query_parse("dirty-bitmap:");
if (!*query) {
-if (client->opt == NBD_OPT_LIST_META_CONTEXT) {
+if (client->opt == NBD_OPT_LIST_META_CONTEXT &&
+meta->exp->nr_export_bitmaps) {
memset(meta->bitmaps, 1, meta->exp->nr_export_bitmaps);
}
trace_nbd_negotiate_meta_query_parse("empty");
@@ -1024,7 +1027,9 @@ static int nbd_negotiate_meta_queries(NBDClient *client,
/* enable all known contexts */
meta->base_allocation = true;
meta->allocation_depth = meta->exp->allocation_depth;
-memset(meta->bitmaps, 1, meta->exp->nr_export_bitmaps);
+if (meta->exp->nr_export_bitmaps) {
+memset(meta->bitmaps, 1, meta->exp->nr_export_bitmaps);
+}
} else {
for (i = 0; i < nb_queries; ++i) {
ret = nbd_negotiate_meta_query(client, meta, errp);
--
2.33.1
[PULL 0/2] NBD patches for 6.2-rc1, 2021-11-16
The following changes since commit 9f0f846465d4c52ce9857787e947dffb64367fae: Merge tag 'machine-core-2025' of https://github.com/philmd/qemu into staging (2021-11-16 12:50:27 +0100) are available in the Git repository at: https://repo.or.cz/qemu/ericb.git tags/pull-nbd-2021-11-16 for you to fetch changes up to 3d212b41e9ccb3f37d04f22c59a960bac099c1d4: nbd/server: Add --selinux-label option (2021-11-16 10:16:38 -0600) nbd patches for 2021-11-16 - Rich Jones: Add 'qemu-nbd --selinux-label' option for running Unix socket with appropriate SELinux labeling - Eric Blake: Address clang sanitizer warning Eric Blake (1): nbd/server: Silence clang sanitizer warning Richard W.M. Jones (1): nbd/server: Add --selinux-label option meson.build | 10 - nbd/server.c | 13 +-- qemu-nbd.c| 46 +++ meson_options.txt | 3 ++ scripts/meson-buildoptions.sh | 3 ++ tests/docker/dockerfiles/centos8.docker | 1 + tests/docker/dockerfiles/fedora-i386-cross.docker | 1 + tests/docker/dockerfiles/fedora.docker| 1 + tests/docker/dockerfiles/opensuse-leap.docker | 1 + tests/docker/dockerfiles/ubuntu1804.docker| 1 + tests/docker/dockerfiles/ubuntu2004.docker| 1 + 11 files changed, 76 insertions(+), 5 deletions(-) -- 2.33.1
[PATCH] tests/9pfs: use g_autofree where possible
Signed-off-by: Christian Schoenebeck
---
tests/qtest/virtio-9p-test.c | 86 +++-
1 file changed, 25 insertions(+), 61 deletions(-)
diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 41fed41de1..11861aaf7d 100644
--- a/tests/qtest/virtio-9p-test.c
+++ b/tests/qtest/virtio-9p-test.c
@@ -84,7 +84,7 @@ static void pci_config(void *obj, void *data, QGuestAllocator
*t_alloc)
QVirtio9P *v9p = obj;
alloc = t_alloc;
size_t tag_len = qvirtio_config_readw(v9p->vdev, 0);
-char *tag;
+g_autofree char *tag = NULL;
int i;
g_assert_cmpint(tag_len, ==, strlen(MOUNT_TAG));
@@ -94,7 +94,6 @@ static void pci_config(void *obj, void *data, QGuestAllocator
*t_alloc)
tag[i] = qvirtio_config_readb(v9p->vdev, i + 2);
}
g_assert_cmpmem(tag, tag_len, MOUNT_TAG, tag_len);
-g_free(tag);
}
#define P9_MAX_SIZE 4096 /* Max size of a T-message or R-message */
@@ -580,7 +579,7 @@ static void do_version(QVirtio9P *v9p)
{
const char *version = "9P2000.L";
uint16_t server_len;
-char *server_version;
+g_autofree char *server_version = NULL;
P9Req *req;
req = v9fs_tversion(v9p, P9_MAX_SIZE, version, P9_NOTAG);
@@ -588,8 +587,6 @@ static void do_version(QVirtio9P *v9p)
v9fs_rversion(req, _len, _version);
g_assert_cmpmem(server_version, server_len, version, strlen(version));
-
-g_free(server_version);
}
/* utility function: walk to requested dir and return fid for that dir */
@@ -637,7 +634,7 @@ static void fs_walk(void *obj, void *data, QGuestAllocator
*t_alloc)
alloc = t_alloc;
char *wnames[P9_MAXWELEM];
uint16_t nwqid;
-v9fs_qid *wqid;
+g_autofree v9fs_qid *wqid = NULL;
int i;
P9Req *req;
@@ -655,8 +652,6 @@ static void fs_walk(void *obj, void *data, QGuestAllocator
*t_alloc)
for (i = 0; i < P9_MAXWELEM; i++) {
g_free(wnames[i]);
}
-
-g_free(wqid);
}
static bool fs_dirents_contain_name(struct V9fsDirent *e, const char* name)
@@ -984,7 +979,8 @@ static void fs_walk_dotdot(void *obj, void *data,
QGuestAllocator *t_alloc)
QVirtio9P *v9p = obj;
alloc = t_alloc;
char *const wnames[] = { g_strdup("..") };
-v9fs_qid root_qid, *wqid;
+v9fs_qid root_qid;
+g_autofree v9fs_qid *wqid = NULL;
P9Req *req;
do_version(v9p);
@@ -998,7 +994,6 @@ static void fs_walk_dotdot(void *obj, void *data,
QGuestAllocator *t_alloc)
g_assert_cmpmem(_qid, 13, wqid[0], 13);
-g_free(wqid);
g_free(wnames[0]);
}
@@ -1027,7 +1022,7 @@ static void fs_write(void *obj, void *data,
QGuestAllocator *t_alloc)
alloc = t_alloc;
static const uint32_t write_count = P9_MAX_SIZE / 2;
char *const wnames[] = { g_strdup(QTEST_V9FS_SYNTH_WRITE_FILE) };
-char *buf = g_malloc0(write_count);
+g_autofree char *buf = g_malloc0(write_count);
uint32_t count;
P9Req *req;
@@ -1045,7 +1040,6 @@ static void fs_write(void *obj, void *data,
QGuestAllocator *t_alloc)
v9fs_rwrite(req, );
g_assert_cmpint(count, ==, write_count);
-g_free(buf);
g_free(wnames[0]);
}
@@ -1125,7 +1119,7 @@ static void fs_flush_ignored(void *obj, void *data,
QGuestAllocator *t_alloc)
static void do_mkdir(QVirtio9P *v9p, const char *path, const char *cname)
{
-char *const name = g_strdup(cname);
+g_autofree char *name = g_strdup(cname);
uint32_t fid;
P9Req *req;
@@ -1134,15 +1128,13 @@ static void do_mkdir(QVirtio9P *v9p, const char *path,
const char *cname)
req = v9fs_tmkdir(v9p, fid, name, 0750, 0, 0);
v9fs_req_wait_for_reply(req, NULL);
v9fs_rmkdir(req, NULL);
-
-g_free(name);
}
/* create a regular file with Tlcreate and return file's fid */
static uint32_t do_lcreate(QVirtio9P *v9p, const char *path,
const char *cname)
{
-char *const name = g_strdup(cname);
+g_autofree char *name = g_strdup(cname);
uint32_t fid;
P9Req *req;
@@ -1152,7 +1144,6 @@ static uint32_t do_lcreate(QVirtio9P *v9p, const char
*path,
v9fs_req_wait_for_reply(req, NULL);
v9fs_rlcreate(req, NULL, NULL);
-g_free(name);
return fid;
}
@@ -1160,8 +1151,8 @@ static uint32_t do_lcreate(QVirtio9P *v9p, const char
*path,
static void do_symlink(QVirtio9P *v9p, const char *path, const char *clink,
const char *to)
{
-char *const name = g_strdup(clink);
-char *const dst = g_strdup(to);
+g_autofree char *name = g_strdup(clink);
+g_autofree char *dst = g_strdup(to);
uint32_t fid;
P9Req *req;
@@ -1170,9 +1161,6 @@ static void do_symlink(QVirtio9P *v9p, const char *path,
const char *clink,
req = v9fs_tsymlink(v9p, fid, name, dst, 0, 0);
v9fs_req_wait_for_reply(req, NULL);
v9fs_rsymlink(req, NULL);
-
-g_free(dst);
-g_free(name);
}
/* create a hard link named @a clink in directory @a path pointing to @a to */
@@
Re: [PATCH] gitlab-ci/cirrus: Increase timeout to 80 minutes
On Tue, Nov 16, 2021 at 05:33:09PM +0100, Thomas Huth wrote: > The jobs on Cirrus-CI sometimes get delayed quite a bit, waiting to > be scheduled, so while the build test itself finishes within 60 minutes, > the total run time of the jobs can be longer due to this waiting time. > Thus let's increase the timeout on the gitlab side a little bit, so > that these jobs are not marked as failing just because of the delay. On a successful pipeline I see freebsd-11 - 28 minutes freebsd-12 - 57 minutes macos - 30 minutes We know cirrus allows 2 concurrent jobs, so from that I infer that the freebsd-12 job was queued for ~30 minutes waiting for either the freebsd-11 or macos job to finish, and then it ran in 30 minutes, giving the ~60 minute total. That's too close to the 60 minute gitlab default job timeout for comfort - it can easily slip over 60 minutes by just a small amount. 80 minutes will certainly help in the case where we randomly take a little longer than 30 minutes to build, and have 1 of the 3 jobs queued. When we're running jobs on both master + staging, we can have 2 jobs running and 4 more queued - 2 of those queued might just finish in time, but 2 will definitely fail. My patch will cut these extra jobs on master, so in common case we only ever get 1 queued, which should work well in combo with your patch here. That should be good enough for the qemu-project namespace, unless someone is triggering pipelines for stable branch staging at the same time as the master branch staging. If we do want to worry about more than 2 queued jobs again for that reason, we might consider putting it upto 100 minutes. That would give us enough slack to have 4 queued jobs behind two running jobs and have them all succeed > Signed-off-by: Thomas Huth > --- > .gitlab-ci.d/cirrus.yml | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/.gitlab-ci.d/cirrus.yml b/.gitlab-ci.d/cirrus.yml > index e7b25e7427..22d42585e4 100644 > --- a/.gitlab-ci.d/cirrus.yml > +++ b/.gitlab-ci.d/cirrus.yml > @@ -14,6 +14,7 @@ >stage: build >image: registry.gitlab.com/libvirt/libvirt-ci/cirrus-run:master >needs: [] > + timeout: 80m >allow_failure: true >script: > - source .gitlab-ci.d/cirrus/$NAME.vars Whether 80 or 100 minute, consider it Reviewed-by: Daniel P. Berrangé Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PULL SUBSYSTEM qemu-pseries] pseries: Update SLOF firmware image
On 11/14/21 01:51, Alexey Kardashevskiy wrote: The following changes since commit d139786e1b3d67991e6cb49a8a59bb2182350285: ppc/mmu_helper.c: do not truncate 'ea' in booke206_invalidate_ea_tlb() (2021-11-11 11:35:13 +0100) are available in the Git repository at: g...@github.com:aik/qemu.git tags/qemu-slof-2022 for you to fetch changes up to 73944a4bf4ab259b489af8128b4aec525484d642: pseries: Update SLOF firmware image (2021-11-13 14:47:56 +1100) Alexey Kardashevskiy (1): pseries: Update SLOF firmware image pc-bios/README | 2 +- pc-bios/slof.bin | Bin 991744 -> 991920 bytes roms/SLOF| 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) Queued for 7.0. Thanks, C.
Re: [PATCH v5 6/6] multifd: Implement zerocopy write in multifd migration (multifd-zerocopy)
On Tue, Nov 16, 2021 at 05:34:50PM +0100, Juan Quintela wrote:
> Daniel P. Berrangé wrote:
>
> >>
> >> if (params->zerocopy &&
> >> (params->parameters.multifd_compression !=
> >> MULTIFD_COMPRESSION_NONE ||
> >> migrate_use_tls())) {
> >>error_setg(,
> >> "Zerocopy only available for non-compressed non-TLS
> >> multifd migration");
> >> return false;
> >> }
> >>
> >> You have to do the equivalent of multifd_compression and tls enablement,
> >> to see that zerocopy is not enabled, of course.
> >>
> >> I would prefer to check for QIO_CHANNEL_FEATUR_WRITE_ZEROCPY there, but
> >> I can't see a way of doing that without a qio.
> >
> > I don't think you need to check that feature flag.
>
> Oh, I mean other thing.
>
> When you set "zerocopy" capability, you don't know if the kernel support
> it. My understanding is that the only way to check if it supported is
> here.
If you reqest it and it isn't supported you'll get an error back from
qio_channel_writev_zerocopy(). That's a bit too late though.
Ideally we should report an error straight after the migration code
creates the I/O channel, by querying for the feature.
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] pci-host: Allow extended config space access for PowerNV PHB4 model
On 11/9/21 15:50, Christophe Lombard wrote: The PCIe extended configuration space on the device is not currently accessible to the host. if by default, it is still inaccessible for conventional for PCIe buses, add the current flag PCI_BUS_EXTENDED_CONFIG_SPACE on the root bus permits PCI-E extended config space access. Signed-off-by: Christophe Lombard --- hw/pci-host/pnv_phb4.c | 1 + 1 file changed, 1 insertion(+) Queued for 7.0. Thanks, C.
Re: [PATCH v4 07/25] assertions for block_int global state API
On 16.11.21 16:43, Emanuele Giuseppe Esposito wrote:
On 12/11/2021 14:51, Hanna Reitz wrote:
On 25.10.21 12:17, Emanuele Giuseppe Esposito wrote:
Signed-off-by: Emanuele Giuseppe Esposito
Reviewed-by: Stefan Hajnoczi
---
block.c | 17 +
block/backup.c | 1 +
block/block-backend.c | 3 +++
block/commit.c | 2 ++
block/dirty-bitmap.c | 1 +
block/io.c | 6 ++
block/mirror.c | 4
block/monitor/bitmap-qmp-cmds.c | 6 ++
block/stream.c | 2 ++
blockdev.c | 7 +++
10 files changed, 49 insertions(+)
diff --git a/block.c b/block.c
index 672f946065..41c5883c5c 100644
--- a/block.c
+++ b/block.c
[...]
@@ -7473,6 +7488,7 @@ static bool
append_strong_runtime_options(QDict *d, BlockDriverState *bs)
* would result in exactly bs->backing. */
bool bdrv_backing_overridden(BlockDriverState *bs)
{
+ assert(qemu_in_main_thread());
if (bs->backing) {
return strcmp(bs->auto_backing_file,
bs->backing->bs->filename);
This function is in block_int-common.h, though.
Can go as GS, since it is under BQL.
(Actually, it is only used in block.c, so if you want I can put it as
static). Otherwise, I will just move it to GS.
Sounds good to me.
Hanna
Re: [PATCH v5 6/6] multifd: Implement zerocopy write in multifd migration (multifd-zerocopy)
Daniel P. Berrangé wrote:
>>
>> if (params->zerocopy &&
>> (params->parameters.multifd_compression != MULTIFD_COMPRESSION_NONE
>> ||
>> migrate_use_tls())) {
>>error_setg(,
>> "Zerocopy only available for non-compressed non-TLS
>> multifd migration");
>> return false;
>> }
>>
>> You have to do the equivalent of multifd_compression and tls enablement,
>> to see that zerocopy is not enabled, of course.
>>
>> I would prefer to check for QIO_CHANNEL_FEATUR_WRITE_ZEROCPY there, but
>> I can't see a way of doing that without a qio.
>
> I don't think you need to check that feature flag.
Oh, I mean other thing.
When you set "zerocopy" capability, you don't know if the kernel support
it. My understanding is that the only way to check if it supported is
here.
I agree with the rest of the arguments.
Later, Juan.
Re: [PATCH 1/2] migration/colo: Optimize COLO start code path
Zhang Chen wrote:
> There is no need to start COLO through MIGRATION_STATUS_ACTIVE.
Hi
I don't understand what you are trying to do. In my reading, at least
the commit message is wrong:
void migrate_start_colo_process(MigrationState *s)
{
...
migrate_set_state(>state, MIGRATION_STATUS_ACTIVE,
MIGRATION_STATUS_COLO);
...
}
and
void *colo_process_incoming_thread(void *opaque)
{
...
migrate_set_state(>state, MIGRATION_STATUS_ACTIVE,
MIGRATION_STATUS_COLO);
So colo starts with MIGRATION_STATUS_ACTIVE.
> Signed-off-by: Zhang Chen
> ---
> migration/colo.c | 2 --
> migration/migration.c | 18 +++---
> 2 files changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/migration/colo.c b/migration/colo.c
> index 2415325262..ad1a4426b3 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -667,8 +667,6 @@ void migrate_start_colo_process(MigrationState *s)
> colo_checkpoint_notify, s);
>
> qemu_sem_init(>colo_exit_sem, 0);
> -migrate_set_state(>state, MIGRATION_STATUS_ACTIVE,
> - MIGRATION_STATUS_COLO);
> colo_process_checkpoint(s);
> qemu_mutex_lock_iothread();
> }
> diff --git a/migration/migration.c b/migration/migration.c
> index abaf6f9e3d..4c8662a839 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -3222,7 +3222,10 @@ static void migration_completion(MigrationState *s)
> goto fail_invalidate;
> }
>
> -if (!migrate_colo_enabled()) {
> +if (migrate_colo_enabled()) {
> +migrate_set_state(>state, current_active_state,
> + MIGRATION_STATUS_COLO);
> +} else {
> migrate_set_state(>state, current_active_state,
>MIGRATION_STATUS_COMPLETED);
> }
This moves the setup to MIGRATION_STATUS_COLO to completion time instead
of the beggining of the process. I have no clue why. I guess you can
put a comment/commit message to say what you ar.e trynig to do.
> @@ -3607,12 +3610,7 @@ static void migration_iteration_finish(MigrationState
> *s)
> migration_calculate_complete(s);
> runstate_set(RUN_STATE_POSTMIGRATE);
> break;
> -
> -case MIGRATION_STATUS_ACTIVE:
> -/*
> - * We should really assert here, but since it's during
> - * migration, let's try to reduce the usage of assertions.
> - */
> +case MIGRATION_STATUS_COLO:
> if (!migrate_colo_enabled()) {
> error_report("%s: critical error: calling COLO code without "
> "COLO enabled", __func__);
> @@ -3622,6 +3620,12 @@ static void migration_iteration_finish(MigrationState
> *s)
> * Fixme: we will run VM in COLO no matter its old running state.
> * After exited COLO, we will keep running.
> */
> + /* Fallthrough */
> +case MIGRATION_STATUS_ACTIVE:
> +/*
> + * We should really assert here, but since it's during
> + * migration, let's try to reduce the usage of assertions.
> + */
> s->vm_was_running = true;
> /* Fallthrough */
> case MIGRATION_STATUS_FAILED:
I guess this change is related to the previous one, but I don't
understand colo enough to review it.
Later, Juan.
Re: [PATCH v2] Fixed a QEMU hang when guest poweroff in COLO mode
"Rao, Lei" wrote: > From: "Rao, Lei" > > When the PVM guest poweroff, the COLO thread may wait a semaphore > in colo_process_checkpoint().So, we should wake up the COLO thread > before migration shutdown. > > Signed-off-by: Lei Rao Reviewed-by: Juan Quintela
Re: [PATCH v5 6/6] multifd: Implement zerocopy write in multifd migration (multifd-zerocopy)
On Tue, Nov 16, 2021 at 04:17:47PM +, Daniel P. Berrangé wrote:
> On Tue, Nov 16, 2021 at 05:08:06PM +0100, Juan Quintela wrote:
> > Leonardo Bras wrote:
> > > Implement zerocopy on nocomp_send_write(), by making use of QIOChannel
> > > zerocopy interface.
> > >
> > > Change multifd_send_sync_main() so it can distinguish each iteration sync
> > > from
> > > the setup and the completion, so a flush_zerocopy() can be called
> > > at the after each iteration in order to make sure all dirty pages are sent
> > > before a new iteration is started.
> > >
> > > Also make it return -1 if flush_zerocopy() fails, in order to cancel
> > > the migration process, and avoid resuming the guest in the target host
> > > without receiving all current RAM.
> > >
> > > This will work fine on RAM migration because the RAM pages are not
> > > usually freed,
> > > and there is no problem on changing the pages content between
> > > async_send() and
> > > the actual sending of the buffer, because this change will dirty the page
> > > and
> > > cause it to be re-sent on a next iteration anyway.
> > >
> > > Given a lot of locked memory may be needed in order to use multid
> > > migration
> > > with zerocopy enabled, make it optional by creating a new migration
> > > parameter
> > > "zerocopy" on qapi, so low-privileged users can still perform multifd
> > > migrations.
> >
> > How much memory can a non-root program use by default?
> >
> >
> > > static void *multifd_send_thread(void *opaque)
> > > @@ -853,6 +875,10 @@ static void multifd_new_send_channel_async(QIOTask
> > > *task, gpointer opaque)
> > > goto cleanup;
> > > }
> > >
> > > +if (migrate_use_zerocopy()) {
> > > +p->write_flags = QIO_CHANNEL_WRITE_FLAG_ZEROCOPY;
> > > +}
> >
> > This belongs
> >
> >
> > > p->c = QIO_CHANNEL(sioc);
> > > qio_channel_set_delay(p->c, false);
> > > p->running = true;
> > > @@ -918,6 +944,7 @@ int multifd_save_setup(Error **errp)
> > > p->packet->version = cpu_to_be32(MULTIFD_VERSION);
> > > p->name = g_strdup_printf("multifdsend_%d", i);
> > > p->tls_hostname = g_strdup(s->hostname);
> > > +p->write_flags = 0;
> >
> > here?
> >
> > > socket_send_channel_create(multifd_new_send_channel_async, p);
> > > }
> > > diff --git a/migration/socket.c b/migration/socket.c
> > > index e26e94aa0c..8e40e0a3fd 100644
> > > --- a/migration/socket.c
> > > +++ b/migration/socket.c
> > > @@ -78,8 +78,13 @@ static void socket_outgoing_migration(QIOTask *task,
> > > trace_migration_socket_outgoing_connected(data->hostname);
> > > }
> > >
> > > -if (migrate_use_zerocopy()) {
> > > -error_setg(, "Zerocopy not available in migration");
> > > +if (migrate_use_zerocopy() &&
> > > +(!migrate_use_multifd() ||
> > > + !qio_channel_has_feature(sioc,
> > > QIO_CHANNEL_FEATURE_WRITE_ZEROCOPY) ||
> > > + migrate_multifd_compression() != MULTIFD_COMPRESSION_NONE ||
> > > + migrate_use_tls())) {
> > > +error_setg(,
> > > + "Zerocopy only available for non-compressed non-TLS
> > > multifd migration");
> > > }
> > >
> > > migration_channel_connect(data->s, sioc, data->hostname, err);
> >
> > Do we really want to do this check here? I think this is really too
> > late.
> >
> > You are not patching migrate_params_check().
> >
> > I think that the proper way of doing this is something like:
> >
> > if (params->zerocopy &&
> > (params->parameters.multifd_compression != MULTIFD_COMPRESSION_NONE
> > ||
> > migrate_use_tls())) {
> >error_setg(,
> > "Zerocopy only available for non-compressed non-TLS
> > multifd migration");
> > return false;
> > }
> >
> > You have to do the equivalent of multifd_compression and tls enablement,
> > to see that zerocopy is not enabled, of course.
> >
> > I would prefer to check for QIO_CHANNEL_FEATUR_WRITE_ZEROCPY there, but
> > I can't see a way of doing that without a qio.
>
> I don't think you need to check that feature flag.
>
> The combination of zerocopy and compression is simply illogical
> and can be rejected unconditionally.
Or we could think of "zerocopy" in a more targetted way.
It is only "zerocopy" in terms the final I/O operation.
Earlier parts of the process may involve copies. IOW, we
can copy as part of the compress operation, but skip the
when then sending the compressed page.
In practice though this is still unlikely to be something
we can practically do, as we would need to keep compressed
pages around for an entire migration iteration until we can
call flush to ensure the data has been sent. This would be
significant memory burden.
> The combination of zerocopy and TLS is somewhat questionable.
> You're always going to have a copy between the plain text and
> cipher text. Avoiding an extra copy for the cipher text will
> require kerenl
[PATCH] gitlab-ci/cirrus: Increase timeout to 80 minutes
The jobs on Cirrus-CI sometimes get delayed quite a bit, waiting to be scheduled, so while the build test itself finishes within 60 minutes, the total run time of the jobs can be longer due to this waiting time. Thus let's increase the timeout on the gitlab side a little bit, so that these jobs are not marked as failing just because of the delay. Signed-off-by: Thomas Huth --- .gitlab-ci.d/cirrus.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitlab-ci.d/cirrus.yml b/.gitlab-ci.d/cirrus.yml index e7b25e7427..22d42585e4 100644 --- a/.gitlab-ci.d/cirrus.yml +++ b/.gitlab-ci.d/cirrus.yml @@ -14,6 +14,7 @@ stage: build image: registry.gitlab.com/libvirt/libvirt-ci/cirrus-run:master needs: [] + timeout: 80m allow_failure: true script: - source .gitlab-ci.d/cirrus/$NAME.vars -- 2.27.0
[PULL 5/7] Jobs based on custom runners: add CentOS Stream 8
From: Cleber Rosa This introduces three different parts of a job designed to run on a custom runner managed by Red Hat. The goals include: a) propose a model for other organizations that want to onboard their own runners, with their specific platforms, build configuration and tests. b) bring awareness to the differences between upstream QEMU and the version available under CentOS Stream, which is "A preview of upcoming Red Hat Enterprise Linux minor and major releases". c) because of b), it should be easier to identify and reduce the gap between Red Hat's downstream and upstream QEMU. The components of this custom job are: I) OS build environment setup code: - additions to the existing "build-environment.yml" playbook that can be used to set up CentOS/EL 8 systems. - a CentOS Stream 8 specific "build-environment.yml" playbook that adds to the generic one. II) QEMU build configuration: a script that will produce binaries with features as similar as possible to the ones built and packaged on CentOS stream 8. III) Scripts that define the minimum amount of testing that the binaries built with the given configuration (point II) under the given OS build environment (point I) should be subjected to. IV) Job definition: GitLab CI jobs that will dispatch the build/test jobs (see points #II and #III) to the machine specifically configured according to #I. Signed-off-by: Cleber Rosa Signed-off-by: Alex Bennée Reviewed-by: Willian Rampazzo Tested-by: Willian Rampazzo Message-Id: <2021160501.862396-2-cr...@redhat.com> Message-Id: <2025142915.3797652-6-alex.ben...@linaro.org> diff --git a/docs/devel/ci-jobs.rst.inc b/docs/devel/ci-jobs.rst.inc index 277975e4ad..db3f571d5f 100644 --- a/docs/devel/ci-jobs.rst.inc +++ b/docs/devel/ci-jobs.rst.inc @@ -49,3 +49,10 @@ S390X_RUNNER_AVAILABLE If you've got access to an IBM Z host that can be used as a gitlab-CI runner, you can set this variable to enable the tests that require this kind of host. The runner should be tagged with "s390x". + +CENTOS_STREAM_8_x86_64_RUNNER_AVAILABLE +~~~ +If you've got access to a CentOS Stream 8 x86_64 host that can be +used as a gitlab-CI runner, you can set this variable to enable the +tests that require this kind of host. The runner should be tagged with +both "centos_stream_8" and "x86_64". diff --git a/.gitlab-ci.d/custom-runners.yml b/.gitlab-ci.d/custom-runners.yml index a89a20da48..1f56297dfa 100644 --- a/.gitlab-ci.d/custom-runners.yml +++ b/.gitlab-ci.d/custom-runners.yml @@ -248,3 +248,32 @@ ubuntu-20.04-aarch64-notcg: - ../configure --disable-libssh --disable-tcg - make --output-sync -j`nproc` - make --output-sync -j`nproc` check V=1 + +centos-stream-8-x86_64: + allow_failure: true + needs: [] + stage: build + tags: + - centos_stream_8 + - x86_64 + rules: + - if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH =~ /^staging/' + - if: "$CENTOS_STREAM_8_x86_64_RUNNER_AVAILABLE" + artifacts: + name: "$CI_JOB_NAME-$CI_COMMIT_REF_SLUG" + when: on_failure + expire_in: 7 days + paths: + - build/tests/results/latest/results.xml + - build/tests/results/latest/test-results + reports: + junit: build/tests/results/latest/results.xml + before_script: + - JOBS=$(expr $(nproc) + 1) + script: + - mkdir build + - cd build + - ../scripts/ci/org.centos/stream/8/x86_64/configure + - make -j"$JOBS" + - make NINJA=":" check + - ../scripts/ci/org.centos/stream/8/x86_64/test-avocado diff --git a/scripts/ci/org.centos/stream/8/build-environment.yml b/scripts/ci/org.centos/stream/8/build-environment.yml new file mode 100644 index 00..42b0471634 --- /dev/null +++ b/scripts/ci/org.centos/stream/8/build-environment.yml @@ -0,0 +1,51 @@ +--- +- name: Installation of extra packages to build QEMU + hosts: all + tasks: +- name: Extra check for CentOS Stream 8 + lineinfile: +path: /etc/redhat-release +line: CentOS Stream release 8 +state: present + check_mode: yes + register: centos_stream_8 + +- name: Enable PowerTools repo on CentOS Stream 8 + ini_file: +path: /etc/yum.repos.d/CentOS-Stream-PowerTools.repo +section: powertools +option: enabled +value: "1" + when: +- ansible_facts['distribution'] == 'CentOS' +- ansible_facts['distribution_major_version'] == '8' +- centos_stream_8 + +- name: Install basic packages to build QEMU on CentOS Stream 8 + dnf: +name: + - device-mapper-multipath-devel + - glusterfs-api-devel + - gnutls-devel + - libcap-ng-devel + - libcurl-devel + - libfdt-devel + - libiscsi-devel + - libpmem-devel + - librados-devel + - librbd-devel + - libseccomp-devel + - libssh-devel + - libxkbcommon-devel +
[PATCH] MAINTAINERS: Add section for Aarch64 GitLab custom runner
Add a MAINTAINERS section to cover the GitLab YAML config file containing the jobs run on the custom runner sponsored by the Works On Arm project [*]. [*] https://developer.arm.com/solutions/infrastructure/works-on-arm Suggested-by: Thomas Huth Signed-off-by: Philippe Mathieu-Daudé --- Based-on: <2026162515.4100231-1-alex.ben...@linaro.org> --- MAINTAINERS | 6 ++ 1 file changed, 6 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index d3879aa3c12..f215e50f502 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3511,6 +3511,12 @@ R: Willian Rampazzo S: Odd Fixes F: tests/avocado/ +GitLab custom runner (Works On Arm Sponsored) +M: Alex Bennée +M: Philippe Mathieu-Daudé +S: Maintained +F: .gitlab-ci.d/custom-runners/ubuntu-20.04-aarch64.yml + Documentation - Build system architecture -- 2.31.1
[PULL 6/7] gitlab-ci: Split custom-runners.yml in one file per runner
From: Philippe Mathieu-Daudé To ease maintenance, add the custom-runners/ directory and split custom-runners.yml in 3 files, all included by the current custom-runners.yml: - ubuntu-18.04-s390x.yml - ubuntu-20.04-aarch64.yml - centos-stream-8-x86_64.yml Signed-off-by: Philippe Mathieu-Daudé Signed-off-by: Alex Bennée Message-Id: <2025095608.2436223-1-phi...@redhat.com> Reviewed-by: Willian Rampazzo Message-Id: <2025142915.3797652-7-alex.ben...@linaro.org> diff --git a/.gitlab-ci.d/custom-runners.yml b/.gitlab-ci.d/custom-runners.yml index 1f56297dfa..056c374619 100644 --- a/.gitlab-ci.d/custom-runners.yml +++ b/.gitlab-ci.d/custom-runners.yml @@ -13,267 +13,7 @@ variables: GIT_STRATEGY: clone -# All ubuntu-18.04 jobs should run successfully in an environment -# setup by the scripts/ci/setup/build-environment.yml task -# "Install basic packages to build QEMU on Ubuntu 18.04/20.04" -ubuntu-18.04-s390x-all-linux-static: - needs: [] - stage: build - tags: - - ubuntu_18.04 - - s390x - rules: - - if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH =~ /^staging/' - - if: "$S390X_RUNNER_AVAILABLE" - script: - # --disable-libssh is needed because of https://bugs.launchpad.net/qemu/+bug/1838763 - # --disable-glusterfs is needed because there's no static version of those libs in distro supplied packages - - mkdir build - - cd build - - ../configure --enable-debug --static --disable-system --disable-glusterfs --disable-libssh - - make --output-sync -j`nproc` - - make --output-sync -j`nproc` check V=1 - - make --output-sync -j`nproc` check-tcg V=1 - -ubuntu-18.04-s390x-all: - needs: [] - stage: build - tags: - - ubuntu_18.04 - - s390x - rules: - - if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH =~ /^staging/' - - if: "$S390X_RUNNER_AVAILABLE" - script: - - mkdir build - - cd build - - ../configure --disable-libssh - - make --output-sync -j`nproc` - - make --output-sync -j`nproc` check V=1 - -ubuntu-18.04-s390x-alldbg: - needs: [] - stage: build - tags: - - ubuntu_18.04 - - s390x - rules: - - if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH =~ /^staging/' - when: manual - allow_failure: true - - if: "$S390X_RUNNER_AVAILABLE" - when: manual - allow_failure: true - script: - - mkdir build - - cd build - - ../configure --enable-debug --disable-libssh - - make clean - - make --output-sync -j`nproc` - - make --output-sync -j`nproc` check V=1 - -ubuntu-18.04-s390x-clang: - needs: [] - stage: build - tags: - - ubuntu_18.04 - - s390x - rules: - - if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH =~ /^staging/' - when: manual - allow_failure: true - - if: "$S390X_RUNNER_AVAILABLE" - when: manual - allow_failure: true - script: - - mkdir build - - cd build - - ../configure --disable-libssh --cc=clang --cxx=clang++ --enable-sanitizers - - make --output-sync -j`nproc` - - make --output-sync -j`nproc` check V=1 - -ubuntu-18.04-s390x-tci: - needs: [] - stage: build - tags: - - ubuntu_18.04 - - s390x - rules: - - if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH =~ /^staging/' - when: manual - allow_failure: true - - if: "$S390X_RUNNER_AVAILABLE" - when: manual - allow_failure: true - script: - - mkdir build - - cd build - - ../configure --disable-libssh --enable-tcg-interpreter - - make --output-sync -j`nproc` - -ubuntu-18.04-s390x-notcg: - needs: [] - stage: build - tags: - - ubuntu_18.04 - - s390x - rules: - - if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH =~ /^staging/' - when: manual - allow_failure: true - - if: "$S390X_RUNNER_AVAILABLE" - when: manual - allow_failure: true - script: - - mkdir build - - cd build - - ../configure --disable-libssh --disable-tcg - - make --output-sync -j`nproc` - - make --output-sync -j`nproc` check V=1 - -# All ubuntu-20.04 jobs should run successfully in an environment -# setup by the scripts/ci/setup/qemu/build-environment.yml task -# "Install basic packages to build QEMU on Ubuntu 18.04/20.04" -ubuntu-20.04-aarch64-all-linux-static: - needs: [] - stage: build - tags: - - ubuntu_20.04 - - aarch64 - rules: - - if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH =~ /^staging/' - - if: "$AARCH64_RUNNER_AVAILABLE" - script: - # --disable-libssh is needed because of https://bugs.launchpad.net/qemu/+bug/1838763 - # --disable-glusterfs is needed because there's no static version of those libs in distro supplied packages - - mkdir build - - cd build - - ../configure --enable-debug --static --disable-system --disable-glusterfs --disable-libssh - - make --output-sync -j`nproc` - - make --output-sync -j`nproc` check V=1 - - make --output-sync -j`nproc` check-tcg V=1 - -ubuntu-20.04-aarch64-all: - needs: [] - stage: build - tags: - - ubuntu_20.04 - - aarch64 - rules: - - if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH =~ /^staging/' - when: manual - allow_failure: true - - if: "$AARCH64_RUNNER_AVAILABLE"
[PULL 4/7] meson: remove useless libdl test
From: Paolo Bonzini
dlopen is never used after it is sought via cc.find_library, because
plugins use gmodule instead; remove the test.
Signed-off-by: Paolo Bonzini
Reviewed-by: Thomas Huth
Message-Id: <2020092454.30916-1-pbonz...@redhat.com>
Signed-off-by: Alex Bennée
Reviewed-by: Richard Henderson
Message-Id: <2025142915.3797652-5-alex.ben...@linaro.org>
diff --git a/meson.build b/meson.build
index 2ece4fe088..baeaee4522 100644
--- a/meson.build
+++ b/meson.build
@@ -566,13 +566,7 @@ endif
spice_headers = spice.partial_dependency(compile_args: true, includes: true)
rt = cc.find_library('rt', required: false)
-libdl = not_found
-if 'CONFIG_PLUGIN' in config_host
- libdl = cc.find_library('dl', required: false)
- if not cc.has_function('dlopen', dependencies: libdl)
-error('dlopen not found')
- endif
-endif
+
libiscsi = not_found
if not get_option('libiscsi').auto() or have_block
libiscsi = dependency('libiscsi', version: '>=1.9.0',
diff --git a/accel/tcg/meson.build b/accel/tcg/meson.build
index 137a1a44cc..7a0a79d731 100644
--- a/accel/tcg/meson.build
+++ b/accel/tcg/meson.build
@@ -10,7 +10,7 @@ tcg_ss.add(files(
))
tcg_ss.add(when: 'CONFIG_USER_ONLY', if_true: files('user-exec.c'))
tcg_ss.add(when: 'CONFIG_SOFTMMU', if_false: files('user-exec-stub.c'))
-tcg_ss.add(when: 'CONFIG_PLUGIN', if_true: [files('plugin-gen.c'), libdl])
+tcg_ss.add(when: 'CONFIG_PLUGIN', if_true: [files('plugin-gen.c')])
specific_ss.add_all(when: 'CONFIG_TCG', if_true: tcg_ss)
specific_ss.add(when: ['CONFIG_SOFTMMU', 'CONFIG_TCG'], if_true: files(
--
2.30.2
