[Xen-devel] [PATCH] arm/dom0: Add check for maximum number of supported vGIC IRQs
Xen's vGIC implementation supports a maximum number of 992 IRQ lines. GICv2 specification allows for 1020 IRQ lines. This commit adds a check for this discrepancy. Signed-off-by: Lukas Juenger (juen...@ice.rwth-aachen.de) --- xen/arch/arm/setup.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index 444857a967..37b3648a18 100644 --- a/xen/arch/arm/setup.c +++ b/xen/arch/arm/setup.c @@ -888,7 +888,13 @@ void __init start_xen(unsigned long boot_phys_offset, /* Create initial domain 0. */ /* The vGIC for DOM0 is exactly emulating the hardware GIC */ dom0_cfg.arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE; -dom0_cfg.arch.nr_spis = gic_number_lines() - 32; +/* + * Xen vGIC supports a maximum of 992 IRQ lines. + * 32 are substracted to cover local IRQs. + */ +dom0_cfg.arch.nr_spis = min(gic_number_lines(), (unsigned int) 992) - 32; +if ( gic_number_lines() > 992 ) +printk(XENLOG_WARNING "Maximum number of vGIC IRQs exceeded.\n"); dom0_cfg.max_vcpus = dom0_max_vcpus(); dom0 = domain_create(0, &dom0_cfg, true); -- 2.21.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Xen 4.12.0-rc Hangs Around masked ExtINT on CPU#
>>> On 26.03.19 at 18:21, wrote: > zeta /usr/local/src/xen # cat xen/.config |grep CONFIG_HVM > # CONFIG_HVM is not set > zeta /usr/local/src/xen # > > # tried 2 boot attempts > log at: https://pastebin.com/nL4BWJ6Y > > Hang points at lines: Thanks for trying anyway; one further possibility eliminated. Looking at the logs I've had another thought (wild guess again, so not really much hope): Could you try "mwait-idle=no"? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 01/14] x86/cpu: Create Hygon Dhyana architecture support file
On 2019/3/26 23:49, Jan Beulich wrote:
> On 25.03.19 at 14:29, wrote:
>> -static unsigned int __initdata opt_cpuid_mask_l7s0_eax = ~0u;
>> -integer_param("cpuid_mask_l7s0_eax", opt_cpuid_mask_l7s0_eax);
>> -static unsigned int __initdata opt_cpuid_mask_l7s0_ebx = ~0u;
>> -integer_param("cpuid_mask_l7s0_ebx", opt_cpuid_mask_l7s0_ebx);
>
> This is no longer needed - all references are now local to amd.c.
Okay, will put them back to amd.c.
>> @@ -116,6 +121,9 @@ bool __init probe_cpuid_faulting(void)
>> uint64_t val;
>> int rc;
>>
>> +if(boot_cpu_data.x86_vendor == X86_VENDOR_HYGON)
>> +return false;
>> +
>> if ((rc = rdmsr_safe(MSR_INTEL_PLATFORM_INFO, val)) == 0)
>
> Isn't this similarly true for AMD, in which case adding both at the
There is no MSR_INTEL_PLATFORM_INFO for AMD family 17h and Hygon family
18h. Reading this MSR will stall on Hygon system. I don't know if it
would successfully returned when reading it on AMD system.
> same time in a separate patch would seem better? Yet then again -
In a separate patch is fine.
> did you look at the description of the commit moving the function
> here (6e2fdc0f89 ["x86: Common cpuid faulting support"])? Hence
> if anything this would need qualifying by !cpu_has_hypervisor.
Then it would be like this:
if (boot_cpu_data.x86_vendor == X86_VENDOR_HYGON &&
!cpu_has_hypervisor)
return false;
> Also the contextual if() tells you that there's a blank missing in the
> one you add. However, there's a wider style question to raise:
> This file is not a Linux clone, so generally I'd expect it to be
> written in plain Xen style.
I'm sorry for the missing blank. Okay, will use the right style.
>> +static void early_init_hygon(struct cpuinfo_x86 *c)
>> +{
>> +early_init_amd(c);
>> +}
>
> Why the wrapper function? It can be introduced once you actually
To keep the functions uniform Hygon ones in hygon_cpu_dev.
> need to do more than just the call.
Okay, will remove the wrapper and directly call early_init_amd().
>> +static void init_hygon(struct cpuinfo_x86 *c)
>> +{
>> +u32 l, h;
>
> As mentioned before, please prefer uint32_t et al over u32 and
> friends. While that's applicable to the entire series (and then
> also to use basic types in preference to the fixed width ones,
Okay.
> where possible), in this particular case it would be even better if
> you dropped the variables altogether, using ...
>> +unsigned long long value;
...
>> +if (cpu_has(c, X86_FEATURE_EFRO)) {
>> +rdmsr(MSR_K7_HWCR, l, h);
>> +l |= (1 << 27); /* Enable read-only APERF/MPERF bit */
>> +wrmsr(MSR_K7_HWCR, l, h);
>> +}
>
> ... "value" and rdmsrl() / wrmsrl() here instead.
Will use rdmsrl()/wrmsrl() instead.
--
Regards,
Pu Wen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 03/14] x86/cpu/vpmu: Add Hygon Dhyana and AMD Zen support for vPMU
On 2019/3/27 0:10, Jan Beulich wrote:
> On 25.03.19 at 14:30, wrote:
>> --- a/xen/arch/x86/cpu/vpmu_amd.c
>> +++ b/xen/arch/x86/cpu/vpmu_amd.c
>> @@ -538,13 +538,37 @@ int svm_vpmu_initialise(struct vcpu *v)
>> return 0;
>> }
>>
>> -int __init amd_vpmu_init(void)
>> +static int _vpmu_init(void)
>
> Despite it having been me (I think) to have suggested this as
> a possible name, now that I see it in use I don't think it's a
> good choice: We're in vPMU code anyway, so the vpmu_
> prefix is pretty pointless. Simply init() would be too short and
> generic for my taste, so how about common_init() or
> shared_init()?
I prefer common_init() here.
>> -for ( i = 0; i < num_counters; i++ )
>> +int __init hygon_vpmu_init(void)
>> +{
>> +switch ( current_cpu_data.x86 )
>> {
>> -rdmsrl(ctrls[i], ctrl_rsvd[i]);
>> -ctrl_rsvd[i] &= CTRL_RSVD_MASK;
>> +case 0x18:
>> +num_counters = F15H_NUM_COUNTERS;
>> +counters = AMD_F15H_COUNTERS;
>> +ctrls = AMD_F15H_CTRLS;
>> +k7_counters_mirrored = 1;
>> +break;
>> +default:
>> +printk(XENLOG_WARNING "VPMU: Unsupported CPU family %#x\n",
>> + current_cpu_data.x86);
>> +return -EINVAL;
>> }
>
> While I'm not going to insist in cases where you add to existing
> switch()-es which lack such blank lines, please add a blank line
> between the case blocks here. Yet then again I wonder whether
> the default case wouldn't better move into the shared function
> as well, keying off of e.g. num_counters still being zero.
I think it's a good idea to move the default case into the shared
function, which would like:
static int common_init(void)
{
unsigned int i;
if (!num_counters) {
printk(XENLOG_WARNING "VPMU: Unsupported CPU family %#x\n",
current_cpu_data.x86);
return -EINVAL;
}
...
Then as there is only one case in hygon_vpmu_init(), how about remove
switch()-es in this function?
--
Regards,
Pu Wen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 01/14] x86/cpu: Create Hygon Dhyana architecture support file
>>> On 27.03.19 at 09:14, wrote: > On 2019/3/26 23:49, Jan Beulich wrote: >> On 25.03.19 at 14:29, wrote: >>> @@ -116,6 +121,9 @@ bool __init probe_cpuid_faulting(void) >>> uint64_t val; >>> int rc; >>> >>> + if(boot_cpu_data.x86_vendor == X86_VENDOR_HYGON) >>> + return false; >>> + >>> if ((rc = rdmsr_safe(MSR_INTEL_PLATFORM_INFO, val)) == 0) >> >> Isn't this similarly true for AMD, in which case adding both at the > > There is no MSR_INTEL_PLATFORM_INFO for AMD family 17h and Hygon family > 18h. Reading this MSR will stall on Hygon system. I don't know if it > would successfully returned when reading it on AMD system. What do you mean by "stall"? Reading an unimplemented MSR should produce #GP(0). >> same time in a separate patch would seem better? Yet then again - > > In a separate patch is fine. > >> did you look at the description of the commit moving the function >> here (6e2fdc0f89 ["x86: Common cpuid faulting support"])? Hence >> if anything this would need qualifying by !cpu_has_hypervisor. > > Then it would be like this: > if (boot_cpu_data.x86_vendor == X86_VENDOR_HYGON && > !cpu_has_hypervisor) > return false; Right, plus perhaps said AMD addition, unless Andrew objects to it for some reason. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 03/14] x86/cpu/vpmu: Add Hygon Dhyana and AMD Zen support for vPMU
>>> On 27.03.19 at 09:16, wrote:
> On 2019/3/27 0:10, Jan Beulich wrote:
>> On 25.03.19 at 14:30, wrote:
>>> -for ( i = 0; i < num_counters; i++ )
>>> +int __init hygon_vpmu_init(void)
>>> +{
>>> +switch ( current_cpu_data.x86 )
>>> {
>>> -rdmsrl(ctrls[i], ctrl_rsvd[i]);
>>> -ctrl_rsvd[i] &= CTRL_RSVD_MASK;
>>> +case 0x18:
>>> +num_counters = F15H_NUM_COUNTERS;
>>> +counters = AMD_F15H_COUNTERS;
>>> +ctrls = AMD_F15H_CTRLS;
>>> +k7_counters_mirrored = 1;
>>> +break;
>>> +default:
>>> +printk(XENLOG_WARNING "VPMU: Unsupported CPU family %#x\n",
>>> + current_cpu_data.x86);
>>> +return -EINVAL;
>>> }
>>
>> While I'm not going to insist in cases where you add to existing
>> switch()-es which lack such blank lines, please add a blank line
>> between the case blocks here. Yet then again I wonder whether
>> the default case wouldn't better move into the shared function
>> as well, keying off of e.g. num_counters still being zero.
>
> I think it's a good idea to move the default case into the shared
> function, which would like:
> static int common_init(void)
> {
> unsigned int i;
>
> if (!num_counters) {
> printk(XENLOG_WARNING "VPMU: Unsupported CPU family %#x\n",
> current_cpu_data.x86);
> return -EINVAL;
> }
> ...
>
> Then as there is only one case in hygon_vpmu_init(), how about remove
> switch()-es in this function?
Well, personally I'd prefer to keep the switch(), as that what's
going to be needed once you introduce a second model, but I
won't insist.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [qemu-mainline test] 134090: regressions - trouble: blocked/broken/fail/pass
flight 134090 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/134090/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-xsm broken build-arm64 broken build-arm64-pvopsbroken build-arm64-pvops 4 host-install(4)broken REGR. vs. 133909 build-arm64 4 host-install(4)broken REGR. vs. 133909 build-arm64-xsm 4 host-install(4)broken REGR. vs. 133909 test-amd64-i386-freebsd10-i386 14 guest-saverestore fail REGR. vs. 133909 test-amd64-i386-freebsd10-amd64 14 guest-saverestore fail REGR. vs. 133909 test-amd64-amd64-xl-qemuu-win7-amd64 13 guest-saverestore fail REGR. vs. 133909 test-amd64-amd64-xl-qemuu-ovmf-amd64 13 guest-saverestore fail REGR. vs. 133909 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 guest-saverestore fail REGR. vs. 133909 test-amd64-i386-xl-qemuu-debianhvm-amd64 13 guest-saverestore fail REGR. vs. 133909 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 13 guest-saverestore fail REGR. vs. 133909 test-amd64-amd64-xl-qemuu-debianhvm-amd64 13 guest-saverestore fail REGR. vs. 133909 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 13 guest-saverestore fail REGR. vs. 133909 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 guest-saverestore fail REGR. vs. 133909 test-amd64-i386-xl-qemuu-ovmf-amd64 13 guest-saverestore fail REGR. vs. 133909 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 13 guest-saverestore fail REGR. vs. 133909 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 13 guest-saverestore fail REGR. vs. 133909 test-amd64-i386-xl-qemuu-win7-amd64 13 guest-saverestore fail REGR. vs. 133909 test-amd64-amd64-xl-qemuu-ws16-amd64 13 guest-saverestore fail REGR. vs. 133909 test-amd64-i386-xl-qemuu-ws16-amd64 13 guest-saverestore fail REGR. vs. 133909 Tests which did not succeed, but are not blocking: test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit1 1 build-check(1) blocked n/a test-arm64-arm64-xl 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 133909 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 133909 test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass ve
Re: [Xen-devel] [PATCH] x86/xen: Add "xen_timer_slop" command line option
On Tue, Mar 26, 2019 at 07:21:31PM -0400, Boris Ostrovsky wrote: > On 3/26/19 5:13 AM, Dario Faggioli wrote: > > On Mon, 2019-03-25 at 09:43 -0400, Boris Ostrovsky wrote: > >> On 3/25/19 8:05 AM, luca abeni wrote: > >>> The picture shows the latencies measured with an unpatched guest > >>> kernel > >>> and with a guest kernel having TIMER_SLOP set to 1000 (arbitrary > >>> small > >>> value :). > >>> All the experiments have been performed booting the hypervisor with > >>> a > >>> small timer_slop (the hypervisor's one) value. So, they show that > >>> decreasing the hypervisor's timer_slop is not enough to measure low > >>> latencies with cyclictest. > >> I have a couple of questions: > >> * Does it make sense to make this a tunable for other clockevent > >> devices > >> as well? > >> > > So, AFAIUI, the thing is as follows. In clockevents_program_event(), we > > keep the delta between now and the next timer event within > > dev->max_delta_ns and dev->min_delta_ns: > > > > delta = min(delta, (int64_t) dev->max_delta_ns); > > delta = max(delta, (int64_t) dev->min_delta_ns); > > > > For Xen (well, for the Xen clock) we have: > > > > .max_delta_ns = 0x, > > .min_delta_ns = TIMER_SLOP, > > > > which means a guest can't ask for a timer to fire earlier than 100us > > ahead, which is a bit too coarse, especially on contemporary hardware. > > > > For "lapic_deadline" (which was what was in use in KVM guests, in our > > experiments) we have: > > > > lapic_clockevent.max_delta_ns = clockevent_delta2ns(0x7F, > > &lapic_clockevent); > > lapic_clockevent.min_delta_ns = clockevent_delta2ns(0xF, > > &lapic_clockevent); > > > > Which means max is 0x7F device ticks, and min is 0xF. > > clockevent_delta2ns() does the conversion from ticks to ns, basing on > > the results of the APIC calibration process. It calls cev_delta2ns() > > which does some scaling, shifting, divs, etc, and, at the very end, > > this: > > > > /* Deltas less than 1usec are pointless noise */ > > return clc > 1000 ? clc : 1000; > > > > So, as Ryan is also saying, the actual minimum, in this case, depends > > on hardware, with a sanity check of "never below 1us" (which is quite > > smaller than 100us!) > > > > Of course, the actual granularity depends on hardware in the Xen case > > as well, but that is handled in Xen itself. And we have mechanisms in > > place in there to avoid timer interrupt storms (like, ahem, the Xen's > > 'timer_slop' boot parameter... :-P) > > > > And this is basically why I was also thinking we can/should lower the > > default value of TIMER_SLOP, here in the Xen clock implementation in > > Linux. > > What do you think would be a sane value? 10us? Should we then still keep > this patch? > > My concern would be that if we change the current value and it turns out > to be very wrong we'd then have no recourse. > > > -boris > Speaking out of turn but as a participant in this thread, I would not assume to change the default value for all cases without significant testing by the community, touching a variety of configurations. It feels like changing the default has a non-trivial amount of unknowns that would need to be addressed. Not surprisingly, I am biased to the approach of my patch which does not change the default but offers flexibility to all. - Ryan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 01/14] x86/cpu: Create Hygon Dhyana architecture support file
On 2019/3/27 16:31, Jan Beulich wrote: > On 27.03.19 at 09:14, wrote: >> On 2019/3/26 23:49, Jan Beulich wrote: >>> On 25.03.19 at 14:29, wrote: @@ -116,6 +121,9 @@ bool __init probe_cpuid_faulting(void) uint64_t val; int rc; + if(boot_cpu_data.x86_vendor == X86_VENDOR_HYGON) + return false; + if ((rc = rdmsr_safe(MSR_INTEL_PLATFORM_INFO, val)) == 0) >>> >>> Isn't this similarly true for AMD, in which case adding both at the >> >> There is no MSR_INTEL_PLATFORM_INFO for AMD family 17h and Hygon family >> 18h. Reading this MSR will stall on Hygon system. I don't know if it >> would successfully returned when reading it on AMD system. > > What do you mean by "stall"? Reading an unimplemented MSR > should produce #GP(0). On certain old Hygon system there is no #GP produced. And the Xen initialization process is stopped. Beyond that it will indeed produce: "traps.c:1574: GPF ()" and return false from the last if() conditional. >>> same time in a separate patch would seem better? Yet then again - >> >> In a separate patch is fine. >> >>> did you look at the description of the commit moving the function >>> here (6e2fdc0f89 ["x86: Common cpuid faulting support"])? Hence >>> if anything this would need qualifying by !cpu_has_hypervisor. >> >> Then it would be like this: >> if (boot_cpu_data.x86_vendor == X86_VENDOR_HYGON && >> !cpu_has_hypervisor) >> return false; > > Right, plus perhaps said AMD addition, unless Andrew objects to it > for some reason. Then it would be like this: if ((boot_cpu_data.x86_vendor == X86_VENDOR_HYGON || boot_cpu_data.x86_vendor == X86_VENDOR_AMD) && !cpu_has_hypervisor) return false; Andrew, any objections? -- Regards, Pu Wen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 03/14] x86/cpu/vpmu: Add Hygon Dhyana and AMD Zen support for vPMU
On 2019/3/27 16:38, Jan Beulich wrote: On 27.03.19 at 09:16, wrote: On 2019/3/27 0:10, Jan Beulich wrote: On 25.03.19 at 14:30, wrote: +default: +printk(XENLOG_WARNING "VPMU: Unsupported CPU family %#x\n", + current_cpu_data.x86); +return -EINVAL; } While I'm not going to insist in cases where you add to existing switch()-es which lack such blank lines, please add a blank line between the case blocks here. Yet then again I wonder whether the default case wouldn't better move into the shared function as well, keying off of e.g. num_counters still being zero. Then as there is only one case in hygon_vpmu_init(), how about remove switch()-es in this function? Well, personally I'd prefer to keep the switch(), as that what's going to be needed once you introduce a second model, but I won't insist. Keeping the switch() is also fine to me. -- Regards, Pu Wen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-coverity test] 134122: regressions - ALL FAIL
flight 134122 xen-unstable-coverity real [real] http://logs.test-lab.xenproject.org/osstest/logs/134122/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: coverity-amd647 coverity-upload fail REGR. vs. 133615 version targeted for testing: xen cb70a26f78848fe45f593f7ebc9cfaac760a791b baseline version: xen eeb31ee522c7bb8541eb4c037be2c42bfcf0a3c3 Last test of basis 133615 2019-03-06 09:18:51 Z 21 days Failing since133682 2019-03-10 09:18:29 Z 17 days6 attempts Testing same since 134045 2019-03-24 09:18:44 Z3 days2 attempts People who touched revisions under test: Andrew Cooper Andrii Anisov Chao Gao George Dunlap Ian Jackson Igor Druzhinin Jan Beulich Juergen Gross Julien Grall Kevin Tian Olaf Hering Petre Pircalabu Pritha Srivastava Roger Pau Monné Ronan Abhamon Sergey Dyasli Tamas K Lengyel Tim Deegan Wei Liu jobs: coverity-amd64 fail 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 Not pushing. (No revision log; it would be 1184 lines long.) ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v1 1/1] hvmloader: allow overriding SMBIOS type 2 info
Hi Jan,
Thanks for reviewing.
> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: Tuesday, March 26, 2019 9:15 PM
> To: Xin Li
> Cc: Andrew Cooper ; Igor Druzhinin
> ; Sergey Dyasli ; Xin Li
> (Talons) ; xen-de...@lists.xen.org
> Subject: Re: [Xen-devel] [PATCH v1 1/1] hvmloader: allow overriding SMBIOS
> type 2 info
>
> >>> On 26.03.19 at 07:45, wrote:
> > Extend smbios type 2 struct to match specification, add support to
> > override strings from toolstack.
> >
> > Signed-off-by: Xin Li
> >
> > ---
> > CC: Igor Druzhinin
> > CC: Sergey Dyasli
> > CC: Andrew Cooper
>
> I wonder why I have not been Cc-ed.
Sorry, adding you.
>
> > @@ -518,7 +520,67 @@ smbios_type_2_init(void *start)
> > return (start + length);
> > }
>
> In the subject you say "overriding", but you add new information only when
> it couldn't be found via get_smbios_pt_struct(). Which in turn already is sort
> of tool stack provided, so a means to override things already exists. Please
> clarify this in title and/or description.
OK. how about:
hvmloader: add SMBIOS type 2 info for customized string
Extend smbios type 2 struct to match specification, add support to
write it when customized string provided and no smbios passed in.
>
> > -/* Only present when passed in */
> > +s = xenstore_read(HVM_XS_BASEBOARD_MANUFACTURER, NULL);
> > +if ( (s != NULL) && (*s != '\0') )
>
> Is it really a good idea to key everything else off of the presence of this
> one
> string in xenstore? Shouldn't it rather be that the structure gets
> instantiated
> whenever any of the strings are there?
OK.
I wanted to avoid structure without manufacturer,
the app we met only read this field.
Can I iterate the 6 keys twice?
first iteration to decide if any string is provided,
the second iteration to initialize structure.
>
> > +{
> > +memset(p, 0, sizeof(*p));
> > +p->header.type = 2;
> > +p->header.length = sizeof(struct smbios_type_2);
> > +p->header.handle = SMBIOS_HANDLE_TYPE2;
> > +p->feature_flags = 0x09; /* Board is a hosting board and
> > + replaceable */
>
> Doesn't setting bit 3 sort of imply also setting bit 2? Yet do we really mean
> to
> mark the board as replaceable in the first place?
For the hosts I've checked, bit 3 is set but bit 2 isn't.
>
> > +p->chassis_handle = SMBIOS_HANDLE_TYPE3;
> > +p->board_type = 0x0a; /* Motherboard */
> > +start += sizeof(struct smbios_type_2);
> > +
> > +strcpy((char *)start, s);
>
> There's at least one example in smbios_type_3_init() showing that casts like
> this one aren't needed.
OK. Will remove this unnecessary cast for void*.
>
> > --- a/tools/firmware/hvmloader/smbios_types.h
> > +++ b/tools/firmware/hvmloader/smbios_types.h
> > @@ -90,6 +90,12 @@ struct smbios_type_2 {
> > uint8_t product_name_str;
> > uint8_t version_str;
> > uint8_t serial_number_str;
> > +uint8_t asset_tag_str;
> > +uint8_t feature_flags;
> > +uint8_t location_in_chassis_str;
> > +uint16_t chassis_handle;
> > +uint8_t board_type;
> > +uint8_t contained_handle_count;
>
> uint16_t contained_handles[];
Sure. Adding this.
>
> > --- a/xen/include/public/hvm/hvm_xs_strings.h
> > +++ b/xen/include/public/hvm/hvm_xs_strings.h
> > @@ -62,18 +62,24 @@
> > /* The following xenstore values are used to override some of the default
> > * string values in the SMBIOS table constructed in hvmloader.
> > */
> > -#define HVM_XS_BIOS_STRINGS"bios-strings"
> > -#define HVM_XS_BIOS_VENDOR "bios-strings/bios-vendor"
> > -#define HVM_XS_BIOS_VERSION"bios-strings/bios-version"
> > -#define HVM_XS_SYSTEM_MANUFACTURER "bios-strings/system-
> manufacturer"
> > -#define HVM_XS_SYSTEM_PRODUCT_NAME "bios-strings/system-
> product-name"
> > -#define HVM_XS_SYSTEM_VERSION "bios-strings/system-version"
> > -#define HVM_XS_SYSTEM_SERIAL_NUMBER"bios-strings/system-serial-
> number"
> > -#define HVM_XS_ENCLOSURE_MANUFACTURER "bios-strings/enclosure-
> manufacturer"
> > -#define HVM_XS_ENCLOSURE_SERIAL_NUMBER "bios-strings/enclosure-
> serial-number"
> > -#define HVM_XS_ENCLOSURE_ASSET_TAG "bios-strings/enclosure-
> asset-tag"
> > -#define HVM_XS_BATTERY_MANUFACTURER"bios-strings/battery-
> manufacturer"
> > -#define HVM_XS_BATTERY_DEVICE_NAME "bios-strings/battery-
> device-name"
> > +#define HVM_XS_BIOS_STRINGS "bios-strings"
> > +#define HVM_XS_BIOS_VENDOR "bios-strings/bios-vendor"
> > +#define HVM_XS_BIOS_VERSION "bios-strings/bios-version"
> > +#define HVM_XS_SYSTEM_MANUFACTURER "bios-strings/system-
> manufacturer"
> > +#define HVM_XS_SYSTEM_PRODUCT_NAME "bios-strings/system-
> product-name"
> > +#define HVM_XS_SYSTEM_VERSION"bios-strings/system-
> version"
> > +#define HVM_XS_SYSTEM_SERIA
[Xen-devel] [xen-unstable-smoke test] 134107: trouble: blocked/broken/pass
flight 134107 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/134107/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-xsm broken build-arm64-xsm 4 host-install(4)broken REGR. vs. 133991 Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen 8eed571409a7f81ec9327cfa95d7c298333e22e4 baseline version: xen cb70a26f78848fe45f593f7ebc9cfaac760a791b Last test of basis 133991 2019-03-22 15:00:46 Z4 days Failing since134068 2019-03-25 12:00:51 Z1 days9 attempts Testing same since 134104 2019-03-26 22:06:44 Z0 days2 attempts People who touched revisions under test: Andrew Cooper Wei Liu jobs: build-arm64-xsm broken build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm blocked test-amd64-amd64-xl-qemuu-debianhvm-i386 pass 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 broken-job build-arm64-xsm broken broken-step build-arm64-xsm host-install(4) Not pushing. commit 8eed571409a7f81ec9327cfa95d7c298333e22e4 Author: Andrew Cooper Date: Tue Mar 26 14:23:03 2019 + CI: Add a CentOS 6 container and build jobs CentOS 6 is probably the most frequently broken build, so adding it to CI would be a very good move. One problem is that CentOS 6 comes with Python 2.6, and Qemu requires 2.7. There appear to be no sensible ways to get Python 2.7 into a CentOS 6 environments, so modify the build script to skip the Qemu upstream build instead. Additionally, SeaBIOS requires GCC 4.6 or later, so skip it as well. Signed-off-by: Andrew Cooper Acked-by: Wei Liu commit 1316369dca610352cce3aaf76e90db1cce75ed9f Author: Andrew Cooper Date: Fri Mar 22 11:12:28 2019 + CI: Fix indentation in containerize script The script is mostly indented with spaces, but there are three tabs. Fix them up to be consistent. No functional change. Signed-off-by: Andrew Cooper Acked-by: Wei Liu (qemu changes not included) ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.12] passthrough/vtd: Drop the "workaround_bios_bug" logic entirely
On 3/25/19 3:24 PM, Jan Beulich wrote: On 21.03.19 at 21:26, wrote: >> It turns out that this code was previously dead. > > If it was entirely dead, why the rush to get the change into 4.12? > (I suppose the later parts of description are then justifying why > the code wasn't actually dead, and why it was getting in the way, > but I think this way of putting it is at least confusing.) > >> c/s dcf41790 " x86/mmcfg/drhd: Move acpi_mmcfg_init() call before calling >> acpi_parse_dmar()" resulted in PCI segment 0 now having been initialised >> enough for acpi_parse_one_drhd() to not take the >> >> /* Skip checking if segment is not accessible yet. */ >> >> path unconditionally. > > Or am I misreading the initial sentence, and you're really suggesting > that prior to said commit the code in question had been dead? How's > that commit related then? Segment 0 is valid irrespective of any > MMCFG information gained from ACPI tables (see pci_segments_init()). > >> However, some systems have DMAR tables which list >> devices which are disabled by user choice (in particular, Dell PowerEdge R740 >> with I/O AT DMA disabled), and turning off all IOMMU functionality in this >> case is entirely unhelpful behaviour. > > As in many other cases, what is or is not unhelpful is often a matter > of perception and hence possibly subjective. I can see your point, > but I also can see why the authors of the code considered it a rather > bad sign if non-existing PCI devices get named by an ACPI table. > What if e.g. later a device gets hot-plugged into that very SBDF? > >> Leave the warning which identifies the problematic devices, but drop the >> remaining logic. This leaves the system in better overall state, and working >> in the same way that it did in previous releases. > > I wonder whether you've taken the time to look at the description > of the commit first introducing this logic (a8059ffced "VT-d: improve > RMRR validity checking"). I find it worrying in particular to > effectively revert a change which claims 'to avoid any security > vulnerability with malicious s/s re-enabling "supposed disabled" > devices' without any discussion of why that may have been a > wrong perspective to take. Having read the patch description, I think you have the polarity of that comment wrong. If I understand correctly, that patch disables part of the IOMMU if it finds something strange, *unless* iommu=force is on. The text gives the idea that it is *less* safe to disable the IOMMU; and that enabling the IOMMU functionality, even with invalid entries, is safer than leaving it off. The patch we checked in (if I understand correctly), enables the IOMMU in more situations, even when iommu=force is not set; and thus (by the logic of the original patch) is more "safe" by default than the previous patch. -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v1 1/1] hvmloader: allow overriding SMBIOS type 2 info
>>> On 27.03.19 at 11:54, wrote:
>> From: Jan Beulich [mailto:jbeul...@suse.com]
>> Sent: Tuesday, March 26, 2019 9:15 PM
>>
>> >>> On 26.03.19 at 07:45, wrote:
>> > @@ -518,7 +520,67 @@ smbios_type_2_init(void *start)
>> > return (start + length);
>> > }
>>
>> In the subject you say "overriding", but you add new information only when
>> it couldn't be found via get_smbios_pt_struct(). Which in turn already is
>> sort
>> of tool stack provided, so a means to override things already exists. Please
>> clarify this in title and/or description.
> OK. how about:
>
> hvmloader: add SMBIOS type 2 info for customized string
>
> Extend smbios type 2 struct to match specification, add support to
> write it when customized string provided and no smbios passed in.
Looks reasonable to me.
>> > -/* Only present when passed in */
>> > +s = xenstore_read(HVM_XS_BASEBOARD_MANUFACTURER, NULL);
>> > +if ( (s != NULL) && (*s != '\0') )
>>
>> Is it really a good idea to key everything else off of the presence of this
>> one
>> string in xenstore? Shouldn't it rather be that the structure gets
>> instantiated
>> whenever any of the strings are there?
> OK.
> I wanted to avoid structure without manufacturer,
> the app we met only read this field.
>
> Can I iterate the 6 keys twice?
> first iteration to decide if any string is provided,
> the second iteration to initialize structure.
I'd suggest to avoid this. Fill the structure without the surrounding
if(), and simply determine the function's return values based on
whether counter is non-zero.
>> > +{
>> > +memset(p, 0, sizeof(*p));
>> > +p->header.type = 2;
>> > +p->header.length = sizeof(struct smbios_type_2);
>> > +p->header.handle = SMBIOS_HANDLE_TYPE2;
>> > +p->feature_flags = 0x09; /* Board is a hosting board and
>> > + replaceable */
>>
>> Doesn't setting bit 3 sort of imply also setting bit 2? Yet do we really
>> mean to
>> mark the board as replaceable in the first place?
> For the hosts I've checked, bit 3 is set but bit 2 isn't.
But my reading of the spec suggests this implication; the question
just is whether that's an implication for the producer to follow or
the consumer. If on actual hardware it's observed as you say, I
won't object you keeping it as is.
>> > --- a/xen/include/public/hvm/hvm_xs_strings.h
>> > +++ b/xen/include/public/hvm/hvm_xs_strings.h
>> > @@ -62,18 +62,24 @@
>> > /* The following xenstore values are used to override some of the default
>> > * string values in the SMBIOS table constructed in hvmloader.
>> > */
>> > -#define HVM_XS_BIOS_STRINGS"bios-strings"
>> > -#define HVM_XS_BIOS_VENDOR "bios-strings/bios-vendor"
>> > -#define HVM_XS_BIOS_VERSION"bios-strings/bios-version"
>> > -#define HVM_XS_SYSTEM_MANUFACTURER "bios-strings/system-
>> manufacturer"
>> > -#define HVM_XS_SYSTEM_PRODUCT_NAME "bios-strings/system-
>> product-name"
>> > -#define HVM_XS_SYSTEM_VERSION "bios-strings/system-version"
>> > -#define HVM_XS_SYSTEM_SERIAL_NUMBER"bios-strings/system-serial-
>> number"
>> > -#define HVM_XS_ENCLOSURE_MANUFACTURER "bios-strings/enclosure-
>> manufacturer"
>> > -#define HVM_XS_ENCLOSURE_SERIAL_NUMBER "bios-strings/enclosure-
>> serial-number"
>> > -#define HVM_XS_ENCLOSURE_ASSET_TAG "bios-strings/enclosure-
>> asset-tag"
>> > -#define HVM_XS_BATTERY_MANUFACTURER"bios-strings/battery-
>> manufacturer"
>> > -#define HVM_XS_BATTERY_DEVICE_NAME "bios-strings/battery-
>> device-name"
>> > +#define HVM_XS_BIOS_STRINGS "bios-strings"
>> > +#define HVM_XS_BIOS_VENDOR "bios-strings/bios-vendor"
>> > +#define HVM_XS_BIOS_VERSION "bios-strings/bios-version"
>> > +#define HVM_XS_SYSTEM_MANUFACTURER "bios-strings/system-
>> manufacturer"
>> > +#define HVM_XS_SYSTEM_PRODUCT_NAME "bios-strings/system-
>> product-name"
>> > +#define HVM_XS_SYSTEM_VERSION"bios-strings/system-
>> version"
>> > +#define HVM_XS_SYSTEM_SERIAL_NUMBER "bios-strings/system-
>> serial-number"
>> > +#define HVM_XS_BASEBOARD_MANUFACTURER"bios-
>> strings/baseboard-manufacturer"
>> > +#define HVM_XS_BASEBOARD_PRODUCT_NAME"bios-
>> strings/baseboard-product-name"
>> > +#define HVM_XS_BASEBOARD_VERSION "bios-strings/baseboard-
>> version"
>> > +#define HVM_XS_BASEBOARD_SERIAL_NUMBER "bios-
>> strings/baseboard-serial-number"
>> > +#define HVM_XS_BASEBOARD_ASSET_TAG "bios-
>> strings/baseboard-asset-tag"
>> > +#define HVM_XS_BASEBOARD_LOCATION_IN_CHASSIS "bios-
>> strings/baseboard-location-in-chassis"
>> > +#define HVM_XS_ENCLOSURE_MANUFACTURER"bios-
>> strings/enclosure-manufacturer"
>> > +#define HVM_XS_ENCLOSURE_SERIAL_NUMBER "bios-
>> strings/enclosure-serial-number"
>> > +#define HVM_XS_ENCLOSURE_ASSET_TAG "bios-strings/enclosure-
>> asset-t
[Xen-devel] [distros-debian-squeeze test] 83830: trouble: blocked/broken
flight 83830 distros-debian-squeeze real [real] http://osstest.xensource.com/osstest/logs/83830/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: build-armhf-pvopsbroken build-i386 broken build-amd64-pvopsbroken build-armhf broken build-amd64 broken build-i386-pvops broken build-armhf-pvops 3 syslog-serverrunning build-armhf 3 syslog-serverrunning Tests which did not succeed, but are not blocking: test-amd64-i386-i386-squeeze-netboot-pygrub 1 build-check(1) blocked n/a test-amd64-amd64-i386-squeeze-netboot-pygrub 1 build-check(1) blocked n/a test-amd64-i386-amd64-squeeze-netboot-pygrub 1 build-check(1) blocked n/a test-amd64-amd64-amd64-squeeze-netboot-pygrub 1 build-check(1)blocked n/a build-armhf-pvops 4 host-install(4) broken like 83757 build-armhf 4 host-install(4) broken like 83757 build-amd64 4 host-install(4) broken like 83757 build-i386-pvops 4 host-install(4) broken like 83757 build-i3864 host-install(4) broken like 83757 build-amd64-pvops 4 host-install(4) broken like 83757 build-armhf-pvops 5 capture-logs broken like 83757 build-armhf 5 capture-logs broken like 83757 baseline version: flight 83757 jobs: build-amd64 broken build-armhf broken build-i386 broken build-amd64-pvopsbroken build-armhf-pvopsbroken build-i386-pvops broken test-amd64-amd64-amd64-squeeze-netboot-pygrubblocked test-amd64-i386-amd64-squeeze-netboot-pygrub blocked test-amd64-amd64-i386-squeeze-netboot-pygrub blocked test-amd64-i386-i386-squeeze-netboot-pygrub blocked sg-report-flight on osstest.xs.citrite.net logs: /home/osstest/logs images: /home/osstest/images Logs, config files, etc. are available at http://osstest.xensource.com/osstest/logs Test harness code can be found at http://xenbits.xensource.com/gitweb?p=osstest.git;a=summary Push not applicable. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.12] passthrough/vtd: Drop the "workaround_bios_bug" logic entirely
>>> On 27.03.19 at 12:02, wrote: > On 3/25/19 3:24 PM, Jan Beulich wrote: > On 21.03.19 at 21:26, wrote: >>> It turns out that this code was previously dead. >> >> If it was entirely dead, why the rush to get the change into 4.12? >> (I suppose the later parts of description are then justifying why >> the code wasn't actually dead, and why it was getting in the way, >> but I think this way of putting it is at least confusing.) >> >>> c/s dcf41790 " x86/mmcfg/drhd: Move acpi_mmcfg_init() call before calling >>> acpi_parse_dmar()" resulted in PCI segment 0 now having been initialised >>> enough for acpi_parse_one_drhd() to not take the >>> >>> /* Skip checking if segment is not accessible yet. */ >>> >>> path unconditionally. >> >> Or am I misreading the initial sentence, and you're really suggesting >> that prior to said commit the code in question had been dead? How's >> that commit related then? Segment 0 is valid irrespective of any >> MMCFG information gained from ACPI tables (see pci_segments_init()). >> >>> However, some systems have DMAR tables which list >>> devices which are disabled by user choice (in particular, Dell PowerEdge > R740 >>> with I/O AT DMA disabled), and turning off all IOMMU functionality in this >>> case is entirely unhelpful behaviour. >> >> As in many other cases, what is or is not unhelpful is often a matter >> of perception and hence possibly subjective. I can see your point, >> but I also can see why the authors of the code considered it a rather >> bad sign if non-existing PCI devices get named by an ACPI table. >> What if e.g. later a device gets hot-plugged into that very SBDF? >> >>> Leave the warning which identifies the problematic devices, but drop the >>> remaining logic. This leaves the system in better overall state, and > working >>> in the same way that it did in previous releases. >> >> I wonder whether you've taken the time to look at the description >> of the commit first introducing this logic (a8059ffced "VT-d: improve >> RMRR validity checking"). I find it worrying in particular to >> effectively revert a change which claims 'to avoid any security >> vulnerability with malicious s/s re-enabling "supposed disabled" >> devices' without any discussion of why that may have been a >> wrong perspective to take. > > Having read the patch description, I think you have the polarity of that > comment wrong. > > If I understand correctly, that patch disables part of the IOMMU if it > finds something strange, *unless* iommu=force is on. The text gives the > idea that it is *less* safe to disable the IOMMU; and that enabling the > IOMMU functionality, even with invalid entries, is safer than leaving it > off. Hmm, indeed, I did associate the security vulnerability statement with the wrong context. Yet still, "iommu=force" is what is precisely meant for such a situation: Enable the IOMMU despite there having been some issues. > The patch we checked in (if I understand correctly), enables the IOMMU > in more situations, even when iommu=force is not set; and thus (by the > logic of the original patch) is more "safe" by default than the previous > patch. I'm not sure about this conclusion of yours: If the goal had been to make things "more secure" by default, why would they have disabled the IOMMU in the first place when finding non-discoverable devices? How do we know (in the abstract general case) that enabling the IOMMU is not going to cause well hidden problems when the firmware provided data is inconsistent? Leaving the IOMMU off in such a case puts the system in a well known (albeit likely less secure) state. Turning the IOMMU on, otoh, puts the system in an unknown (albeit likely more secure) state. This calls for an admin decision, and I continue to think that the well known state is preferable as the default, because of the risk that the firmware flaw opens an unknown security hole. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v1 1/1] hvmloader: allow overriding SMBIOS type 2 info
> -Original Message- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: Wednesday, March 27, 2019 7:24 PM > To: Xin Li (Talons) > Cc: Andrew Cooper ; Igor Druzhinin > ; Sergey Dyasli ; Xin Li > ; xen-de...@lists.xen.org > Subject: RE: [Xen-devel] [PATCH v1 1/1] hvmloader: allow overriding SMBIOS > type 2 info > >> > -/* Only present when passed in */ > >> > +s = xenstore_read(HVM_XS_BASEBOARD_MANUFACTURER, NULL); > >> > +if ( (s != NULL) && (*s != '\0') ) > >> > >> Is it really a good idea to key everything else off of the presence > >> of this one string in xenstore? Shouldn't it rather be that the > >> structure gets instantiated whenever any of the strings are there? > > OK. > > I wanted to avoid structure without manufacturer, the app we met only > > read this field. > > > > Can I iterate the 6 keys twice? > > first iteration to decide if any string is provided, the second > > iteration to initialize structure. > > I'd suggest to avoid this. Fill the structure without the surrounding if(), > and > simply determine the function's return values based on whether counter is > non-zero. OK. Other structs can override this header. > >> > --- a/xen/include/public/hvm/hvm_xs_strings.h > >> > +++ b/xen/include/public/hvm/hvm_xs_strings.h > >> > @@ -62,18 +62,24 @@ > >> > /* The following xenstore values are used to override some of the > default > >> > * string values in the SMBIOS table constructed in hvmloader. > >> > */ > >> > -#define HVM_XS_BIOS_STRINGS"bios-strings" > >> > -#define HVM_XS_BIOS_VENDOR "bios-strings/bios-vendor" > >> > -#define HVM_XS_BIOS_VERSION"bios-strings/bios-version" > >> > -#define HVM_XS_SYSTEM_MANUFACTURER "bios-strings/system- > >> manufacturer" > >> > -#define HVM_XS_SYSTEM_PRODUCT_NAME "bios-strings/system- > >> product-name" > >> > -#define HVM_XS_SYSTEM_VERSION "bios-strings/system- > version" > >> > -#define HVM_XS_SYSTEM_SERIAL_NUMBER"bios-strings/system- > serial- > >> number" > >> > -#define HVM_XS_ENCLOSURE_MANUFACTURER "bios- > strings/enclosure- > >> manufacturer" > >> > -#define HVM_XS_ENCLOSURE_SERIAL_NUMBER "bios- > strings/enclosure- > >> serial-number" > >> > -#define HVM_XS_ENCLOSURE_ASSET_TAG "bios-strings/enclosure- > >> asset-tag" > >> > -#define HVM_XS_BATTERY_MANUFACTURER"bios-strings/battery- > >> manufacturer" > >> > -#define HVM_XS_BATTERY_DEVICE_NAME "bios-strings/battery- > >> device-name" > >> > +#define HVM_XS_BIOS_STRINGS "bios-strings" > >> > +#define HVM_XS_BIOS_VENDOR "bios-strings/bios-vendor" > >> > +#define HVM_XS_BIOS_VERSION "bios-strings/bios-version" > >> > +#define HVM_XS_SYSTEM_MANUFACTURER "bios- > strings/system- > >> manufacturer" > >> > +#define HVM_XS_SYSTEM_PRODUCT_NAME "bios- > strings/system- > >> product-name" > >> > +#define HVM_XS_SYSTEM_VERSION"bios-strings/system- > >> version" > >> > +#define HVM_XS_SYSTEM_SERIAL_NUMBER "bios- > strings/system- > >> serial-number" > >> > +#define HVM_XS_BASEBOARD_MANUFACTURER"bios- > >> strings/baseboard-manufacturer" > >> > +#define HVM_XS_BASEBOARD_PRODUCT_NAME"bios- > >> strings/baseboard-product-name" > >> > +#define HVM_XS_BASEBOARD_VERSION "bios- > strings/baseboard- > >> version" > >> > +#define HVM_XS_BASEBOARD_SERIAL_NUMBER "bios- > >> strings/baseboard-serial-number" > >> > +#define HVM_XS_BASEBOARD_ASSET_TAG "bios- > >> strings/baseboard-asset-tag" > >> > +#define HVM_XS_BASEBOARD_LOCATION_IN_CHASSIS "bios- > >> strings/baseboard-location-in-chassis" > >> > +#define HVM_XS_ENCLOSURE_MANUFACTURER"bios- > >> strings/enclosure-manufacturer" > >> > +#define HVM_XS_ENCLOSURE_SERIAL_NUMBER "bios- > >> strings/enclosure-serial-number" > >> > +#define HVM_XS_ENCLOSURE_ASSET_TAG "bios- > strings/enclosure- > >> asset-tag" > >> > +#define HVM_XS_BATTERY_MANUFACTURER "bios- > strings/battery- > >> manufacturer" > >> > +#define HVM_XS_BATTERY_DEVICE_NAME "bios-strings/battery- > >> device-name" > >> > >> To be honest I'd prefer if you avoided the re-formatting, accepting > >> the one definition that then doesn't properly align with the rest. > >> But if others think differently, so be it. > > Can I keep this style? This seems fit current code style. > > I'm afraid I don't understand the question in the light of me having asked to > avoid the re-formatting. Is the question perhaps targeted at others, not me? OK. Then HVM_XS_BASEBOARD_LOCATION_IN_CHASSIS won't align with the rest. > Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 2/6] xen: add helper for calling notifier_call_chain() to common/cpu.c
On 3/18/19 1:11 PM, Juergen Gross wrote: > Add a helper cpu_notifier_call_chain() to call notifier_call_chain() > for a cpu with a specified action, returning an errno value. > > This avoids coding the same pattern multiple times. > > Signed-off-by: Juergen Gross Reviewed-by: George Dunlap ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Xen 4.12.0-rc Hangs Around masked ExtINT on CPU#
On 3/27/2019 1:14 AM, Jan Beulich wrote: On 26.03.19 at 18:21, wrote: zeta /usr/local/src/xen # cat xen/.config |grep CONFIG_HVM # CONFIG_HVM is not set zeta /usr/local/src/xen # # tried 2 boot attempts log at: https://pastebin.com/nL4BWJ6Y Hang points at lines: Thanks for trying anyway; one further possibility eliminated. Looking at the logs I've had another thought (wild guess again, so not really much hope): Could you try "mwait-idle=no"? Jan I modified man_xen.cfg by adding at the end the kernel parameter: mwait-idle=no Rebooted. Result: hung: ... (XEN) [ 200.939353] TSC deadline timer enabled (XEN) [2019-03-27 13:21:21] Platform timer appears to have unexpectedly wrapped 1 times. (XEN) [2019-03-27 13:21:21] mwait-idle: disabled (XEN) [2019-03-27 13:21:21] Booting processor 1/2 eip 3e000 (XEN) [2019-03-27 13:21:21] Setting warm reset code and vector. (XEN) [2019-03-27 13:21:21] 1. (XEN) [2019-03-27 13:21:21] 2. (XEN) [2019-03-27 13:21:21] 3. (XEN) [2019-03-27 13:21:21] Asserting INIT. (XEN) [2019-03-27 13:21:22] Waiting for send to finish... Log at: https://pastebin.com/zdyhCtGv ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] xen disk spec and emulation
Hi, Currently, when specifying a virtual disk, the only way to determine the emulation model used is by selecting a particular 'vdev' numbering scheme as detailed in xl-disk-configuration in the docs. That document refers the reader to xen-vbd-interface which says: xvd* -> xen virtual disk (from which one infers PV-only) hd* -> IDE emulation sd* -> SCSI emulation d*p* -> unknown emulation The first choice actually results in IDE emulation, which is not stated anywhere AFAIK, and the fourth choice actually results in no emulation, although that's not stated either. These two things can, of course, be fixed in the documentation but I also think that the behaviour of vdev=xvd* is surprising and wrong. But there is another problem... Modern OS typically expect to use an NVMe device and we have no way to specify this kind of emulation. The obvious choice might be yet another vdev naming scheme, but this has a problem. The vdev name also dictates the vbd encoding (i.e. enumeration schemed used for PV devices), which seems to have little to do with the emulation model chosen. Adding a new encoding would also mean having to modify all PV frontends in existence to recognise it. My proposal to start to dig us out of this mess, is to add a new parameter for disks: emu= where model would be 'ide', 'scsi', 'ahci' (for which we already have half-hearted and completely undocumented support), 'nvme', or 'none'. To maintain compatibility with existing brokenness, I proposed that this parameter only apples if diskspec is of the d*p* form and that we document all other forms as deprecated. I'm fairly sure that all existent PV frontends recognise xen virtual disk vbd encoding so I don't believe it would be necessary, for instance, for 'vdev=d0p0,emu=ide' to force hd encoding and so I think we should also consider deprecating all encodings other than xen virtual disk. Furthermore, the 'hvm-emulated-unplug' doc. specifies that emulated nvme devices are unplugged differently (using code 3) from IDE or SCSI (using code 0). I propose this also be changed so that code 0 unplugs all emulated disks (similarly to how 1 unplugs all emulated NICs). The unplug mechanism was never intended (I believe) to offer any finer grained control (although code 2 is somewhat of an anomaly) and I am not aware of any OS/driver that uses code 3. I propose this change, again, because we want to avoid having to modify all PV frontend code, which would be the case if NVMe devices has to be separately unplugged using code 3. Thoughts? Paul ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Xen 4.12.0-rc Hangs Around masked ExtINT on CPU#
>>> On 27.03.19 at 14:25, wrote: > On 3/27/2019 1:14 AM, Jan Beulich wrote: > On 26.03.19 at 18:21, wrote: >>> zeta /usr/local/src/xen # cat xen/.config |grep CONFIG_HVM >>> # CONFIG_HVM is not set >>> zeta /usr/local/src/xen # >>> >>> # tried 2 boot attempts >>> log at: https://pastebin.com/nL4BWJ6Y >>> >>> Hang points at lines: >> Thanks for trying anyway; one further possibility eliminated. Looking >> at the logs I've had another thought (wild guess again, so not really >> much hope): Could you try "mwait-idle=no"? >> > I modified man_xen.cfg by adding at the end the kernel parameter: > > mwait-idle=no > > Rebooted. > Result: hung: Thanks. I'm afraid I'm out of ideas for the moment. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] xen disk spec and emulation
> -Original Message- > From: Paul Durrant > Sent: 27 March 2019 13:54 > To: xen-devel (xen-devel@lists.xenproject.org) > > Subject: xen disk spec and emulation > > Hi, > > Currently, when specifying a virtual disk, the only way to determine the > emulation model used is by > selecting a particular 'vdev' numbering scheme as detailed in > xl-disk-configuration in the docs. That > document refers the reader to xen-vbd-interface which says: > > xvd* -> xen virtual disk (from which one infers PV-only) > hd* -> IDE emulation > sd* -> SCSI emulation > d*p* -> unknown emulation > > The first choice actually results in IDE emulation, which is not stated > anywhere AFAIK, and the > fourth choice actually results in no emulation, although that's not stated > either. Actually I'll correct myself... d*p* also results in IDE emulation and so is equivalent to xvd. Thus in my proposal below a missing 'emu' parameter needs to imply 'emu=ide'. Paul > These two things > can, of course, be fixed in the documentation but I also think that the > behaviour of vdev=xvd* is > surprising and wrong. But there is another problem... > > Modern OS typically expect to use an NVMe device and we have no way to > specify this kind of > emulation. The obvious choice might be yet another vdev naming scheme, but > this has a problem. The > vdev name also dictates the vbd encoding (i.e. enumeration schemed used for > PV devices), which seems > to have little to do with the emulation model chosen. Adding a new encoding > would also mean having to > modify all PV frontends in existence to recognise it. > > My proposal to start to dig us out of this mess, is to add a new parameter > for disks: emu= > where model would be 'ide', 'scsi', 'ahci' (for which we already have > half-hearted and completely > undocumented support), 'nvme', or 'none'. To maintain compatibility with > existing brokenness, I > proposed that this parameter only apples if diskspec is of the d*p* form and > that we document all > other forms as deprecated. I'm fairly sure that all existent PV frontends > recognise xen virtual disk > vbd encoding so I don't believe it would be necessary, for instance, for > 'vdev=d0p0,emu=ide' to force > hd encoding and so I think we should also consider deprecating all encodings > other than xen virtual > disk. > > Furthermore, the 'hvm-emulated-unplug' doc. specifies that emulated nvme > devices are unplugged > differently (using code 3) from IDE or SCSI (using code 0). I propose this > also be changed so that > code 0 unplugs all emulated disks (similarly to how 1 unplugs all emulated > NICs). The unplug mechanism > was never intended (I believe) to offer any finer grained control (although > code 2 is somewhat of an > anomaly) and I am not aware of any OS/driver that uses code 3. I propose this > change, again, because > we want to avoid having to modify all PV frontend code, which would be the > case if NVMe devices has to > be separately unplugged using code 3. > > Thoughts? > > Paul ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] xen disk spec and emulation
>>> On 27.03.19 at 14:54, wrote: > Currently, when specifying a virtual disk, the only way to determine the > emulation model used is by selecting a particular 'vdev' numbering scheme as > detailed in xl-disk-configuration in the docs. That document refers the > reader to xen-vbd-interface which says: > > xvd* -> xen virtual disk (from which one infers PV-only) > hd* -> IDE emulation > sd* -> SCSI emulation > d*p* -> unknown emulation > > The first choice actually results in IDE emulation, which is not stated > anywhere AFAIK, and the fourth choice actually results in no emulation, > although that's not stated either. These two things can, of course, be fixed > in the documentation but I also think that the behaviour of vdev=xvd* is > surprising and wrong. Wasn't this behavior intended to allow a guest to be installed without PV drivers, but to have them enabled once they got installed? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/xen: Add "xen_timer_slop" command line option
On 3/27/19 6:00 AM, Ryan Thibodeaux wrote: > On Tue, Mar 26, 2019 at 07:21:31PM -0400, Boris Ostrovsky wrote: >> On 3/26/19 5:13 AM, Dario Faggioli wrote: >>> On Mon, 2019-03-25 at 09:43 -0400, Boris Ostrovsky wrote: On 3/25/19 8:05 AM, luca abeni wrote: > The picture shows the latencies measured with an unpatched guest > kernel > and with a guest kernel having TIMER_SLOP set to 1000 (arbitrary > small > value :). > All the experiments have been performed booting the hypervisor with > a > small timer_slop (the hypervisor's one) value. So, they show that > decreasing the hypervisor's timer_slop is not enough to measure low > latencies with cyclictest. I have a couple of questions: * Does it make sense to make this a tunable for other clockevent devices as well? >>> So, AFAIUI, the thing is as follows. In clockevents_program_event(), we >>> keep the delta between now and the next timer event within >>> dev->max_delta_ns and dev->min_delta_ns: >>> >>> delta = min(delta, (int64_t) dev->max_delta_ns); >>> delta = max(delta, (int64_t) dev->min_delta_ns); >>> >>> For Xen (well, for the Xen clock) we have: >>> >>> .max_delta_ns = 0x, >>> .min_delta_ns = TIMER_SLOP, >>> >>> which means a guest can't ask for a timer to fire earlier than 100us >>> ahead, which is a bit too coarse, especially on contemporary hardware. >>> >>> For "lapic_deadline" (which was what was in use in KVM guests, in our >>> experiments) we have: >>> >>> lapic_clockevent.max_delta_ns = clockevent_delta2ns(0x7F, >>> &lapic_clockevent); >>> lapic_clockevent.min_delta_ns = clockevent_delta2ns(0xF, >>> &lapic_clockevent); >>> >>> Which means max is 0x7F device ticks, and min is 0xF. >>> clockevent_delta2ns() does the conversion from ticks to ns, basing on >>> the results of the APIC calibration process. It calls cev_delta2ns() >>> which does some scaling, shifting, divs, etc, and, at the very end, >>> this: >>> >>> /* Deltas less than 1usec are pointless noise */ >>> return clc > 1000 ? clc : 1000; >>> >>> So, as Ryan is also saying, the actual minimum, in this case, depends >>> on hardware, with a sanity check of "never below 1us" (which is quite >>> smaller than 100us!) >>> >>> Of course, the actual granularity depends on hardware in the Xen case >>> as well, but that is handled in Xen itself. And we have mechanisms in >>> place in there to avoid timer interrupt storms (like, ahem, the Xen's >>> 'timer_slop' boot parameter... :-P) >>> >>> And this is basically why I was also thinking we can/should lower the >>> default value of TIMER_SLOP, here in the Xen clock implementation in >>> Linux. >> What do you think would be a sane value? 10us? Should we then still keep >> this patch? >> >> My concern would be that if we change the current value and it turns out >> to be very wrong we'd then have no recourse. >> >> >> -boris >> > Speaking out of turn but as a participant in this thread, I would not > assume to change the default value for all cases without significant > testing by the community, touching a variety of configurations. > > It feels like changing the default has a non-trivial amount of > unknowns that would need to be addressed. > > Not surprisingly, I am biased to the approach of my patch which > does not change the default but offers flexibility to all. If we are to change the default it would be good to at least collect some data on distribution of delta values in clockevents_program_event(). But as I said, I'd keep the patch. Also, as far as the comment describing TIMER_SLOP, I agree that it is rather misleading. I can replace it with /* Minimum amount of time until next clock event fires */, I can do it while committing so no need to resend. -boris ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/3] mwait-idle: add support for using halt
>>> On 26.03.19 at 22:56, wrote:
> On 3/26/19 10:54 AM, Jan Beulich wrote:
> On 19.03.19 at 17:12, wrote:
>>> On 3/15/19 3:37 AM, Jan Beulich wrote:
Furthermore I'm then once again wondering what the gain is
over using the ACPI driver: The suggested _CST looks to exactly
match the data you enter into the table in the later patch. IOW
my fundamental concern didn't go away yet: As per the name
of the driver, it shouldn't really need to support HLT (or anything
other than MWAIT) as an entry method. Hence I think that at
the very least you need to extend the description of the change
quite a bit to explain why the ACPI driver is not suitable.
Depending on how this comes out, it may then still be a matter
of discussing whether, rather than fiddling with mwait-idle, it
wouldn't be better to have an AMD-specific driver instead. Are
there any thoughts in similar directions for Linux?
>>>
>>> Because:
>>> #1 getting the ACPI tables from dom0 is either unreliable (PV dom0) or
>>> not possible (PVH dom0).
>>> #2 the changes to the Intel code are minimal.
>>> #3 worse case, Xen thinks it's using CC6 when it's using CC1. Not
>>> perfect but far from fatal or breaking.
>>
>> Having thought about this some more, I agree that an AMD-specific
>> driver would likely go too far. However, that's still no reason to fiddle
>> with the mwait-idle one - I think you could as well populate the data
>> as necessary for the ACPI driver to use, removing the dependency
>> on Dom0. After all that driver already knows of all the entry methods
>> you may want/need to use (see acpi_idle_do_entry()).
>>
> I did a rough example of how that might work and lines of code changed
> for adding it to cpu_idle was roughly 125. Seeing as this doesn't
> compile and doesn't even have comments, I'd say at least 140 lines of
> code/change (most of those are additive too), a lot of is functionally
> copied from mwait-idle and how it reads data out of the structures,
> checks, and populates the cx structures. The first set of mwait patches
> is 87 lines changed total.
>
> I _could_ try and refactor some of the code and get it down from
> 125-140, but that would most likely make porting changes even harder for
> mwait-idle.
Well, I was rather thinking about something like the change below,
taking slightly over 100 lines of new code, and not touching
mwait-idle.c at all. Otoh there are a couple of TBDs in there which
may cause the patch to further grow once addressed.
Note that this goes on top of
https://lists.xenproject.org/archives/html/xen-devel/2018-08/msg00089.html
which sadly there still wasn't sufficient feedback on to decide where
to go with the series; all I know is that Andrew (understandably)
doesn't want to see the last patch go in without vendor confirmation
(and I'd be fine to drop that last patch if need be, but this shouldn't
block the earlier patches in the series).
Jan
x86/AMD: make C-state handling independent of Dom0
At least for more recent CPUs, following what BKDG / PPR suggest for the
BIOS to surface via ACPI we can make ourselves independent of Dom0
uploading respective data.
Signed-off-by: Jan Beulich
---
TBD: Can we set local_apic_timer_c2_ok to true? I can't seem to find any
statement in the BKDG / PPR as to whether the LAPIC timer continues
running in CC6.
TBD: We may want to verify that HLT indeed is configured to enter CC6.
TBD: I guess we could extend this to families older then Fam15 as well.
--- a/xen/arch/x86/acpi/cpu_idle.c
+++ b/xen/arch/x86/acpi/cpu_idle.c
@@ -120,6 +120,8 @@ boolean_param("lapic_timer_c2_ok", local
struct acpi_processor_power *__read_mostly processor_powers[NR_CPUS];
+static int8_t __read_mostly vendor_override;
+
struct hw_residencies
{
uint64_t mc0;
@@ -1220,6 +1222,9 @@ long set_cx_pminfo(uint32_t acpi_id, str
if ( pm_idle_save && pm_idle != acpi_processor_idle )
return 0;
+if ( vendor_override > 0 )
+return 0;
+
print_cx_pminfo(acpi_id, power);
cpu_id = get_cpu_id(acpi_id);
@@ -1292,6 +1297,98 @@ long set_cx_pminfo(uint32_t acpi_id, str
return 0;
}
+static void amd_cpuidle_init(struct acpi_processor_power *power)
+{
+unsigned int i, nr = 0;
+const struct cpuinfo_x86 *c = ¤t_cpu_data;
+const unsigned int ecx_req = CPUID5_ECX_EXTENSIONS_SUPPORTED |
+ CPUID5_ECX_INTERRUPT_BREAK;
+const struct acpi_processor_cx *cx = NULL;
+static const struct acpi_processor_cx fam17[] = {
+{
+.type = ACPI_STATE_C1,
+.entry_method = ACPI_CSTATE_EM_FFH,
+.address = 0,
+.latency = 1,
+},
+{
+.type = ACPI_STATE_C2,
+.entry_method = ACPI_CSTATE_EM_HALT,
+.latency = 400,
+},
+};
+
+if ( pm_idle_save && pm_idle != acpi_processor_idle )
+return;
+
+if ( vendor_override < 0 )
+
Re: [Xen-devel] [PATCH] xen/pv: Add PV specific legacy_pic struct to expose legacy IRQs.
On 3/25/19 10:40 AM, Paul Durrant wrote: >> -Original Message- >> From: Xen-devel [mailto:xen-devel-boun...@lists.xenproject.org] On Behalf Of >> Jennifer Herbert >> Sent: 25 March 2019 14:24 >> To: Boris Ostrovsky ; x...@kernel.org; >> xen-devel@lists.xenproject.org; >> linux-ker...@vger.kernel.org >> Cc: Juergen Gross ; Stefano Stabellini >> ; Ingo Molnar >> ; Borislav Petkov ; H. Peter Anvin >> ; Thomas Gleixner >> >> Subject: Re: [Xen-devel] [PATCH] xen/pv: Add PV specific legacy_pic struct >> to expose legacy IRQs. >> >> >> >> On 21/03/19 17:49, Jennifer Herbert wrote: >>> >>> On 19/03/19 23:06, Boris Ostrovsky wrote: On 3/19/19 4:02 PM, Jennifer Herbert wrote: > The ACPI tables doesn't always contain all IRQs for legacy devices > such as RTC. Since no PIC controller is visible for a PV linux guest, > under Xen, legacy_pic currently defaults to the null_legacy_pic - with > reports no legacy IRQs. Since the commit "rtc: cmos: Do not assume > irq 8 for rtc when there are no legacy irqs" by Hans de Goede > (commit id: a1e23a42f1bdc00e32fc4869caef12e4e6272f26), the rtc now > incorrectly decides it has no irq it can use, for some hardware. > > This patch rectifies the problem by providing a xen legacy_pic > struct, which is much like the null_legacy_pic except that it > reports NR_IRQS_LEGACY irqs. I assume this is for dom0? Could there be the same problem with PVH dom0? (and if yes then this should probably go into arch/x86/xen/enlighten.c). -boris >>> I am doing this to fix a problem with dom0. DomU doesn't seem to have >>> an RTC, and so it is unaffected. >>> >>> I'm not familiar with PVH, but have now done some experiments. The RTC >>> on PVH seems broken - but not quite in the same way as PV. More >>> research is needed, however simply doing the same trick I did with PV >>> will not fix the issue. >>> >>> I'll look further into it. >>> >> The same problem does exist with PVH - however its worse with the >> presence of the IO-APIC, as with my patch it tries to set up with IRQ, >> and fails. I'm not sure how would be best to deal with this. >> However, the RTC seems broken even for machines without the ACPI omission. >> I can see fixing it for just PV doesn't seem too nice, but unsure how to >> fix this for PVH. I'm open to suggestions, but otherwise I'll put this >> on hold. > AFAICT from the code in libxl__arch_domain_prepare_config(), PVH domains > don't get an RTC, just a local APIC. > That's true for domU but not for PVH dom0 I believe. Roger? -boris ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] xen disk spec and emulation
> -Original Message- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: 27 March 2019 14:27 > To: Paul Durrant > Cc: xen-devel > Subject: Re: [Xen-devel] xen disk spec and emulation > > >>> On 27.03.19 at 14:54, wrote: > > Currently, when specifying a virtual disk, the only way to determine the > > emulation model used is by selecting a particular 'vdev' numbering scheme as > > detailed in xl-disk-configuration in the docs. That document refers the > > reader to xen-vbd-interface which says: > > > > xvd* -> xen virtual disk (from which one infers PV-only) > > hd* -> IDE emulation > > sd* -> SCSI emulation > > d*p* -> unknown emulation > > > > The first choice actually results in IDE emulation, which is not stated > > anywhere AFAIK, and the fourth choice actually results in no emulation, > > although that's not stated either. These two things can, of course, be fixed > > in the documentation but I also think that the behaviour of vdev=xvd* is > > surprising and wrong. > > Wasn't this behavior intended to allow a guest to be installed > without PV drivers, but to have them enabled once they got > installed? > I don't think so. AIUI that's what hd* and sd* were for. They seem to be the only vdev types that are documented to have emulated equivalents. I also appear to have been wrong about AHCI being completely undocumented. There is a line in xen-vbd-interface that specifies an extra 'hdtype=ahci' option, to add add ich9 controller emulation to the VM. Paul ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.12] passthrough/vtd: Drop the "workaround_bios_bug" logic entirely
On 26/03/2019 13:39, Jan Beulich wrote: On 26.03.19 at 13:43, wrote: >> On 26/03/2019 09:08, Jan Beulich wrote: >> Leave the warning which identifies the problematic devices, but drop the >> remaining logic. This leaves the system in better overall state, and >> working >> in the same way that it did in previous releases. > I wonder whether you've taken the time to look at the description > of the commit first introducing this logic (a8059ffced "VT-d: improve > RMRR validity checking"). I find it worrying in particular to > effectively revert a change which claims 'to avoid any security > vulnerability with malicious s/s re-enabling "supposed disabled" > devices' without any discussion of why that may have been a > wrong perspective to take. I had, and as a maintainer, I'd reject a patch like that were it presented today. >>> Understood. But whether you'd accept it with a better description >>> is unknown, I assume. >> I severely doubt I'd accept it at all, because it is entirely >> unreasonable behaviour. >> >> At best, it is the equivalent of throwing your hands up in the air and >> saying "I give up", and that is not good enough behaviour for Xen. >> There is a nebulous claim of security, but it is exactly that - nebulous. There isn't enough information to work out what the concern was, and even if the concern was valid, disabling VT-d across the system isn't an appropriate action to take. >>> This heavily depends on the position the system's admin takes: >>> Enabling VT-d in an incomplete fashion may as well be considered >>> worse than not enabling it at all. >> No - that's simply not true, or a reasonable position to take. > As is every way of thinking differently than you do? No, but I do expect common sense to be used in the judgement of what is appropriate and/or reasonable end user behaviour. > I'm sorry to > be putting it this way, but you continue to make claims about > how people ought to think without giving any reason why that's > the only valid way. I can't see anything wrong with someone > putting themselves on the position that if they see an enabled > IOMMU, they assume that pass-through is as safe as it can > (currently) be. Once again, we get back to a un-justified (and disputed) claim of "security". *What* is unsafe about having non-active devices behind an IOMMU? How does this scenario differ from one of PCI hotplug where the device really doesn't exist at boot time, and comes into existence later? > Just to then be caught by surprise that there is > a device not actually handled by any IOMMU? After all a non- > existent device listed in a table may as well be a hint that it's > just its SBDF which the firmware got wrong. and where does Xen currently check this? (this is a rhetorical question - Xen cannot check this.) There is absolutely nothing *at all* which guarantees that just because a number of devices are identified to be behind specific IOMMUs, that DMA wont start appearing from elsewhere in the system. Security of the system when it comes to IOMMUs *is and always will be* a mutually cooperative and trusting relationship between Xen and the firmware. The notion of "I'm safe because there were no inconsistencies in a piece of information I have to trust fully" is security theatre, not security. > >> Disabling the IOMMU prevents the system from booting with a PVH dom0. > But doing what you did is not the only way of getting around this. > Defaulting to workaround_bios_bug=1 in the PVH case would be > another, as would be a mode in which the IOMMU exists for Dom0's > purposes only (i.e. still disallowing any pass-through to DomU-s). A discussion along these lines might be appropriate in the middle of a dev cycle, and might even be valid for a discussion of future improvements. It is not appropriate for resolving an issue identified as a 4.12 blocker by the RM, on a timescale which needs to fit into the 4.12 release plans. >> I am not aware of a credible case where partially enabled VT-d is less >> secure than no VT-d, and there is one headline case now where disabled >> VT-d causes a failure to boot. >> >>> Furthermore, as much as the security related claim there is >>> nebulous, your description - I'm sorry to say that - isn't much >>> better, as you don't clarify why there's _no_ security aspect >>> there. Stating that "this leaves the system in better overall >>> state" without making clear why that is _for everyone_ is not >>> helpful at all. >> The nebulous security claim is not relevant to this patch. >> >> This code was not run previously. An unexpected consequence of a change >> in 4.12 caused it to run, and break booting on some (sadly rather >> common) systems. >> >> This is a regression in 4.12 and needs resolving. The choice is between >> reverting dcf41790 or removing this code, and reverting dcf41790 is >> obviously not a valid thing to do. > As explained before, th
Re: [Xen-devel] [PATCH] x86/xen: Add "xen_timer_slop" command line option
On Wed, Mar 27, 2019 at 10:46:21AM -0400, Boris Ostrovsky wrote: > On 3/27/19 6:00 AM, Ryan Thibodeaux wrote: > > On Tue, Mar 26, 2019 at 07:21:31PM -0400, Boris Ostrovsky wrote: > >> On 3/26/19 5:13 AM, Dario Faggioli wrote: > >>> On Mon, 2019-03-25 at 09:43 -0400, Boris Ostrovsky wrote: > On 3/25/19 8:05 AM, luca abeni wrote: > > The picture shows the latencies measured with an unpatched guest > > kernel > > and with a guest kernel having TIMER_SLOP set to 1000 (arbitrary > > small > > value :). > > All the experiments have been performed booting the hypervisor with > > a > > small timer_slop (the hypervisor's one) value. So, they show that > > decreasing the hypervisor's timer_slop is not enough to measure low > > latencies with cyclictest. > I have a couple of questions: > * Does it make sense to make this a tunable for other clockevent > devices > as well? > > >>> So, AFAIUI, the thing is as follows. In clockevents_program_event(), we > >>> keep the delta between now and the next timer event within > >>> dev->max_delta_ns and dev->min_delta_ns: > >>> > >>> delta = min(delta, (int64_t) dev->max_delta_ns); > >>> delta = max(delta, (int64_t) dev->min_delta_ns); > >>> > >>> For Xen (well, for the Xen clock) we have: > >>> > >>> .max_delta_ns = 0x, > >>> .min_delta_ns = TIMER_SLOP, > >>> > >>> which means a guest can't ask for a timer to fire earlier than 100us > >>> ahead, which is a bit too coarse, especially on contemporary hardware. > >>> > >>> For "lapic_deadline" (which was what was in use in KVM guests, in our > >>> experiments) we have: > >>> > >>> lapic_clockevent.max_delta_ns = clockevent_delta2ns(0x7F, > >>> &lapic_clockevent); > >>> lapic_clockevent.min_delta_ns = clockevent_delta2ns(0xF, > >>> &lapic_clockevent); > >>> > >>> Which means max is 0x7F device ticks, and min is 0xF. > >>> clockevent_delta2ns() does the conversion from ticks to ns, basing on > >>> the results of the APIC calibration process. It calls cev_delta2ns() > >>> which does some scaling, shifting, divs, etc, and, at the very end, > >>> this: > >>> > >>> /* Deltas less than 1usec are pointless noise */ > >>> return clc > 1000 ? clc : 1000; > >>> > >>> So, as Ryan is also saying, the actual minimum, in this case, depends > >>> on hardware, with a sanity check of "never below 1us" (which is quite > >>> smaller than 100us!) > >>> > >>> Of course, the actual granularity depends on hardware in the Xen case > >>> as well, but that is handled in Xen itself. And we have mechanisms in > >>> place in there to avoid timer interrupt storms (like, ahem, the Xen's > >>> 'timer_slop' boot parameter... :-P) > >>> > >>> And this is basically why I was also thinking we can/should lower the > >>> default value of TIMER_SLOP, here in the Xen clock implementation in > >>> Linux. > >> What do you think would be a sane value? 10us? Should we then still keep > >> this patch? > >> > >> My concern would be that if we change the current value and it turns out > >> to be very wrong we'd then have no recourse. > >> > >> > >> -boris > >> > > Speaking out of turn but as a participant in this thread, I would not > > assume to change the default value for all cases without significant > > testing by the community, touching a variety of configurations. > > > > It feels like changing the default has a non-trivial amount of > > unknowns that would need to be addressed. > > > > Not surprisingly, I am biased to the approach of my patch which > > does not change the default but offers flexibility to all. > > > If we are to change the default it would be good to at least collect > some data on distribution of delta values in > clockevents_program_event(). But as I said, I'd keep the patch. > > Also, as far as the comment describing TIMER_SLOP, I agree that it is > rather misleading. > > I can replace it with /* Minimum amount of time until next clock event > fires */, I can do it while committing so no need to resend. > > -boris I like that. Thanks Boris! - Ryan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] x86/Xen: streamline (and fix) PV CPU enumeration
This started out with me noticing that "dom0_max_vcpus=" with
larger than the number of physical CPUs reported through ACPI tables
would not bring up the "excess" vCPU-s.
Noticing that xen_fill_possible_map() gets called way too early, whereas
xen_filter_cpu_maps() gets called too late (after per-CPU areas were
already set up), and further observing that each of the functions serves
only one of Dom0 or DomU, it looked like it was better to simplify this.
Use the .get_smp_config hook instead, uniformly for Dom0 and DomU.
xen_fill_possible_map() can be dropped altogether, while
xen_filter_cpu_maps() gets re-purposed but not otherwise changed.
Signed-off-by: Jan Beulich
---
arch/x86/xen/enlighten_pv.c |4
arch/x86/xen/smp_pv.c | 26 ++
2 files changed, 6 insertions(+), 24 deletions(-)
--- 5.1-rc2/arch/x86/xen/enlighten_pv.c
+++ 5.1-rc2-xen-x86-Dom0-more-vCPUs/arch/x86/xen/enlighten_pv.c
@@ -1381,10 +1381,6 @@ asmlinkage __visible void __init xen_sta
xen_acpi_sleep_register();
- /* Avoid searching for BIOS MP tables */
- x86_init.mpparse.find_smp_config = x86_init_noop;
- x86_init.mpparse.get_smp_config = x86_init_uint_noop;
-
xen_boot_params_init_edd();
}
--- 5.1-rc2/arch/x86/xen/smp_pv.c
+++ 5.1-rc2-xen-x86-Dom0-more-vCPUs/arch/x86/xen/smp_pv.c
@@ -146,28 +146,12 @@ int xen_smp_intr_init_pv(unsigned int cp
return rc;
}
-static void __init xen_fill_possible_map(void)
-{
- int i, rc;
-
- if (xen_initial_domain())
- return;
-
- for (i = 0; i < nr_cpu_ids; i++) {
- rc = HYPERVISOR_vcpu_op(VCPUOP_is_up, i, NULL);
- if (rc >= 0) {
- num_processors++;
- set_cpu_possible(i, true);
- }
- }
-}
-
-static void __init xen_filter_cpu_maps(void)
+static void __init _get_smp_config(unsigned int early)
{
int i, rc;
unsigned int subtract = 0;
- if (!xen_initial_domain())
+ if (early)
return;
num_processors = 0;
@@ -217,7 +201,6 @@ static void __init xen_pv_smp_prepare_bo
loadsegment(es, __USER_DS);
#endif
- xen_filter_cpu_maps();
xen_setup_vcpu_info_placement();
/*
@@ -503,5 +486,8 @@ static const struct smp_ops xen_smp_ops
void __init xen_smp_init(void)
{
smp_ops = xen_smp_ops;
- xen_fill_possible_map();
+
+ /* Avoid searching for BIOS MP tables */
+ x86_init.mpparse.find_smp_config = x86_init_noop;
+ x86_init.mpparse.get_smp_config = _get_smp_config;
}
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/xen: Add "xen_timer_slop" command line option
On Wed, 2019-03-27 at 10:46 -0400, Boris Ostrovsky wrote: > On 3/27/19 6:00 AM, Ryan Thibodeaux wrote: > > On Tue, Mar 26, 2019 at 07:21:31PM -0400, Boris Ostrovsky wrote: > > > On 3/26/19 5:13 AM, Dario Faggioli wrote: > > > > > > > > And this is basically why I was also thinking we can/should > > > > lower the > > > > default value of TIMER_SLOP, here in the Xen clock > > > > implementation in > > > > Linux. > > > What do you think would be a sane value? 10us? Should we then > > > still keep > > > this patch? > > > > > > My concern would be that if we change the current value and it > > > turns out > > > to be very wrong we'd then have no recourse. > > > > > Speaking out of turn but as a participant in this thread, I would > > not > > assume to change the default value for all cases without > > significant > > testing by the community, touching a variety of configurations. > > > If we are to change the default it would be good to at least collect > some data on distribution of delta values in > clockevents_program_event(). But as I said, I'd keep the patch. > I would definitely take/keep this patch. Choosing a more sane (IMO) default and making things flexible and configurable are not mutually exclusive things. :-) I think that having this set to 100us stands in the way of a lot of people wanting to do time sensitive stuff in Xen VMs. I'd at least halve that to 50us, but 10us is even better. But sure we can do this at a later point. And even at that point, a patch like this is valuable, because there might be people that might, probably after some testing of their own setup, want to lower it even further. > Also, as far as the comment describing TIMER_SLOP, I agree that it is > rather misleading. > > I can replace it with /* Minimum amount of time until next clock > event > fires */, I can do it while committing so no need to resend. > Yeah, much better, yes. Dario -- <> (Raistlin Majere) - Dario Faggioli, Ph.D, http://about.me/dario.faggioli Software Engineer @ SUSE https://www.suse.com/ ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH RFC v4] x86/mm: Clean up p2m_finish_type_change return value
In the case of any errors, finish_type_change() passes values returned
from p2m->recalc() up the stack (with some exceptions in the case where
an error is expected); this eventually ends up being returned to the
XEN_DOMOP_map_mem_type_to_ioreq_server hypercall.
However, on Intel processors (but not on AMD processor), p2m->recalc()
can also return '1' as well as '0'. This case is handled very
inconsistently: finish_type_change() will return the value of the final
entry it attempts, discarding results for other entries;
p2m_finish_type_change() will attempt to accumulate '1's, so that it
returns '1' if any of the calls to finish_type_change() returns '1'; and
dm_op() will again return '1' only if the very last call to
p2m_finish_type_change() returns '1'. The result is that the
XEN_DMOP_map_mem_type_to_ioreq_server() hypercall will sometimes return
0 and sometimes return 1 on success, in an unpredictable manner.
The hypercall documentation doesn't mention return values; but it's not
clear what the caller could do with the information about whether
entries had been changed or not. At the moment it's always 0 on AMD
boxes, and *usually* 1 on Intel boxes; so nothing can be relying on a
'1' return value for correctness (or if it is, it's broken).
Make the return value on success consistently '0' by only returning
0/-ERROR from finish_type_change(). Also remove the accumulation code
from p2m_finish_type_change().
Suggested-by: George Dunlap
Signed-off-by: Alexandru Isaila
---
Changes since V3:
- Remove positive returned values from p2m->recalc.
---
xen/arch/x86/mm/p2m-ept.c | 10 ++
xen/arch/x86/mm/p2m.c | 15 +--
2 files changed, 11 insertions(+), 14 deletions(-)
diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
index e3044bee2e..d336c138b0 100644
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -459,8 +459,7 @@ static int ept_invalidate_emt_range(struct p2m_domain *p2m,
* for entries not involved in the translation of the given GFN.
* Returns:
* - negative errno values in error,
- * - zero if no adjustment was done,
- * - a positive value if at least one adjustment was done.
+ * - zero for ok
*/
static int resolve_misconfig(struct p2m_domain *p2m, unsigned long gfn)
{
@@ -600,6 +599,9 @@ static int resolve_misconfig(struct p2m_domain *p2m,
unsigned long gfn)
v->arch.hvm.vmx.ept_spurious_misconfig = 1;
}
+if ( rc > 0 )
+rc = 0;
+
return rc;
}
@@ -621,7 +623,7 @@ bool_t ept_handle_misconfig(uint64_t gpa)
p2m_unlock(p2m);
-return spurious ? (rc >= 0) : (rc > 0);
+return spurious && !rc;
}
/*
@@ -668,7 +670,7 @@ ept_set_entry(struct p2m_domain *p2m, gfn_t gfn_, mfn_t mfn,
/* Carry out any eventually pending earlier changes first. */
ret = resolve_misconfig(p2m, gfn);
-if ( ret < 0 )
+if ( ret )
return ret;
ASSERT((target == 2 && hap_has_1gb) ||
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index b9bbb8f485..d5690b96bf 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1172,7 +1172,7 @@ static int finish_type_change(struct p2m_domain *p2m,
{
rc = p2m->recalc(p2m, gfn);
/*
- * ept->recalc could return 0/1/-ENOMEM. pt->recalc could return
+ * ept->recalc could return 0/-ENOMEM. pt->recalc could return
* 0/-ENOMEM/-ENOENT, -ENOENT isn't an error as we are looping
* gfn here.
*/
@@ -1201,7 +1201,7 @@ int p2m_finish_type_change(struct domain *d,
rc = finish_type_change(hostp2m, first_gfn, max_nr);
-if ( rc < 0 )
+if ( rc )
goto out;
#ifdef CONFIG_HVM
@@ -1213,22 +1213,17 @@ int p2m_finish_type_change(struct domain *d,
if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
{
struct p2m_domain *altp2m = d->arch.altp2m_p2m[i];
-int rc1;
p2m_lock(altp2m);
-rc1 = finish_type_change(altp2m, first_gfn, max_nr);
+rc = finish_type_change(altp2m, first_gfn, max_nr);
p2m_unlock(altp2m);
-if ( rc1 < 0 )
-{
-rc = rc1;
+if ( rc < 0 )
goto out;
-}
-
-rc |= rc1;
}
}
#endif
+rc = 0;
out:
p2m_unlock(hostp2m);
--
2.17.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/4] libx86: Introduce x86_cpuid_lookup_vendor()
On 26/03/2019 14:39, Jan Beulich wrote: On 26.03.19 at 15:23, wrote: >> IMO especially in the CPUID case it is desirable to explicitly specify >> the width of the data. Looking at nodes 0x8002 and following this >> should be rather clear (and I even think get_model_name() should be >> modified to use a pointer to uint32_t instead of unsigned int). Using >> a type with size >= 4 doesn't fit really well. You want size == 4. > Why? Fixed width types only introduce unnecessary restrictions > when wanting to re-use code in other environments. And I don't > see why CPUID nodes 0x800[234] would be any better (or > worse) as an example here. If anything they tell us that neither > uint32_t nor unsigned int are right, and it should be char[4] or > uint8_t[4] instead (depending on whether we want to tie > ourselves to CHAR_BIT == 8, which clearly is more restrictive > than sizeof(int) >= 4, but otoh is also less likely to get in the > way). The ABI of CPUID really is 2x uint32_t input, 4x uint32_t output. It is only by current convention that the data is also valid ASCII, and there is absolutely nothing which prevents a new vendor choosing to put non-ASCII content in their id string. Given the recent developments from China, I'm slightly surprised that we haven't seen any UTF-8 (or UTF-16 for that matter) yet. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/6] xen/sched: call cpu_disable_scheduler() via cpu notifier
On 3/18/19 1:11 PM, Juergen Gross wrote: > cpu_disable_scheduler() is being called from __cpu_disable() today. > There is no need to call it on the cpu just being disabled, so use > the CPU_DEAD case of the cpu notifier chain. > > Signed-off-by: Juergen Gross Scheduler change: Acked-by: George Dunlap ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.12] passthrough/vtd: Drop the "workaround_bios_bug" logic entirely
>>> On 27.03.19 at 15:38, wrote: > There is absolutely nothing *at all* which guarantees that just because > a number of devices are identified to be behind specific IOMMUs, that > DMA wont start appearing from elsewhere in the system. I fully agree here. > Security of the system when it comes to IOMMUs *is and always will be* a > mutually cooperative and trusting relationship between Xen and the firmware. > > The notion of "I'm safe because there were no inconsistencies in a piece > of information I have to trust fully" is security theatre, not security. Doing our best to sanity check information we're handed is, I think, not just "security theater". >>> Disabling the IOMMU prevents the system from booting with a PVH dom0. >> But doing what you did is not the only way of getting around this. >> Defaulting to workaround_bios_bug=1 in the PVH case would be >> another, as would be a mode in which the IOMMU exists for Dom0's >> purposes only (i.e. still disallowing any pass-through to DomU-s). > > A discussion along these lines might be appropriate in the middle of a > dev cycle, and might even be valid for a discussion of future improvements. > > It is not appropriate for resolving an issue identified as a 4.12 > blocker by the RM, on a timescale which needs to fit into the 4.12 > release plans. Okay, I clearly must have missed the mail where this was flagged as release critical. Irrespective of this I don't think, though, that a pending release is sufficient justification to rush in a controversial patch. We're yet to hear from Kevin, but as you can see I would not have ack- ed the patch. Emergency ack-s from The Rest maintainers ought to be given on the assumption that the actual maintainer(s) would likely not object. I don't think George tried to violate this, i.e. I think he was assuming that the maintainer(s) would agree, but as we see this was wrong in this case. > And if you'd not broken the behaviour back in 4.2, this class of system > would have been discovered the first time someone tried booting Xen on > it, not at the point someone is trying to upgrade 4.11 to 4.12. Correct, and I take the blame for this, but I don't think it helps the situation. If the problem had been discovered earlier, I don't think the fix would have been to rip out that code altogether. >>> I have made a statement, backed up with specific reference to the code >>> which, to the best of my ability, demonstrates it to be true. >>> >>> If you believe contrary then clearly identify the fault in my reasoning. >> I did, by pointing out the earlier regression, which you elected to >> ignore altogether in your reply. > > You identified why Xen 4.11 didn't behave in the way you expected. > > In doing so, you also demonstrated why the commit message was, in fact, > correct. Well, we continue to disagree here. It was at best misleading and/or incomplete. > Like other parts of this thread, it was deviating sufficiently > off-topic/relevance that I chose to trim it. I will continue doing so > in an effort to reduce the amount of my time that this thread is > wasting. I don't know for certain, but I expect you've also got better > things to do with your time than arguing over off-topic aspects of this > thread. Purely from a technical pov the discussion on whether this should have been rushed in may indeed be considered off topic. But this doesn't means it's irrelevant. Would you have liked it better if I had started a separate thread, e.g. by formally requesting a revert? I can understand your frustration, but it's not you alone who is frustrated. Throughout this discussion I've not seen a single sign that you would be willing to find a compromise. I'm sorry to repeat myself, but it imo is not reasonable to assume that your way of thinking is the only possible or reasonable one. Claiming that anything else is beyond "common sense" is, well, offending. And yes, I do have better things to do with my time. But I don't think I can leave uncommented how things have gone here, if nothing else then in the hopes that it wouldn't repeat again. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH V2 00/10] block: enable multi-page bvec for passthrough IO
On 3/17/19 4:01 AM, Ming Lei wrote: > Hi, > > Now the whole IO stack is capable of handling multi-page bvec, and it has > been enabled in the normal FS IO path. However, it isn't done for > passthrough IO. > > Without enabling multi-bvec for passthough IO, we won't go ahead for > optimizing related IO paths, such as bvec merging, bio_add_pc_page > simplification. > > This patch enables multi-page bvec for passthrough IO. Turns out > bio_add_pc_page() is simpliefied a lot, especially the physical segment > number of passthrough bio is always same with bio.bi_vcnt. Also the > bvec merging inside bio is killed. > > blktests(block/029) is added for covering passthough IO path, and this > patchset does pass the new block/029 test. > > https://marc.info/?l=linux-block&m=155175063417139&w=2 Merged for 5.2, thanks Ming. -- Jens Axboe ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/6] xen: add new cpu notifier action CPU_RESUME_FAILED
On 3/18/19 1:11 PM, Juergen Gross wrote: > Add a new cpu notifier action CPU_RESUME_FAILED which is called for all > cpus which failed to come up at resume. The calls will be done after > all other cpus are already up. > > Signed-off-by: Juergen Gross Reviewed-by: George Dunlap ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/6] xen/sched: call cpu_disable_scheduler() via cpu notifier
On 18/03/2019 13:11, Juergen Gross wrote:
> cpu_disable_scheduler() is being called from __cpu_disable() today.
> There is no need to call it on the cpu just being disabled, so use
> the CPU_DEAD case of the cpu notifier chain.
>
> Signed-off-by: Juergen Gross
> ---
> xen/arch/x86/smpboot.c | 3 ---
> xen/common/schedule.c | 12 +---
> 2 files changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
> index 7d1226d7bc..b7a0a4a419 100644
> --- a/xen/arch/x86/smpboot.c
> +++ b/xen/arch/x86/smpboot.c
> @@ -1221,9 +1221,6 @@ void __cpu_disable(void)
> cpumask_clear_cpu(cpu, &cpu_online_map);
> fixup_irqs(&cpu_online_map, 1);
> fixup_eoi();
> -
> -if ( cpu_disable_scheduler(cpu) )
> -BUG();
> }
It looks like ARM needs an equivalent adjustment.
>
> void __cpu_die(unsigned int cpu)
> diff --git a/xen/common/schedule.c b/xen/common/schedule.c
> index 60755a631e..665747f247 100644
> --- a/xen/common/schedule.c
> +++ b/xen/common/schedule.c
> @@ -773,8 +773,9 @@ void restore_vcpu_affinity(struct domain *d)
> }
>
> /*
> - * This function is used by cpu_hotplug code from stop_machine context
> + * This function is used by cpu_hotplug code via cpu notifier chain
> * and from cpupools to switch schedulers on a cpu.
> + * Caller must get domlist_read_lock.
s/get/hold/ ?
With at least the ARM side adjusted, Reviewed-by: Andrew Cooper
> */
> int cpu_disable_scheduler(unsigned int cpu)
> {
> @@ -789,12 +790,6 @@ int cpu_disable_scheduler(unsigned int cpu)
> if ( c == NULL )
> return ret;
>
> -/*
> - * We'd need the domain RCU lock, but:
> - * - when we are called from cpupool code, it's acquired there already;
> - * - when we are called for CPU teardown, we're in stop-machine context,
> - *so that's not be a problem.
> - */
> for_each_domain_in_cpupool ( d, c )
> {
> for_each_vcpu ( d, v )
> @@ -1738,6 +1733,9 @@ static int cpu_schedule_callback(
> rc = cpu_schedule_up(cpu);
> break;
> case CPU_DEAD:
> +rcu_read_lock(&domlist_read_lock);
> +cpu_disable_scheduler(cpu);
> +rcu_read_unlock(&domlist_read_lock);
> SCHED_OP(sched, deinit_pdata, sd->sched_priv, cpu);
> /* Fallthrough */
> case CPU_UP_CANCELED:
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/6] xen: add new cpu notifier action CPU_RESUME_FAILED
On Mon, 2019-03-25 at 13:29 +0100, Juergen Gross wrote: > On 25/03/2019 13:21, Dario Faggioli wrote: > > > Reviewed-by: Dario Faggioli > > > > One more (minor) thing... > > > > > /* CPU_REMOVE: CPU was removed. */ > > > -#define CPU_REMOVE (0x0009 | NOTIFY_REVERSE) > > > +#define CPU_REMOVE(0x0009 | NOTIFY_REVERSE) > > > +/* CPU_RESUME_FAILED: CPU failed to come up in resume, all other > > > CPUs up. */ > > > +#define CPU_RESUME_FAILED (0x000a | NOTIFY_REVERSE) > > > > > ... technically, when we're dealing with CPU_RESUME_FAILED on one > > CPU, > > we don't know if _all_ others really went up, so I think I'd remove > > what follows the ','. > > The point is that for the CPU_RESUME_FAILED case we can be sure that > no > cpu will come up due to resume just a little bit later. > Ah, I see what you mean... that's the fact that this notifier is invoked from another loop, and although we don't know which CPU did manage to come up and which don't, we do know that all the ones that could come up, are up already. > So we can test > for e.g. a cpupool suddenly having no more cpus available. This is in > contrast to CPU_UP_CANCELLED being signalled just after the one cpu > failing to come up, but before the next cpu is triggered to come up. > Right. Well, it still looks to me that "all other CPUs up" is not entirely accurate. But I can't propose a better alternative (unless we write something very long, which is probably not worth it). Perhaps you can explain this a little in another comment, like in enable_nonboot_cpus(), before the for_each_cpu() loop itself. But I don't feel too strong about that, and the RoB stands, whatever you decide to do. Regards, Dario -- <> (Raistlin Majere) - Dario Faggioli, Ph.D, http://about.me/dario.faggioli Software Engineer @ SUSE https://www.suse.com/ ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 5/6] xen/cpupool: simplify suspend/resume handling
On 3/18/19 1:11 PM, Juergen Gross wrote:
> Instead of removing cpus temporarily from cpupools during
> suspend/resume only remove cpus finally which didn't come up when
> resuming.
>
> Signed-off-by: Juergen Gross
Looks good overall -- one comment...
> @@ -774,10 +741,15 @@ static int cpu_callback(
> {
> case CPU_DOWN_FAILED:
> case CPU_ONLINE:
> -rc = cpupool_cpu_add(cpu);
> +if ( system_state <= SYS_STATE_active )
> +rc = cpupool_cpu_add(cpu);
> break;
> case CPU_DOWN_PREPARE:
> -rc = cpupool_cpu_remove(cpu);
> +if ( system_state <= SYS_STATE_active )
> +rc = cpupool_cpu_remove(cpu);
> +break;
> +case CPU_RESUME_FAILED:
> +cpupool_cpu_remove_forced(cpu);
> break;
> default:
It would be good to have some comments here just explaining this; maybe
just to the effect of, "Suspend/resume operations don't affect cpupool
placement".
With that:
Reviewed-by: George Dunlap
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 2/6] xen: add helper for calling notifier_call_chain() to common/cpu.c
On 18/03/2019 13:11, Juergen Gross wrote:
> Add a helper cpu_notifier_call_chain() to call notifier_call_chain()
> for a cpu with a specified action, returning an errno value.
>
> This avoids coding the same pattern multiple times.
>
> Signed-off-by: Juergen Gross
> ---
> xen/common/cpu.c | 50 +-
> 1 file changed, 21 insertions(+), 29 deletions(-)
>
> diff --git a/xen/common/cpu.c b/xen/common/cpu.c
> index 836c62f97f..c436c0de7f 100644
> --- a/xen/common/cpu.c
> +++ b/xen/common/cpu.c
> @@ -71,11 +71,18 @@ void __init register_cpu_notifier(struct notifier_block
> *nb)
> spin_unlock(&cpu_add_remove_lock);
> }
>
> +static int cpu_notifier_call_chain(unsigned int cpu, unsigned long action,
> + struct notifier_block **nb)
> +{
> +void *hcpu = (void *)(long)cpu;
> +int notifier_rc = notifier_call_chain(&cpu_chain, action, hcpu, nb);
> +
> +return (notifier_rc == NOTIFY_DONE) ? 0 : notifier_to_errno(notifier_rc);
> +}
> +
> static void _take_cpu_down(void *unused)
> {
> -void *hcpu = (void *)(long)smp_processor_id();
> -int notifier_rc = notifier_call_chain(&cpu_chain, CPU_DYING, hcpu, NULL);
> -BUG_ON(notifier_rc != NOTIFY_DONE);
> +BUG_ON(cpu_notifier_call_chain(smp_processor_id(), CPU_DYING, NULL));
Where possible, we're trying to remove side effects from macros.
Could I please talk you into writing this as:
int rc = cpu_notifier_call_chain(smp_processor_id(), CPU_DYING, NULL);
BUG_ON(rc);
An alternative might be to have a:
static void cpu_notifier_call_chain_nofail(...)
wrapper as this seems to be a common pattern. (Ideally longterm, it
might be better to pass the nofail intention into the notifier chain
itself so we can identify which callback had a problem, but thats
definitely not something for here.)
~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH RFCv2 0/4] mm/memory_hotplug: Introduce memory block types
On 20.12.18 14:08, Michal Hocko wrote:
> On Thu 20-12-18 13:58:16, David Hildenbrand wrote:
>> On 30.11.18 18:59, David Hildenbrand wrote:
>>> This is the second approach, introducing more meaningful memory block
>>> types and not changing online behavior in the kernel. It is based on
>>> latest linux-next.
>>>
>>> As we found out during dicussion, user space should always handle onlining
>>> of memory, in any case. However in order to make smart decisions in user
>>> space about if and how to online memory, we have to export more information
>>> about memory blocks. This way, we can formulate rules in user space.
>>>
>>> One such information is the type of memory block we are talking about.
>>> This helps to answer some questions like:
>>> - Does this memory block belong to a DIMM?
>>> - Can this DIMM theoretically ever be unplugged again?
>>> - Was this memory added by a balloon driver that will rely on balloon
>>> inflation to remove chunks of that memory again? Which zone is advised?
>>> - Is this special standby memory on s390x that is usually not automatically
>>> onlined?
>>>
>>> And in short it helps to answer to some extend (excluding zone imbalances)
>>> - Should I online this memory block?
>>> - To which zone should I online this memory block?
>>> ... of course special use cases will result in different anwers. But that's
>>> why user space has control of onlining memory.
>>>
>>> More details can be found in Patch 1 and Patch 3.
>>> Tested on x86 with hotplugged DIMMs. Cross-compiled for PPC and s390x.
>>>
>>>
>>> Example:
>>> $ udevadm info -q all -a /sys/devices/system/memory/memory0
>>> KERNEL=="memory0"
>>> SUBSYSTEM=="memory"
>>> DRIVER==""
>>> ATTR{online}=="1"
>>> ATTR{phys_device}=="0"
>>> ATTR{phys_index}==""
>>> ATTR{removable}=="0"
>>> ATTR{state}=="online"
>>> ATTR{type}=="boot"
>>> ATTR{valid_zones}=="none"
>>> $ udevadm info -q all -a /sys/devices/system/memory/memory90
>>> KERNEL=="memory90"
>>> SUBSYSTEM=="memory"
>>> DRIVER==""
>>> ATTR{online}=="1"
>>> ATTR{phys_device}=="0"
>>> ATTR{phys_index}=="005a"
>>> ATTR{removable}=="1"
>>> ATTR{state}=="online"
>>> ATTR{type}=="dimm"
>>> ATTR{valid_zones}=="Normal"
>>>
>>>
>>> RFC -> RFCv2:
>>> - Now also taking care of PPC (somehow missed it :/ )
>>> - Split the series up to some degree (some ideas on how to split up patch 3
>>> would be very welcome)
>>> - Introduce more memory block types. Turns out abstracting too much was
>>> rather confusing and not helpful. Properly document them.
>>>
>>> Notes:
>>> - I wanted to convert the enum of types into a named enum but this
>>> provoked all kinds of different errors. For now, I am doing it just like
>>> the other types (e.g. online_type) we are using in that context.
>>> - The "removable" property should never have been named like that. It
>>> should have been "offlinable". Can we still rename that? E.g. boot memory
>>> is sometimes marked as removable ...
>>>
>>
>>
>> Any feedback regarding the suggested block types would be very much
>> appreciated!
>
> I still do not like this much to be honest. I just didn't get to think
> through this properly. My fear is that this is conflating an actual API
> with the current implementation and as such will cause problems in
> future. But I haven't really looked into your patches closely so I might
> be wrong. Anyway I won't be able to look into it by the end of year.
>
So I started to think about this again, and I guess somehow exposing an
identification of the device driver that added the memory section could
be sufficient.
E.g. "hyperv", "xen", "acpi", "sclp", "virtio-mem" ...
Via separate device driver interfaces, other information about the
memory could be exposed. (e.g. for ACPI: which memory devices belong to
one physical device). So stuff would not have to centered around
/sys/devices/system/memory/ , uglifying it for special cases.
We would have to write udev rules to deal with these values, should be
easy. If no DRIVER is given, it is simply memory detected and detected
during boot. ACPI changing the DRIVER might be tricky (from no DRIVER ->
ACPI), but I guess it could be done.
Now, the question would be how to get the DRIVER value in there. Adding
a bunch of fake device drivers would work, however this might get a
little messy ... and then there is unbining and rebinding which can be
triggered by userspace. Thinks to care about? Most probably not.
--
Thanks,
David / dhildenb
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 2/6] xen: add helper for calling notifier_call_chain() to common/cpu.c
On 27/03/2019 16:39, Andrew Cooper wrote:
> On 18/03/2019 13:11, Juergen Gross wrote:
>> Add a helper cpu_notifier_call_chain() to call notifier_call_chain()
>> for a cpu with a specified action, returning an errno value.
>>
>> This avoids coding the same pattern multiple times.
>>
>> Signed-off-by: Juergen Gross
>> ---
>> xen/common/cpu.c | 50 +-
>> 1 file changed, 21 insertions(+), 29 deletions(-)
>>
>> diff --git a/xen/common/cpu.c b/xen/common/cpu.c
>> index 836c62f97f..c436c0de7f 100644
>> --- a/xen/common/cpu.c
>> +++ b/xen/common/cpu.c
>> @@ -71,11 +71,18 @@ void __init register_cpu_notifier(struct notifier_block
>> *nb)
>> spin_unlock(&cpu_add_remove_lock);
>> }
>>
>> +static int cpu_notifier_call_chain(unsigned int cpu, unsigned long action,
>> + struct notifier_block **nb)
>> +{
>> +void *hcpu = (void *)(long)cpu;
>> +int notifier_rc = notifier_call_chain(&cpu_chain, action, hcpu, nb);
>> +
>> +return (notifier_rc == NOTIFY_DONE) ? 0 :
>> notifier_to_errno(notifier_rc);
>> +}
>> +
>> static void _take_cpu_down(void *unused)
>> {
>> -void *hcpu = (void *)(long)smp_processor_id();
>> -int notifier_rc = notifier_call_chain(&cpu_chain, CPU_DYING, hcpu,
>> NULL);
>> -BUG_ON(notifier_rc != NOTIFY_DONE);
>> +BUG_ON(cpu_notifier_call_chain(smp_processor_id(), CPU_DYING, NULL));
>
> Where possible, we're trying to remove side effects from macros.
>
> Could I please talk you into writing this as:
>
> int rc = cpu_notifier_call_chain(smp_processor_id(), CPU_DYING, NULL);
>
> BUG_ON(rc);
Fine with me.
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-4.4 test] 134102: trouble: blocked/broken/fail/pass
flight 134102 linux-4.4 real [real] http://logs.test-lab.xenproject.org/osstest/logs/134102/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64 broken build-arm64-xsm broken build-arm64-pvopsbroken build-arm64 4 host-install(4)broken REGR. vs. 133468 build-arm64-xsm 4 host-install(4)broken REGR. vs. 133468 build-arm64-pvops 4 host-install(4)broken REGR. vs. 133468 Tests which are failing intermittently (not blocking): test-amd64-amd64-examine4 memdisk-try-append fail in 134072 pass in 134102 test-amd64-i386-examine 8 reboot fail pass in 134072 test-amd64-i386-freebsd10-amd64 7 xen-bootfail pass in 134072 Tests which did not succeed, but are not blocking: build-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit1 1 build-check(1) blocked n/a test-arm64-arm64-xl 1 build-check(1) blocked n/a test-arm64-arm64-examine 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-amd64-xl-pvhv2-amd 12 guest-start fail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass te
Re: [Xen-devel] [PATCH RFC v4] x86/mm: Clean up p2m_finish_type_change return value
>>> On 27.03.19 at 16:21, wrote: > @@ -621,7 +623,7 @@ bool_t ept_handle_misconfig(uint64_t gpa) > > p2m_unlock(p2m); > > -return spurious ? (rc >= 0) : (rc > 0); > +return spurious && !rc; > } I think you've gone too far now: This is - afaict - the one place where the distinction matters. Looking back at Paul's reply and my subsequent one on v3, I'm afraid I've managed to misguide you by not looking closely enough at what Paul did sketch out. I'm sorry for this. I think you either want to leave EPT code untouched, and zap the positive return value in finish_type_change() instead. Or EPT's resolve_misconfig() would need to gain a wrapper for use as the ->recalc hook, to squash the positive value for the outside world. Iirc I've avoided introducing such a wrapper originally just to limit the number of functions layered on top of one another, while using resolve_misconfig() directly appeared to be fine. > @@ -668,7 +670,7 @@ ept_set_entry(struct p2m_domain *p2m, gfn_t gfn_, mfn_t > mfn, > > /* Carry out any eventually pending earlier changes first. */ > ret = resolve_misconfig(p2m, gfn); > -if ( ret < 0 ) > +if ( ret ) > return ret; This would then need undoing as well. > @@ -1201,7 +1201,7 @@ int p2m_finish_type_change(struct domain *d, > > rc = finish_type_change(hostp2m, first_gfn, max_nr); > > -if ( rc < 0 ) > +if ( rc ) > goto out; While I don't really object to this change, I also don't think it's strictly necessary. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 4/6] xen: don't free percpu areas during suspend
On 18/03/2019 13:11, Juergen Gross wrote: > Instead of freeing percpu areas during suspend and allocating them > again when resuming keep them. Only free an area in case a cpu didn't > come up again when resuming. > > Signed-off-by: Juergen Gross Hmm - this is slightly problematic, given the dual nature of this code. I agree that it this change is beneficial for the suspend case, but it is a problem when we are parking an individual CPU for smt=0 or xen-hptool reasons. Do we have any hint we can use when taking the CPU down as to whether we're expecting it to come straight back up again? ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 4/6] xen: don't free percpu areas during suspend
On 27/03/2019 16:55, Andrew Cooper wrote: > On 18/03/2019 13:11, Juergen Gross wrote: >> Instead of freeing percpu areas during suspend and allocating them >> again when resuming keep them. Only free an area in case a cpu didn't >> come up again when resuming. >> >> Signed-off-by: Juergen Gross > > Hmm - this is slightly problematic, given the dual nature of this code. > > I agree that it this change is beneficial for the suspend case, but it > is a problem when we are parking an individual CPU for smt=0 or > xen-hptool reasons. > > Do we have any hint we can use when taking the CPU down as to whether > we're expecting it to come straight back up again? Did you look into the patch? I did this by testing system_state. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/6] xen/sched: call cpu_disable_scheduler() via cpu notifier
>>> On 18.03.19 at 14:11, wrote: > @@ -1738,6 +1733,9 @@ static int cpu_schedule_callback( > rc = cpu_schedule_up(cpu); > break; > case CPU_DEAD: > +rcu_read_lock(&domlist_read_lock); > +cpu_disable_scheduler(cpu); > +rcu_read_unlock(&domlist_read_lock); > SCHED_OP(sched, deinit_pdata, sd->sched_priv, cpu); > /* Fallthrough */ > case CPU_UP_CANCELED: cpu_disable_scheduler() has a return value (and hence means to fail) - is ignoring this here really appropriate? Also while indeed (as the description says) there's no need to run the function on the CPU itself, it's not obvious to me that it's safe to run it outside of stop_machine() context. Or to be more precise, it's not clear to me that leaving stop_machine() context with the adjustments not done yet is not going to lead to problems (due to the gap between leaving that context and acquiring the RCU lock). Could you clarify this in the description, please (if it indeed is fine this way)? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/6] xen/sched: call cpu_disable_scheduler() via cpu notifier
On Mon, 2019-03-18 at 14:11 +0100, Juergen Gross wrote: > cpu_disable_scheduler() is being called from __cpu_disable() today. > There is no need to call it on the cpu just being disabled, so use > the CPU_DEAD case of the cpu notifier chain. > So, what do you mean with "There is no need to call it on the cpu just being disabled"? Because we still (even after this patch, I mean) call cpu_disable_scheduler() on all non-boot CPUs, aren't we? It's just that right now we call it from __cpu_disable(), with the patch we call it slightly later. And another difference looks to me to be that right now we call cpu_disable_scheduler() from stop-machine context, which I think is no longer true with this patch. Perhaps the changelog could tell why that is ok? Regards, Dario -- <> (Raistlin Majere) - Dario Faggioli, Ph.D, http://about.me/dario.faggioli Software Engineer @ SUSE https://www.suse.com/ signature.asc Description: This is a digitally signed message part ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/6] xen: add new cpu notifier action CPU_RESUME_FAILED
>>> On 18.03.19 at 14:11, wrote:
> --- a/xen/common/cpu.c
> +++ b/xen/common/cpu.c
> @@ -214,7 +214,12 @@ void enable_nonboot_cpus(void)
> printk("Error bringing CPU%d up: %d\n", cpu, error);
> BUG_ON(error == -EBUSY);
> }
> +else
> +__cpumask_clear_cpu(cpu, &frozen_cpus);
> }
>
> +for_each_cpu ( cpu, &frozen_cpus )
> +BUG_ON(cpu_notifier_call_chain(cpu, CPU_RESUME_FAILED, NULL));
> +
> cpumask_clear(&frozen_cpus);
> }
Is there a particular reason you add a second loop here?
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/6] xen/sched: call cpu_disable_scheduler() via cpu notifier
On 27/03/2019 17:24, Dario Faggioli wrote: > On Mon, 2019-03-18 at 14:11 +0100, Juergen Gross wrote: >> cpu_disable_scheduler() is being called from __cpu_disable() today. >> There is no need to call it on the cpu just being disabled, so use >> the CPU_DEAD case of the cpu notifier chain. >> > So, what do you mean with "There is no need to call it on the cpu just > being disabled"? > > Because we still (even after this patch, I mean) call > cpu_disable_scheduler() on all non-boot CPUs, aren't we? It's just that > right now we call it from __cpu_disable(), with the patch we call it > slightly later. The CPU_DEAD notifier chain is called on the CPU requesting the other one to go down (so on the boot CPU in suspend case). So we call it _for_ all non-boot CPUs in the boot CPU. > And another difference looks to me to be that right now we call > cpu_disable_scheduler() from stop-machine context, which I think is no > longer true with this patch. Perhaps the changelog could tell why that > is ok? Okay, I'll add something. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/6] xen: add new cpu notifier action CPU_RESUME_FAILED
On 27/03/2019 17:29, Jan Beulich wrote:
On 18.03.19 at 14:11, wrote:
>> --- a/xen/common/cpu.c
>> +++ b/xen/common/cpu.c
>> @@ -214,7 +214,12 @@ void enable_nonboot_cpus(void)
>> printk("Error bringing CPU%d up: %d\n", cpu, error);
>> BUG_ON(error == -EBUSY);
>> }
>> +else
>> +__cpumask_clear_cpu(cpu, &frozen_cpus);
>> }
>>
>> +for_each_cpu ( cpu, &frozen_cpus )
>> +BUG_ON(cpu_notifier_call_chain(cpu, CPU_RESUME_FAILED, NULL));
>> +
>> cpumask_clear(&frozen_cpus);
>> }
>
> Is there a particular reason you add a second loop here?
Yes, I want to know which cpus did come up again.
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 5/6] xen/cpupool: simplify suspend/resume handling
On Mon, 2019-03-18 at 14:11 +0100, Juergen Gross wrote: > Instead of removing cpus temporarily from cpupools during > suspend/resume only remove cpus finally which didn't come up when > resuming. > > Signed-off-by: Juergen Gross > --- > xen/common/cpupool.c | 130 ++- > -- > xen/include/xen/sched-if.h | 1 - > 2 files changed, 51 insertions(+), 80 deletions(-) > Cool diffstat! :-) And I'm particularly happy to see 'c->cpu_suspended' go away. ;-D Reviewed-by: Dario Faggioli Regards, Dario -- <> (Raistlin Majere) - Dario Faggioli, Ph.D, http://about.me/dario.faggioli Software Engineer @ SUSE https://www.suse.com/ signature.asc Description: This is a digitally signed message part ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 4/6] xen: don't free percpu areas during suspend
>>> On 27.03.19 at 17:18, wrote: > On 27/03/2019 16:55, Andrew Cooper wrote: >> On 18/03/2019 13:11, Juergen Gross wrote: >>> Instead of freeing percpu areas during suspend and allocating them >>> again when resuming keep them. Only free an area in case a cpu didn't >>> come up again when resuming. >>> >>> Signed-off-by: Juergen Gross >> >> Hmm - this is slightly problematic, given the dual nature of this code. >> >> I agree that it this change is beneficial for the suspend case, but it >> is a problem when we are parking an individual CPU for smt=0 or >> xen-hptool reasons. >> >> Do we have any hint we can use when taking the CPU down as to whether >> we're expecting it to come straight back up again? > > Did you look into the patch? I did this by testing system_state. I think there's a wider problem here: enable_nonboot_cpus() only brings back up the CPUs that were previously online. Parked ones would be left alone, yet after resume they'd need to be put back into parked state. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH RFC v4] x86/mm: Clean up p2m_finish_type_change return value
On 3/27/19 3:21 PM, Alexandru Stefan ISAILA wrote: > In the case of any errors, finish_type_change() passes values returned > from p2m->recalc() up the stack (with some exceptions in the case where > an error is expected); this eventually ends up being returned to the > XEN_DOMOP_map_mem_type_to_ioreq_server hypercall. > > However, on Intel processors (but not on AMD processor), p2m->recalc() > can also return '1' as well as '0'. This case is handled very > inconsistently: finish_type_change() will return the value of the final > entry it attempts, discarding results for other entries; > p2m_finish_type_change() will attempt to accumulate '1's, so that it > returns '1' if any of the calls to finish_type_change() returns '1'; and > dm_op() will again return '1' only if the very last call to > p2m_finish_type_change() returns '1'. The result is that the > XEN_DMOP_map_mem_type_to_ioreq_server() hypercall will sometimes return > 0 and sometimes return 1 on success, in an unpredictable manner. > > The hypercall documentation doesn't mention return values; but it's not > clear what the caller could do with the information about whether > entries had been changed or not. At the moment it's always 0 on AMD > boxes, and *usually* 1 on Intel boxes; so nothing can be relying on a > '1' return value for correctness (or if it is, it's broken). > > Make the return value on success consistently '0' by only returning > 0/-ERROR from finish_type_change(). Also remove the accumulation code > from p2m_finish_type_change(). Thanks for putting in the effort to clean this up. One comment: this is the second instance of the patch you posted where this paragraph is not true. What I wrote was meant to be an example of how a good patch description should be written, which includes a sketch of what the patch does technically. You need to make sure the sketch matches the patch. As it happens, it looks like you'll have to modify the patch such that v5 actually matches the description again. :-) Also, you probably should have dropped the RFC at v2. -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/6] xen/sched: call cpu_disable_scheduler() via cpu notifier
On 27/03/2019 17:22, Jan Beulich wrote: On 18.03.19 at 14:11, wrote: >> @@ -1738,6 +1733,9 @@ static int cpu_schedule_callback( >> rc = cpu_schedule_up(cpu); >> break; >> case CPU_DEAD: >> +rcu_read_lock(&domlist_read_lock); >> +cpu_disable_scheduler(cpu); >> +rcu_read_unlock(&domlist_read_lock); >> SCHED_OP(sched, deinit_pdata, sd->sched_priv, cpu); >> /* Fallthrough */ >> case CPU_UP_CANCELED: > > cpu_disable_scheduler() has a return value (and hence means to > fail) - is ignoring this here really appropriate? You are right, I should handle those cases in cpu_disable_scheduler(). Without the complete series committed this will result in a case not handled correctly: dom0 trying to pin a vcpu to a physical cpu other than cpu 0 via SCHEDOP_pin_override and suspending in that state. I'm not aware of any dom0 trying to do that. > Also while indeed (as the description says) there's no need to > run the function on the CPU itself, it's not obvious to me that > it's safe to run it outside of stop_machine() context. Or to be > more precise, it's not clear to me that leaving stop_machine() > context with the adjustments not done yet is not going to > lead to problems (due to the gap between leaving that context > and acquiring the RCU lock). Could you clarify this in the > description, please (if it indeed is fine this way)? It is fine, as the chances are zero that any code will run on the cpu just taken down and that cpu is not holding any locks we might need. I'll add that to the commit message. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 4/6] xen: don't free percpu areas during suspend
On 27/03/2019 17:38, Jan Beulich wrote: On 27.03.19 at 17:18, wrote: >> On 27/03/2019 16:55, Andrew Cooper wrote: >>> On 18/03/2019 13:11, Juergen Gross wrote: Instead of freeing percpu areas during suspend and allocating them again when resuming keep them. Only free an area in case a cpu didn't come up again when resuming. Signed-off-by: Juergen Gross >>> >>> Hmm - this is slightly problematic, given the dual nature of this code. >>> >>> I agree that it this change is beneficial for the suspend case, but it >>> is a problem when we are parking an individual CPU for smt=0 or >>> xen-hptool reasons. >>> >>> Do we have any hint we can use when taking the CPU down as to whether >>> we're expecting it to come straight back up again? >> >> Did you look into the patch? I did this by testing system_state. > > I think there's a wider problem here: enable_nonboot_cpus() > only brings back up the CPUs that were previously online. > Parked ones would be left alone, yet after resume they'd > need to be put back into parked state. I can add that handling in the respin of the series. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/6] xen/sched: call cpu_disable_scheduler() via cpu notifier
On Wed, 2019-03-27 at 17:31 +0100, Juergen Gross wrote: > On 27/03/2019 17:24, Dario Faggioli wrote: > > On Mon, 2019-03-18 at 14:11 +0100, Juergen Gross wrote: > > > cpu_disable_scheduler() is being called from __cpu_disable() > > > today. > > > There is no need to call it on the cpu just being disabled, so > > > use > > > the CPU_DEAD case of the cpu notifier chain. > > > > > So, what do you mean with "There is no need to call it on the cpu > > just > > being disabled"? > > > > Because we still (even after this patch, I mean) call > > cpu_disable_scheduler() on all non-boot CPUs, aren't we? It's just > > that > > right now we call it from __cpu_disable(), with the patch we call > > it > > slightly later. > > The CPU_DEAD notifier chain is called on the CPU requesting the other > one to go down (so on the boot CPU in suspend case). So we call it > _for_ > all non-boot CPUs in the boot CPU. > Mmm... ok, I see what you mean now. I guess part of "the problem" is that "call func on cpu A" reads, at least to me, as both 1) call func so that it acts on and change the state of cpu A, and 2) call func in such a way that it executes on cpu A. But I'm no native speaker, so it may very well be that the confusion is all and only mine. Dario -- <> (Raistlin Majere) - Dario Faggioli, Ph.D, http://about.me/dario.faggioli Software Engineer @ SUSE https://www.suse.com/ signature.asc Description: This is a digitally signed message part ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/6] xen/sched: call cpu_disable_scheduler() via cpu notifier
On 27/03/2019 17:51, Dario Faggioli wrote: > On Wed, 2019-03-27 at 17:31 +0100, Juergen Gross wrote: >> On 27/03/2019 17:24, Dario Faggioli wrote: >>> On Mon, 2019-03-18 at 14:11 +0100, Juergen Gross wrote: cpu_disable_scheduler() is being called from __cpu_disable() today. There is no need to call it on the cpu just being disabled, so use the CPU_DEAD case of the cpu notifier chain. >>> So, what do you mean with "There is no need to call it on the cpu >>> just >>> being disabled"? >>> >>> Because we still (even after this patch, I mean) call >>> cpu_disable_scheduler() on all non-boot CPUs, aren't we? It's just >>> that >>> right now we call it from __cpu_disable(), with the patch we call >>> it >>> slightly later. >> >> The CPU_DEAD notifier chain is called on the CPU requesting the other >> one to go down (so on the boot CPU in suspend case). So we call it >> _for_ >> all non-boot CPUs in the boot CPU. >> > Mmm... ok, I see what you mean now. > > I guess part of "the problem" is that "call func on cpu A" reads, at > least to me, as both 1) call func so that it acts on and change the > state of cpu A, and 2) call func in such a way that it executes on cpu > A. I'll rephrase to "execute func on cpu...". Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/6] xen/sched: call cpu_disable_scheduler() via cpu notifier
>>> On 27.03.19 at 17:45, wrote: > On 27/03/2019 17:22, Jan Beulich wrote: >> Also while indeed (as the description says) there's no need to >> run the function on the CPU itself, it's not obvious to me that >> it's safe to run it outside of stop_machine() context. Or to be >> more precise, it's not clear to me that leaving stop_machine() >> context with the adjustments not done yet is not going to >> lead to problems (due to the gap between leaving that context >> and acquiring the RCU lock). Could you clarify this in the >> description, please (if it indeed is fine this way)? > > It is fine, as the chances are zero that any code will run on the cpu > just taken down and that cpu is not holding any locks we might need. Well, of course nothing's going to run on that CPU anymore. But vCPU-s may still have associations with it, so what I'm worried about is e.g. something finding v->processor pointing at an offline CPU and getting confused. Another, more exotic (or should I say contrived) scenario might be a soft-online request coming very quickly after a prior soft-offline one, with this function not having got around to run yet. Or basically anything else that accesses the same state the function means to update (or use). Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/Xen: streamline (and fix) PV CPU enumeration
On 3/27/19 11:12 AM, Jan Beulich wrote:
> -
> -static void __init xen_filter_cpu_maps(void)
> +static void __init _get_smp_config(unsigned int early)
> {
> int i, rc;
> unsigned int subtract = 0;
>
> - if (!xen_initial_domain())
> + if (early)
> return;
>
> num_processors = 0;
Is there a reason to set_cpu_possible() here (not in the diff, but in
this routine)? This will be called from setup_arch() before
prefill_possible_map(), which will clear and then re-initialize
__cpu_possible_mask.
-boris
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/6] xen/sched: call cpu_disable_scheduler() via cpu notifier
On 27/03/2019 17:58, Jan Beulich wrote: On 27.03.19 at 17:45, wrote: >> On 27/03/2019 17:22, Jan Beulich wrote: >>> Also while indeed (as the description says) there's no need to >>> run the function on the CPU itself, it's not obvious to me that >>> it's safe to run it outside of stop_machine() context. Or to be >>> more precise, it's not clear to me that leaving stop_machine() >>> context with the adjustments not done yet is not going to >>> lead to problems (due to the gap between leaving that context >>> and acquiring the RCU lock). Could you clarify this in the >>> description, please (if it indeed is fine this way)? >> >> It is fine, as the chances are zero that any code will run on the cpu >> just taken down and that cpu is not holding any locks we might need. > > Well, of course nothing's going to run on that CPU anymore. > But vCPU-s may still have associations with it, so what I'm > worried about is e.g. something finding v->processor pointing > at an offline CPU and getting confused. Another, more exotic v->processor is allowed to have a stale value as long as the vcpu isn't running. > (or should I say contrived) scenario might be a soft-online > request coming very quickly after a prior soft-offline one, with > this function not having got around to run yet. Or basically > anything else that accesses the same state the function > means to update (or use). The CPU_DEAD notifier chain is activated before calling cpu_hotplug_done(). I don't see how an online request could make it in between. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 134127: trouble: blocked/broken/pass
flight 134127 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/134127/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-xsm broken build-arm64-xsm 4 host-install(4)broken REGR. vs. 133991 Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen 8eed571409a7f81ec9327cfa95d7c298333e22e4 baseline version: xen cb70a26f78848fe45f593f7ebc9cfaac760a791b Last test of basis 133991 2019-03-22 15:00:46 Z5 days Failing since134068 2019-03-25 12:00:51 Z2 days 10 attempts Testing same since 134104 2019-03-26 22:06:44 Z0 days3 attempts People who touched revisions under test: Andrew Cooper Wei Liu jobs: build-arm64-xsm broken build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm blocked test-amd64-amd64-xl-qemuu-debianhvm-i386 pass 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 broken-job build-arm64-xsm broken broken-step build-arm64-xsm host-install(4) Not pushing. commit 8eed571409a7f81ec9327cfa95d7c298333e22e4 Author: Andrew Cooper Date: Tue Mar 26 14:23:03 2019 + CI: Add a CentOS 6 container and build jobs CentOS 6 is probably the most frequently broken build, so adding it to CI would be a very good move. One problem is that CentOS 6 comes with Python 2.6, and Qemu requires 2.7. There appear to be no sensible ways to get Python 2.7 into a CentOS 6 environments, so modify the build script to skip the Qemu upstream build instead. Additionally, SeaBIOS requires GCC 4.6 or later, so skip it as well. Signed-off-by: Andrew Cooper Acked-by: Wei Liu commit 1316369dca610352cce3aaf76e90db1cce75ed9f Author: Andrew Cooper Date: Fri Mar 22 11:12:28 2019 + CI: Fix indentation in containerize script The script is mostly indented with spaces, but there are three tabs. Fix them up to be consistent. No functional change. Signed-off-by: Andrew Cooper Acked-by: Wei Liu (qemu changes not included) ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/3] mwait-idle: add support for using halt
On 3/27/19 9:48 AM, Jan Beulich wrote:
On 26.03.19 at 22:56, wrote:
>> On 3/26/19 10:54 AM, Jan Beulich wrote:
>> On 19.03.19 at 17:12, wrote:
On 3/15/19 3:37 AM, Jan Beulich wrote:
> Furthermore I'm then once again wondering what the gain is
> over using the ACPI driver: The suggested _CST looks to exactly
> match the data you enter into the table in the later patch. IOW
> my fundamental concern didn't go away yet: As per the name
> of the driver, it shouldn't really need to support HLT (or anything
> other than MWAIT) as an entry method. Hence I think that at
> the very least you need to extend the description of the change
> quite a bit to explain why the ACPI driver is not suitable.
>
> Depending on how this comes out, it may then still be a matter
> of discussing whether, rather than fiddling with mwait-idle, it
> wouldn't be better to have an AMD-specific driver instead. Are
> there any thoughts in similar directions for Linux?
Because:
#1 getting the ACPI tables from dom0 is either unreliable (PV dom0) or
not possible (PVH dom0).
#2 the changes to the Intel code are minimal.
#3 worse case, Xen thinks it's using CC6 when it's using CC1. Not
perfect but far from fatal or breaking.
>>>
>>> Having thought about this some more, I agree that an AMD-specific
>>> driver would likely go too far. However, that's still no reason to fiddle
>>> with the mwait-idle one - I think you could as well populate the data
>>> as necessary for the ACPI driver to use, removing the dependency
>>> on Dom0. After all that driver already knows of all the entry methods
>>> you may want/need to use (see acpi_idle_do_entry()).
>>>
>> I did a rough example of how that might work and lines of code changed
>> for adding it to cpu_idle was roughly 125. Seeing as this doesn't
>> compile and doesn't even have comments, I'd say at least 140 lines of
>> code/change (most of those are additive too), a lot of is functionally
>> copied from mwait-idle and how it reads data out of the structures,
>> checks, and populates the cx structures. The first set of mwait patches
>> is 87 lines changed total.
>>
>> I _could_ try and refactor some of the code and get it down from
>> 125-140, but that would most likely make porting changes even harder for
>> mwait-idle.
>
> Well, I was rather thinking about something like the change below,
> taking slightly over 100 lines of new code, and not touching
> mwait-idle.c at all. Otoh there are a couple of TBDs in there which
> may cause the patch to further grow once addressed.
>
> Note that this goes on top of
> https://lists.xenproject.org/archives/html/xen-devel/2018-08/msg00089.html
> which sadly there still wasn't sufficient feedback on to decide where
> to go with the series; all I know is that Andrew (understandably)
> doesn't want to see the last patch go in without vendor confirmation
> (and I'd be fine to drop that last patch if need be, but this shouldn't
> block the earlier patches in the series).
>
> Jan
>
> x86/AMD: make C-state handling independent of Dom0
>
> At least for more recent CPUs, following what BKDG / PPR suggest for the
> BIOS to surface via ACPI we can make ourselves independent of Dom0
> uploading respective data.
>
> Signed-off-by: Jan Beulich
> ---
> TBD: Can we set local_apic_timer_c2_ok to true? I can't seem to find any
> statement in the BKDG / PPR as to whether the LAPIC timer continues
> running in CC6.
> TBD: We may want to verify that HLT indeed is configured to enter CC6.
> TBD: I guess we could extend this to families older then Fam15 as well.
>
> --- a/xen/arch/x86/acpi/cpu_idle.c
> +++ b/xen/arch/x86/acpi/cpu_idle.c
> @@ -120,6 +120,8 @@ boolean_param("lapic_timer_c2_ok", local
>
> struct acpi_processor_power *__read_mostly processor_powers[NR_CPUS];
>
> +static int8_t __read_mostly vendor_override;
> +
> struct hw_residencies
> {
> uint64_t mc0;
> @@ -1220,6 +1222,9 @@ long set_cx_pminfo(uint32_t acpi_id, str
> if ( pm_idle_save && pm_idle != acpi_processor_idle )
> return 0;
>
> +if ( vendor_override > 0 )
> +return 0;
> +
> print_cx_pminfo(acpi_id, power);
>
> cpu_id = get_cpu_id(acpi_id);
> @@ -1292,6 +1297,98 @@ long set_cx_pminfo(uint32_t acpi_id, str
> return 0;
> }
>
> +static void amd_cpuidle_init(struct acpi_processor_power *power)
> +{
> +unsigned int i, nr = 0;
> +const struct cpuinfo_x86 *c = ¤t_cpu_data;
> +const unsigned int ecx_req = CPUID5_ECX_EXTENSIONS_SUPPORTED |
> + CPUID5_ECX_INTERRUPT_BREAK;
> +const struct acpi_processor_cx *cx = NULL;
> +static const struct acpi_processor_cx fam17[] = {
> +{
> +.type = ACPI_STATE_C1,
> +.entry_method = ACPI_CSTATE_EM_FFH,
> +.address = 0,
> +.latency = 1,
> +},
> +{
> +.type = A
[Xen-devel] [PATCH v2 1/2] xen-block: scale sector based quantities correctly
The Xen blkif protocol requires that sector based quantities should be
interpreted strictly as multiples of 512 bytes. Specifically:
"first_sect and last_sect in blkif_request_segment, as well as
sector_number in blkif_request, are always expressed in 512-byte units."
This patch modifies the xen-block code accordingly.
Signed-off-by: Paul Durrant
---
Cc: Stefan Hajnoczi
Cc: Stefano Stabellini
Cc: Anthony Perard
Cc: Kevin Wolf
Cc: Max Reitz
---
hw/block/dataplane/xen-block.c | 28 +---
hw/block/xen_blkif.h | 2 ++
2 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/hw/block/dataplane/xen-block.c b/hw/block/dataplane/xen-block.c
index f1523c5b45..bb8f1186e4 100644
--- a/hw/block/dataplane/xen-block.c
+++ b/hw/block/dataplane/xen-block.c
@@ -49,7 +49,6 @@ struct XenBlockDataPlane {
unsigned int *ring_ref;
unsigned int nr_ring_ref;
void *sring;
-int64_t file_blk;
int protocol;
blkif_back_rings_t rings;
int more_work;
@@ -168,7 +167,7 @@ static int xen_block_parse_request(XenBlockRequest *request)
goto err;
}
-request->start = request->req.sector_number * dataplane->file_blk;
+request->start = request->req.sector_number * XEN_BLKIF_SECTOR_SIZE;
for (i = 0; i < request->req.nr_segments; i++) {
if (i == BLKIF_MAX_SEGMENTS_PER_REQUEST) {
error_report("error: nr_segments too big");
@@ -178,14 +177,14 @@ static int xen_block_parse_request(XenBlockRequest
*request)
error_report("error: first > last sector");
goto err;
}
-if (request->req.seg[i].last_sect * dataplane->file_blk >=
+if (request->req.seg[i].last_sect * XEN_BLKIF_SECTOR_SIZE >=
XC_PAGE_SIZE) {
error_report("error: page crossing");
goto err;
}
len = (request->req.seg[i].last_sect -
- request->req.seg[i].first_sect + 1) * dataplane->file_blk;
+ request->req.seg[i].first_sect + 1) * XEN_BLKIF_SECTOR_SIZE;
request->size += len;
}
if (request->start + request->size > blk_getlength(dataplane->blk)) {
@@ -205,7 +204,6 @@ static int xen_block_copy_request(XenBlockRequest *request)
XenDevice *xendev = dataplane->xendev;
XenDeviceGrantCopySegment segs[BLKIF_MAX_SEGMENTS_PER_REQUEST];
int i, count;
-int64_t file_blk = dataplane->file_blk;
bool to_domain = (request->req.operation == BLKIF_OP_READ);
void *virt = request->buf;
Error *local_err = NULL;
@@ -220,16 +218,17 @@ static int xen_block_copy_request(XenBlockRequest
*request)
if (to_domain) {
segs[i].dest.foreign.ref = request->req.seg[i].gref;
segs[i].dest.foreign.offset = request->req.seg[i].first_sect *
-file_blk;
+XEN_BLKIF_SECTOR_SIZE;
segs[i].source.virt = virt;
} else {
segs[i].source.foreign.ref = request->req.seg[i].gref;
segs[i].source.foreign.offset = request->req.seg[i].first_sect *
-file_blk;
+XEN_BLKIF_SECTOR_SIZE;
segs[i].dest.virt = virt;
}
segs[i].len = (request->req.seg[i].last_sect -
- request->req.seg[i].first_sect + 1) * file_blk;
+ request->req.seg[i].first_sect + 1) *
+ XEN_BLKIF_SECTOR_SIZE;
virt += segs[i].len;
}
@@ -331,22 +330,22 @@ static bool xen_block_split_discard(XenBlockRequest
*request,
XenBlockDataPlane *dataplane = request->dataplane;
int64_t byte_offset;
int byte_chunk;
-uint64_t byte_remaining, limit;
+uint64_t byte_remaining;
uint64_t sec_start = sector_number;
uint64_t sec_count = nr_sectors;
/* Wrap around, or overflowing byte limit? */
if (sec_start + sec_count < sec_count ||
-sec_start + sec_count > INT64_MAX / dataplane->file_blk) {
+sec_start + sec_count > INT64_MAX / XEN_BLKIF_SECTOR_SIZE) {
return false;
}
-limit = BDRV_REQUEST_MAX_SECTORS * dataplane->file_blk;
-byte_offset = sec_start * dataplane->file_blk;
-byte_remaining = sec_count * dataplane->file_blk;
+byte_offset = sec_start * XEN_BLKIF_SECTOR_SIZE;
+byte_remaining = sec_count * XEN_BLKIF_SECTOR_SIZE;
do {
-byte_chunk = byte_remaining > limit ? limit : byte_remaining;
+byte_chunk = byte_remaining > BDRV_REQUEST_MAX_BYTES ?
+BDRV_REQUEST_MAX_BYTES : byte_remaining;
request->aio_inflight++;
blk_aio_pdiscard(dataplane->blk, byte_offset, byte_chunk,
xen_block_complete_aio, request);
@@ -632,7 +631,6 @@ XenBlockDataPlane *xen_block_dataplane_create(XenDevice
*xendev,
XenBlockDataPlane *dataplane = g_new0(XenBlockDataPlane, 1);
dataplane->xendev = xendev;
-dataplane->file_blk = conf->logical_block_size;
dataplane->blk = conf->blk;
[Xen-devel] [PATCH v2 0/2] xen-block: fix sector size confusion
The Xen blkif protocol is confusing but discussion with the maintainer has clarified that sector based quantities in requests and the 'sectors' value advertized in xenstore should always be in terms of 512-byte units and not the advertised logical 'sector-size' value. This series fixes xen-block to adhere to the spec. Paul Durrant (2): xen-block: scale sector based quantities correctly xen-block: always report 'sectors' in terms of 512-byte units hw/block/dataplane/xen-block.c | 28 +--- hw/block/xen-block.c | 2 +- hw/block/xen_blkif.h | 2 ++ 3 files changed, 16 insertions(+), 16 deletions(-) --- v2: - Split up previous single patch Cc: Anthony Perard Cc: Kevin Wolf Cc: Max Reitz Cc: Stefan Hajnoczi Cc: Stefano Stabellini -- 2.20.1.2.gb21ebb6 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 2/2] xen-block: always report 'sectors' in terms of 512-byte units
The mail thread at [1] clarifies that the Xen blkif protocol requires that 'sectors' value reported in xenstore is strictly in terms of 512-byte units and is not dependent on the logical sector size reported in 'sector-size'. [1] https://lists.xenproject.org/archives/html/xen-devel/2019-03/msg01600.html Signed-off-by: Paul Durrant --- Cc: Stefano Stabellini Cc: Anthony Perard Cc: Kevin Wolf Cc: Max Reitz --- hw/block/xen-block.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c index a848849f48..57e9da7e1c 100644 --- a/hw/block/xen-block.c +++ b/hw/block/xen-block.c @@ -149,7 +149,7 @@ static void xen_block_set_size(XenBlockDevice *blockdev) const char *type = object_get_typename(OBJECT(blockdev)); XenBlockVdev *vdev = &blockdev->props.vdev; BlockConf *conf = &blockdev->props.conf; -int64_t sectors = blk_getlength(conf->blk) / conf->logical_block_size; +int64_t sectors = blk_getlength(conf->blk) / XEN_BLKIF_SECTOR_SIZE; XenDevice *xendev = XEN_DEVICE(blockdev); trace_xen_block_size(type, vdev->disk, vdev->partition, sectors); -- 2.20.1.2.gb21ebb6 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 0/2] xen-block: fix sector size confusion
On 27/03/2019 17:32, Paul Durrant wrote: > The Xen blkif protocol is confusing but discussion with the maintainer > has clarified that sector based quantities in requests and the 'sectors' > value advertized in xenstore should always be in terms of 512-byte > units and not the advertised logical 'sector-size' value. > > This series fixes xen-block to adhere to the spec. I thought we agreed that hardcoding things to 512 bytes was the wrong thing to do. I was expecting something like: 1) Clarify the spec with the intended meaning, (which is what some implementations actually use already) and wont cripple 4k datapaths. 2) Introduce a compatibility key for "I don't rely on sector-size being 512", which fixed implementations should advertise. 3) Specify that because of bugs in the spec which got out into the wild, drivers which don't find the key being advertised by the other end should emulate sector-size=512 for compatibility with broken implementations. Whatever the eventual way out, the first thing which needs to happen is an update to the spec, before actions are taken to alter existing implementations. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 09/12] xen/arm: guest_walk: Avoid theoritical unitialized value in get_top_bit
Clang 8.0 throws an error in the get_top_bit function:
guest_walk.c:328:15: error: variable 'topbit' is used uninitialized
whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
else if ( is_64bit_domain(d) )
^~
This is happening because clang thinks that is_32bit_domain(d) is not
the exact inverse of is_64bit_domain(d). So it expects a else case to
handle the case where the latter call is false.
In other part of the code, dealing with difference between 32-bit and
64-bit domain, we usually use if ( is_XXbit_domain ) ... else ...
So use the same pattern here.
Signed-off-by: Julien Grall
---
xen/arch/arm/guest_walk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/arch/arm/guest_walk.c b/xen/arch/arm/guest_walk.c
index 7db7a7321b..1bee198777 100644
--- a/xen/arch/arm/guest_walk.c
+++ b/xen/arch/arm/guest_walk.c
@@ -325,7 +325,7 @@ static unsigned int get_top_bit(struct domain *d, vaddr_t
gva, register_t tcr)
*/
if ( is_32bit_domain(d) )
topbit = 31;
-else if ( is_64bit_domain(d) )
+else
{
if ( ((gva & BIT_ULL(55)) && (tcr & TCR_EL1_TBI1)) ||
(!(gva & BIT_ULL(55)) && (tcr & TCR_EL1_TBI0)) )
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 08/12] xen/arm: cpufeature: Match register size with value size in cpus_have_const_cap
Clang is pickier than GCC for the register size in asm statement. It
expects the register size to match the value size.
The asm statement expects a 32-bit (resp. 64-bit) value on Arm32
(resp. Arm64) whereas the value is a boolean (Clang consider to be
32-bit).
It would be possible to impose 32-bit register for both architecture
but this require the code to use __OP32. However, it does no really
improve the assembly generated. Instead, replace switch the variable to
use register_t.
Signed-off-by: Julien Grall
---
xen/include/asm-arm/cpufeature.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
index c2c8f3417c..d06f09ecfa 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -67,7 +67,7 @@ static inline bool cpus_have_cap(unsigned int num)
/* System capability check for constant cap */
#define cpus_have_const_cap(num) ({ \
-bool __ret; \
+register_t __ret; \
\
asm volatile (ALTERNATIVE("mov %0, #0", \
"mov %0, #1", \
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 01/12] xen: clang: Support correctly cross-compile
Clang uses "-target" option for cross-compilation. Signed-off-by: Julien Grall --- config/StdGNU.mk | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/config/StdGNU.mk b/config/StdGNU.mk index 039274ea61..48c50b5ad7 100644 --- a/config/StdGNU.mk +++ b/config/StdGNU.mk @@ -1,8 +1,13 @@ AS = $(CROSS_COMPILE)as LD = $(CROSS_COMPILE)ld ifeq ($(clang),y) -CC = $(CROSS_COMPILE)clang -CXX= $(CROSS_COMPILE)clang++ +ifneq ($(CROSS_COMPILE),) +CC = clang -target $(CROSS_COMPILE:-=) +CXX= clang++ -target $(CROSS_COMPILE:-=) +else +CC = clang +CXX= clang++ +endif LD_LTO = $(CROSS_COMPILE)llvm-ld else CC = $(CROSS_COMPILE)gcc -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 02/12] xen/arm: fix get_cpu_info() when built with clang
Clang understands the GCCism in use here, but still complains that sp is
unitialised. In such cases, resort to the older versions of this code,
which directly read sp into the temporary variable.
Note that we still keep the GCCism in the default case, as it causes GCC
to create rather better assembly.
This is based on the x86 counterpart.
Signed-off-by: Julien Grall
---
xen/include/asm-arm/current.h | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/xen/include/asm-arm/current.h b/xen/include/asm-arm/current.h
index c4af66fbb9..6b7c1df64d 100644
--- a/xen/include/asm-arm/current.h
+++ b/xen/include/asm-arm/current.h
@@ -28,8 +28,16 @@ struct cpu_info {
static inline struct cpu_info *get_cpu_info(void)
{
+#ifdef __clang__
+unsigned long sp;
+
+asm ("mov %0, sp" : "=r" (sp));
+#else
register unsigned long sp asm ("sp");
-return (struct cpu_info *)((sp & ~(STACK_SIZE - 1)) + STACK_SIZE -
sizeof(struct cpu_info));
+#endif
+
+return (struct cpu_info *)((sp & ~(STACK_SIZE - 1)) +
+ STACK_SIZE - sizeof(struct cpu_info));
}
#define guest_cpu_user_regs() (&get_cpu_info()->guest_cpu_user_regs)
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 06/12] xen/arm64: sysreg: Implement the 32-bit helpers using the 64-bit helpers
Clang is pickier than GCC for the register size in asm statement. It
expects the register size to match the value size.
The instructions msr/mrs are expecting a 64-bit register. This means the
implementation of the 32-bit helpers is not correct. The easiest
solution is to implement the 32-bit helpers using the 64-bit helpers.
Signed-off-by: Julien Grall
---
xen/include/asm-arm/arm64/sysregs.h | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/xen/include/asm-arm/arm64/sysregs.h
b/xen/include/asm-arm/arm64/sysregs.h
index 08585a969e..c60029d38f 100644
--- a/xen/include/asm-arm/arm64/sysregs.h
+++ b/xen/include/asm-arm/arm64/sysregs.h
@@ -59,14 +59,9 @@
/* Access to system registers */
-#define READ_SYSREG32(name) ({ \
-uint32_t _r;\
-asm volatile("mrs %0, "__stringify(name) : "=r" (_r)); \
-_r; })
-#define WRITE_SYSREG32(v, name) do {\
-uint32_t _r = v;\
-asm volatile("msr "__stringify(name)", %0" : : "r" (_r)); \
-} while (0)
+#define READ_SYSREG32(name) ((uint32_t)READ_SYSREG64(name))
+
+#define WRITE_SYSREG32(v, name) WRITE_SYSREG64((uint64_t)v, name)
#define WRITE_SYSREG64(v, name) do {\
uint64_t _r = v;\
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 05/12] xen/arm64: bitops: Match the register size with the value size in flsl
Clang is pickier than GCC for the register size in asm statement. It expects
the register size to match the value size.
The instruction clz is expecting the two operands to be the same size
(i.e 32-bit or 64-bit). As the flsl function is dealing with 64-bit
value, we need to make the destination variable 64-bit as well.
While at it, add a newline before the return statement.
Signed-off-by: Julien Grall
---
xen/include/asm-arm/arm64/bitops.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/xen/include/asm-arm/arm64/bitops.h
b/xen/include/asm-arm/arm64/bitops.h
index 6bf1922680..05045f1109 100644
--- a/xen/include/asm-arm/arm64/bitops.h
+++ b/xen/include/asm-arm/arm64/bitops.h
@@ -34,12 +34,13 @@ static /*__*/always_inline unsigned long __ffs(unsigned
long word)
static inline int flsl(unsigned long x)
{
-int ret;
+uint64_t ret;
if (__builtin_constant_p(x))
return generic_flsl(x);
asm("clz\t%0, %1" : "=r" (ret) : "r" (x));
+
return BITS_PER_LONG - ret;
}
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 04/12] xen/arm: memaccess: Initialize correctly *access in __p2m_get_mem_access
The commit 8d84e701fd "xen/arm: initialize access" initializes
*access using the wrong enumeration type. This result to a warning
using clang:
mem_access.c:50:20: error: implicit conversion from enumeration type
'p2m_access_t' to different enumeration type 'xenmem_access_t'
[-Werror,-Wenum-conversion]
*access = p2m->default_access;
~ ~^~
The correct solution is to use the array memaccess that will do the
conversion between the 2 enums.
Fixes: 8d84e701fd ("xen/arm: initialize access")
Signed-off-by: Julien Grall
---
This patch is candidate for backporting in 4.12.
---
xen/arch/arm/mem_access.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/arch/arm/mem_access.c b/xen/arch/arm/mem_access.c
index db49372a2c..3e3620294c 100644
--- a/xen/arch/arm/mem_access.c
+++ b/xen/arch/arm/mem_access.c
@@ -47,7 +47,7 @@ static int __p2m_get_mem_access(struct domain *d, gfn_t gfn,
};
ASSERT(p2m_is_locked(p2m));
-*access = p2m->default_access;
+*access = memaccess[p2m->default_access];
/* If no setting was ever set, just return rwx. */
if ( !p2m->mem_access_enabled )
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 07/12] xen/arm: cpuerrata: Match register size with value size in check_workaround_*
Clang is pickier than GCC for the register size in asm statement. It
expects the register size to match the value size.
The asm statement expects a 32-bit (resp. 64-bit) value on Arm32
(resp. Arm64) whereas the value is a boolean (Clang consider to be
32-bit).
It would be possible to impose 32-bit register for both architecture
but this require the code to use __OP32. However, it does not really
improve the assembly generated. Instead, replace switch the variable
to use register_t.
Signed-off-by: Julien Grall
---
xen/include/asm-arm/cpuerrata.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/include/asm-arm/cpuerrata.h b/xen/include/asm-arm/cpuerrata.h
index 55ddfda272..88ef3ca934 100644
--- a/xen/include/asm-arm/cpuerrata.h
+++ b/xen/include/asm-arm/cpuerrata.h
@@ -14,7 +14,7 @@ static inline bool check_workaround_##erratum(void)
\
return false; \
else\
{ \
-bool ret; \
+register_t ret; \
\
asm volatile (ALTERNATIVE("mov %0, #0", \
"mov %0, #1", \
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 11/12] xen/arm: traps: Mark check_stack_alignment_constraints as unused
Clang will throw an error if a function is unused unless you tell
to ignore it. This can be done using __maybe_unused.
Signed-off-by: Julien Grall
---
xen/arch/arm/traps.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index d8b9a8a0f0..661475666a 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -54,7 +54,8 @@
* that both the kernel half of struct cpu_user_regs (which is pushed in
* entry.S) and struct cpu_info (which lives at the bottom of a Xen
* stack) must be doubleword-aligned in size. */
-static inline void check_stack_alignment_constraints(void) {
+static void __maybe_unused check_stack_alignment_constraints(void)
+{
#ifdef CONFIG_ARM_64
BUILD_BUG_ON((sizeof (struct cpu_user_regs)) & 0xf);
BUILD_BUG_ON((offsetof(struct cpu_user_regs, spsr_el1)) & 0xf);
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 12/12] xen/arm64: __cmpxchg and __cmpxchg_mb should always be inline
Currently __cmpxchg_mb and __cmpxchg are only marked inline. The
compiler is free to decide to not honor the inline. This will result to
generate code use __bad_cmpxchg and lead a link failure.
This was caught by Clang 8.0.
Signed-off-by: Julien Grall
---
xen/include/asm-arm/arm64/cmpxchg.h | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/xen/include/asm-arm/arm64/cmpxchg.h
b/xen/include/asm-arm/arm64/cmpxchg.h
index ae42b2f5ff..359271173e 100644
--- a/xen/include/asm-arm/arm64/cmpxchg.h
+++ b/xen/include/asm-arm/arm64/cmpxchg.h
@@ -63,8 +63,9 @@ static inline unsigned long __xchg(unsigned long x, volatile
void *ptr, int size
extern void __bad_cmpxchg(volatile void *ptr, int size);
-static inline unsigned long __cmpxchg(volatile void *ptr, unsigned long old,
- unsigned long new, int size)
+static always_inline unsigned long __cmpxchg(volatile void *ptr,
+unsigned long old,
+unsigned long new, int size)
{
unsigned long oldval = 0, res;
@@ -137,8 +138,9 @@ static inline unsigned long __cmpxchg(volatile void *ptr,
unsigned long old,
return oldval;
}
-static inline unsigned long __cmpxchg_mb(volatile void *ptr, unsigned long old,
-unsigned long new, int size)
+static always_inline unsigned long __cmpxchg_mb(volatile void *ptr,
+ unsigned long old,
+ unsigned long new, int size)
{
unsigned long ret;
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 03/12] xen/arm: zynqmp: Fix header guard for xilinx-zynqmp-eemi.h
The header guard for xilinx-zynqmp-eemi.h is not followed by a #define of the macro used in the guard. Signed-off-by: Julien Grall --- xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h b/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h index 72aadf7a44..cf25a9014d 100644 --- a/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h +++ b/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h @@ -12,7 +12,7 @@ */ #ifndef __ASM_ARM_PLATFORMS_ZYNQMP_H -#define __ASM_ASM_PLATFORMS_ZYNQMP_H +#define __ASM_ARM_PLATFORMS_ZYNQMP_H #include #include -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 00/12] xen/arm: Add support to build with clang
Hi all, This series adds support to build Xen Arm with clang. This series was tested with clang 8.0. Note that I only did build for arm64. I still need to look at the arm32 build. Cheers, Julien Grall (12): xen: clang: Support correctly cross-compile xen/arm: fix get_cpu_info() when built with clang xen/arm: zynqmp: Fix header guard for xilinx-zynqmp-eemi.h xen/arm: memaccess: Initialize correctly *access in __p2m_get_mem_access xen/arm64: bitops: Match the register size with the value size in flsl xen/arm64: sysreg: Implement the 32-bit helpers using the 64-bit helpers xen/arm: cpuerrata: Match register size with value size in check_workaround_* xen/arm: cpufeature: Match register size with value size in cpus_have_const_cap xen/arm: guest_walk: Avoid theoritical unitialized value in get_top_bit xen/arm: mm: Mark check_memory_layout_alignment_constraints as unused xen/arm: traps: Mark check_stack_alignment_constraints as unused xen/arm64: __cmpxchg and __cmpxchg_mb should always be inline config/StdGNU.mk | 9 +++-- xen/arch/arm/guest_walk.c | 2 +- xen/arch/arm/mem_access.c | 2 +- xen/arch/arm/mm.c | 3 ++- xen/arch/arm/traps.c | 3 ++- xen/include/asm-arm/arm64/bitops.h | 3 ++- xen/include/asm-arm/arm64/cmpxchg.h| 10 ++ xen/include/asm-arm/arm64/sysregs.h| 11 +++ xen/include/asm-arm/cpuerrata.h| 2 +- xen/include/asm-arm/cpufeature.h | 2 +- xen/include/asm-arm/current.h | 10 +- xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h | 2 +- 12 files changed, 36 insertions(+), 23 deletions(-) -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 10/12] xen/arm: mm: Mark check_memory_layout_alignment_constraints as unused
Clang will throw an error if a function is unused unless you tell
to ignore it. This can be done using __maybe_unused.
Signed-off-by: Julien Grall
---
xen/arch/arm/mm.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 01ae20..d37925051a 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -160,7 +160,8 @@ unsigned long total_pages;
extern char __init_begin[], __init_end[];
/* Checking VA memory layout alignment. */
-static inline void check_memory_layout_alignment_constraints(void) {
+static __maybe_unused void check_memory_layout_alignment_constraints(void)
+{
/* 2MB aligned regions */
BUILD_BUG_ON(XEN_VIRT_START & ~SECOND_MASK);
BUILD_BUG_ON(FIXMAP_ADDR(0) & ~SECOND_MASK);
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 05/12] xen/arm64: bitops: Match the register size with the value size in flsl
On 27/03/2019 18:45, Julien Grall wrote:
> Clang is pickier than GCC for the register size in asm statement. It expects
> the register size to match the value size.
>
> The instruction clz is expecting the two operands to be the same size
> (i.e 32-bit or 64-bit). As the flsl function is dealing with 64-bit
> value, we need to make the destination variable 64-bit as well.
>
> While at it, add a newline before the return statement.
>
> Signed-off-by: Julien Grall
> ---
> xen/include/asm-arm/arm64/bitops.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/xen/include/asm-arm/arm64/bitops.h
> b/xen/include/asm-arm/arm64/bitops.h
> index 6bf1922680..05045f1109 100644
> --- a/xen/include/asm-arm/arm64/bitops.h
> +++ b/xen/include/asm-arm/arm64/bitops.h
> @@ -34,12 +34,13 @@ static /*__*/always_inline unsigned long __ffs(unsigned
> long word)
>
> static inline int flsl(unsigned long x)
> {
> -int ret;
> +uint64_t ret;
>
> if (__builtin_constant_p(x))
> return generic_flsl(x);
>
> asm("clz\t%0, %1" : "=r" (ret) : "r" (x));
As x is fixed by the ABI, ret should be unsigned long to match, surely?
This will compile as it is arm64 specific, but it looks just as wrong
(per the commit message) as using int in the first place.
~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 04/12] xen/arm: memaccess: Initialize correctly *access in __p2m_get_mem_access
On 3/27/19 8:45 PM, Julien Grall wrote:
> The commit 8d84e701fd "xen/arm: initialize access" initializes
> *access using the wrong enumeration type. This result to a warning
> using clang:
>
> mem_access.c:50:20: error: implicit conversion from enumeration type
> 'p2m_access_t' to different enumeration type 'xenmem_access_t'
> [-Werror,-Wenum-conversion]
> *access = p2m->default_access;
> ~ ~^~
>
> The correct solution is to use the array memaccess that will do the
> conversion between the 2 enums.
>
> Fixes: 8d84e701fd ("xen/arm: initialize access")
> Signed-off-by: Julien Grall
Acked-by: Razvan Cojocaru
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 10/12] xen/arm: mm: Mark check_memory_layout_alignment_constraints as unused
On 27/03/2019 18:45, Julien Grall wrote:
> Clang will throw an error if a function is unused unless you tell
> to ignore it. This can be done using __maybe_unused.
>
> Signed-off-by: Julien Grall
> ---
> xen/arch/arm/mm.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 01ae20..d37925051a 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -160,7 +160,8 @@ unsigned long total_pages;
> extern char __init_begin[], __init_end[];
>
> /* Checking VA memory layout alignment. */
> -static inline void check_memory_layout_alignment_constraints(void) {
> +static __maybe_unused void check_memory_layout_alignment_constraints(void)
As you're already changing this...
The style used elsewhere is
static void __init __maybe_unused build_assertions(void)
which has the added advantage that it more obvious to future developers
that trying to put code other than BUILD_BUG_ON() in it is a bad idea.
It took me a moment to realise that this function deliberately wasn't
called.
~Andrew
> +{
> /* 2MB aligned regions */
> BUILD_BUG_ON(XEN_VIRT_START & ~SECOND_MASK);
> BUILD_BUG_ON(FIXMAP_ADDR(0) & ~SECOND_MASK);
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 06/12] xen/arm64: sysreg: Implement the 32-bit helpers using the 64-bit helpers
On 27/03/2019 18:45, Julien Grall wrote: > Clang is pickier than GCC for the register size in asm statement. It > expects the register size to match the value size. > > The instructions msr/mrs are expecting a 64-bit register. This means the > implementation of the 32-bit helpers is not correct. The easiest > solution is to implement the 32-bit helpers using the 64-bit helpers. > > Signed-off-by: Julien Grall (I don't have an opinion on how to fix the issue, but) Are SYSREGS actually always 64 bits even on arm32, and these helpers just a shorthand for the lower 32 bits, or is this an effectively-unnecessary constraint imposed by Clang? ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] x86/vvmx: Fix debug prints to not have 17 unnecessary spaces
This has been problematic since its introduction in Xen 4.3
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
CC: Jun Nakajima
CC: Kevin Tian
---
xen/arch/x86/hvm/vmx/vvmx.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index 6644ea9..0f16688 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -924,8 +924,8 @@ static void vvmcs_to_shadow_bulk(struct vcpu *v, unsigned
int n,
if ( !value || n > VMCS_BUF_SIZE )
{
-gdprintk(XENLOG_DEBUG, "vmcs sync fall back to non-bulk mode, \
- buffer: %p, buffer size: %d, fields number: %d.\n",
+gdprintk(XENLOG_DEBUG, "vmcs sync fall back to non-bulk mode, "
+ "buffer: %p, buffer size: %d, fields number: %d.\n",
value, VMCS_BUF_SIZE, n);
goto fallback;
}
@@ -964,8 +964,8 @@ static void shadow_to_vvmcs_bulk(struct vcpu *v, unsigned
int n,
if ( !value || n > VMCS_BUF_SIZE )
{
-gdprintk(XENLOG_DEBUG, "vmcs sync fall back to non-bulk mode, \
- buffer: %p, buffer size: %d, fields number: %d.\n",
+gdprintk(XENLOG_DEBUG, "vmcs sync fall back to non-bulk mode, "
+ "buffer: %p, buffer size: %d, fields number: %d.\n",
value, VMCS_BUF_SIZE, n);
goto fallback;
}
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-4.19 test] 134109: regressions - trouble: blocked/broken/fail/pass
flight 134109 linux-4.19 real [real] http://logs.test-lab.xenproject.org/osstest/logs/134109/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-xsm broken build-arm64-pvopsbroken build-arm64 broken build-arm64 4 host-install(4)broken REGR. vs. 129313 build-arm64-xsm 4 host-install(4)broken REGR. vs. 129313 build-arm64-pvops 4 host-install(4)broken REGR. vs. 129313 test-amd64-i386-qemut-rhel6hvm-amd 12 guest-start/redhat.repeat fail in 134080 REGR. vs. 129313 Tests which are failing intermittently (not blocking): test-amd64-amd64-rumprun-amd64 17 rumprun-demo-xenstorels/xenstorels.repeat fail in 134080 pass in 134109 test-amd64-amd64-xl-qcow219 guest-start/debian.repeat fail pass in 134050 test-amd64-i386-qemut-rhel6hvm-amd 10 redhat-install fail pass in 134080 Tests which did not succeed, but are not blocking: test-arm64-arm64-xl 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit1 1 build-check(1) blocked n/a test-arm64-arm64-examine 1 build-check(1) blocked n/a test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass version targeted for testing: linuxa2cddfe2ce6e9108341820fff8af467
Re: [Xen-devel] [PATCH 05/12] xen/arm64: bitops: Match the register size with the value size in flsl
Hi,
On 27/03/2019 19:03, Andrew Cooper wrote:
> On 27/03/2019 18:45, Julien Grall wrote:
>> Clang is pickier than GCC for the register size in asm statement. It expects
>> the register size to match the value size.
>>
>> The instruction clz is expecting the two operands to be the same size
>> (i.e 32-bit or 64-bit). As the flsl function is dealing with 64-bit
>> value, we need to make the destination variable 64-bit as well.
>>
>> While at it, add a newline before the return statement.
>>
>> Signed-off-by: Julien Grall
>> ---
>> xen/include/asm-arm/arm64/bitops.h | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/include/asm-arm/arm64/bitops.h
>> b/xen/include/asm-arm/arm64/bitops.h
>> index 6bf1922680..05045f1109 100644
>> --- a/xen/include/asm-arm/arm64/bitops.h
>> +++ b/xen/include/asm-arm/arm64/bitops.h
>> @@ -34,12 +34,13 @@ static /*__*/always_inline unsigned long __ffs(unsigned
>> long word)
>>
>> static inline int flsl(unsigned long x)
>> {
>> -int ret;
>> +uint64_t ret;
>>
>> if (__builtin_constant_p(x))
>> return generic_flsl(x);
>>
>> asm("clz\t%0, %1" : "=r" (ret) : "r" (x));
>
> As x is fixed by the ABI, ret should be unsigned long to match, surely?
No need for it. The result of the instruction clz will always be smaller
than 64.
I suspect they require a 64-bit register just for simplicity as they
encode the size for the 2 registers in only a single bit (i.e sf).
>
> This will compile as it is arm64 specific, but it looks just as wrong
> (per the commit message) as using int in the first place.
See above. I can clarify it in the commit message.
Cheers,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 06/12] xen/arm64: sysreg: Implement the 32-bit helpers using the 64-bit helpers
Hi,
On 27/03/2019 19:15, Andrew Cooper wrote:
> On 27/03/2019 18:45, Julien Grall wrote:
>> Clang is pickier than GCC for the register size in asm statement. It
>> expects the register size to match the value size.
>>
>> The instructions msr/mrs are expecting a 64-bit register. This means the
>> implementation of the 32-bit helpers is not correct. The easiest
>> solution is to implement the 32-bit helpers using the 64-bit helpers.
>>
>> Signed-off-by: Julien Grall
>
> (I don't have an opinion on how to fix the issue, but)
>
> Are SYSREGS actually always 64 bits even on arm32, and these helpers
> just a shorthand for the lower 32 bits, or is this an
> effectively-unnecessary constraint imposed by Clang?
This code is Arm64 specific. On Arm64, system registers are always
64-bits, and therefore the instructions msr/mrs require a 64-bit register.
So the complain from Clang is valid here. It happens that some of the
registers have the top 32-bit RES0 in current architecture. One could
argue that this is not very future-proof and we should get rid of {READ,
WRITE)_SYSREG32.
Unfortunately, the callers are expecting a 32-bit value. I need to
investigate all the callers to ensure no one is transforming the value
to 64-bit again. I don't really want to block clang support on that, so
I have added an action for fixing this later on.
Cheers,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 0/2] xen-block: fix sector size confusion
> -Original Message- > From: Andrew Cooper > Sent: 27 March 2019 18:20 > To: Paul Durrant ; xen-devel@lists.xenproject.org; > qemu-bl...@nongnu.org; > qemu-de...@nongnu.org > Cc: Kevin Wolf ; Stefano Stabellini > ; Max Reitz > ; Stefan Hajnoczi ; Anthony Perard > > Subject: Re: [Xen-devel] [PATCH v2 0/2] xen-block: fix sector size confusion > > On 27/03/2019 17:32, Paul Durrant wrote: > > The Xen blkif protocol is confusing but discussion with the maintainer > > has clarified that sector based quantities in requests and the 'sectors' > > value advertized in xenstore should always be in terms of 512-byte > > units and not the advertised logical 'sector-size' value. > > > > This series fixes xen-block to adhere to the spec. > > I thought we agreed that hardcoding things to 512 bytes was the wrong > thing to do. To some extent we decided it was the *only* thing to do. > > I was expecting something like: > > 1) Clarify the spec with the intended meaning, (which is what some > implementations actually use already) and wont cripple 4k datapaths. > 2) Introduce a compatibility key for "I don't rely on sector-size being > 512", which fixed implementations should advertise. > 3) Specify that because of bugs in the spec which got out into the wild, > drivers which don't find the key being advertised by the other end > should emulate sector-size=512 for compatibility with broken > implementations. Yes, that's how we are going to fix things. > > Whatever the eventual way out, the first thing which needs to happen is > an update to the spec, before actions are taken to alter existing > implementations. Well the implementation is currently wrong w.r.t. the spec and these patches fix that. As long as sector-size remains at 512 then no existing frontend should break, so I guess you could argue that patch #2 should also make sure that sector-size is also 512... but that is not yet in the spec. I guess I'm ok to defer patch #2 until a revised spec. is agreed, but the ship has already sailed as far as patch #1 goes. Anthony, thoughts? Paul > > ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 134136: trouble: blocked/broken/pass
flight 134136 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/134136/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-xsm broken build-arm64-xsm 4 host-install(4)broken REGR. vs. 133991 Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen e88afede8cbc18032bcab49b3a25b472d5516cf5 baseline version: xen cb70a26f78848fe45f593f7ebc9cfaac760a791b Last test of basis 133991 2019-03-22 15:00:46 Z5 days Failing since134068 2019-03-25 12:00:51 Z2 days 11 attempts Testing same since 134136 2019-03-27 18:00:28 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Wei Liu jobs: build-arm64-xsm broken build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm blocked test-amd64-amd64-xl-qemuu-debianhvm-i386 pass 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 broken-job build-arm64-xsm broken broken-step build-arm64-xsm host-install(4) Not pushing. commit e88afede8cbc18032bcab49b3a25b472d5516cf5 Author: Andrew Cooper Date: Tue Jul 10 13:53:21 2018 +0100 libx86: Recalculate synthesised cpuid_policy fields when appropriate When filling a policy, either from CPUID or an incomming leaf stream, recalculate the synthesised vendor value. All callers are expected to want this behaviour. Signed-off-by: Andrew Cooper Reviewed-by: Jan Beulich commit 1c2c9f85dd36bd908441b37ab73172358509c9b5 Author: Andrew Cooper Date: Wed Mar 20 14:56:15 2019 + tools/libxc: Use x86_cpuid_lookup_vendor() rather than opencoding the logic This doesn't address any of the assumptions that "anything which isn't AMD is Intel". This logic is expected to be replaced wholesale with libx86 in the longterm. Signed-off-by: Andrew Cooper Reviewed-by: Jan Beulich commit 00b4f4d0fb75dc183b499e78d1abcb865dbc30d7 Author: Andrew Cooper Date: Tue Jul 10 13:53:21 2018 +0100 x86/cpuid: Drop get_cpu_vendor() completely get_cpu_vendor() tries to do a number of things, and ends up doing none of them well. For calculating the vendor itself, use x86_cpuid_lookup_vendor() which is implemented in a far more efficient manner than looping over cpu_devs[]. For setting up this_cpu, set it up once on the BSP only, rather than latest-takes-precident across the APs. Such a system is probably not going to boot, but this feels like a less dangerous course of action. Adjust the printed errors to be more clear in the mismatch case. This removes the only user of cpu_dev->c_ident[], so drop that field as well. Signed-off-by: Andrew Cooper Reviewed-by: Jan Beulich commit e72309ffbe7c4e507649c74749f130cda691131c Author: Andrew Cooper Date: Wed Mar 20 14:05:11 2019 + libx86: Introduce x86_cpuid_lookup_vendor() Also introduce constants for the vendor strings in CPUID leaf 0. Signed-off-by: Andrew Cooper Reviewed-by: Jan Beulich commit 8eed571409a7f81ec9327cfa95d7c298333e22e4 Author: Andrew Cooper Date: Tue Mar 26 14:23:03 2019 + CI: Add a CentOS 6 container and build jobs CentOS 6 is probably the most frequently broken build, so adding it to CI would be a very good move. One problem is that CentOS 6 comes with Python 2.6, and Qemu requires 2.7. There appear to be no sensible ways
[Xen-devel] [ovmf test] 134113: all pass - PUSHED
flight 134113 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/134113/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf c455bc8c8d78ad51c24426a500914ea32504bf06 baseline version: ovmf 2f2c51acfb70efe3dd02022ca09dd853601d8acd Last test of basis 134071 2019-03-25 13:34:53 Z2 days Failing since134097 2019-03-26 15:53:37 Z1 days2 attempts Testing same since 134113 2019-03-27 05:12:07 Z0 days1 attempts People who touched revisions under test: Ard Biesheuvel Bob Feng Bret Barkelew Feng, Bob C Jordan Justen Laszlo Ersek Leif Lindholm Rebecca Cran Rebecca Cran via edk2-devel Shenglei Zhang Zhichao Gao 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 2f2c51acfb..c455bc8c8d c455bc8c8d78ad51c24426a500914ea32504bf06 -> xen-tested-master ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [libvirt test] 134039: regressions - trouble: blocked/broken/fail/pass
flight 134039 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/134039/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64 broken build-arm64-pvopsbroken build-arm64-xsm broken build-arm64-xsm 4 host-install(4)broken REGR. vs. 133846 build-arm64-pvops 4 host-install(4)broken REGR. vs. 133846 build-arm64 4 host-install(4)broken REGR. vs. 133846 build-i386-libvirt6 libvirt-buildfail REGR. vs. 133846 Tests which did not succeed, but are not blocking: test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-qcow2 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-arm64-arm64-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 133846 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 133846 test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass version targeted for testing: libvirt 530c1671e120c4415bfc20c318199738f2ae795c baseline version: libvirt 25e2e4e04f13901b3db903b2301bd11381bdf128 Last test of basis 133846 2019-03-16 02:09:09 Z 11 days Failing since133876 2019-03-17 11:33:04 Z 10 days7 attempts Testing same since 134004 2019-03-23 07:11:40 Z4 days2 attempts People who touched revisions under test: Andrea Bolognani Cole Robinson Daniel P. Berrangé Eric Blake Jason Dillaman Michal Privoznik Nikolay Shirokovskiy Peter Krempa jobs: build-amd64-xsm pass build-arm64-xsm broken build-i386-xsm pass build-amd64 pass build-arm64 broken build-armhf pass build-i386 pass build-amd64-libvirt pass build-arm64-libvirt blocked build-armhf-libvirt pass build-i386-libvirt fail build-amd64-pvopspass build-arm64-pvopsbroken build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmblocked test-amd64-amd64-libvirt-xsm pass test-arm64-arm64-libvirt-xsm blocked test-amd64-i386-libvirt-xsm blocked test-amd64-amd64-libvirt pass test-arm64-arm64-libvirt blocked test-armhf-armhf-libvirt pass test-amd64-i386-libvirt blocked test-amd64-amd64-libvirt-pairpass test-amd64-i386-libvirt-pair blocked test-arm64-arm64-libvirt-qcow2 blocked test-armhf-armhf-libvirt-raw pass test-amd64-amd64-libvirt-vhd 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
Re: [Xen-devel] [PATCH 1/6] xen/sched: call cpu_disable_scheduler() via cpu notifier
On Wed, 2019-03-27 at 18:06 +0100, Juergen Gross wrote: > On 27/03/2019 17:58, Jan Beulich wrote: > > > > > > > Well, of course nothing's going to run on that CPU anymore. > > But vCPU-s may still have associations with it, so what I'm > > worried about is e.g. something finding v->processor pointing > > at an offline CPU and getting confused. Another, more exotic > > v->processor is allowed to have a stale value as long as the vcpu > isn't running. > I was also concerned about things being done no longer in stop-machine, like Jan, and in fact, I asked for similar reassurances. Thinking more about this, I agree with Juergen that it should work. In fact, it's important for v->processor to point to a CPU that has its scheduling data structure allocated and valid. In current approach, right before suspend, that is only true for the BSP, and that's why we move every vcpu there. But after this series, since we don't deallocate such structs any longer, it should be ok that v->processor retains its value. Regards, Dario -- <> (Raistlin Majere) - Dario Faggioli, Ph.D, http://about.me/dario.faggioli Software Engineer @ SUSE https://www.suse.com/ signature.asc Description: This is a digitally signed message part ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 6/6] xen/sched: don't disable scheduler on cpus during suspend
On Mon, 2019-03-18 at 14:11 +0100, Juergen Gross wrote: > Today there is special handling in cpu_disable_scheduler() for > suspend > by forcing all vcpus to the boot cpu. In fact there is no need for > that > as during resume the vcpus are put on the correct cpus again. > > So we can just omit the call of cpu_disable_scheduler() when > offlining > a cpu due to suspend and on resuming we can omit taking the schedule > lock for selecting the new processor. > > In restore_vcpu_affinity() we should be careful when applying > affinity > as the cpu might not have come back to life. This in turn enables us > to even support affinity_broken across suspend/resume. > > Avoid all other scheduler dealloc - alloc dance when doing suspend > and > resume, too. It is enough to react on cpus failing to come up on > resume > again. > > Signed-off-by: Juergen Gross > Ok, I think this patch is fine. Reviewed-by: Dario Faggioli I guess you have tested both cpu off/on-lining and suspend/resume, or do you need help with that? (One of my testboxes that I have here, should be able to do suspend) Regards, Dario -- <> (Raistlin Majere) - Dario Faggioli, Ph.D, http://about.me/dario.faggioli Software Engineer @ SUSE https://www.suse.com/ signature.asc Description: This is a digitally signed message part ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [RFC PATCH 35/68] vfs: Convert xenfs to use the new mount API
Convert the xenfs filesystem to the new internal mount API as the old
one will be obsoleted and removed. This allows greater flexibility in
communication of mount parameters between userspace, the VFS and the
filesystem.
See Documentation/filesystems/mount_api.txt for more information.
Signed-off-by: David Howells
cc: Boris Ostrovsky
cc: Juergen Gross
cc: Stefano Stabellini
cc: xen-devel@lists.xenproject.org
---
drivers/xen/xenfs/super.c | 21 +++--
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/xen/xenfs/super.c b/drivers/xen/xenfs/super.c
index 71ddfb4cf61c..2e16214e9c7f 100644
--- a/drivers/xen/xenfs/super.c
+++ b/drivers/xen/xenfs/super.c
@@ -13,6 +13,7 @@
#include
#include
#include
+#include
#include
#include
@@ -42,7 +43,7 @@ static const struct file_operations capabilities_file_ops = {
.llseek = default_llseek,
};
-static int xenfs_fill_super(struct super_block *sb, void *data, int silent)
+static int xenfs_fill_super(struct super_block *sb, struct fs_context *fc)
{
static const struct tree_descr xenfs_files[] = {
[2] = { "xenbus", &xen_xenbus_fops, S_IRUSR|S_IWUSR },
@@ -67,17 +68,25 @@ static int xenfs_fill_super(struct super_block *sb, void
*data, int silent)
xen_initial_domain() ? xenfs_init_files : xenfs_files);
}
-static struct dentry *xenfs_mount(struct file_system_type *fs_type,
- int flags, const char *dev_name,
- void *data)
+static int xenfs_get_tree(struct fs_context *fc)
{
- return mount_single(fs_type, flags, data, xenfs_fill_super);
+ return vfs_get_super(fc, vfs_get_single_super, xenfs_fill_super);
+}
+
+static const struct fs_context_operations xenfs_context_ops = {
+ .get_tree = xenfs_get_tree,
+};
+
+static int xenfs_init_fs_context(struct fs_context *fc)
+{
+ fc->ops = &xenfs_context_ops;
+ return 0;
}
static struct file_system_type xenfs_type = {
.owner =THIS_MODULE,
.name = "xenfs",
- .mount =xenfs_mount,
+ .init_fs_context = xenfs_init_fs_context,
.kill_sb = kill_litter_super,
};
MODULE_ALIAS_FS("xenfs");
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 134140: trouble: blocked/broken/pass
flight 134140 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/134140/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-xsm broken build-arm64-xsm 4 host-install(4)broken REGR. vs. 133991 Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen e88afede8cbc18032bcab49b3a25b472d5516cf5 baseline version: xen cb70a26f78848fe45f593f7ebc9cfaac760a791b Last test of basis 133991 2019-03-22 15:00:46 Z5 days Failing since134068 2019-03-25 12:00:51 Z2 days 12 attempts Testing same since 134136 2019-03-27 18:00:28 Z0 days2 attempts People who touched revisions under test: Andrew Cooper Wei Liu jobs: build-arm64-xsm broken build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm blocked test-amd64-amd64-xl-qemuu-debianhvm-i386 pass 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 broken-job build-arm64-xsm broken broken-step build-arm64-xsm host-install(4) Not pushing. commit e88afede8cbc18032bcab49b3a25b472d5516cf5 Author: Andrew Cooper Date: Tue Jul 10 13:53:21 2018 +0100 libx86: Recalculate synthesised cpuid_policy fields when appropriate When filling a policy, either from CPUID or an incomming leaf stream, recalculate the synthesised vendor value. All callers are expected to want this behaviour. Signed-off-by: Andrew Cooper Reviewed-by: Jan Beulich commit 1c2c9f85dd36bd908441b37ab73172358509c9b5 Author: Andrew Cooper Date: Wed Mar 20 14:56:15 2019 + tools/libxc: Use x86_cpuid_lookup_vendor() rather than opencoding the logic This doesn't address any of the assumptions that "anything which isn't AMD is Intel". This logic is expected to be replaced wholesale with libx86 in the longterm. Signed-off-by: Andrew Cooper Reviewed-by: Jan Beulich commit 00b4f4d0fb75dc183b499e78d1abcb865dbc30d7 Author: Andrew Cooper Date: Tue Jul 10 13:53:21 2018 +0100 x86/cpuid: Drop get_cpu_vendor() completely get_cpu_vendor() tries to do a number of things, and ends up doing none of them well. For calculating the vendor itself, use x86_cpuid_lookup_vendor() which is implemented in a far more efficient manner than looping over cpu_devs[]. For setting up this_cpu, set it up once on the BSP only, rather than latest-takes-precident across the APs. Such a system is probably not going to boot, but this feels like a less dangerous course of action. Adjust the printed errors to be more clear in the mismatch case. This removes the only user of cpu_dev->c_ident[], so drop that field as well. Signed-off-by: Andrew Cooper Reviewed-by: Jan Beulich commit e72309ffbe7c4e507649c74749f130cda691131c Author: Andrew Cooper Date: Wed Mar 20 14:05:11 2019 + libx86: Introduce x86_cpuid_lookup_vendor() Also introduce constants for the vendor strings in CPUID leaf 0. Signed-off-by: Andrew Cooper Reviewed-by: Jan Beulich commit 8eed571409a7f81ec9327cfa95d7c298333e22e4 Author: Andrew Cooper Date: Tue Mar 26 14:23:03 2019 + CI: Add a CentOS 6 container and build jobs CentOS 6 is probably the most frequently broken build, so adding it to CI would be a very good move. One problem is that CentOS 6 comes with Python 2.6, and Qemu requires 2.7. There appear to be no sensible ways
[Xen-devel] [ovmf baseline-only test] 83833: trouble: blocked/broken
This run is configured for baseline tests only. flight 83833 ovmf real [real] http://osstest.xensource.com/osstest/logs/83833/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm broken build-i386 broken build-amd64-pvopsbroken build-i386-xsm broken build-amd64 broken build-i386-pvops broken Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a build-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a build-amd64 4 host-install(4) broken baseline untested build-amd64-xsm 4 host-install(4) broken baseline untested build-amd64-pvops 4 host-install(4) broken baseline untested build-i386-xsm4 host-install(4) broken baseline untested build-i3864 host-install(4) broken baseline untested build-i386-pvops 4 host-install(4) broken baseline untested version targeted for testing: ovmf c455bc8c8d78ad51c24426a500914ea32504bf06 baseline version: ovmf 2f2c51acfb70efe3dd02022ca09dd853601d8acd Last test of basis83827 2019-03-26 15:54:48 Z1 days Testing same since83833 2019-03-27 22:25:02 Z0 days1 attempts People who touched revisions under test: Ard Biesheuvel Bob Feng Bret Barkelew Feng, Bob C Jordan Justen Laszlo Ersek Leif Lindholm Rebecca Cran Rebecca Cran via edk2-devel Shenglei Zhang Zhichao Gao jobs: build-amd64-xsm broken build-i386-xsm broken build-amd64 broken build-i386 broken build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopsbroken build-i386-pvops broken test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 blocked sg-report-flight on osstest.xs.citrite.net logs: /home/osstest/logs images: /home/osstest/images Logs, config files, etc. are available at http://osstest.xensource.com/osstest/logs Test harness code can be found at http://xenbits.xensource.com/gitweb?p=osstest.git;a=summary broken-job build-amd64-xsm broken broken-job build-i386 broken broken-job build-amd64-pvops broken broken-job build-i386-xsm broken broken-job build-amd64 broken broken-job build-i386-pvops broken broken-step build-amd64 host-install(4) broken-step build-amd64-xsm host-install(4) broken-step build-amd64-pvops host-install(4) broken-step build-i386-xsm host-install(4) broken-step build-i386 host-install(4) broken-step build-i386-pvops host-install(4) Push not applicable. commit c455bc8c8d78ad51c24426a500914ea32504bf06 Author: Shenglei Zhang Date: Thu Mar 21 09:47:31 2019 +0800 EdkCompatibilityPkg: Remove EdkCompatibilityPkg https://bugzilla.tianocore.org/show_bug.cgi?id=1103 Cc: Liming Gao Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Shenglei Zhang Reviewed-by: Liming Gao commit 5bca07268acabe7f31407358e875ccf89cb5e386 Author: Shenglei Zhang Date: Fri Mar 22 09:05:22 2019 +0800 Maintainers.txt: Remove EdkCompatibilityPkg information EdkCompatibilityPkg will be deleted from edk2/master. So update the maintainer information of EdkCompatibilityPkg. https://bugzilla.tianocore.org/show_bug.cgi?id=1103 Cc: Andrew Fish Cc: Laszlo Ersek Cc: Leif Lindholm Cc: Michael D Kinney Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Shenglei Zhang Reviewed-by: Liming Gao Reviewed-by: Laszlo Ersek commit fdebdc961bfdc600d4040213ef4e757f3fff3574 Author: Shenglei Zhang Date: Fri Mar 22 09:25:56 2019 +0800 Nt32Pkg/Nt32Pkg.dsc: Remove EdkCompatibilityPkg information EdkCompatibilityPkg will be removed from edk2/master. The dependency about EdkCompatibilityPkg in Nt32Pkg.dsc should also be removed. Cc: Ray Ni Cc: Hao Wu
[Xen-devel] [linux-4.9 test] 134117: regressions - trouble: blocked/broken/fail/pass
flight 134117 linux-4.9 real [real] http://logs.test-lab.xenproject.org/osstest/logs/134117/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-pvopsbroken build-arm64 broken build-arm64-xsm broken build-arm64-xsm 4 host-install(4)broken REGR. vs. 134015 build-arm64-pvops 4 host-install(4)broken REGR. vs. 134015 build-arm64 4 host-install(4)broken REGR. vs. 134015 test-armhf-armhf-libvirt-raw 10 debian-di-installfail REGR. vs. 134015 Tests which did not succeed, but are not blocking: test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a test-arm64-arm64-examine 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-xl 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit1 1 build-check(1) blocked n/a test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 134015 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 134015 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 134015 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 134015 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 134015 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-amd64-xl-pvhv2-amd 12 guest-start fail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass version targeted for testing: linux60771fc402877163d07569addadcf18b86acb455 baseline version: linux1c453afcda4f68f634475f166418e937ac235200 Last test of basis 134015 2019-03-23 12:49:59 Z4 days Tes
