[linux-linus test] 180172: tolerable trouble: fail/pass/starved - PUSHED
flight 180172 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/180172/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 180159 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 180159 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180159 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180159 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 180159 test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-examine 1 build-check(1) starved n/a test-armhf-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) starved n/a test-armhf-armhf-libvirt-raw 1 build-check(1) starved n/a test-armhf-armhf-xl 1 build-check(1) starved n/a test-armhf-armhf-xl-credit1 1 build-check(1) starved n/a test-armhf-armhf-xl-credit2 1 build-check(1) starved n/a test-armhf-armhf-xl-cubietruck 1 build-check(1) starved n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) starved n/a test-armhf-armhf-xl-rtds 1 build-check(1) starved n/a test-armhf-armhf-xl-vhd 1 build-check(1) starved n/a build-armhf-libvirt 1 build-check(1) starved n/a build-armhf 2 hosts-allocate starved n/a version targeted for testing: linuxf2afccfefe7be1f7346564fe619277110d341f9b baseline version: linux99ddf2254febae9eab7fb0bcc02c5322243f5c49 Last test of basis 180159 2023-04-05 18:14:53 Z1 days Testing same since 180172 2023-04-06 19:10:15 Z0 days1 attempts People who touched revisions under test: Amit Pundir Andrea Righi Andy Chi Andy Roulin Anh Tuan Phan Armin Wolf Arnd Bergmann Arseniy Krasnov Bard Liao Ben Greear Benjamin Asbach Bobby Eshleman Boris Brezillon chowtom Christian Brauner Christian König Christian König Corinna Vinschen Daniel Golle Daniel Vetter Daniele Ceraolo Spurio David S. Miller Duy Nguyen Eric Dumazet Eugene Huang Felix Fietkau Frank Wunderlich Ge-org Brohammer Greg Ungerer Guennadi Liakhovetski Guenter Roeck Guillaume Nault Gustav Ekelund Hangbin Liu Hans de Goede Jacek Lawrynowicz Jakub Kicinski Jani Nikula Jason Gunthorpe Jason Montleon Jeremy Soller Jiri Slaby (SUSE) Johannes Berg Junfeng Guo Kalle Valo Kalle Valo Karol Herbst Karol Wachowski Khanh Le Kuninori Morimoto Kuniyuki Iwashima Lai Peter Jun Ann Lingyu Liu Linus Torvalds Lorenzo Bianconi Marc Kleine-Budde Marek Szyprowski Mario Limonciello Mark Brown Mark Pearson Martin Blumenstingl Matthew Auld Michael Sit Wei Hong Michal Sojka Min Li Mirsad Goran Todorovac Oleksij Rempel Oliver Hartkopp Paolo Abeni Pengfei Xu Peter Ujfalusi Pierre-Louis Bossart Rafal Romanowski Ram Kumar Dharuman Ranjani
[qemu-mainline test] 180168: tolerable trouble: fail/pass/starved - PUSHED
flight 180168 qemu-mainline real [real] flight 180174 qemu-mainline real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/180168/ http://logs.test-lab.xenproject.org/osstest/logs/180174/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-i386-pair10 xen-install/src_host fail pass in 180174-retest test-amd64-i386-libvirt-raw 19 guest-start/debian.repeat fail pass in 180174-retest Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 180153 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180153 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 180153 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180153 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 180153 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) starved n/a test-armhf-armhf-libvirt-raw 1 build-check(1) starved n/a test-armhf-armhf-xl 1 build-check(1) starved n/a test-armhf-armhf-xl-credit2 1 build-check(1) starved n/a test-armhf-armhf-xl-cubietruck 1 build-check(1) starved n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) starved n/a test-armhf-armhf-xl-rtds 1 build-check(1) starved n/a test-armhf-armhf-xl-vhd 1 build-check(1) starved n/a build-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-xl-credit1 1 build-check(1) starved n/a build-armhf 2 hosts-allocate starved n/a version targeted for testing: qemuuc6f3cbca32bde9ee94d9949aa63e8a7ef2d7bc5b baseline version: qemuub5fba99ec7969054ab2f3727d2df014b5b72e4f1 Last test of basis 180153 2023-04-05 12:38:48 Z1 days Testing same since 180168 2023-04-06 10:37:17 Z0 days1 attempts People who touched revisions under test: Michael S. Tsirkin Peter Maydell jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf starved build-i386 pass build-amd64-libvirt pass build-arm64-libvirt pass build-armhf-libvirt
Re: [PATCH 9/9] x86emul+VMX: support {RD,WR}MSRLIST
On 04/04/2023 3:55 pm, Jan Beulich wrote:
> These are "compound" instructions to issue a series of RDMSR / WRMSR
> respectively. In the emulator we can therefore implement them by using
> the existing msr_{read,write}() hooks. The memory accesses utilize that
> the HVM ->read() / ->write() hooks are already linear-address
> (x86_seg_none) aware (by way of hvmemul_virtual_to_linear() handling
> this case).
>
> Signed-off-by: Jan Beulich
> ---
> TODO: Use VMX tertiary execution control (once bit is known; see
> //todo-s) and then further adjust cpufeatureset.h.
>
> RFC: In vmx_vmexit_handler() handling is forwarded to the emulator
> blindly. Alternatively we could consult the exit qualification and
> process just a single MSR at a time (without involving the
> emulator), exiting back to the guest after every iteration. (I
> don't think a mix of both models makes a lot of sense.)
{RD,WR}MSRLIST are supposed to be used for context switch paths. They
really shouldn't be exiting in the common case.
What matters here is the conditional probability of a second MSR being
intercepted too, given that one already has been. And I don't have a
clue how to answer this.
I would not expect Introspection to be intercepting a fastpath MSR.
(And if it does, frankly it can live with the consequences.)
For future scenarios such as reloading PMU/LBR/whatever, these will be
all-or-nothing and we'd expect to have them eagerly in context anyway.
If I were going to guess, I'd say that probably MSR_XSS or MSR_SPEC_CTRL
will be giving us the most grief here, because they're both ones that
are liable to be touched on a context switch path, and have split
host/guest bits.
> RFC: For PV priv_op_ops would need to gain proper read/write hooks,
> which doesn't look desirable (albeit there we could refuse to
> handle anything else than x86_seg_none); we may want to consider to
> instead not support the feature for PV guests, requiring e.g. Linux
> to process the lists in new pvops hooks.
Ah - funny you should ask. See patch 2. These are even better reasons
not to support on PV guests.
> RFC: I wasn't sure whether to add preemption checks to the loops -
> thoughts?
I'd be tempted to. Mostly because a guest can exert 64* longest MSR
worth of pressure here, along with associated emulation overhead.
64* "write hypercall page" sounds expensive. So too does 64* MSR_PAT,
given all the EPT actions behind the scenes.
Its probably one of those
> With the VMX side of the spec still unclear (tertiary execution control
> bit unspecified in ISE 046) we can't enable the insn yet for (HVM) guest
> use. The precise behavior of MSR_BARRIER is also not spelled out, so the
> (minimal) implementation is a guess for now.
MSRs are expensive for several reasons. The %ecx decode alone isn't
cheap, nor is the reserved bit checking, and that's before starting the
action itself.
What the list form gets you is the ability to pipeline the checks on the
following MSR while performing the action of the current one. I suspect
there are plans to try and parallelise the actions too if possible.
As I understand it, BARRIER just halts the piplineing of processing the
next MSR.
~Andrew
Re: [PATCH 8/9] x86emul: support AVX-NE-CONVERT insns
On 04/04/2023 3:54 pm, Jan Beulich wrote:
> Matching what was done earlier, explicit tests are added only for
> irregular insn / memory access patterns.
>
> Signed-off-by: Jan Beulich
Reviewed-by: Andrew Cooper , with two minor
requests.
> --- a/tools/misc/xen-cpuid.c
> +++ b/tools/misc/xen-cpuid.c
> @@ -214,7 +214,7 @@ static const char *const str_7c1[32] =
>
> static const char *const str_7d1[32] =
> {
> -[ 4] = "avx-vnni-int8",
> +[ 4] = "avx-vnni-int8", [ 5] = "avx-ne-convert",
I'd leave a bit more horizontal space. These names are getting rather
long, and we're only 10% into this word.
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -6208,6 +6208,19 @@ x86_emulate(
> host_and_vcpu_must_have(avx512_vbmi2);
> goto avx512f_no_sae;
>
> +case X86EMUL_OPC_VEX (0x0f38, 0xb0): /* vcvtneoph2ps mem,[xy]mm */
> +case X86EMUL_OPC_VEX_66(0x0f38, 0xb0): /* vcvtneeph2ps mem,[xy]mm */
> +case X86EMUL_OPC_VEX_F3(0x0f38, 0xb0): /* vcvtneebf162ps mem,[xy]mm */
> +case X86EMUL_OPC_VEX_F2(0x0f38, 0xb0): /* vcvtneobf162ps mem,[xy]mm */
> +generate_exception_if(ea.type != OP_MEM, EXC_UD);
> +/* fall through */
Only just occurred to me, but we should probably be using fallthrough;
in new code, now there's a real attribute to use.
~Andrew
Re: [PATCH v9 4/5] xen/arm: switch ARM to use generic implementation of bug.h
On Wed, 5 Apr 2023, Julien Grall wrote:
> On 03/04/2023 00:15, Stefano Stabellini wrote:
> > On Fri, 31 Mar 2023, Julien Grall wrote:
> > > Hi Oleksii,
> > >
> > > I was going to ack the patch but then I spotted something that would want
> > > some
> > > clarification.
> > >
> > > On 29/03/2023 11:50, Oleksii Kurochko wrote:
> > > > diff --git a/xen/arch/arm/include/asm/bug.h
> > > > b/xen/arch/arm/include/asm/bug.h
> > > > index cacaf014ab..3fb0471a9b 100644
> > > > --- a/xen/arch/arm/include/asm/bug.h
> > > > +++ b/xen/arch/arm/include/asm/bug.h
> > > > @@ -1,6 +1,24 @@
> > > >#ifndef __ARM_BUG_H__
> > > >#define __ARM_BUG_H__
> > > >+/*
> > > > + * Please do not include in the header any header that might
> > > > + * use BUG/ASSERT/etc maros asthey will be defined later after
> > > > + * the return to from the current header:
> > > > + *
> > > > + * :
> > > > + * ...
> > > > + * :
> > > > + * ...
> > > > + *
> > > > + * ...
> > > > + * ...
> > > > + * #define BUG() ...
> > > > + * ...
> > > > + * #define ASSERT() ...
> > > > + * ...
> > > > + */
> > > > +
> > > >#include
> > > > #if defined(CONFIG_ARM_32)
> > > > @@ -11,76 +29,7 @@
> > > ># error "unknown ARM variant"
> > > >#endif
> > > >-#define BUG_FRAME_STRUCT
> > > > -
> > > > -struct bug_frame {
> > > > -signed int loc_disp;/* Relative address to the bug address */
> > > > -signed int file_disp; /* Relative address to the filename */
> > > > -signed int msg_disp;/* Relative address to the predicate (for
> > > > ASSERT) */
> > > > -uint16_t line; /* Line number */
> > > > -uint32_t pad0:16; /* Padding for 8-bytes align */
> > > > -};
> > > > -
> > > > -#define bug_loc(b) ((const void *)(b) + (b)->loc_disp)
> > > > -#define bug_file(b) ((const void *)(b) + (b)->file_disp);
> > > > -#define bug_line(b) ((b)->line)
> > > > -#define bug_msg(b) ((const char *)(b) + (b)->msg_disp)
> > > > -
> > > > -/* Many versions of GCC doesn't support the asm %c parameter which
> > > > would
> > > > - * be preferable to this unpleasantness. We use mergeable string
> > > > - * sections to avoid multiple copies of the string appearing in the
> > > > - * Xen image. BUGFRAME_run_fn needs to be handled separately.
> > > > - */
> > >
> > > Given this comment ...
> > >
> > > > -#define BUG_FRAME(type, line, file, has_msg, msg) do {
> > > > \
> > > > -BUILD_BUG_ON((line) >> 16);
> > > > \
> > > > -BUILD_BUG_ON((type) >= BUGFRAME_NR);
> > > > \
> > > > -asm ("1:"BUG_INSTR"\n"
> > > > \
> > > > - ".pushsection .rodata.str, \"aMS\", %progbits, 1\n"
> > > > \
> > > > - "2:\t.asciz " __stringify(file) "\n"
> > > > \
> > > > - "3:\n"
> > > > \
> > > > - ".if " #has_msg "\n"
> > > > \
> > > > - "\t.asciz " #msg "\n"
> > > > \
> > > > - ".endif\n"
> > > > \
> > > > - ".popsection\n"
> > > > \
> > > > - ".pushsection .bug_frames." __stringify(type) ", \"a\",
> > > > %progbits\n"\
> > > > - "4:\n"
> > > > \
> > > > - ".p2align 2\n"
> > > > \
> > > > - ".long (1b - 4b)\n"
> > > > \
> > > > - ".long (2b - 4b)\n"
> > > > \
> > > > - ".long (3b - 4b)\n"
> > > > \
> > > > - ".hword " __stringify(line) ", 0\n"
> > > > \
> > > > - ".popsection");
> > > > \
> > > > -} while (0)
> > > > -
> > > > -/*
> > > > - * GCC will not allow to use "i" when PIE is enabled (Xen doesn't set
> > > > the
> > > > - * flag but instead rely on the default value from the compiler). So
> > > > the
> > > > - * easiest way to implement run_in_exception_handler() is to pass the
> > > > to
> > > > - * be called function in a fixed register.
> > > > - */
> > > > -#define run_in_exception_handler(fn) do {
> > > > \
> > > > -asm ("mov " __stringify(BUG_FN_REG) ", %0\n"
> > > > \
> > > > - "1:"BUG_INSTR"\n"
> > > > \
> > > > - ".pushsection .bug_frames." __stringify(BUGFRAME_run_fn) ","
> > > > \
> > > > - " \"a\", %%progbits\n"
> > > > \
> > > > - "2:\n"
> > > > \
> > > > - ".p2align 2\n"
> > > > \
> > > > - ".long (1b - 2b)\n"
> > > > \
> > > > - ".long 0, 0, 0\n"
> > > > \
> > > > - ".popsection" :: "r" (fn) : __stringify(BUG_FN_REG) );
> > > > \
> > > > -} while (0)
> > > > -
> > > > -#define WARN() BUG_FRAME(BUGFRAME_warn, __LINE__, __FILE__, 0, "")
> > > > -
> > > > -#define BUG() do { \
> > > > -BUG_FRAME(BUGFRAME_bug, __LINE__, __FILE__, 0, "");\
> > > > -unreachable(); \
> > > > -} while (0)
> > > > -
> > > > -#define assert_failed(msg) do { \
> > > > -BUG_FRAME(BUGFRAME_assert, __LINE__, __FILE__, 1, msg); \
> > > > -unreachable(); \
> > > > -} while (0)
> > > > +#define BUG_ASM_CONST "c"
> > >
> > >
Re: [PATCH 7/9] x86emul: support AVX-VNNI-INT8
On 04/04/2023 3:54 pm, Jan Beulich wrote: > These are close relatives of the AVX-VNNI ISA extension. Since the insns > here and in particular their memory access patterns follow the usual > scheme (and especially the byte variants of AVX-VNNI), I didn't think it > was necessary to add a contrived test specifically for them. > > While making the addition also re-wire AVX-VNNI's handling to > simd_0f_ymm: There's no reason to check the AVX feature alongside the > one actually of interest (there are a few features where two checks are > actually necessary, e.g. GFNI+AVX, but this isn't the case here). > > Signed-off-by: Jan Beulich Reviewed-by: Andrew Cooper
[xen-unstable-smoke test] 180171: tolerable trouble: pass/starved - PUSHED
flight 180171 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/180171/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 1 build-check(1) starved n/a build-armhf 2 hosts-allocate starved n/a version targeted for testing: xen ddaf7bb0cfd27369252de52e4b03410c4065bad2 baseline version: xen a5087069a8c40541ba81fa0e2850471c949932b3 Last test of basis 180169 2023-04-06 14:03:37 Z0 days Testing same since 180171 2023-04-06 19:00:24 Z0 days1 attempts People who touched revisions under test: Andrew Cooper jobs: build-arm64-xsm pass build-amd64 pass build-armhf starved build-amd64-libvirt pass test-armhf-armhf-xl starved test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git a5087069a8..ddaf7bb0cf ddaf7bb0cfd27369252de52e4b03410c4065bad2 -> smoke
Re: [PATCH 6/9] x86emul: support AVX-IFMA insns
On 04/04/2023 3:53 pm, Jan Beulich wrote: > As in a few cases before (in particular: AVX512_IFMA), since the insns > here and in particular their memory access patterns follow the usual > scheme, I didn't think it was necessary to add a contrived test > specifically for them. > > Signed-off-by: Jan Beulich Reviewed-by: Andrew Cooper
Re: [PATCH 5/9] x86emul: re-use new stub_exn field in state structure
On 04/04/2023 3:53 pm, Jan Beulich wrote: > This can now also be used to reduce the number of parameters > x86emul_fpu() needs to take. > > Signed-off-by: Jan Beulich As said in the previous patch, I think this patch wants reordering forwards and picking up the addition into state. "Because we're going to need it in another hook, and it simplifies an existing one" is a perfectly fine justification in isolation. With that done, Acked-by: Andrew Cooper , although... > --- > We could of course set the struct field once early in x86_emulate(), but > for now I think we're better off leaving it as NULL where not actually > needed. Do we gain anything useful from not doing it once? it's certainly more to remember, and more code overall, to assign when needed. > > --- a/xen/arch/x86/x86_emulate/fpu.c > +++ b/xen/arch/x86/x86_emulate/fpu.c > @@ -90,9 +90,8 @@ int x86emul_fpu(struct x86_emulate_state > unsigned int *insn_bytes, > enum x86_emulate_fpu_type *fpu_type, > #define fpu_type (*fpu_type) /* for get_fpu() */ > -struct stub_exn *stub_exn, > -#define stub_exn (*stub_exn) /* for invoke_stub() */ > mmval_t *mmvalp) > +#define stub_exn (*s->stub_exn) /* for invoke_stub() */ ... honestly, I'd really like to see these macros purged. I know the general style was done like this to avoid code churn, but hiding indirection is a very rude thing to do, and none of these are usefully shortening the expressions they replace. Also, putting stub_exn in the K type space is still weird to read. ~Andrew
[xen-unstable test] 180163: tolerable trouble: fail/pass/starved - PUSHED
flight 180163 xen-unstable real [real] flight 180170 xen-unstable real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/180163/ http://logs.test-lab.xenproject.org/osstest/logs/180170/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-i386-qemuu-rhel6hvm-amd 7 xen-install fail pass in 180170-retest test-amd64-amd64-xl-qemuu-win7-amd64 12 windows-install fail pass in 180170-retest Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stop fail in 180170 like 180148 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 180148 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 180148 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180148 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180148 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 180148 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 180148 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 180148 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 180148 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass build-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-examine 1 build-check(1) starved n/a test-armhf-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) starved n/a test-armhf-armhf-libvirt-raw 1 build-check(1) starved n/a test-armhf-armhf-xl 1 build-check(1) starved n/a test-armhf-armhf-xl-credit1 1 build-check(1) starved n/a test-armhf-armhf-xl-credit2 1 build-check(1) starved n/a test-armhf-armhf-xl-cubietruck 1 build-check(1) starved n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) starved n/a test-armhf-armhf-xl-rtds 1 build-check(1) starved n/a test-armhf-armhf-xl-vhd 1 build-check(1) starved n/a build-armhf 2 hosts-allocate starved n/a version targeted for testing: xen 881ba20eb0222305a9d2cd090c9345992794f4f5 baseline version: xen 415f7d9404171cbc968b1ea22e7d3523ac2f3fc1 Last test of basis 180148 2023-04-05 06:43:52 Z1 days Testing same since 180163 2023-04-05 23:38:37 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Jan Beulich Roger Pau Monné jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64-xtf pass
Re: [PATCH 4/9] x86emul: support CMPccXADD
On 04/04/2023 3:52 pm, Jan Beulich wrote: > Unconditionally wire this through the ->rmw() hook. Since x86_emul_rmw() > now wants to construct and invoke a stub, make stub_exn available to it > via a new field in the emulator state structure. IMO, patch 5 should be re-ordered with this, because it removes one incidental change that's not really related to CMPccXADD. > > Signed-off-by: Jan Beulich > --- > # SDE: -grr or -srf The ISE makes a point of noting that CMPccXADD is implicitly locked, like XCHG. (Unlike XCHG, there isn't a valid reg/reg encoding.) Right now, the xchg emulation overrides lock_prefix, but I have a feeling that's stale now with the rmw() hook in place. But it is dubious that we let xchg fall back to a non-atomic exchange if the rmw() hook is missing. Either way, I think it would be nice to clean that up so we don't have differences in the handling of instructions which the ISE at least claims are similar. Tangentially, what about the RAO instructions? > --- a/tools/tests/x86_emulator/x86-emulate.h > +++ b/tools/tests/x86_emulator/x86-emulate.h > @@ -934,6 +935,8 @@ decode_0f38(struct x86_emulate_state *s, > ctxt->opcode |= MASK_INSR(s->vex.pfx, X86EMUL_OPC_PFX_MASK); > break; > > +case X86EMUL_OPC_VEX_66(0, 0xe0) > + ... X86EMUL_OPC_VEX_66(0, 0xef): /* cmpxadd */ I know the style is a little mixed in the emulator, but + case X86EMUL_OPC_VEX_66(0, 0xe0) ... + X86EMUL_OPC_VEX_66(0, 0xef): /* cmpxadd */ is more consistent with Xen style (because it's somewhat of a binary operator), and more readable IMO. > --- a/xen/include/public/arch-x86/cpufeatureset.h > +++ b/xen/include/public/arch-x86/cpufeatureset.h > @@ -278,6 +278,7 @@ XEN_CPUFEATURE(SSBD, 9*32+31) / > /* Intel-defined CPU features, CPUID level 0x0007:1.eax, word 10 */ > XEN_CPUFEATURE(AVX_VNNI, 10*32+ 4) /*A AVX-VNNI Instructions */ > XEN_CPUFEATURE(AVX512_BF16, 10*32+ 5) /*A AVX512 BFloat16 Instructions */ > +XEN_CPUFEATURE(CMPCCXADD,10*32+ 7) /*A CMPccXADD Instructions */ Given the non-triviality of this instruction, I'd prefer to keep this "a" until we've tried it on real hardware. ~Andrew
[xen-unstable-smoke test] 180169: tolerable trouble: pass/starved - PUSHED
flight 180169 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/180169/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 1 build-check(1) starved n/a build-armhf 2 hosts-allocate starved n/a version targeted for testing: xen a5087069a8c40541ba81fa0e2850471c949932b3 baseline version: xen 881ba20eb0222305a9d2cd090c9345992794f4f5 Last test of basis 180160 2023-04-05 20:01:54 Z0 days Testing same since 180169 2023-04-06 14:03:37 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Jan Beulich jobs: build-arm64-xsm pass build-amd64 pass build-armhf starved build-amd64-libvirt pass test-armhf-armhf-xl starved test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git 881ba20eb0..a5087069a8 a5087069a8c40541ba81fa0e2850471c949932b3 -> smoke
Re: [PATCH] x86emul: avoid triggering event related assertions
On 06.04.2023 17:33, Andrew Cooper wrote:
> On 06/04/2023 3:15 pm, Jan Beulich wrote:
>> The assertion at the end of x86_emulate_wrapper() as well as the ones
>> in x86_emul_{hw_exception,pagefault}() can trigger if we ignore
>> X86EMUL_EXCEPTION coming back from certain hook functions.
>
> Looking at the comment I wrote back then, I don't think I'd considered
> this case.
>
> What I was fixing at the time was the case where
> hvm_inject_hw_exception() had raised an exception behind the back of the
> emulator, and any subsequent exception escalates to #DF.
>
> I guess the comment wants updating to discuss this problem too, where
> the hook queued an exception but we didn't squash it properly when
> deciding to ignore it.
Can do.
> As it's debugging-only anyway, it might be worth rearranging the
> expression to be
>
> if ( ctxt->event_pending )
> ASSERT(rc == X86EMUL_EXCEPTION);
> else
> ASSERT(rc != X86EMUL_EXCEPTION);
>
> because it distinguishes the two cases without an intermediate round of
> debugging.
Maybe, but I don't think this adjustment would belong here.
>> ---
>> EFER reads won't fault in any of the handlers we have, so in principle
>> the respective check could be omitted (and hence has no Fixes: tag).
>> Thoughts?
>
> We already require LMA as input state, and don't have an execution model
> where EFER is actually absent in the first place, so passing the whole
> thing wouldn't really be an issue.
>
> I have previously considered doing the same for CR0 too, but at best
> it's marginal so I haven't got around to trying it.
Well, that's more longer term plans (and not very clear as you say). I'm
afraid this doesn't answer my question, though: Do you think the respective
adjustment should stay, or be dropped?
>> --- a/xen/arch/x86/x86_emulate/0fae.c
>> +++ b/xen/arch/x86/x86_emulate/0fae.c
>> @@ -67,7 +67,10 @@ int x86emul_0fae(struct x86_emulate_stat
>> cr4 = X86_CR4_OSFXSR;
>> if ( !ops->read_msr ||
>> ops->read_msr(MSR_EFER, _val, ctxt) !=
>> X86EMUL_OKAY )
>> +{
>> +x86_emul_reset_event(ctxt);
>
> This is the only path you've introduced that doesn't restrict the reset
> to the X86EMUL_EXCEPTION case.
Right, other similar ones had to go into individual other patches I have
pending. The distinction I made was whether the call was in a helper
function (where I want to be careful, because I don't know what state we
may have accumulated) vs mainline code. IOW ...
> In principle you can get things like RETRY for introspection.
> Internally, UNHANDLEABLE is used but I hope that never escapes from
> guest_{rd,wr}msr().
... other errors are possible, yes, but those shouldn't cause an event
to be recorded. Hence it seemed reasonable to me to flush events
unconditionally here, i.e. even if none is pending in the first place.
Jan
Re: [PATCH 3/9] x86emul: drop regs field from emulator state structure
On 04/04/2023 3:51 pm, Jan Beulich wrote:
> For an unclear reason 0552a8cfda43 ("x86emul: track only rIP in emulator
> state") converted the original struct cpu_user_regs instance to a
> pointer, rather than dropping the field altogether: The pointer merely
> aliases the one in the context structure.
>
> Signed-off-by: Jan Beulich
Acked-by: Andrew Cooper
Re: [PATCH] x86emul: avoid triggering event related assertions
On 06/04/2023 3:15 pm, Jan Beulich wrote:
> The assertion at the end of x86_emulate_wrapper() as well as the ones
> in x86_emul_{hw_exception,pagefault}() can trigger if we ignore
> X86EMUL_EXCEPTION coming back from certain hook functions.
Looking at the comment I wrote back then, I don't think I'd considered
this case.
What I was fixing at the time was the case where
hvm_inject_hw_exception() had raised an exception behind the back of the
emulator, and any subsequent exception escalates to #DF.
I guess the comment wants updating to discuss this problem too, where
the hook queued an exception but we didn't squash it properly when
deciding to ignore it.
As it's debugging-only anyway, it might be worth rearranging the
expression to be
if ( ctxt->event_pending )
ASSERT(rc == X86EMUL_EXCEPTION);
else
ASSERT(rc != X86EMUL_EXCEPTION);
because it distinguishes the two cases without an intermediate round of
debugging.
> Squash
> exceptions when merely probing MSRs, plus on SWAPGS'es "best effort"
> error handling path.
>
> In adjust_bnd() add another assertion after the read_xcr(0, ...)
> invocation, paralleling the one in x86emul_get_fpu() - XCR0 reads should
> never fault when XSAVE is (implicitly) known to be available.
>
> Fixes: 14a6be89ec04 ("x86emul: correct EFLAGS.TF handling")
> Fixes: cb2626c75813 ("x86emul: conditionally clear BNDn for branches")
> Fixes: 6eb43fcf8a0b ("x86emul: support SWAPGS")
> Reported-by: AFL
> Signed-off-by: Jan Beulich
> ---
> EFER reads won't fault in any of the handlers we have, so in principle
> the respective check could be omitted (and hence has no Fixes: tag).
> Thoughts?
We already require LMA as input state, and don't have an execution model
where EFER is actually absent in the first place, so passing the whole
thing wouldn't really be an issue.
I have previously considered doing the same for CR0 too, but at best
it's marginal so I haven't got around to trying it.
> --- a/xen/arch/x86/x86_emulate/0fae.c
> +++ b/xen/arch/x86/x86_emulate/0fae.c
> @@ -67,7 +67,10 @@ int x86emul_0fae(struct x86_emulate_stat
> cr4 = X86_CR4_OSFXSR;
> if ( !ops->read_msr ||
> ops->read_msr(MSR_EFER, _val, ctxt) != X86EMUL_OKAY
> )
> +{
> +x86_emul_reset_event(ctxt);
This is the only path you've introduced that doesn't restrict the reset
to the X86EMUL_EXCEPTION case.
In principle you can get things like RETRY for introspection.
Internally, UNHANDLEABLE is used but I hope that never escapes from
guest_{rd,wr}msr().
~Andrew
Re: [PULL 14/27] hw/xen: Move xenstore_store_pv_console_info to xen_console.c
On Tue, 7 Mar 2023 at 18:28, David Woodhouse wrote:
>
> From: David Woodhouse
>
> There's no need for this to be in the Xen accel code, and as we want to
> use the Xen console support with KVM-emulated Xen we'll want to have a
> platform-agnostic version of it. Make it use GString to build up the
> path while we're at it.
>
> Signed-off-by: David Woodhouse
> Reviewed-by: Paul Durrant
Hi; Coverity points out a double-free here (CID 1508254):
> +static int store_con_info(struct XenConsole *con)
> +{
> +Chardev *cs = qemu_chr_fe_get_driver(>chr);
> +char *pts = NULL;
> +char *dom_path;
> +GString *path;
> +int ret = -1;
> +
> +/* Only continue if we're talking to a pty. */
> +if (!CHARDEV_IS_PTY(cs)) {
> +return 0;
> +}
> +pts = cs->filename + 4;
> +
> +dom_path = qemu_xen_xs_get_domain_path(xenstore, xen_domid);
> +if (!dom_path) {
> +return 0;
> +}
> +
> +path = g_string_new(dom_path);
> +free(dom_path);
> +
> +if (con->xendev.dev) {
> +g_string_append_printf(path, "/device/console/%d", con->xendev.dev);
> +} else {
> +g_string_append(path, "/console");
> +}
> +g_string_append(path, "/tty");
> +
> +if (xenstore_write_str(con->console, path->str, pts)) {
> +fprintf(stderr, "xenstore_write_str for '%s' fail", path->str);
> +goto out;
> +}
> +ret = 0;
> +
> +out:
> +g_string_free(path, true);
> +free(path);
g_string_free frees the GString, but then we call free() on it
as well. Presumably the free() should just be deleted ?
> +
> +return ret;
> +}
thanks
-- PMM
Re: [PATCH] tools/xendomains: Only save/restore/migrate if supported by xenlight
On 04/04/2023 17:28, Anthony PERARD wrote: On Wed, Mar 22, 2023 at 01:58:00PM +, Peter Hoyes wrote: From: Peter Hoyes Saving, restoring and migrating domains are not currently supported on arm and arm64 platforms, so xendomains prints the warning: An error occurred while saving domain: command not implemented when attempting to run `xendomains stop`. It otherwise continues to shut down the domains cleanly, with the unsupported steps skipped. The patch looks kind of ok, but shouldn't $XENDOMAINS_SAVE be set to an empty string in the config by the admin instead? Or is the issue that $XENDOMAINS_SAVE is set by default, even on arm* ? Yea the default is the issue. We are building for embedded, using Yocto, so there isn't really an admin. Maybe it's easier to check that the command is implemented at run time rather than trying to have a good default value for XENDOMAINS_SAVE at install/package time. It would be cleaner to do this at build time for sure, but I'm not sure the autotools config file approach for sysconfig.xendomains.in can handle the logic for this? Thanks, Peter
Re: [PATCH] x86/svm: Provide EXITINFO decodes for Exceptions/NPT intercepts
On 06/04/2023 3:28 pm, Jan Beulich wrote:
> On 06.04.2023 15:27, Andrew Cooper wrote:
>> Exceptions and NPT intercepts almost have the same layout, but NPT has bits
>> above 31 in the error code, and the name for exitinfo2 really does want
>> distinguishing between cr2 and gpa.
>>
>> In nsvm_vcpu_vmexit_inject() rearrange VMEXIT_NPF to fall through instead of
>> repeating the exitinfo1 write. Use the fallthrough pseudo keyword instead of
>> a comment.
>>
>> In VMEXIT_NPF, as we're editing the printk() anyway, switch to using the
>> newer
>> domain_crash() form.
>>
>> No functional change.
>>
>> Signed-off-by: Andrew Cooper
> Reviewed-by: Jan Beulich
>
> with one remark / suggestion:
>
>> @@ -455,6 +461,10 @@ struct vmcb_struct {
>> uint64_t :59;
>> bool mov_insn:1; /* MOV, as opposed to LMSW, CLTS, etc
>> */
>> } mov_cr;
>> +struct {
>> +uint64_t ec;
>> +uint64_t gpa;
>> +} npt;
> Maybe better "npf" than "npt", as it's describing the exit/fault?
Oh yes, of course. That is what I'd intended to put here.
Thanks.
~Andrew
Re: [PATCH] x86/svm: Provide EXITINFO decodes for Exceptions/NPT intercepts
On 06.04.2023 15:27, Andrew Cooper wrote:
> Exceptions and NPT intercepts almost have the same layout, but NPT has bits
> above 31 in the error code, and the name for exitinfo2 really does want
> distinguishing between cr2 and gpa.
>
> In nsvm_vcpu_vmexit_inject() rearrange VMEXIT_NPF to fall through instead of
> repeating the exitinfo1 write. Use the fallthrough pseudo keyword instead of
> a comment.
>
> In VMEXIT_NPF, as we're editing the printk() anyway, switch to using the newer
> domain_crash() form.
>
> No functional change.
>
> Signed-off-by: Andrew Cooper
Reviewed-by: Jan Beulich
with one remark / suggestion:
> @@ -455,6 +461,10 @@ struct vmcb_struct {
> uint64_t :59;
> bool mov_insn:1; /* MOV, as opposed to LMSW, CLTS, etc */
> } mov_cr;
> +struct {
> +uint64_t ec;
> +uint64_t gpa;
> +} npt;
Maybe better "npf" than "npt", as it's describing the exit/fault?
Jan
[PATCH] x86emul: avoid triggering event related assertions
The assertion at the end of x86_emulate_wrapper() as well as the ones
in x86_emul_{hw_exception,pagefault}() can trigger if we ignore
X86EMUL_EXCEPTION coming back from certain hook functions. Squash
exceptions when merely probing MSRs, plus on SWAPGS'es "best effort"
error handling path.
In adjust_bnd() add another assertion after the read_xcr(0, ...)
invocation, paralleling the one in x86emul_get_fpu() - XCR0 reads should
never fault when XSAVE is (implicitly) known to be available.
Fixes: 14a6be89ec04 ("x86emul: correct EFLAGS.TF handling")
Fixes: cb2626c75813 ("x86emul: conditionally clear BNDn for branches")
Fixes: 6eb43fcf8a0b ("x86emul: support SWAPGS")
Reported-by: AFL
Signed-off-by: Jan Beulich
---
EFER reads won't fault in any of the handlers we have, so in principle
the respective check could be omitted (and hence has no Fixes: tag).
Thoughts?
The Fixes: tags are for the commits introducing the affected code; I'm
not entirely sure whether the raising of exceptions from hook functions
actually pre-dates them, but even if not the issues were at least latent
ones already before.
--- a/xen/arch/x86/x86_emulate/0f01.c
+++ b/xen/arch/x86/x86_emulate/0f01.c
@@ -198,8 +198,10 @@ int x86emul_0f01(struct x86_emulate_stat
if ( (rc = ops->write_segment(x86_seg_gs, ,
ctxt)) != X86EMUL_OKAY )
{
-/* Best effort unwind (i.e. no error checking). */
-ops->write_msr(MSR_SHADOW_GS_BASE, msr_val, ctxt);
+/* Best effort unwind (i.e. no real error checking). */
+if ( ops->write_msr(MSR_SHADOW_GS_BASE, msr_val,
+ctxt) == X86EMUL_EXCEPTION )
+x86_emul_reset_event(ctxt);
goto done;
}
break;
--- a/xen/arch/x86/x86_emulate/0fae.c
+++ b/xen/arch/x86/x86_emulate/0fae.c
@@ -67,7 +67,10 @@ int x86emul_0fae(struct x86_emulate_stat
cr4 = X86_CR4_OSFXSR;
if ( !ops->read_msr ||
ops->read_msr(MSR_EFER, _val, ctxt) != X86EMUL_OKAY )
+{
+x86_emul_reset_event(ctxt);
msr_val = 0;
+}
if ( !(cr4 & X86_CR4_OSFXSR) ||
(mode_64bit() && mode_ring0() && (msr_val & EFER_FFXSE)) )
s->op_bytes = offsetof(struct x86_fxsr, xmm[0]);
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1177,10 +1177,18 @@ static bool is_branch_step(struct x86_em
const struct x86_emulate_ops *ops)
{
uint64_t debugctl;
+int rc = X86EMUL_UNHANDLEABLE;
-return ops->read_msr &&
- ops->read_msr(MSR_IA32_DEBUGCTLMSR, , ctxt) ==
X86EMUL_OKAY &&
- (debugctl & IA32_DEBUGCTLMSR_BTF);
+if ( !ops->read_msr ||
+ (rc = ops->read_msr(MSR_IA32_DEBUGCTLMSR, ,
+ ctxt)) != X86EMUL_OKAY )
+{
+if ( rc == X86EMUL_EXCEPTION )
+x86_emul_reset_event(ctxt);
+debugctl = 0;
+}
+
+return debugctl & IA32_DEBUGCTLMSR_BTF;
}
static void adjust_bnd(struct x86_emulate_ctxt *ctxt,
@@ -1194,13 +1202,21 @@ static void adjust_bnd(struct x86_emulat
if ( !ops->read_xcr || ops->read_xcr(0, , ctxt) != X86EMUL_OKAY ||
!(xcr0 & X86_XCR0_BNDREGS) || !(xcr0 & X86_XCR0_BNDCSR) )
+{
+ASSERT(!ctxt->event_pending);
return;
+}
if ( !mode_ring0() )
bndcfg = read_bndcfgu();
else if ( !ops->read_msr ||
- ops->read_msr(MSR_IA32_BNDCFGS, , ctxt) != X86EMUL_OKAY )
+ (rc = ops->read_msr(MSR_IA32_BNDCFGS, ,
+ ctxt)) != X86EMUL_OKAY )
+{
+if ( rc == X86EMUL_EXCEPTION )
+x86_emul_reset_event(ctxt);
return;
+}
if ( (bndcfg & IA32_BNDCFGS_ENABLE) && !(bndcfg & IA32_BNDCFGS_PRESERVE) )
{
/*
[PATCH] x86/svm: Provide EXITINFO decodes for Exceptions/NPT intercepts
Exceptions and NPT intercepts almost have the same layout, but NPT has bits
above 31 in the error code, and the name for exitinfo2 really does want
distinguishing between cr2 and gpa.
In nsvm_vcpu_vmexit_inject() rearrange VMEXIT_NPF to fall through instead of
repeating the exitinfo1 write. Use the fallthrough pseudo keyword instead of
a comment.
In VMEXIT_NPF, as we're editing the printk() anyway, switch to using the newer
domain_crash() form.
No functional change.
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Roger Pau Monné
CC: Wei Liu
---
xen/arch/x86/hvm/svm/nestedsvm.c| 9 +++--
xen/arch/x86/hvm/svm/svm.c | 23 ++-
xen/arch/x86/include/asm/hvm/svm/vmcb.h | 10 ++
3 files changed, 23 insertions(+), 19 deletions(-)
diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index 2003f28f66f4..51e437e73a20 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -824,14 +824,11 @@ nsvm_vcpu_vmexit_inject(struct vcpu *v, struct
cpu_user_regs *regs,
ns_vmcb->exit_int_info = ns_vmcb->event_inj;
break;
case VMEXIT_EXCEPTION_PF:
-ns_vmcb->_cr2 = ns_vmcb->exitinfo2;
-/* fall through */
+ns_vmcb->_cr2 = ns_vmcb->ei.exc.cr2;
+fallthrough;
case VMEXIT_NPF:
-/* PF error code */
-ns_vmcb->exitinfo1 = svm->ns_vmexit.exitinfo1;
-/* fault address */
ns_vmcb->exitinfo2 = svm->ns_vmexit.exitinfo2;
-break;
+fallthrough;
case VMEXIT_EXCEPTION_NP:
case VMEXIT_EXCEPTION_SS:
case VMEXIT_EXCEPTION_GP:
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 1b32ef77b643..17359047b396 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2808,9 +2808,9 @@ void svm_vmexit_handler(void)
case VMEXIT_EXCEPTION_PF:
{
-unsigned long va;
-va = vmcb->exitinfo2;
-regs->error_code = vmcb->exitinfo1;
+unsigned long va = vmcb->ei.exc.cr2;
+
+regs->error_code = vmcb->ei.exc.ec;
HVM_DBG_LOG(DBG_LEVEL_VMMU,
"eax=%lx, ebx=%lx, ecx=%lx, edx=%lx, esi=%lx, edi=%lx",
regs->rax, regs->rbx, regs->rcx,
@@ -2838,7 +2838,7 @@ void svm_vmexit_handler(void)
case VMEXIT_EXCEPTION_AC:
HVMTRACE_1D(TRAP, X86_EXC_AC);
-hvm_inject_hw_exception(X86_EXC_AC, vmcb->exitinfo1);
+hvm_inject_hw_exception(X86_EXC_AC, vmcb->ei.exc.ec);
break;
case VMEXIT_EXCEPTION_UD:
@@ -3051,18 +3051,15 @@ void svm_vmexit_handler(void)
case VMEXIT_NPF:
if ( cpu_has_svm_decode )
v->arch.hvm.svm.cached_insn_len = vmcb->guest_ins_len & 0xf;
-rc = vmcb->exitinfo1 & PFEC_page_present
- ? p2m_pt_handle_deferred_changes(vmcb->exitinfo2) : 0;
+rc = vmcb->ei.npt.ec & PFEC_page_present
+ ? p2m_pt_handle_deferred_changes(vmcb->ei.npt.gpa) : 0;
if ( rc == 0 )
/* If no recal adjustments were being made - handle this fault */
-svm_do_nested_pgfault(v, regs, vmcb->exitinfo1, vmcb->exitinfo2);
+svm_do_nested_pgfault(v, regs, vmcb->ei.npt.ec, vmcb->ei.npt.gpa);
else if ( rc < 0 )
-{
-printk(XENLOG_G_ERR
- "%pv: Error %d handling NPF (gpa=%08lx ec=%04lx)\n",
- v, rc, vmcb->exitinfo2, vmcb->exitinfo1);
-domain_crash(v->domain);
-}
+domain_crash(v->domain,
+ "%pv: Error %d handling NPF (gpa=%08lx ec=%04lx)\n",
+ v, rc, vmcb->ei.npt.gpa, vmcb->ei.npt.ec);
v->arch.hvm.svm.cached_insn_len = 0;
break;
diff --git a/xen/arch/x86/include/asm/hvm/svm/vmcb.h
b/xen/arch/x86/include/asm/hvm/svm/vmcb.h
index 6cd1c4cfe4fa..05b3fb82bd12 100644
--- a/xen/arch/x86/include/asm/hvm/svm/vmcb.h
+++ b/xen/arch/x86/include/asm/hvm/svm/vmcb.h
@@ -436,6 +436,12 @@ struct vmcb_struct {
uint64_t exitinfo2; /* offset 0x80 */
};
union {
+struct {
+uint32_t ec; /* #NP, #SS, #GP, #PF, #AC */
+uint32_t :32;
+
+uint64_t cr2; /* #PF */
+} exc;
struct {
bool in:1;
bool :1;
@@ -455,6 +461,10 @@ struct vmcb_struct {
uint64_t :59;
bool mov_insn:1; /* MOV, as opposed to LMSW, CLTS, etc */
} mov_cr;
+struct {
+uint64_t ec;
+uint64_t gpa;
+} npt;
struct {
uint16_t sel;
uint64_t :48;
--
2.30.2
Re: [PATCH] x86/emul: Use existing X86_EXC_* constants
On 06/04/2023 1:50 pm, Jan Beulich wrote: > On 06.04.2023 14:17, Andrew Cooper wrote: >> ... rather than having separate definitions locally. EXC_HAS_EC in >> particular >> is missing #CP, #VC and #SX vs X86_EXC_HAVE_EC. >> >> No functional change. >> >> Signed-off-by: Andrew Cooper > Acked-by: Jan Beulich Thanks. > Yet more re-basing for me to do ... And me... I'm somewhat alarmed with how may branch collisions I've had with the TRAP_* change. > But yes, it needs to happen at some > point, and I guess there never really is a good time. Yeah, but doing it together with the TRAP_* change is going to be least disruptive (overall) to others. ~Andrew
Re: [PATCH] x86/emul: Use existing X86_EXC_* constants
On 06.04.2023 14:17, Andrew Cooper wrote: > ... rather than having separate definitions locally. EXC_HAS_EC in particular > is missing #CP, #VC and #SX vs X86_EXC_HAVE_EC. > > No functional change. > > Signed-off-by: Andrew Cooper Acked-by: Jan Beulich Yet more re-basing for me to do ... But yes, it needs to happen at some point, and I guess there never really is a good time. Jan
[PATCH] x86/emul: Use existing X86_EXC_* constants
... rather than having separate definitions locally. EXC_HAS_EC in particular
is missing #CP, #VC and #SX vs X86_EXC_HAVE_EC.
No functional change.
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Roger Pau Monné
CC: Wei Liu
---
xen/arch/x86/x86_emulate/x86_emulate.c | 608 -
1 file changed, 294 insertions(+), 314 deletions(-)
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c
b/xen/arch/x86/x86_emulate/x86_emulate.c
index c69f7c65f526..8aa23b929c07 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -135,28 +135,6 @@ static const uint8_t sse_prefix[] = { 0x66, 0xf3, 0xf2 };
/* MXCSR bit definitions. */
#define MXCSR_MM (1U << 17)
-/* Exception definitions. */
-#define EXC_DE 0
-#define EXC_DB 1
-#define EXC_BP 3
-#define EXC_OF 4
-#define EXC_BR 5
-#define EXC_UD 6
-#define EXC_NM 7
-#define EXC_DF 8
-#define EXC_TS 10
-#define EXC_NP 11
-#define EXC_SS 12
-#define EXC_GP 13
-#define EXC_PF 14
-#define EXC_MF 16
-#define EXC_AC 17
-#define EXC_XM 19
-
-#define EXC_HAS_EC \
-((1u << EXC_DF) | (1u << EXC_TS) | (1u << EXC_NP) | \
- (1u << EXC_SS) | (1u << EXC_GP) | (1u << EXC_PF) | (1u << EXC_AC))
-
/* Segment selector error code bits. */
#define ECODE_EXT (1 << 0)
#define ECODE_IDT (1 << 1)
@@ -360,11 +338,11 @@ do {
\
#define validate_far_branch(cs, ip) ({ \
if ( sizeof(ip) <= 4 ) {\
ASSERT(!ctxt->lma); \
-generate_exception_if((ip) > (cs)->limit, EXC_GP, 0); \
+generate_exception_if((ip) > (cs)->limit, X86_EXC_GP, 0); \
} else \
generate_exception_if(ctxt->lma && (cs)->l \
? !is_canonical_address(ip) \
- : (ip) > (cs)->limit, EXC_GP, 0); \
+ : (ip) > (cs)->limit, X86_EXC_GP, 0); \
})
#define commit_far_branch(cs, newip) ({ \
@@ -431,7 +409,7 @@ int x86emul_get_fpu(
return rc;
generate_exception_if(!(cr4 & ((type == X86EMUL_FPU_xmm)
? X86_CR4_OSFXSR :
X86_CR4_OSXSAVE)),
- EXC_UD);
+ X86_EXC_UD);
}
rc = ops->read_cr(0, , ctxt);
@@ -444,13 +422,13 @@ int x86emul_get_fpu(
}
if ( cr0 & X86_CR0_EM )
{
-generate_exception_if(type == X86EMUL_FPU_fpu, EXC_NM);
-generate_exception_if(type == X86EMUL_FPU_mmx, EXC_UD);
-generate_exception_if(type == X86EMUL_FPU_xmm, EXC_UD);
+generate_exception_if(type == X86EMUL_FPU_fpu, X86_EXC_NM);
+generate_exception_if(type == X86EMUL_FPU_mmx, X86_EXC_UD);
+generate_exception_if(type == X86EMUL_FPU_xmm, X86_EXC_UD);
}
generate_exception_if((cr0 & X86_CR0_TS) &&
(type != X86EMUL_FPU_wait || (cr0 & X86_CR0_MP)),
- EXC_NM);
+ X86_EXC_NM);
}
done:
@@ -776,7 +754,7 @@ static int ioport_access_check(
return rc == X86EMUL_DONE ? X86EMUL_OKAY : rc;
/* Ensure the TSS has an io-bitmap-offset field. */
-generate_exception_if(tr.type != 0xb, EXC_GP, 0);
+generate_exception_if(tr.type != 0xb, X86_EXC_GP, 0);
switch ( rc = read_ulong(x86_seg_tr, 0x66, , 2, ctxt, ops) )
{
@@ -784,7 +762,7 @@ static int ioport_access_check(
break;
case X86EMUL_EXCEPTION:
-generate_exception_if(!ctxt->event_pending, EXC_GP, 0);
+generate_exception_if(!ctxt->event_pending, X86_EXC_GP, 0);
/* fallthrough */
default:
@@ -799,7 +777,7 @@ static int ioport_access_check(
break;
case X86EMUL_EXCEPTION:
-generate_exception_if(!ctxt->event_pending, EXC_GP, 0);
+generate_exception_if(!ctxt->event_pending, X86_EXC_GP, 0);
/* fallthrough */
default:
@@ -807,7 +785,7 @@ static int ioport_access_check(
}
generate_exception_if(iobmp & (((1 << bytes) - 1) << (first_port & 7)),
- EXC_GP, 0);
+ X86_EXC_GP, 0);
done:
return rc;
@@ -854,7 +832,7 @@ protmode_load_seg(
uint8_t dpl, rpl;
int cpl = x86emul_get_cpl(ctxt, ops);
uint32_t a_flag = 0x100;
-int rc, fault_type = EXC_GP;
+int rc, fault_type = X86_EXC_GP;
if ( cpl < 0 )
return X86EMUL_UNHANDLEABLE;
@@ -982,7 +960,7 @@ protmode_load_seg(
/* Segment present in memory? */
if ( !(desc.b & (1 << 15)) && seg !=
Re: [PATCH v2] livepatch-tools: remove usage of error.h
On 06/04/2023 12:41 pm, Roger Pau Monne wrote: > It's a GNU libc specific header which prevents building on musl for > example. Instead use errx() in ERROR() and DIFF_FATAL() macros. > > Signed-off-by: Roger Pau Monné Acked-by: Andrew Cooper
[linux-linus test] 180159: tolerable trouble: fail/pass/starved - PUSHED
flight 180159 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/180159/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 180145 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 180145 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180145 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180145 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 180145 test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-examine 1 build-check(1) starved n/a test-armhf-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) starved n/a test-armhf-armhf-libvirt-raw 1 build-check(1) starved n/a test-armhf-armhf-xl 1 build-check(1) starved n/a test-armhf-armhf-xl-credit1 1 build-check(1) starved n/a test-armhf-armhf-xl-credit2 1 build-check(1) starved n/a test-armhf-armhf-xl-cubietruck 1 build-check(1) starved n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) starved n/a test-armhf-armhf-xl-rtds 1 build-check(1) starved n/a test-armhf-armhf-xl-vhd 1 build-check(1) starved n/a build-armhf-libvirt 1 build-check(1) starved n/a build-armhf 2 hosts-allocate starved n/a version targeted for testing: linux99ddf2254febae9eab7fb0bcc02c5322243f5c49 baseline version: linux76f598ba7d8e2bfb4855b5298caedd5af0c374a8 Last test of basis 180145 2023-04-05 00:11:48 Z1 days Testing same since 180159 2023-04-05 18:14:53 Z0 days1 attempts People who touched revisions under test: Daniel Bristot de Oliveira John Keeping Linus Torvalds Masami Hiramatsu (Google) Mirsad Todorovac Steven Rostedt (Google) Tze-nan Wu Zheng Yejian jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf starved build-i386 pass build-amd64-libvirt pass build-arm64-libvirt pass build-armhf-libvirt starved build-i386-libvirt pass build-amd64-pvopspass build-arm64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-xl pass
[PATCH v2] livepatch-tools: remove usage of error.h
It's a GNU libc specific header which prevents building on musl for
example. Instead use errx() in ERROR() and DIFF_FATAL() macros.
Signed-off-by: Roger Pau Monné
---
Cc: Konrad Rzeszutek Wilk
Cc: Ross Lagerwall
---
Changes since v1:
- Use errx().
---
common.h | 9 ++---
create-diff-object.c | 1 -
lookup.c | 7 +--
prelink.c| 1 -
4 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/common.h b/common.h
index 9a9da79..bbaa950 100644
--- a/common.h
+++ b/common.h
@@ -1,18 +1,21 @@
#ifndef _COMMON_H_
#define _COMMON_H_
-#include
+#include
extern char *childobj;
#define ERROR(format, ...) \
- error(1, 0, "ERROR: %s: %s: %d: " format, childobj, __FUNCTION__,
__LINE__, ##__VA_ARGS__)
+({ \
+ fflush(stdout); \
+ errx(1, "ERROR: %s: %s: %d: " format "\n", childobj, __FUNCTION__,
__LINE__, ##__VA_ARGS__); \
+})
#define DIFF_FATAL(format, ...) \
({ \
fflush(stdout); \
fprintf(stderr, "ERROR: %s: " format "\n", childobj, ##__VA_ARGS__); \
- error(2, 0, "unreconcilable difference"); \
+ errx(2, "unreconcilable difference"); \
})
#define log_debug(format, ...) log(DEBUG, format, ##__VA_ARGS__)
diff --git a/create-diff-object.c b/create-diff-object.c
index 780e6c8..d8a0032 100644
--- a/create-diff-object.c
+++ b/create-diff-object.c
@@ -45,7 +45,6 @@
#include
#include
#include
-#include
#include
#include
#include
diff --git a/lookup.c b/lookup.c
index 39125c6..9633ea2 100644
--- a/lookup.c
+++ b/lookup.c
@@ -28,14 +28,17 @@
#include
#include
#include
-#include
+#include
#include
#include
#include "lookup.h"
#define ERROR(format, ...) \
- error(1, 0, "%s: %d: " format, __FUNCTION__, __LINE__, ##__VA_ARGS__)
+({ \
+ fflush(stdout); \
+ errx(1, "%s: %d: " format, __FUNCTION__, __LINE__, ##__VA_ARGS__); \
+})
struct symbol {
unsigned long value;
diff --git a/prelink.c b/prelink.c
index 2039e5b..18c5159 100644
--- a/prelink.c
+++ b/prelink.c
@@ -27,7 +27,6 @@
#include
#include
#include
-#include
#include
#include
--
2.40.0
[PATCH AUTOSEL 4.19 8/8] xen/netback: use same error messages for same errors
From: Juergen Gross
[ Upstream commit 2eca98e5b24d01c02b46c67be05a5f98cc9789b1 ]
Issue the same error message in case an illegal page boundary crossing
has been detected in both cases where this is tested.
Suggested-by: Jan Beulich
Signed-off-by: Juergen Gross
Reviewed-by: Jan Beulich
Link: https://lore.kernel.org/r/20230329080259.14823-1-jgr...@suse.com
Signed-off-by: Paolo Abeni
Signed-off-by: Sasha Levin
---
drivers/net/xen-netback/netback.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/xen-netback/netback.c
b/drivers/net/xen-netback/netback.c
index ed644b6824cef..d2b79d7c0b881 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -989,10 +989,8 @@ static void xenvif_tx_build_gops(struct xenvif_queue
*queue,
/* No crossing a page as the payload mustn't fragment. */
if (unlikely((txreq.offset + txreq.size) > XEN_PAGE_SIZE)) {
- netdev_err(queue->vif->dev,
- "txreq.offset: %u, size: %u, end: %lu\n",
- txreq.offset, txreq.size,
- (unsigned long)(txreq.offset&~XEN_PAGE_MASK)
+ txreq.size);
+ netdev_err(queue->vif->dev, "Cross page boundary,
txreq.offset: %u, size: %u\n",
+ txreq.offset, txreq.size);
xenvif_fatal_tx_err(queue->vif);
break;
}
--
2.39.2
[PATCH AUTOSEL 4.14 7/7] xen/netback: use same error messages for same errors
From: Juergen Gross
[ Upstream commit 2eca98e5b24d01c02b46c67be05a5f98cc9789b1 ]
Issue the same error message in case an illegal page boundary crossing
has been detected in both cases where this is tested.
Suggested-by: Jan Beulich
Signed-off-by: Juergen Gross
Reviewed-by: Jan Beulich
Link: https://lore.kernel.org/r/20230329080259.14823-1-jgr...@suse.com
Signed-off-by: Paolo Abeni
Signed-off-by: Sasha Levin
---
drivers/net/xen-netback/netback.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/xen-netback/netback.c
b/drivers/net/xen-netback/netback.c
index 252414a9293db..a141db3f0dc7c 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -991,10 +991,8 @@ static void xenvif_tx_build_gops(struct xenvif_queue
*queue,
/* No crossing a page as the payload mustn't fragment. */
if (unlikely((txreq.offset + txreq.size) > XEN_PAGE_SIZE)) {
- netdev_err(queue->vif->dev,
- "txreq.offset: %u, size: %u, end: %lu\n",
- txreq.offset, txreq.size,
- (unsigned long)(txreq.offset&~XEN_PAGE_MASK)
+ txreq.size);
+ netdev_err(queue->vif->dev, "Cross page boundary,
txreq.offset: %u, size: %u\n",
+ txreq.offset, txreq.size);
xenvif_fatal_tx_err(queue->vif);
break;
}
--
2.39.2
[PATCH AUTOSEL 5.4 9/9] xen/netback: use same error messages for same errors
From: Juergen Gross
[ Upstream commit 2eca98e5b24d01c02b46c67be05a5f98cc9789b1 ]
Issue the same error message in case an illegal page boundary crossing
has been detected in both cases where this is tested.
Suggested-by: Jan Beulich
Signed-off-by: Juergen Gross
Reviewed-by: Jan Beulich
Link: https://lore.kernel.org/r/20230329080259.14823-1-jgr...@suse.com
Signed-off-by: Paolo Abeni
Signed-off-by: Sasha Levin
---
drivers/net/xen-netback/netback.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/xen-netback/netback.c
b/drivers/net/xen-netback/netback.c
index 3dfc5c66f1408..a3078755939e3 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -989,10 +989,8 @@ static void xenvif_tx_build_gops(struct xenvif_queue
*queue,
/* No crossing a page as the payload mustn't fragment. */
if (unlikely((txreq.offset + txreq.size) > XEN_PAGE_SIZE)) {
- netdev_err(queue->vif->dev,
- "txreq.offset: %u, size: %u, end: %lu\n",
- txreq.offset, txreq.size,
- (unsigned long)(txreq.offset&~XEN_PAGE_MASK)
+ txreq.size);
+ netdev_err(queue->vif->dev, "Cross page boundary,
txreq.offset: %u, size: %u\n",
+ txreq.offset, txreq.size);
xenvif_fatal_tx_err(queue->vif);
break;
}
--
2.39.2
[PATCH AUTOSEL 5.10 9/9] xen/netback: use same error messages for same errors
From: Juergen Gross
[ Upstream commit 2eca98e5b24d01c02b46c67be05a5f98cc9789b1 ]
Issue the same error message in case an illegal page boundary crossing
has been detected in both cases where this is tested.
Suggested-by: Jan Beulich
Signed-off-by: Juergen Gross
Reviewed-by: Jan Beulich
Link: https://lore.kernel.org/r/20230329080259.14823-1-jgr...@suse.com
Signed-off-by: Paolo Abeni
Signed-off-by: Sasha Levin
---
drivers/net/xen-netback/netback.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/xen-netback/netback.c
b/drivers/net/xen-netback/netback.c
index 67614e7166ac8..379ac9ca60b70 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -996,10 +996,8 @@ static void xenvif_tx_build_gops(struct xenvif_queue
*queue,
/* No crossing a page as the payload mustn't fragment. */
if (unlikely((txreq.offset + txreq.size) > XEN_PAGE_SIZE)) {
- netdev_err(queue->vif->dev,
- "txreq.offset: %u, size: %u, end: %lu\n",
- txreq.offset, txreq.size,
- (unsigned long)(txreq.offset&~XEN_PAGE_MASK)
+ txreq.size);
+ netdev_err(queue->vif->dev, "Cross page boundary,
txreq.offset: %u, size: %u\n",
+ txreq.offset, txreq.size);
xenvif_fatal_tx_err(queue->vif);
break;
}
--
2.39.2
[PATCH AUTOSEL 5.15 10/11] xen/netback: use same error messages for same errors
From: Juergen Gross
[ Upstream commit 2eca98e5b24d01c02b46c67be05a5f98cc9789b1 ]
Issue the same error message in case an illegal page boundary crossing
has been detected in both cases where this is tested.
Suggested-by: Jan Beulich
Signed-off-by: Juergen Gross
Reviewed-by: Jan Beulich
Link: https://lore.kernel.org/r/20230329080259.14823-1-jgr...@suse.com
Signed-off-by: Paolo Abeni
Signed-off-by: Sasha Levin
---
drivers/net/xen-netback/netback.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/xen-netback/netback.c
b/drivers/net/xen-netback/netback.c
index 303d8ebbaafc4..63118b56c5289 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -996,10 +996,8 @@ static void xenvif_tx_build_gops(struct xenvif_queue
*queue,
/* No crossing a page as the payload mustn't fragment. */
if (unlikely((txreq.offset + txreq.size) > XEN_PAGE_SIZE)) {
- netdev_err(queue->vif->dev,
- "txreq.offset: %u, size: %u, end: %lu\n",
- txreq.offset, txreq.size,
- (unsigned long)(txreq.offset&~XEN_PAGE_MASK)
+ txreq.size);
+ netdev_err(queue->vif->dev, "Cross page boundary,
txreq.offset: %u, size: %u\n",
+ txreq.offset, txreq.size);
xenvif_fatal_tx_err(queue->vif);
break;
}
--
2.39.2
[PATCH AUTOSEL 6.1 14/17] xen/netback: use same error messages for same errors
From: Juergen Gross
[ Upstream commit 2eca98e5b24d01c02b46c67be05a5f98cc9789b1 ]
Issue the same error message in case an illegal page boundary crossing
has been detected in both cases where this is tested.
Suggested-by: Jan Beulich
Signed-off-by: Juergen Gross
Reviewed-by: Jan Beulich
Link: https://lore.kernel.org/r/20230329080259.14823-1-jgr...@suse.com
Signed-off-by: Paolo Abeni
Signed-off-by: Sasha Levin
---
drivers/net/xen-netback/netback.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/xen-netback/netback.c
b/drivers/net/xen-netback/netback.c
index 5c266062c08f0..c35c085dbc877 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -996,10 +996,8 @@ static void xenvif_tx_build_gops(struct xenvif_queue
*queue,
/* No crossing a page as the payload mustn't fragment. */
if (unlikely((txreq.offset + txreq.size) > XEN_PAGE_SIZE)) {
- netdev_err(queue->vif->dev,
- "txreq.offset: %u, size: %u, end: %lu\n",
- txreq.offset, txreq.size,
- (unsigned long)(txreq.offset&~XEN_PAGE_MASK)
+ txreq.size);
+ netdev_err(queue->vif->dev, "Cross page boundary,
txreq.offset: %u, size: %u\n",
+ txreq.offset, txreq.size);
xenvif_fatal_tx_err(queue->vif);
break;
}
--
2.39.2
[PATCH AUTOSEL 6.2 14/17] xen/netback: use same error messages for same errors
From: Juergen Gross
[ Upstream commit 2eca98e5b24d01c02b46c67be05a5f98cc9789b1 ]
Issue the same error message in case an illegal page boundary crossing
has been detected in both cases where this is tested.
Suggested-by: Jan Beulich
Signed-off-by: Juergen Gross
Reviewed-by: Jan Beulich
Link: https://lore.kernel.org/r/20230329080259.14823-1-jgr...@suse.com
Signed-off-by: Paolo Abeni
Signed-off-by: Sasha Levin
---
drivers/net/xen-netback/netback.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/xen-netback/netback.c
b/drivers/net/xen-netback/netback.c
index 5c266062c08f0..c35c085dbc877 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -996,10 +996,8 @@ static void xenvif_tx_build_gops(struct xenvif_queue
*queue,
/* No crossing a page as the payload mustn't fragment. */
if (unlikely((txreq.offset + txreq.size) > XEN_PAGE_SIZE)) {
- netdev_err(queue->vif->dev,
- "txreq.offset: %u, size: %u, end: %lu\n",
- txreq.offset, txreq.size,
- (unsigned long)(txreq.offset&~XEN_PAGE_MASK)
+ txreq.size);
+ netdev_err(queue->vif->dev, "Cross page boundary,
txreq.offset: %u, size: %u\n",
+ txreq.offset, txreq.size);
xenvif_fatal_tx_err(queue->vif);
break;
}
--
2.39.2
Re: [PATCH] x86/svm: Provide EXITINFO decodes for MOV CR/DR intercepts
On 06/04/2023 10:59 am, Jan Beulich wrote:
> On 06.04.2023 11:52, Andrew Cooper wrote:
>> On 06/04/2023 10:31 am, Jan Beulich wrote:
>>> On 05.04.2023 22:44, Andrew Cooper wrote:
--- a/xen/arch/x86/include/asm/hvm/svm/vmcb.h
+++ b/xen/arch/x86/include/asm/hvm/svm/vmcb.h
@@ -450,6 +450,11 @@ struct vmcb_struct {
uint64_t nrip;
} io;
+struct {
+uint64_t gpr:4;
+uint64_t :59;
+bool mov_insn:1; /* MOV, as opposed to LMSW, CLTS,
etc */
+} mov;
>>> The field being named just "mov" makes it apparently applicable to DRn
>>> moves, too (and the title supports this), yet the top bit doesn't have
>>> this meaning there. So perhaps say "MOV-CR" (or alike) in the comment?
>> Hmm - I'd not spotted that distinction.
>>
>> Xen never decodes the exitinfo for a DR access - we just resync dr
>> state, drop the intercept and reenter the guest.
>>
>> Therefore I think it would be better to rename "mov" to "mov_cr" so you
>> can't use the mov_insn field in a context that plausibly looks like a dr
>> access.
>>
>> Thoughts?
> That was my other thought; it merely seemed to me that updating the comment
> would allow future (if ever) use with MOV-DR. So yes, if you prefer going
> that route: Fine with me.
Will do. I don't foresee us ever needing to inspect the dr decode
information.
~Andrew
Re: [PATCH] livepatch-tools: remove usage of error.h
On 06/04/2023 11:54 am, Roger Pau Monné wrote:
> On Thu, Apr 06, 2023 at 10:36:37AM +0100, Andrew Cooper wrote:
>> On 06/04/2023 10:18 am, Roger Pau Monne wrote:
>>> It's a GNU libc specific header which prevents building on musl for
>>> example. Instead open code an equivalent replacement for the usage
>>> of ERROR() and DIFF_FATAL() macros.
>>>
>>> Signed-off-by: Roger Pau Monné
>>> ---
>>> Cc: Konrad Rzeszutek Wilk
>>> Cc: Ross Lagerwall
>>> ---
>>> common.h | 10 ++
>>> create-diff-object.c | 1 -
>>> lookup.c | 7 +--
>>> prelink.c| 1 -
>>> 4 files changed, 11 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/common.h b/common.h
>>> index 9a9da79..ec2ea33 100644
>>> --- a/common.h
>>> +++ b/common.h
>>> @@ -1,18 +1,20 @@
>>> #ifndef _COMMON_H_
>>> #define _COMMON_H_
>>>
>>> -#include
>>> -
>>> extern char *childobj;
>>>
>>> #define ERROR(format, ...) \
>>> - error(1, 0, "ERROR: %s: %s: %d: " format, childobj, __FUNCTION__,
>>> __LINE__, ##__VA_ARGS__)
>>> +({ \
>>> + fflush(stdout); \
>>> + fprintf(stderr, "ERROR: %s: %s: %d: " format "\n", childobj,
>>> __FUNCTION__, __LINE__, ##__VA_ARGS__); \
>>> + exit(1); \
>>> +})
>>>
>>> #define DIFF_FATAL(format, ...) \
>>> ({ \
>>> fflush(stdout); \
>>> fprintf(stderr, "ERROR: %s: " format "\n", childobj, ##__VA_ARGS__); \
>>> - error(2, 0, "unreconcilable difference"); \
>>> + exit(2); \
>>> })
>> Looking at the usage, can't we just use err() instead?
> Hm, err() will unconditionaly use errno, which doesn't seem wanted
> here, as in both cases errnum is passed as 0, effectively preventing
> printing it.
>
> I could use errx(), as that doesn't append an error message, I think
> that's available on musl.
>
> Let me know if you agree with that substitution.
Yeah, anything in err.h ought to be fine.
>
>> Also, I suspect you don't intend to delete the error message in
>> DIFF_FATAL() ?
> I didn't think it was that helpful, but I could keep it.
I'd be hesitant to drop it, considering how much shell parsing there is
of these tools.
But ultimately it's up to Konrad/Ross.
~Andrew
Re: [PATCH] livepatch-tools: remove usage of error.h
On Thu, Apr 06, 2023 at 10:36:37AM +0100, Andrew Cooper wrote:
> On 06/04/2023 10:18 am, Roger Pau Monne wrote:
> > It's a GNU libc specific header which prevents building on musl for
> > example. Instead open code an equivalent replacement for the usage
> > of ERROR() and DIFF_FATAL() macros.
> >
> > Signed-off-by: Roger Pau Monné
> > ---
> > Cc: Konrad Rzeszutek Wilk
> > Cc: Ross Lagerwall
> > ---
> > common.h | 10 ++
> > create-diff-object.c | 1 -
> > lookup.c | 7 +--
> > prelink.c| 1 -
> > 4 files changed, 11 insertions(+), 8 deletions(-)
> >
> > diff --git a/common.h b/common.h
> > index 9a9da79..ec2ea33 100644
> > --- a/common.h
> > +++ b/common.h
> > @@ -1,18 +1,20 @@
> > #ifndef _COMMON_H_
> > #define _COMMON_H_
> >
> > -#include
> > -
> > extern char *childobj;
> >
> > #define ERROR(format, ...) \
> > - error(1, 0, "ERROR: %s: %s: %d: " format, childobj, __FUNCTION__,
> > __LINE__, ##__VA_ARGS__)
> > +({ \
> > + fflush(stdout); \
> > + fprintf(stderr, "ERROR: %s: %s: %d: " format "\n", childobj,
> > __FUNCTION__, __LINE__, ##__VA_ARGS__); \
> > + exit(1); \
> > +})
> >
> > #define DIFF_FATAL(format, ...) \
> > ({ \
> > fflush(stdout); \
> > fprintf(stderr, "ERROR: %s: " format "\n", childobj, ##__VA_ARGS__); \
> > - error(2, 0, "unreconcilable difference"); \
> > + exit(2); \
> > })
>
> Looking at the usage, can't we just use err() instead?
Hm, err() will unconditionaly use errno, which doesn't seem wanted
here, as in both cases errnum is passed as 0, effectively preventing
printing it.
I could use errx(), as that doesn't append an error message, I think
that's available on musl.
Let me know if you agree with that substitution.
> Also, I suspect you don't intend to delete the error message in
> DIFF_FATAL() ?
I didn't think it was that helpful, but I could keep it.
Thanks, Roger.
Re: [PATCH 1/3] xen/arm: vpl011: Fix misleading comments
On 05/04/2023 12:17, Michal Orzel wrote:
In both vpl011_read_data() and vpl011_read_data_xen(), there is a comment
stating that the guest is expected to read the DR register only if the
TXFE bit of FR register is not set. This is obviously logically wrong and
it should be RXFE (i.e. RX FIFO empty bit set -> nothing to read).
NIT:- I will prefer if the PL011 TRM is mentioned with the relevant section.
Signed-off-by: Michal Orzel
Reviewed-by: Ayan Kumar Halder
---
xen/arch/arm/vpl011.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index 2fa80bc15ac4..0186d8a31834 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -143,8 +143,8 @@ static uint8_t vpl011_read_data_xen(struct domain *d)
/*
* It is expected that there will be data in the ring buffer when this
* function is called since the guest is expected to read the data
register
- * only if the TXFE flag is not set.
- * If the guest still does read when TXFE bit is set then 0 will be
returned.
+ * only if the RXFE flag is not set.
+ * If the guest still does read when RXFE bit is set then 0 will be
returned.
*/
if ( xencons_queued(in_prod, in_cons, sizeof(intf->in)) > 0 )
{
@@ -202,8 +202,8 @@ static uint8_t vpl011_read_data(struct domain *d)
/*
* It is expected that there will be data in the ring buffer when this
* function is called since the guest is expected to read the data
register
- * only if the TXFE flag is not set.
- * If the guest still does read when TXFE bit is set then 0 will be
returned.
+ * only if the RXFE flag is not set.
+ * If the guest still does read when RXFE bit is set then 0 will be
returned.
*/
if ( xencons_queued(in_prod, in_cons, sizeof(intf->in)) > 0 )
{
[qemu-mainline test] 180153: tolerable trouble: fail/pass/starved - PUSHED
flight 180153 qemu-mainline real [real] flight 180167 qemu-mainline real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/180153/ http://logs.test-lab.xenproject.org/osstest/logs/180167/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-i386-xl-vhd 21 guest-start/debian.repeat fail pass in 180167-retest Tests which did not succeed, but are not blocking: test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail blocked in 180146 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 180146 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180146 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 180146 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180146 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) starved n/a test-armhf-armhf-libvirt-raw 1 build-check(1) starved n/a test-armhf-armhf-xl 1 build-check(1) starved n/a test-armhf-armhf-xl-credit2 1 build-check(1) starved n/a test-armhf-armhf-xl-cubietruck 1 build-check(1) starved n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) starved n/a test-armhf-armhf-xl-rtds 1 build-check(1) starved n/a test-armhf-armhf-xl-vhd 1 build-check(1) starved n/a build-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-xl-credit1 1 build-check(1) starved n/a build-armhf 2 hosts-allocate starved n/a version targeted for testing: qemuub5fba99ec7969054ab2f3727d2df014b5b72e4f1 baseline version: qemuu7d0334e49111787ae19fbc8d29ff6e7347f0605e Last test of basis 180146 2023-04-05 03:09:37 Z1 days Testing same since 180153 2023-04-05 12:38:48 Z0 days1 attempts People who touched revisions under test: Paolo Bonzini Peter Maydell Peter Xu jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf starved build-i386 pass build-amd64-libvirt pass build-arm64-libvirt pass build-armhf-libvirt starved build-i386-libvirt
Re: [PATCH v8 0/7] Add pci_dev_for_each_resource() helper and update users
On Wed, Apr 05, 2023 at 03:18:32PM -0500, Bjorn Helgaas wrote: > On Wed, Apr 05, 2023 at 11:28:27AM +0300, Andy Shevchenko wrote: > > On Tue, Apr 04, 2023 at 11:11:01AM -0500, Bjorn Helgaas wrote: > > > On Thu, Mar 30, 2023 at 07:24:27PM +0300, Andy Shevchenko wrote: ... > > > I omitted > > > > > > [1/7] kernel.h: Split out COUNT_ARGS() and CONCATENATE()" > > > > > > only because it's not essential to this series and has only a trivial > > > one-line impact on include/linux/pci.h. > > > > I'm not sure I understood what exactly "essentiality" means to you, but > > I included that because it makes the split which can be used later by > > others and not including kernel.h in the header is the objective I want > > to achieve. Without this patch the achievement is going to be deferred. > > Yet, this, as you have noticed, allows to compile and use the macros in > > the rest of the patches. > > I haven't followed the kernel.h splitting, and I try to avoid > incidental changes outside of the files I maintain, so I just wanted > to keep this series purely PCI and avoid any possible objections to a > new include file or discussion about how it should be done. Okay, fair enough :-) Thank you for elaboration, I will send the new version of patch 7 separately. -- With Best Regards, Andy Shevchenko
Re: [PATCH] x86/svm: Provide EXITINFO decodes for MOV CR/DR intercepts
On 06.04.2023 11:52, Andrew Cooper wrote:
> On 06/04/2023 10:31 am, Jan Beulich wrote:
>> On 05.04.2023 22:44, Andrew Cooper wrote:
>>> --- a/xen/arch/x86/include/asm/hvm/svm/vmcb.h
>>> +++ b/xen/arch/x86/include/asm/hvm/svm/vmcb.h
>>> @@ -450,6 +450,11 @@ struct vmcb_struct {
>>>
>>> uint64_t nrip;
>>> } io;
>>> +struct {
>>> +uint64_t gpr:4;
>>> +uint64_t :59;
>>> +bool mov_insn:1; /* MOV, as opposed to LMSW, CLTS, etc
>>> */
>>> +} mov;
>> The field being named just "mov" makes it apparently applicable to DRn
>> moves, too (and the title supports this), yet the top bit doesn't have
>> this meaning there. So perhaps say "MOV-CR" (or alike) in the comment?
>
> Hmm - I'd not spotted that distinction.
>
> Xen never decodes the exitinfo for a DR access - we just resync dr
> state, drop the intercept and reenter the guest.
>
> Therefore I think it would be better to rename "mov" to "mov_cr" so you
> can't use the mov_insn field in a context that plausibly looks like a dr
> access.
>
> Thoughts?
That was my other thought; it merely seemed to me that updating the comment
would allow future (if ever) use with MOV-DR. So yes, if you prefer going
that route: Fine with me.
Jan
Re: [PATCH] x86/svm: Provide EXITINFO decodes for MOV CR/DR intercepts
On 06/04/2023 10:31 am, Jan Beulich wrote:
> On 05.04.2023 22:44, Andrew Cooper wrote:
>> This removes raw number manipulation, and makes the logic easier to follow.
>>
>> No functional change.
>>
>> Signed-off-by: Andrew Cooper
> Reviewed-by: Jan Beulich
Thanks.
>
> One remark though:
>
>> --- a/xen/arch/x86/include/asm/hvm/svm/vmcb.h
>> +++ b/xen/arch/x86/include/asm/hvm/svm/vmcb.h
>> @@ -450,6 +450,11 @@ struct vmcb_struct {
>>
>> uint64_t nrip;
>> } io;
>> +struct {
>> +uint64_t gpr:4;
>> +uint64_t :59;
>> +bool mov_insn:1; /* MOV, as opposed to LMSW, CLTS, etc
>> */
>> +} mov;
> The field being named just "mov" makes it apparently applicable to DRn
> moves, too (and the title supports this), yet the top bit doesn't have
> this meaning there. So perhaps say "MOV-CR" (or alike) in the comment?
Hmm - I'd not spotted that distinction.
Xen never decodes the exitinfo for a DR access - we just resync dr
state, drop the intercept and reenter the guest.
Therefore I think it would be better to rename "mov" to "mov_cr" so you
can't use the mov_insn field in a context that plausibly looks like a dr
access.
Thoughts?
~Andrew
Re: [PATCH] livepatch-tools: remove usage of error.h
On 06/04/2023 10:18 am, Roger Pau Monne wrote:
> It's a GNU libc specific header which prevents building on musl for
> example. Instead open code an equivalent replacement for the usage
> of ERROR() and DIFF_FATAL() macros.
>
> Signed-off-by: Roger Pau Monné
> ---
> Cc: Konrad Rzeszutek Wilk
> Cc: Ross Lagerwall
> ---
> common.h | 10 ++
> create-diff-object.c | 1 -
> lookup.c | 7 +--
> prelink.c| 1 -
> 4 files changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/common.h b/common.h
> index 9a9da79..ec2ea33 100644
> --- a/common.h
> +++ b/common.h
> @@ -1,18 +1,20 @@
> #ifndef _COMMON_H_
> #define _COMMON_H_
>
> -#include
> -
> extern char *childobj;
>
> #define ERROR(format, ...) \
> - error(1, 0, "ERROR: %s: %s: %d: " format, childobj, __FUNCTION__,
> __LINE__, ##__VA_ARGS__)
> +({ \
> + fflush(stdout); \
> + fprintf(stderr, "ERROR: %s: %s: %d: " format "\n", childobj,
> __FUNCTION__, __LINE__, ##__VA_ARGS__); \
> + exit(1); \
> +})
>
> #define DIFF_FATAL(format, ...) \
> ({ \
> fflush(stdout); \
> fprintf(stderr, "ERROR: %s: " format "\n", childobj, ##__VA_ARGS__); \
> - error(2, 0, "unreconcilable difference"); \
> + exit(2); \
> })
Looking at the usage, can't we just use err() instead?
Also, I suspect you don't intend to delete the error message in
DIFF_FATAL() ?
~Andrew
Re: [PATCH] x86/svm: Provide EXITINFO decodes for MOV CR/DR intercepts
On 05.04.2023 22:44, Andrew Cooper wrote:
> This removes raw number manipulation, and makes the logic easier to follow.
>
> No functional change.
>
> Signed-off-by: Andrew Cooper
Reviewed-by: Jan Beulich
One remark though:
> --- a/xen/arch/x86/include/asm/hvm/svm/vmcb.h
> +++ b/xen/arch/x86/include/asm/hvm/svm/vmcb.h
> @@ -450,6 +450,11 @@ struct vmcb_struct {
>
> uint64_t nrip;
> } io;
> +struct {
> +uint64_t gpr:4;
> +uint64_t :59;
> +bool mov_insn:1; /* MOV, as opposed to LMSW, CLTS, etc */
> +} mov;
The field being named just "mov" makes it apparently applicable to DRn
moves, too (and the title supports this), yet the top bit doesn't have
this meaning there. So perhaps say "MOV-CR" (or alike) in the comment?
Jan
[PATCH] livepatch-tools: remove usage of error.h
It's a GNU libc specific header which prevents building on musl for
example. Instead open code an equivalent replacement for the usage
of ERROR() and DIFF_FATAL() macros.
Signed-off-by: Roger Pau Monné
---
Cc: Konrad Rzeszutek Wilk
Cc: Ross Lagerwall
---
common.h | 10 ++
create-diff-object.c | 1 -
lookup.c | 7 +--
prelink.c| 1 -
4 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/common.h b/common.h
index 9a9da79..ec2ea33 100644
--- a/common.h
+++ b/common.h
@@ -1,18 +1,20 @@
#ifndef _COMMON_H_
#define _COMMON_H_
-#include
-
extern char *childobj;
#define ERROR(format, ...) \
- error(1, 0, "ERROR: %s: %s: %d: " format, childobj, __FUNCTION__,
__LINE__, ##__VA_ARGS__)
+({ \
+ fflush(stdout); \
+ fprintf(stderr, "ERROR: %s: %s: %d: " format "\n", childobj,
__FUNCTION__, __LINE__, ##__VA_ARGS__); \
+ exit(1); \
+})
#define DIFF_FATAL(format, ...) \
({ \
fflush(stdout); \
fprintf(stderr, "ERROR: %s: " format "\n", childobj, ##__VA_ARGS__); \
- error(2, 0, "unreconcilable difference"); \
+ exit(2); \
})
#define log_debug(format, ...) log(DEBUG, format, ##__VA_ARGS__)
diff --git a/create-diff-object.c b/create-diff-object.c
index 780e6c8..d8a0032 100644
--- a/create-diff-object.c
+++ b/create-diff-object.c
@@ -45,7 +45,6 @@
#include
#include
#include
-#include
#include
#include
#include
diff --git a/lookup.c b/lookup.c
index 39125c6..b440102 100644
--- a/lookup.c
+++ b/lookup.c
@@ -28,14 +28,17 @@
#include
#include
#include
-#include
#include
#include
#include "lookup.h"
#define ERROR(format, ...) \
- error(1, 0, "%s: %d: " format, __FUNCTION__, __LINE__, ##__VA_ARGS__)
+({ \
+ fflush(stdout); \
+ fprintf(stderr, "%s: %d: " format, __FUNCTION__, __LINE__,
##__VA_ARGS__); \
+ exit(1); \
+})
struct symbol {
unsigned long value;
diff --git a/prelink.c b/prelink.c
index 2039e5b..18c5159 100644
--- a/prelink.c
+++ b/prelink.c
@@ -27,7 +27,6 @@
#include
#include
#include
-#include
#include
#include
--
2.40.0
[ovmf test] 180166: all pass - PUSHED
flight 180166 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/180166/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 3e3be2cbc286e773ed5bd3afd5942440546888de baseline version: ovmf 8d185dfb66700e65035d51f149570aeab728c665 Last test of basis 180164 2023-04-06 01:42:38 Z0 days Testing same since 180166 2023-04-06 07:10:40 Z0 days1 attempts People who touched revisions under test: Ard Biesheuvel Jiewen Yao jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/osstest/ovmf.git 8d185dfb66..3e3be2cbc2 3e3be2cbc286e773ed5bd3afd5942440546888de -> xen-tested-master
[PATCH V3 1/2] docs: Allow generic virtio device types to contain device-id
For generic virtio devices, where we don't need to add compatible or
other special DT properties, the type field is set to "virtio,device".
But this misses the case where the user sets the type with a valid
virtio device id as well, like "virtio,device1a" for file system device.
The complete list of virtio device ids is mentioned here:
https://docs.oasis-open.org/virtio/virtio/v1.2/cs01/virtio-v1.2-cs01.html#x1-2160005
Update documentation to support that as well.
Fixes: dd54ea500be8 ("docs: add documentation for generic virtio devices")
Signed-off-by: Viresh Kumar
Reviewed-by: Oleksandr Tyshchenko
---
V2->V3:
- Updated commit log and clarified / fixed doc.
- Tag from Oleksandr.
docs/man/xl.cfg.5.pod.in | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
index 10f37990be57..24ac92718288 100644
--- a/docs/man/xl.cfg.5.pod.in
+++ b/docs/man/xl.cfg.5.pod.in
@@ -1608,8 +1608,11 @@ example, "type=virtio,device22" for the I2C device,
whose device-tree binding is
L
-For generic virtio devices, where we don't need to set special or compatible
-properties in the Device Tree, the type field must be set to "virtio,device".
+For other generic virtio devices, where we don't need to set special or
+compatible properties in the Device Tree, the type field must be set to
+"virtio,device" or "virtio,device", where "N" is the virtio device id in
+hexadecimal format, without the "0x" prefix and all in lower case, like
+"virtio,device1a" for the file system device.
=item B
--
2.31.1.272.g89b43f80a514
[PATCH V3 2/2] libxl: fix matching of generic virtio device
The strings won't be an exact match, as we are only looking to match the
prefix here, i.e. "virtio,device". This is already done properly in
libxl_virtio.c file, lets do the same here too.
Fixes: 43ba5202e2ee ("libxl: add support for generic virtio device")
Signed-off-by: Viresh Kumar
Reviewed-by: Oleksandr Tyshchenko
---
V2->V3:
- Tag from Oleksandr.
tools/libs/light/libxl_arm.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index ddc7b2a15975..97c80d7ed0fa 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -1033,10 +1033,14 @@ static int make_virtio_mmio_node_device(libxl__gc *gc,
void *fdt, uint64_t base,
} else if (!strcmp(type, VIRTIO_DEVICE_TYPE_GPIO)) {
res = make_virtio_mmio_node_gpio(gc, fdt);
if (res) return res;
-} else if (strcmp(type, VIRTIO_DEVICE_TYPE_GENERIC)) {
-/* Doesn't match generic virtio device */
-LOG(ERROR, "Invalid type for virtio device: %s", type);
-return -EINVAL;
+} else {
+int len = sizeof(VIRTIO_DEVICE_TYPE_GENERIC) - 1;
+
+if (strncmp(type, VIRTIO_DEVICE_TYPE_GENERIC, len)) {
+/* Doesn't match generic virtio device */
+LOG(ERROR, "Invalid type for virtio device: %s", type);
+return -EINVAL;
+}
}
return fdt_end_node(fdt);
--
2.31.1.272.g89b43f80a514
Re: [PATCH v9 4/5] xen/arm: switch ARM to use generic implementation of bug.h
Hi, On 06/04/2023 07:35, Jan Beulich wrote: On 05.04.2023 18:39, Julien Grall wrote: To reduce the amount of patch to resend, I was actually thinking to merge patch #1-3 and #5 (so leave this patch alone) and modify the default in a follow-up. Any thoughts? Well, yes, that's what I did a couple of days ago already. Ah. I didn't check the tree when replying. So ignore me. Cheers, -- Julien Grall
[linux-5.4 test] 180149: tolerable trouble: fail/pass/starved - PUSHED
flight 180149 linux-5.4 real [real] flight 180165 linux-5.4 real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/180149/ http://logs.test-lab.xenproject.org/osstest/logs/180165/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-i386-xl7 xen-install fail pass in 180165-retest test-amd64-i386-xl-qemuu-ovmf-amd64 7 xen-install fail pass in 180165-retest Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 180066 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 180066 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 180066 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 180066 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180066 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180066 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 180066 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 180066 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 180066 test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass build-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-examine 1 build-check(1) starved n/a test-armhf-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) starved n/a test-armhf-armhf-libvirt-raw 1 build-check(1) starved n/a test-armhf-armhf-xl 1 build-check(1) starved n/a test-armhf-armhf-xl-credit1 1 build-check(1) starved n/a test-armhf-armhf-xl-credit2 1 build-check(1) starved n/a test-armhf-armhf-xl-cubietruck 1 build-check(1) starved n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) starved n/a test-armhf-armhf-xl-rtds 1 build-check(1) starved n/a test-armhf-armhf-xl-vhd 1 build-check(1) starved n/a build-armhf 2 hosts-allocate starved n/a version targeted for testing: linux32bea3bac5ca484c6f7e302c8c96fc686f62e7b4 baseline version: linux09b1a76e7879184fb35d71a221cae9451b895fff Last test of basis 180066 2023-03-30 13:12:10 Z6 days Testing same since 180149 2023-04-05 09:43:16 Z0 days1 attempts People who touched revisions under test: Adrien Thierry Akihiko Odaki Al Viro Alexander Aring Alexander Lobakin Alexander Stein Alexandre Ghiti Alexei Starovoitov Alvin Šipraga Anand Jain Andreas Gruenbacher Andrew Morton Anna Schumaker Arpana Arland (A Contingent worker at Intel) Arseniy Krasnov Bharath SM Caleb Sander Chris Paterson (CIP)
Re: [PATCH] x86/vmx: Revert "x86/VMX: sanitize rIP before re-entering guest"
On 05.04.2023 23:52, Andrew Cooper wrote:
> At the time of XSA-170, the x86 instruction emulator was genuinely broken. It
> would load arbitrary values into %rip and putting a check here probably was
> the best stopgap security fix. It should have been reverted following c/s
> 81d3a0b26c1 "x86emul: limit-check branch targets" which corrected the emulator
> behaviour.
>
> However, everyone involved in XSA-170, myself included, failed to read the SDM
> correctly. On the subject of %rip consistency checks, the SDM stated:
>
> If the processor supports N < 64 linear-address bits, bits 63:N must be
> identical
>
> A non-canonical %rip (and SSP more recently) is an explicitly legal state in
> x86, and the VMEntry consistency checks are intentionally off-by-one from a
> regular canonical check.
>
> The consequence of this bug is that Xen will currently take a legal x86 state
> which would successfully VMEnter, and corrupt it into having non-architectural
> behaviour.
>
> Furthermore, in the time this bugfix has been pending in public, I
> successfully persuaded Intel to clarify the SDM, adding the following
> clarification:
>
> The guest RIP value is not required to be canonical; the value of bit N-1
> may differ from that of bit N.
>
> Fixes: ffbbfda377 ("x86/VMX: sanitize rIP before re-entering guest")
> Signed-off-by: Andrew Cooper
I am kind of okay with such a full revert, but I'd consider it quite helpful
if the description made clear why the alternative of instead doing the spec
mandated check isn't necessary / useful. The emulator having gained respective
checking is only part of the reason for this, aiui. Plus bugs may be
introduced into the emulator again, where the checking here could be a guard
against needing to issue an XSA in such a case.
Jan
Re: [PATCH v4 2/3] x86/platform: introduce XENPF_get_ucode_revision
On 06.04.2023 01:02, Andrew Cooper wrote:
> On 05/04/2023 9:56 am, Jan Beulich wrote:
>> On 04.04.2023 18:06, Sergey Dyasli wrote:
>>> --- a/tools/libs/ctrl/xc_misc.c
>>> +++ b/tools/libs/ctrl/xc_misc.c
>>> @@ -243,6 +243,24 @@ int xc_get_cpu_version(xc_interface *xch, struct
>>> xenpf_pcpu_version *cpu_ver)
>>> return 0;
>>> }
>>>
>>> +int xc_get_ucode_revision(xc_interface *xch,
>>> + struct xenpf_ucode_revision *ucode_rev)
>>> +{
>>> +int ret;
>>> +struct xen_platform_op op = {
>>> +.cmd = XENPF_get_ucode_revision,
>>> +.u.ucode_revision.cpu = ucode_rev->cpu,
>>> +};
>>> +
>>> +ret = do_platform_op(xch, );
>>> +if ( ret != 0 )
>>> +return ret;
>> Is there anything wrong with omitting this if() and ...
>>
>>> +*ucode_rev = op.u.ucode_revision;
>>> +
>>> +return 0;
>> ... using "return ret" here?
>
> Conceptually, yes. *ucode_rev oughtn't to be written to on failure.
>
> More importantly though, what Sergey wrote is consistent with the vast
> majority of libxc, and consistency is far more important than a marginal
> perf improvement which you won't be able to measure.
My remark was entirely unrelated to performance, and instead solely to
(source) code size.
Jan
Re: [PATCH 2/9] x86emul: support WRMSRNS
On 05.04.2023 23:29, Andrew Cooper wrote: > On 04/04/2023 3:50 pm, Jan Beulich wrote: >> This insn differs from WRMSR solely in the lack of serialization. Hence >> the code used there can simply be used here as well, plus a feature >> check of course. > > I have a feeling this is a bit stale now that 0f01.c has moved into a > separate file ? Hmm, no, not really. This code was written long after the split anyway. "Used here as well" is meant as "copy that code", not "goto ...". >> As there's no other infrastructure needed beyond >> permitting the insn for PV privileged-op emulation (in particular no >> separate new VMEXIT) we can expose the insn to guests right away. > > There are good reasons not to expose this to PV guests. > > The #GP fault to trigger emulation is serialising, so from the guest's > point of view there is literally no difference between WRMSR and WRMSRNS. > > All code is going to need to default to WRMSR for compatibility reasons, > so not advertising WRMSRNS will save the guest going through and > self-modifying itself to use a "more efficient" instruction which is not > actually more efficient. Well, sure, I can drop that again. I don't view "self-modifying itself" as meaningful wastage. Plus I'd consider it reasonable to expose the feature by default only to HVM, while permitting (non-default) exposure to PV (in which case the emul-priv-op.c change would want retaining), except that we can't express this afaict. Even without exposing at all I'd consider it kind of reasonable to allow use of the insn, in case a guest infers its availability (or simply knows from executing raw CPUID). > The emulation implementation is fine, so Reviewed-by: Andrew Cooper > but dependent on the PV guest changes being > dropped. Thanks. Jan
Re: [PATCH v9 4/5] xen/arm: switch ARM to use generic implementation of bug.h
On 05.04.2023 18:39, Julien Grall wrote: > To reduce the amount of patch to resend, I was actually thinking to > merge patch #1-3 and #5 (so leave this patch alone) and modify the > default in a follow-up. Any thoughts? Well, yes, that's what I did a couple of days ago already. Jan
Re: [PATCH v3 2/4] tools/xendevicemodel: Introduce ..._get_ioreq_server_info_ext
On 06.04.23 05:57, Marek Marczykowski-Górecki wrote: Add xendevicemodel_get_ioreq_server_info_ext() which additionally returns output flags that XEN_DMOP_get_ioreq_server_info can now return. Do not change signature of existing xendevicemodel_get_ioreq_server_info() so existing users will not need to be changed. This advertises behavior change of "x86/msi: passthrough all MSI-X vector ctrl writes to device model" patch. Signed-off-by: Marek Marczykowski-Górecki --- v3: - new patch Should there be some HAVE_* #define in the header? Does this change require soname bump (I hope it doesn't...). You need to add version 1.5 to libxendevicemodel.map which should define the new function. Juergen OpenPGP_0xB0DE9DD628BF132F.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature
