Re: [Xen-devel] [PATCH 4/6] xen: don't free percpu areas during suspend
On 27/03/2019 17:52, Juergen Gross wrote: > 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. Looking deeper into that mess I believe that should be a series of its own. Cpu parking needs to be handled for cpu hotplug and core parking (XENPF_core_parking), too. 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 18.03.19 at 14:11, 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. There's another aspect here which needs at least mentioning: Right now, CPUs coming back up have their per-CPU data cleared. Not doing so may indeed be an improvement in some cases (like statistics collected), but may also get in the way in other cases. 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 28/03/2019 08:46, Jan Beulich wrote: On 18.03.19 at 14:11, 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. > > There's another aspect here which needs at least mentioning: > Right now, CPUs coming back up have their per-CPU data > cleared. Not doing so may indeed be an improvement in > some cases (like statistics collected), but may also get in the > way in other cases. I have checked the respective cpu notifier hooks and I think this is no problem. I hope I didn't overlook something. I'll add a note 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 28.03.19 at 07:59, wrote: > On 27/03/2019 17:52, Juergen Gross wrote: >> 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. > > Looking deeper into that mess I believe that should be a series of its > own. Cpu parking needs to be handled for cpu hotplug and core parking > (XENPF_core_parking), too. What issue do you see for CPU hotplug? cpu_up_helper() has been modified by the parking series. For core parking I wonder whether core_parking_helper() shouldn't, first of all, invoke cpu_{up,down}_helper(). This wouldn't be enough, though - the policy hooks need to honor opt_smt as well. As to this wanting to be a patch / series of its own - I don't mind, but preferably it would come ahead of your changes here, so that it can be backported independently and (sufficiently) easily (unless of course there's really no collision between the two). 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 28.03.19 at 08:53, wrote: > On 28/03/2019 08:46, Jan Beulich wrote: > On 18.03.19 at 14:11, 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. >> >> There's another aspect here which needs at least mentioning: >> Right now, CPUs coming back up have their per-CPU data >> cleared. Not doing so may indeed be an improvement in >> some cases (like statistics collected), but may also get in the >> way in other cases. > > I have checked the respective cpu notifier hooks and I think this is no > problem. I hope I didn't overlook something. Is checking the hooks sufficient? I'd rather expect we need to check all per-CPU data objects. Jan ___ 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 27.03.19 at 18:28, wrote: > This also lacks some of the features of mwait-idle has and duplicates > the limited functionality. Would you mind clarifying the lack-of-features aspect? The only difference to your patches that I can spot is that you set .target_residency in the static tables. If the value wanted for CC6 is really 1000 instead of the 800 the default calculation would produce, then this would be easy enough to arrange for in my variant of the patch as well. The mwait-idle driver would not have been needed at all if all BIOSes surfaced complete data via ACPI. Therefore, by suitably populating tables, it ought to be possible in theory to use just that one driver. It's just that for Intel CPUs we've decided to follow what Linux does, hence the separate driver. There's no Linux precedent for AMD (afaict). > There's also a lack of comments which may or > may not be needed. So that would add to the line change count if you > care about that. > > I'm not sure why you're so adverse to the mwait-idle patches. We're > hard coding values in and using mwait (just like Intel is), but the only > real change we need is using halt for one c-state. But that's precisely what I dislike, as getting us further away from the Linux driver. And btw, if we were to go that route, then I think we'd better call acpi_idle_do_entry() than to duplicate further parts of it. But that would also remove some of that other part of the benefits of mwait_idle() over acpi_processor_idle(): The former is much more streamlined, due to not having to care about anything other than MWAIT. As an aside, despite having followed the HLT approach in my draft patch, I continue to be unconvinced that this is what we actually want. There's a respective TBD remark there. 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 28/03/2019 09:03, Jan Beulich wrote: On 28.03.19 at 07:59, wrote: >> On 27/03/2019 17:52, Juergen Gross wrote: >>> 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. >> >> Looking deeper into that mess I believe that should be a series of its >> own. Cpu parking needs to be handled for cpu hotplug and core parking >> (XENPF_core_parking), too. > > What issue do you see for CPU hotplug? cpu_up_helper() has > been modified by the parking series. I was thinking of hot unplug. cpu_down() won't do the job for a parked cpu. > For core parking I wonder whether core_parking_helper() > shouldn't, first of all, invoke cpu_{up,down}_helper(). This > wouldn't be enough, though - the policy hooks need to honor > opt_smt as well. Right. > As to this wanting to be a patch / series of its own - I don't > mind, but preferably it would come ahead of your changes > here, so that it can be backported independently and > (sufficiently) easily (unless of course there's really no > collision between the two). In case there is a collision it should be fairly minimal. I'd prefer not to block my series as it is a prerequisite for my core scheduling series, which I believe should go in rather sooner than later as it probably should see lot of testing before the next release. 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 28/03/2019 09:04, Jan Beulich wrote: On 28.03.19 at 08:53, wrote: >> On 28/03/2019 08:46, Jan Beulich wrote: >> On 18.03.19 at 14:11, 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. >>> >>> There's another aspect here which needs at least mentioning: >>> Right now, CPUs coming back up have their per-CPU data >>> cleared. Not doing so may indeed be an improvement in >>> some cases (like statistics collected), but may also get in the >>> way in other cases. >> >> I have checked the respective cpu notifier hooks and I think this is no >> problem. I hope I didn't overlook something. > > Is checking the hooks sufficient? I'd rather expect we need to > check all per-CPU data objects. Why? The main concern would be a hook not doing its job due to some data not being zeroed. In case there is a dependency somewhere else I'd rather expect subtle negative effects with today's solution as suddenly percpu data will be zero after suspend/resume without a component having been involved with suspending. Sure, there could be negative effects with my series, but I'd asset the chances lower as without my series. Suspend/resume tends to be not tested very often, it seems. I guess I'm the only one having it tried recently (it was broken for 3 months at least before my patch repairing an issue with IOMMU). Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [freebsd-master test] 134123: all pass - PUSHED
flight 134123 freebsd-master real [real] http://logs.test-lab.xenproject.org/osstest/logs/134123/ Perfect :-) All tests in this flight passed as required version targeted for testing: freebsd 4871e6f10ee5265bebc03fd9395874187de091b0 baseline version: freebsd 17b9e44c40f1d40251c09600e89ad6c1752318bf Last test of basis 134066 2019-03-25 09:19:15 Z2 days Testing same since 134123 2019-03-27 09:19:57 Z0 days1 attempts People who touched revisions under test: andrew avos bde cem emaste gonzo grembo hselasky jhb jhibbits kevans markj mm philip rrs sobomax trasz tuexen jobs: build-amd64-freebsd-againpass build-amd64-freebsd pass build-amd64-xen-freebsd 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/freebsd.git 17b9e44c40f..4871e6f10ee 4871e6f10ee5265bebc03fd9395874187de091b0 -> tested/master ___ 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 27.03.19 at 18:07, wrote: > 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. I don't think it's needed before my change either, so I'd call removing this an orthogonal change. As said in the commit message, the goal was to leave this function alone as far as possible. Before my patch, xen_filter_cpu_maps() gets called long after prefill_possible_map(), and by the purpose of the latter function the possible map shouldn't be altered anymore once that has run. Adding bits to it is surely not going to have the intended effect (setup_per_cpu_areas() has already run), while removing bits may have some effect, but comes too late at least for setup_per_cpu_areas(). And if we were to remove this, I think the CONFIG_HOTPLUG_CPU section should go away as well. After all prefill_possible_map() also sets nr_cpu_ids. To be honest, it was largely this code fragment which made me want not touch the function more than necessary: The comment there makes not clear to me at all why all of this needs to be in an #ifdef in the first place. Let me know whether you really want me to fold this extra cleanup into this patch. If so, I'd then wonder whether the set_cpu_present() from xen_pv_smp_prepare_cpus() shouldn't be moved here, too. And the fiddling with the possible map there looks bogus as well: Bring-up of CPUs past the command line option should be avoided at boot time, but they shouldn't be excluded from getting brought up later on. Note how native_smp_prepare_cpus() ignores its parameter altogether. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 134146: trouble: blocked/broken/pass
flight 134146 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/134146/ 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 13 attempts Testing same since 134136 2019-03-27 18:00:28 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 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] [PATCH v2 1/1] 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. Signed-off-by: Xin Li --- CC: Jan Beulich CC: Igor Druzhinin CC: Sergey Dyasli CC: Andrew Cooper v2 1: write the struct if any of the strings is provided 2: add contained_handles as flexible array member 3: update commit message and fix style issue --- tools/firmware/hvmloader/smbios.c | 69 - tools/firmware/hvmloader/smbios_types.h | 7 +++ xen/include/public/hvm/hvm_xs_strings.h | 6 +++ 3 files changed, 80 insertions(+), 2 deletions(-) diff --git a/tools/firmware/hvmloader/smbios.c b/tools/firmware/hvmloader/smbios.c index 40d8399be1..226c38efb2 100644 --- a/tools/firmware/hvmloader/smbios.c +++ b/tools/firmware/hvmloader/smbios.c @@ -497,9 +497,11 @@ static void * smbios_type_2_init(void *start) { struct smbios_type_2 *p = (struct smbios_type_2 *)start; +const char *s; uint8_t *ptr; void *pts; uint32_t length; +uint32_t counter = 0; pts = get_smbios_pt_struct(2, &length); if ( (pts != NULL)&&(length > 0) ) @@ -518,8 +520,71 @@ smbios_type_2_init(void *start) return (start + length); } -/* Only present when passed in */ -return start; +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 */ +p->chassis_handle = SMBIOS_HANDLE_TYPE3; +p->board_type = 0x0a; /* Motherboard */ +start += sizeof(struct smbios_type_2); + +s = xenstore_read(HVM_XS_BASEBOARD_MANUFACTURER, NULL); +if ( (s != NULL) && (*s != '\0') ) +{ +strcpy(start, s); +start += strlen(s) + 1; +p->manufacturer_str = ++counter; +} + +s = xenstore_read(HVM_XS_BASEBOARD_PRODUCT_NAME, NULL); +if ( (s != NULL) && (*s != '\0') ) +{ +strcpy(start, s); +start += strlen(s) + 1; +p->product_name_str = ++counter; +} + +s = xenstore_read(HVM_XS_BASEBOARD_VERSION, NULL); +if ( (s != NULL) && (*s != '\0') ) +{ +strcpy(start, s); +start += strlen(s) + 1; +p->version_str = ++counter; +} + +s = xenstore_read(HVM_XS_BASEBOARD_SERIAL_NUMBER, NULL); +if ( (s != NULL) && (*s != '\0') ) +{ +strcpy(start, s); +start += strlen(s) + 1; +p->serial_number_str = ++counter; +} + +s = xenstore_read(HVM_XS_BASEBOARD_ASSET_TAG, NULL); +if ( (s != NULL) && (*s != '\0') ) +{ +strcpy(start, s); +start += strlen(s) + 1; +p->asset_tag_str = ++counter; +} + +s = xenstore_read(HVM_XS_BASEBOARD_LOCATION_IN_CHASSIS, NULL); +if ( (s != NULL) && (*s != '\0') ) +{ +strcpy(start, s); +start += strlen(s) + 1; +p->location_in_chassis_str = ++counter; +} + +if ( counter ) +{ +*((uint8_t *)start) = 0; +return (start + 1); +} +else +/* Only present when passed in or with customized string */ +return (start - sizeof(struct smbios_type_2)); } /* Type 3 -- System Enclosure */ diff --git a/tools/firmware/hvmloader/smbios_types.h b/tools/firmware/hvmloader/smbios_types.h index acb63e2fe9..7c648ece71 100644 --- a/tools/firmware/hvmloader/smbios_types.h +++ b/tools/firmware/hvmloader/smbios_types.h @@ -90,6 +90,13 @@ 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[]; } __attribute__ ((packed)); /* System Enclosure - Contained Elements */ diff --git a/xen/include/public/hvm/hvm_xs_strings.h b/xen/include/public/hvm/hvm_xs_strings.h index fea1dd4407..fba2546424 100644 --- a/xen/include/public/hvm/hvm_xs_strings.h +++ b/xen/include/public/hvm/hvm_xs_strings.h @@ -69,6 +69,12 @@ #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"
Re: [Xen-devel] [PATCH RFC v4] x86/mm: Clean up p2m_finish_type_change return value
On 27.03.2019 18:07, Jan Beulich wrote: 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. > It's alright, it's an honest mistake and in this case I will go back and have finish_type_change() cut the positive value if that is ok with everyone. Regards, Alex >> @@ -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. > ___ 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 28.03.19 at 09:35, wrote: > On 28/03/2019 09:03, Jan Beulich wrote: > On 28.03.19 at 07:59, wrote: >>> On 27/03/2019 17:52, Juergen Gross wrote: 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. >>> >>> Looking deeper into that mess I believe that should be a series of its >>> own. Cpu parking needs to be handled for cpu hotplug and core parking >>> (XENPF_core_parking), too. >> >> What issue do you see for CPU hotplug? cpu_up_helper() has >> been modified by the parking series. > > I was thinking of hot unplug. cpu_down() won't do the job for a parked > cpu. There's nothing to be done when soft-offlining a CPU. And I'm not convinced physical CPU unplug was actually ever tested to work. >> As to this wanting to be a patch / series of its own - I don't >> mind, but preferably it would come ahead of your changes >> here, so that it can be backported independently and >> (sufficiently) easily (unless of course there's really no >> collision between the two). > > In case there is a collision it should be fairly minimal. > > I'd prefer not to block my series as it is a prerequisite for my core > scheduling series, which I believe should go in rather sooner than later > as it probably should see lot of testing before the next release. Understood. 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 27.03.2019 18:07, Jan Beulich wrote: 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. > Sorry the last reply got mixed up this was my reply: It's alright, it's an honest mistake and in this case I will go back and have finish_type_change() cut the positive value if that is ok with everyone. Regards, Alex >> @@ -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. > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/vvmx: Fix debug prints to not have 17 unnecessary spaces
>>> On 27.03.19 at 20:53, wrote: > This has been problematic since its introduction in Xen 4.3 > > Signed-off-by: Andrew Cooper Reviewed-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 01/12] xen: clang: Support correctly cross-compile
>>> On 27.03.19 at 19:45, wrote: > Clang uses "-target" option for cross-compilation. And all possible targets are always available? I'd like to point out that CROSS_COMPILE can be used for other than actual cross compilation, e.g. building with just an alternative tool chain built for the same target. Dropping the $(CROSS_COMPILE) prefixes makes this impossible afaict. Requiring suitable wrapper scripts to be put in place would seem better to me. I also wonder why this change is needed for Arm, but wasn't needed so far for x86. But perhaps no-one ever tried using it so far ... > --- 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:-=) Is dropping dashes from the variable uniformly correct? If so, could you please clarify in the commit message why that is? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 01/12] xen: clang: Support correctly cross-compile
On 28/03/2019 09:55, Jan Beulich wrote: On 27.03.19 at 19:45, wrote: >> Clang uses "-target" option for cross-compilation. > And all possible targets are always available? I'd like to point out > that CROSS_COMPILE can be used for other than actual cross > compilation, e.g. building with just an alternative tool chain > built for the same target. Dropping the $(CROSS_COMPILE) > prefixes makes this impossible afaict. Requiring suitable wrapper > scripts to be put in place would seem better to me. > > I also wonder why this change is needed for Arm, but wasn't > needed so far for x86. But perhaps no-one ever tried using it > so far ... It seems that CROSS_COMPILE is a GNU-ism, which is not shared by the clang world. I can't find anything which will make you a $FOO-$BAR-clang binary, whereas you do typically get clang-$X aliases for the different versions of clang. Using -target is from the Clang instructions on cross compilation, which say to do it this way. https://clang.llvm.org/docs/CrossCompilation.html The targets supported will depend on the configuration Clang was compiled with, but Clang specifically opposes GCC's way of requiring the user to recompile for every different target. It is expected that a packager of clang will enable all of the supported targets in the package they distribute. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 01/12] xen: clang: Support correctly cross-compile
>>> On 28.03.19 at 11:14, wrote: > On 28/03/2019 09:55, Jan Beulich wrote: > On 27.03.19 at 19:45, wrote: >>> Clang uses "-target" option for cross-compilation. >> And all possible targets are always available? I'd like to point out >> that CROSS_COMPILE can be used for other than actual cross >> compilation, e.g. building with just an alternative tool chain >> built for the same target. Dropping the $(CROSS_COMPILE) >> prefixes makes this impossible afaict. Requiring suitable wrapper >> scripts to be put in place would seem better to me. >> >> I also wonder why this change is needed for Arm, but wasn't >> needed so far for x86. But perhaps no-one ever tried using it >> so far ... > > It seems that CROSS_COMPILE is a GNU-ism, which is not shared by the > clang world. I can't find anything which will make you a > $FOO-$BAR-clang binary, whereas you do typically get clang-$X aliases > for the different versions of clang. Oh, interesting. For my own re-built tool chains I actually can't use CROSS_COMPILE either, because traditionally all my wrapper scripts have a suffix like you say clang uses too. I patch in cross-compile ?= $(CROSS_COMPILE)$(1) locally as a fallback, to then use it as AS = $(call cross-compile,as) and then override things in the build root directory .config to suite my actual needs, e.g. in its simplest possible form cross-compile=$(1)x But of course this doesn't fit clang either, as it's (aiui) only the compiler which wants to be overridden this way. > Using -target is from the Clang instructions on cross compilation, which > say to do it this way. https://clang.llvm.org/docs/CrossCompilation.html > > The targets supported will depend on the configuration Clang was > compiled with, but Clang specifically opposes GCC's way of requiring the > user to recompile for every different target. It is expected that a > packager of clang will enable all of the supported targets in the > package they distribute. Are you sure a distro caring about, say, only x86 would indeed enable Arm and all sorts of other architectures in the compiler, just because it can be enabled? IOW I assume the need for an override to the system default clang binaries would still exist. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 01/12] xen: clang: Support correctly cross-compile
On 28/03/2019 10:28, Jan Beulich wrote: On 28.03.19 at 11:14, wrote: >> On 28/03/2019 09:55, Jan Beulich wrote: >> On 27.03.19 at 19:45, wrote: Clang uses "-target" option for cross-compilation. >>> And all possible targets are always available? I'd like to point out >>> that CROSS_COMPILE can be used for other than actual cross >>> compilation, e.g. building with just an alternative tool chain >>> built for the same target. Dropping the $(CROSS_COMPILE) >>> prefixes makes this impossible afaict. Requiring suitable wrapper >>> scripts to be put in place would seem better to me. >>> >>> I also wonder why this change is needed for Arm, but wasn't >>> needed so far for x86. But perhaps no-one ever tried using it >>> so far ... >> It seems that CROSS_COMPILE is a GNU-ism, which is not shared by the >> clang world. I can't find anything which will make you a >> $FOO-$BAR-clang binary, whereas you do typically get clang-$X aliases >> for the different versions of clang. > Oh, interesting. For my own re-built tool chains I actually can't use > CROSS_COMPILE either, because traditionally all my wrapper scripts > have a suffix like you say clang uses too. I patch in > > cross-compile ?= $(CROSS_COMPILE)$(1) > > locally as a fallback, to then use it as > > AS = $(call cross-compile,as) > > and then override things in the build root directory .config to > suite my actual needs, e.g. in its simplest possible form > > cross-compile=$(1)x > > But of course this doesn't fit clang either, as it's (aiui) only the > compiler which wants to be overridden this way. > >> Using -target is from the Clang instructions on cross compilation, which >> say to do it this way. https://clang.llvm.org/docs/CrossCompilation.html >> >> The targets supported will depend on the configuration Clang was >> compiled with, but Clang specifically opposes GCC's way of requiring the >> user to recompile for every different target. It is expected that a >> packager of clang will enable all of the supported targets in the >> package they distribute. > Are you sure a distro caring about, say, only x86 would indeed > enable Arm and all sorts of other architectures in the compiler, > just because it can be enabled? IOW I assume the need for an > override to the system default clang binaries would still exist. I've just tried, and Ubuntu 16.04's default clang-3.8 is perfectly happy compiling Aarch64, and makes a suitable looking elf object. (I can't actually disassemble it because objdump chokes, but .text is the expected length) As the cross-compilation documentation states, this is a deliberate design decision which, amongst other things, prevents distros from needing to maintain per-arch packages. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 1/1] hvmloader: add SMBIOS type 2 info for customized string
>>> On 28.03.19 at 10:05, wrote: > --- a/tools/firmware/hvmloader/smbios.c > +++ b/tools/firmware/hvmloader/smbios.c > @@ -497,9 +497,11 @@ static void * > smbios_type_2_init(void *start) > { > struct smbios_type_2 *p = (struct smbios_type_2 *)start; > +const char *s; > uint8_t *ptr; > void *pts; > uint32_t length; > +uint32_t counter = 0; I think this wants to be unsigned int. > @@ -518,8 +520,71 @@ smbios_type_2_init(void *start) > return (start + length); > } > > -/* Only present when passed in */ > -return start; > +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 */ > +p->chassis_handle = SMBIOS_HANDLE_TYPE3; > +p->board_type = 0x0a; /* Motherboard */ > +start += sizeof(struct smbios_type_2); sizeof(*p) (also below) please. > +s = xenstore_read(HVM_XS_BASEBOARD_MANUFACTURER, NULL); > +if ( (s != NULL) && (*s != '\0') ) > +{ > +strcpy(start, s); > +start += strlen(s) + 1; > +p->manufacturer_str = ++counter; > +} > + > +s = xenstore_read(HVM_XS_BASEBOARD_PRODUCT_NAME, NULL); > +if ( (s != NULL) && (*s != '\0') ) > +{ > +strcpy(start, s); > +start += strlen(s) + 1; > +p->product_name_str = ++counter; > +} > + > +s = xenstore_read(HVM_XS_BASEBOARD_VERSION, NULL); > +if ( (s != NULL) && (*s != '\0') ) > +{ > +strcpy(start, s); > +start += strlen(s) + 1; > +p->version_str = ++counter; > +} > + > +s = xenstore_read(HVM_XS_BASEBOARD_SERIAL_NUMBER, NULL); > +if ( (s != NULL) && (*s != '\0') ) > +{ > +strcpy(start, s); > +start += strlen(s) + 1; > +p->serial_number_str = ++counter; > +} > + > +s = xenstore_read(HVM_XS_BASEBOARD_ASSET_TAG, NULL); > +if ( (s != NULL) && (*s != '\0') ) > +{ > +strcpy(start, s); > +start += strlen(s) + 1; > +p->asset_tag_str = ++counter; > +} > + > +s = xenstore_read(HVM_XS_BASEBOARD_LOCATION_IN_CHASSIS, NULL); > +if ( (s != NULL) && (*s != '\0') ) > +{ > +strcpy(start, s); > +start += strlen(s) + 1; > +p->location_in_chassis_str = ++counter; > +} > + > +if ( counter ) > +{ > +*((uint8_t *)start) = 0; > +return (start + 1); > +} > +else > +/* Only present when passed in or with customized string */ > +return (start - sizeof(struct smbios_type_2)); This will work, but is slightly more cumbersome to read than if you simply used a local variable and kept "start" unchanged. If the local variable was of "char *" type, you could also avoid the uint8_t * cast above. And I think there are a number of unnecessary parentheses in these last few lines. The "else" is unnecessary as well. If you don't want to go the route of a separate local variable, then with the cosmetic issues fixed this patch can have my Acked-by: Jan Beulich right away, and these cosmetic adjustments could be done while committing. But please let me know whether to wait for a v3 instead. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 01/12] xen: clang: Support correctly cross-compile
>>> On 28.03.19 at 11:43, wrote: > On 28/03/2019 10:28, Jan Beulich wrote: > On 28.03.19 at 11:14, wrote: >>> Using -target is from the Clang instructions on cross compilation, which >>> say to do it this way. https://clang.llvm.org/docs/CrossCompilation.html >>> >>> The targets supported will depend on the configuration Clang was >>> compiled with, but Clang specifically opposes GCC's way of requiring the >>> user to recompile for every different target. It is expected that a >>> packager of clang will enable all of the supported targets in the >>> package they distribute. >> Are you sure a distro caring about, say, only x86 would indeed >> enable Arm and all sorts of other architectures in the compiler, >> just because it can be enabled? IOW I assume the need for an >> override to the system default clang binaries would still exist. > > I've just tried, and Ubuntu 16.04's default clang-3.8 is perfectly happy > compiling Aarch64, and makes a suitable looking elf object. (I can't > actually disassemble it because objdump chokes, but .text is the > expected length) > > As the cross-compilation documentation states, this is a deliberate > design decision which, amongst other things, prevents distros from > needing to maintain per-arch packages. All understood, just that Ubuntu may not be a good example, as there looks to be Ubuntu 16.04 for 64-bit Arm. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [qemu-mainline test] 134124: regressions - trouble: blocked/broken/fail/pass
flight 134124 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/134124/ 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. 133909 build-arm64-pvops 4 host-install(4)broken REGR. vs. 133909 build-arm64-xsm 4 host-install(4)broken REGR. vs. 133909 test-amd64-i386-freebsd10-amd64 14 guest-saverestore fail REGR. vs. 133909 test-amd64-i386-freebsd10-i386 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-amd64-xl-qemuu-debianhvm-amd64-shadow 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-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-xl-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl 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-libvirt-xsm 1 build-check(1) blocked n/a build-arm64-libvirt 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-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 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-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-rtds 13 migrate-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 00/12] xen/arm: Add support to build with clang
Hi Julien, On Wed, 2019-03-27 at 18:45 +, Julien Grall wrote: > 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. > I wonder if you have time to try the series with Arm Compiler 6? I am asking because AFAIK it is based on clang/llvm [1] and there's a safety-compliant version of it certified by TUV [2]. I don't have a license yet so cannot try it myself but maybe you have access. > > 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(-) > [1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.subset.swdev.comp6/index.html [2] https://store.developer.arm.com/store/embedded-iot-software-tools/arm-compiler-6-functional-safety?_ga=2.198817711.1237223028.1553771353-424456504.1550666847 ___ 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 Wed, Mar 27, 2019 at 08:32:28PM +, Paul Durrant wrote: > > -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? So QEMU used to always set "sector-size" to 512, and used that for request. The new implementation (not released yet) doesn't do that anymore, and may set "sector-size" to a different value and used that for requests. patch #1 is one way to fix the requests (and avoid regression) and more clearly spell out the weird thing about the spec. I also think patch #2 is too soon and should point to a commit in xen.git instead of a thread on xen-devel. In the meantime, we should probably set "sector-size" to 512, like QEMU used to do anyway, with a comment about the fact that different implementations uses sector-size differently and a value of 512 would work fine. -- Anthony PERARD ___ 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 28/03/2019 11:40, Anthony PERARD wrote: > On Wed, Mar 27, 2019 at 08:32:28PM +, Paul Durrant wrote: >>> -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? > So QEMU used to always set "sector-size" to 512, and used that for > request. The new implementation (not released yet) doesn't do that > anymore, and may set "sector-size" to a different value and used that > for requests. > > patch #1 is one way to fix the requests (and avoid regression) and > more clearly spell out the weird thing about the spec. > > I also think patch #2 is too soon and should point to a commit in > xen.git instead of a thread on xen-devel. > > In the meantime, we should probably set "sector-size" to 512, like QEMU > used to do anyway, with a comment about the fact that different > implementations uses sector-size differently and a value of 512 would > work fine. Hmm - I hadn't realised this is an unreleased issue in qemu. So, Qemu used to unconditionally set sector-size=512, and your work to qdev-ify everything introduced a change which has identified a spec/protocol issue? If so, then I think it is fine for this series to state (much more clearly than it does) that it is returning qemu's behaviour to match the currently released version, because we've discovered an issue in the spec/protocol, and that we will subsequently work address the issue in the spec and provide a forwards path which doesn't involve nailing our feet to the floor. ~Andrew ___ 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
Am 28.03.2019 um 12:46 hat Andrew Cooper geschrieben: > On 28/03/2019 11:40, Anthony PERARD wrote: > > On Wed, Mar 27, 2019 at 08:32:28PM +, Paul Durrant wrote: > >>> -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? > > So QEMU used to always set "sector-size" to 512, and used that for > > request. The new implementation (not released yet) doesn't do that > > anymore, and may set "sector-size" to a different value and used that > > for requests. > > > > patch #1 is one way to fix the requests (and avoid regression) and > > more clearly spell out the weird thing about the spec. > > > > I also think patch #2 is too soon and should point to a commit in > > xen.git instead of a thread on xen-devel. > > > > In the meantime, we should probably set "sector-size" to 512, like QEMU > > used to do anyway, with a comment about the fact that different > > implementations uses sector-size differently and a value of 512 would > > work fine. > > Hmm - I hadn't realised this is an unreleased issue in qemu. > > So, Qemu used to unconditionally set sector-size=512, and your work to > qdev-ify everything introduced a change which has identified a > spec/protocol issue? The old implementation has the sector size hardcoded: #define BLOCK_SIZE 512 Whereas the qdevified version uses DEFINE_BLOCK_PROPERTIES(), which includes user-visible options for logical/physical_block_size. So before, you couldn't even define a different sector size and the question whether 512 or the sector size should be used didn't make a difference anyway. > If so, then I think it is fine for this series to state (much more > clearly than it does) that it is returning qemu's behaviour to match the > currently released version, because we've discovered an issue in the > spec/protocol, and that we will subsequently work address the issue in > the spec and provide a forwards path which doesn't involve nailing our > feet to the floor. The closest thing to returning to the old behaviour would be erroring out during device initialisation if logical_block_size != 512. Kevin ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 1/6] xen/sched: call cpu_disable_scheduler() via cpu notifier
cpu_disable_scheduler() is being called from __cpu_disable() today. There is no need to execute it on the cpu just being disabled, so use the CPU_DEAD case of the cpu notifier chain. Moving the call out of stop_machine() context is fine, as we just need to hold the domain RCU lock and need the scheduler percpu data to be still allocated. Add another hook for CPU_DOWN_PREPARE to bail out early in case cpu_disable_scheduler() would fail. This will avoid crashes in rare cases for cpu hotplug or suspend. While at it remove a superfluous smp_mb() in the ARM __cpu_disable() incarnation. Signed-off-by: Juergen Gross --- V2: - add CPU_DOWN_PREPARE hook - BUG() in case of cpu_disable_scheduler() failing in CPU_DEAD (Jan Beulich) - modify ARM __cpu_disable(), too (Andrew Cooper) --- xen/arch/arm/smpboot.c | 4 xen/arch/x86/smpboot.c | 3 --- xen/common/schedule.c | 42 +++--- 3 files changed, 35 insertions(+), 14 deletions(-) diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c index 25cd44549c..0728a9b505 100644 --- a/xen/arch/arm/smpboot.c +++ b/xen/arch/arm/smpboot.c @@ -386,10 +386,6 @@ void __cpu_disable(void) /* It's now safe to remove this processor from the online map */ cpumask_clear_cpu(cpu, &cpu_online_map); -if ( cpu_disable_scheduler(cpu) ) -BUG(); -smp_mb(); - /* Return to caller; eventually the IPI mechanism will unwind and the * scheduler will drop to the idle loop, which will call stop_cpu(). */ } 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(); } void __cpu_die(unsigned int cpu) diff --git a/xen/common/schedule.c b/xen/common/schedule.c index 60755a631e..5d2bbd5198 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. */ 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 ) @@ -893,6 +888,30 @@ int cpu_disable_scheduler(unsigned int cpu) return ret; } +static int cpu_disable_scheduler_check(unsigned int cpu) +{ +struct domain *d; +struct vcpu *v; +struct cpupool *c; + +c = per_cpu(cpupool, cpu); +if ( c == NULL ) +return 0; + +for_each_domain_in_cpupool ( d, c ) +{ +for_each_vcpu ( d, v ) +{ +if ( v->affinity_broken ) +return -EADDRINUSE; +if ( system_state != SYS_STATE_suspend && v->processor == cpu ) +return -EAGAIN; +} +} + +return 0; +} + /* * In general, this must be called with the scheduler lock held, because the * adjust_affinity hook may want to modify the vCPU state. However, when the @@ -1737,7 +1756,16 @@ static int cpu_schedule_callback( case CPU_UP_PREPARE: rc = cpu_schedule_up(cpu); break; +case CPU_DOWN_PREPARE: +rcu_read_lock(&domlist_read_lock); +rc = cpu_disable_scheduler_check(cpu); +rcu_read_unlock(&domlist_read_lock); +break; case CPU_DEAD: +rcu_read_lock(&domlist_read_lock); +rc = cpu_disable_scheduler(cpu); +BUG_ON(rc); +rcu_read_unlock(&domlist_read_lock); SCHED_OP(sched, deinit_pdata, sd->sched_priv, cpu); /* Fallthrough */ case CPU_UP_CANCELED: -- 2.16.4 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 6/6] xen/sched: don't disable scheduler on cpus during suspend
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 --- xen/common/schedule.c | 161 -- 1 file changed, 52 insertions(+), 109 deletions(-) diff --git a/xen/common/schedule.c b/xen/common/schedule.c index 5d2bbd5198..6b5d454630 100644 --- a/xen/common/schedule.c +++ b/xen/common/schedule.c @@ -560,33 +560,6 @@ static void vcpu_move_locked(struct vcpu *v, unsigned int new_cpu) v->processor = new_cpu; } -/* - * Move a vcpu from its current processor to a target new processor, - * without asking the scheduler to do any placement. This is intended - * for being called from special contexts, where things are quiet - * enough that no contention is supposed to happen (i.e., during - * shutdown or software suspend, like ACPI S3). - */ -static void vcpu_move_nosched(struct vcpu *v, unsigned int new_cpu) -{ -unsigned long flags; -spinlock_t *lock, *new_lock; - -ASSERT(system_state == SYS_STATE_suspend); -ASSERT(!vcpu_runnable(v) && (atomic_read(&v->pause_count) || - atomic_read(&v->domain->pause_count))); - -lock = per_cpu(schedule_data, v->processor).schedule_lock; -new_lock = per_cpu(schedule_data, new_cpu).schedule_lock; - -sched_spin_lock_double(lock, new_lock, &flags); -ASSERT(new_cpu != v->processor); -vcpu_move_locked(v, new_cpu); -sched_spin_unlock_double(lock, new_lock, flags); - -sched_move_irqs(v); -} - /* * Initiating migration * @@ -735,31 +708,36 @@ void restore_vcpu_affinity(struct domain *d) ASSERT(!vcpu_runnable(v)); -lock = vcpu_schedule_lock_irq(v); - -if ( v->affinity_broken ) -{ -sched_set_affinity(v, v->cpu_hard_affinity_saved, NULL); -v->affinity_broken = 0; - -} - /* - * During suspend (in cpu_disable_scheduler()), we moved every vCPU - * to BSP (which, as of now, is pCPU 0), as a temporary measure to - * allow the nonboot processors to have their data structure freed - * and go to sleep. But nothing guardantees that the BSP is a valid - * pCPU for a particular domain. + * Re-assign the initial processor as after resume we have no + * guarantee the old processor has come back to life again. * * Therefore, here, before actually unpausing the domains, we should * set v->processor of each of their vCPUs to something that will * make sense for the scheduler of the cpupool in which they are in. */ cpumask_and(cpumask_scratch_cpu(cpu), v->cpu_hard_affinity, -cpupool_domain_cpumask(v->domain)); -v->processor = cpumask_any(cpumask_scratch_cpu(cpu)); +cpupool_domain_cpumask(d)); +if ( cpumask_empty(cpumask_scratch_cpu(cpu)) ) +{ +if ( v->affinity_broken ) +{ +sched_set_affinity(v, v->cpu_hard_affinity_saved, NULL); +v->affinity_broken = 0; +cpumask_and(cpumask_scratch_cpu(cpu), v->cpu_hard_affinity, +cpupool_domain_cpumask(d)); +} -spin_unlock_irq(lock); +if ( cpumask_empty(cpumask_scratch_cpu(cpu)) ) +{ +printk(XENLOG_DEBUG "Breaking affinity for %pv\n", v); +sched_set_affinity(v, &cpumask_all, NULL); +cpumask_and(cpumask_scratch_cpu(cpu), v->cpu_hard_affinity, +cpupool_domain_cpumask(d)); +} +} + +v->processor = cpumask_any(cpumask_scratch_cpu(cpu)); lock = vcpu_schedule_lock_irq(v); v->processor = SCHED_OP(vcpu_scheduler(v), pick_cpu, v); @@ -783,7 +761,6 @@ int cpu_disable_scheduler(unsigned int cpu) struct vcpu *v; struct cpupool *c; cpumask_t online_affinity; -unsigned int new_cpu; int ret = 0; c = per_cpu(cpupool, cpu); @@ -809,14 +786,7 @@ int cpu_disable_scheduler(unsigned int cpu) break; } -if (system_state == SYS_STATE_suspend) -{ -cpumask_copy(v->cpu_hard_affinity_saved, - v->cpu_hard
[Xen-devel] [PATCH v2 5/6] xen/cpupool: simplify suspend/resume handling
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 Reviewed-by: George Dunlap Reviewed-by: Dario Faggioli --- V2: - add comment (George Dunlap) --- xen/common/cpupool.c | 131 ++--- xen/include/xen/sched-if.h | 1 - 2 files changed, 52 insertions(+), 80 deletions(-) diff --git a/xen/common/cpupool.c b/xen/common/cpupool.c index e89bb67e71..31ac323e40 100644 --- a/xen/common/cpupool.c +++ b/xen/common/cpupool.c @@ -47,12 +47,6 @@ static struct cpupool *alloc_cpupool_struct(void) xfree(c); c = NULL; } -else if ( !zalloc_cpumask_var(&c->cpu_suspended) ) -{ -free_cpumask_var(c->cpu_valid); -xfree(c); -c = NULL; -} return c; } @@ -60,10 +54,7 @@ static struct cpupool *alloc_cpupool_struct(void) static void free_cpupool_struct(struct cpupool *c) { if ( c ) -{ -free_cpumask_var(c->cpu_suspended); free_cpumask_var(c->cpu_valid); -} xfree(c); } @@ -477,10 +468,6 @@ void cpupool_rm_domain(struct domain *d) /* * Called to add a cpu to a pool. CPUs being hot-plugged are added to pool0, * as they must have been in there when unplugged. - * - * If, on the other hand, we are adding CPUs because we are resuming (e.g., - * after ACPI S3) we put the cpu back in the pool where it was in prior when - * we suspended. */ static int cpupool_cpu_add(unsigned int cpu) { @@ -490,42 +477,15 @@ static int cpupool_cpu_add(unsigned int cpu) cpumask_clear_cpu(cpu, &cpupool_locked_cpus); cpumask_set_cpu(cpu, &cpupool_free_cpus); -if ( system_state == SYS_STATE_suspend || system_state == SYS_STATE_resume ) -{ -struct cpupool **c; - -for_each_cpupool(c) -{ -if ( cpumask_test_cpu(cpu, (*c)->cpu_suspended ) ) -{ -ret = cpupool_assign_cpu_locked(*c, cpu); -if ( ret ) -goto out; -cpumask_clear_cpu(cpu, (*c)->cpu_suspended); -break; -} -} +/* + * If we are not resuming, we are hot-plugging cpu, and in which case + * we add it to pool0, as it certainly was there when hot-unplagged + * (or unplugging would have failed) and that is the default behavior + * anyway. + */ +per_cpu(cpupool, cpu) = NULL; +ret = cpupool_assign_cpu_locked(cpupool0, cpu); -/* - * Either cpu has been found as suspended in a pool, and added back - * there, or it stayed free (if it did not belong to any pool when - * suspending), and we don't want to do anything. - */ -ASSERT(cpumask_test_cpu(cpu, &cpupool_free_cpus) || - cpumask_test_cpu(cpu, (*c)->cpu_valid)); -} -else -{ -/* - * If we are not resuming, we are hot-plugging cpu, and in which case - * we add it to pool0, as it certainly was there when hot-unplagged - * (or unplugging would have failed) and that is the default behavior - * anyway. - */ -per_cpu(cpupool, cpu) = NULL; -ret = cpupool_assign_cpu_locked(cpupool0, cpu); -} - out: spin_unlock(&cpupool_lock); return ret; @@ -535,42 +495,14 @@ static int cpupool_cpu_add(unsigned int cpu) * Called to remove a CPU from a pool. The CPU is locked, to forbid removing * it from pool0. In fact, if we want to hot-unplug a CPU, it must belong to * pool0, or we fail. - * - * However, if we are suspending (e.g., to ACPI S3), we mark the CPU in such - * a way that it can be put back in its pool when resuming. */ static int cpupool_cpu_remove(unsigned int cpu) { int ret = -ENODEV; spin_lock(&cpupool_lock); -if ( system_state == SYS_STATE_suspend ) -{ -struct cpupool **c; - -for_each_cpupool(c) -{ -if ( cpumask_test_cpu(cpu, (*c)->cpu_valid ) ) -{ -cpumask_set_cpu(cpu, (*c)->cpu_suspended); -cpumask_clear_cpu(cpu, (*c)->cpu_valid); -break; -} -} -/* - * Either we found cpu in a pool, or it must be free (if it has been - * hot-unplagged, then we must have found it in pool0). It is, of - * course, fine to suspend or shutdown with CPUs not assigned to a - * pool, and (in case of suspend) they will stay free when resuming. - */ -ASSERT(cpumask_test_cpu(cpu, &cpupool_free_cpus) || - cpumask_test_cpu(cpu, (*c)->cpu_suspended)); -ASSERT(cpumask_test_cpu(cpu, &cpu_online_map) || - cpumask_test_cpu(cpu, cpupool0->cpu_suspended)); -ret = 0; -} -else if ( cpumask_test_cpu(cpu, cpupool0->cpu_valid) ) +if ( cpumask_test_cpu(cpu, cpupool0->cpu_valid) ) { /* * If we are not suspending, w
[Xen-devel] [PATCH v2 2/6] xen: add helper for calling notifier_call_chain() to common/cpu.c
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. While at it avoid side effects from using BUG_ON() by not using cpu_online(cpu) as a parameter. Signed-off-by: Juergen Gross --- V2: - add nofail parameter to cpu_notifier_call_chain() - avoid side effects from using BUG_ON() macro (Andrew Cooper) --- xen/common/cpu.c | 56 ++-- 1 file changed, 26 insertions(+), 30 deletions(-) diff --git a/xen/common/cpu.c b/xen/common/cpu.c index 836c62f97f..8bf69600a6 100644 --- a/xen/common/cpu.c +++ b/xen/common/cpu.c @@ -71,11 +71,21 @@ 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, bool nofail) +{ +void *hcpu = (void *)(long)cpu; +int notifier_rc = notifier_call_chain(&cpu_chain, action, hcpu, nb); +int ret = (notifier_rc == NOTIFY_DONE) ? 0 : notifier_to_errno(notifier_rc); + +BUG_ON(ret && nofail); + +return ret; +} + 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); +cpu_notifier_call_chain(smp_processor_id(), CPU_DYING, NULL, true); __cpu_disable(); } @@ -87,8 +97,7 @@ static int take_cpu_down(void *arg) int cpu_down(unsigned int cpu) { -int err, notifier_rc; -void *hcpu = (void *)(long)cpu; +int err; struct notifier_block *nb = NULL; if ( !cpu_hotplug_begin() ) @@ -100,12 +109,9 @@ int cpu_down(unsigned int cpu) return -EINVAL; } -notifier_rc = notifier_call_chain(&cpu_chain, CPU_DOWN_PREPARE, hcpu, &nb); -if ( notifier_rc != NOTIFY_DONE ) -{ -err = notifier_to_errno(notifier_rc); +err = cpu_notifier_call_chain(cpu, CPU_DOWN_PREPARE, &nb, false); +if ( err ) goto fail; -} if ( unlikely(system_state < SYS_STATE_active) ) on_selected_cpus(cpumask_of(cpu), _take_cpu_down, NULL, true); @@ -113,26 +119,24 @@ int cpu_down(unsigned int cpu) goto fail; __cpu_die(cpu); -BUG_ON(cpu_online(cpu)); +err = cpu_online(cpu); +BUG_ON(err); -notifier_rc = notifier_call_chain(&cpu_chain, CPU_DEAD, hcpu, NULL); -BUG_ON(notifier_rc != NOTIFY_DONE); +cpu_notifier_call_chain(cpu, CPU_DEAD, NULL, true); send_global_virq(VIRQ_PCPU_STATE); cpu_hotplug_done(); return 0; fail: -notifier_rc = notifier_call_chain(&cpu_chain, CPU_DOWN_FAILED, hcpu, &nb); -BUG_ON(notifier_rc != NOTIFY_DONE); +cpu_notifier_call_chain(cpu, CPU_DOWN_FAILED, &nb, true); cpu_hotplug_done(); return err; } int cpu_up(unsigned int cpu) { -int notifier_rc, err = 0; -void *hcpu = (void *)(long)cpu; +int err; struct notifier_block *nb = NULL; if ( !cpu_hotplug_begin() ) @@ -144,19 +148,15 @@ int cpu_up(unsigned int cpu) return -EINVAL; } -notifier_rc = notifier_call_chain(&cpu_chain, CPU_UP_PREPARE, hcpu, &nb); -if ( notifier_rc != NOTIFY_DONE ) -{ -err = notifier_to_errno(notifier_rc); +err = cpu_notifier_call_chain(cpu, CPU_UP_PREPARE, &nb, false); +if ( err ) goto fail; -} err = __cpu_up(cpu); if ( err < 0 ) goto fail; -notifier_rc = notifier_call_chain(&cpu_chain, CPU_ONLINE, hcpu, NULL); -BUG_ON(notifier_rc != NOTIFY_DONE); +cpu_notifier_call_chain(cpu, CPU_ONLINE, NULL, true); send_global_virq(VIRQ_PCPU_STATE); @@ -164,18 +164,14 @@ int cpu_up(unsigned int cpu) return 0; fail: -notifier_rc = notifier_call_chain(&cpu_chain, CPU_UP_CANCELED, hcpu, &nb); -BUG_ON(notifier_rc != NOTIFY_DONE); +cpu_notifier_call_chain(cpu, CPU_UP_CANCELED, &nb, true); cpu_hotplug_done(); return err; } void notify_cpu_starting(unsigned int cpu) { -void *hcpu = (void *)(long)cpu; -int notifier_rc = notifier_call_chain( -&cpu_chain, CPU_STARTING, hcpu, NULL); -BUG_ON(notifier_rc != NOTIFY_DONE); +cpu_notifier_call_chain(cpu, CPU_STARTING, NULL, true); } static cpumask_t frozen_cpus; -- 2.16.4 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 4/6] xen: don't free percpu areas during suspend
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. It should be noted that there is a potential change in behaviour as the percpu areas are no longer zeroed out during suspend/resume. While I have checked the called cpu notifier hooks to cope with that there might be some well hidden dependency on the previous behaviour. OTOH a component not registering itself for cpu down/up and expecting to see a zeroed percpu variable after suspend/resume is kind of broken already. And the opposite case, where a component is not registered to be called for cpu down/up and is not expecting a percpu variable suddenly to be zero due to suspend/resume is much more probable, especially as the suspend/resume functionality seems not to be tested that often. Signed-off-by: Juergen Gross Reviewed-by: Dario Faggioli --- xen/arch/x86/percpu.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/xen/arch/x86/percpu.c b/xen/arch/x86/percpu.c index 8be4ebddf4..5ea14b6ec3 100644 --- a/xen/arch/x86/percpu.c +++ b/xen/arch/x86/percpu.c @@ -76,7 +76,8 @@ static int cpu_percpu_callback( break; case CPU_UP_CANCELED: case CPU_DEAD: -if ( !park_offline_cpus ) +case CPU_RESUME_FAILED: +if ( !park_offline_cpus && system_state != SYS_STATE_suspend ) free_percpu_area(cpu); break; case CPU_REMOVE: -- 2.16.4 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 0/6] xen: simplify suspend/resume handling
Especially in the scheduler area (schedule.c, cpupool.c) there is a rather complex handling involved when doing suspend and resume. This can be simplified a lot by not performing a complete cpu down and up cycle for the non-boot cpus, but keeping the pure software related state and freeing it only in case a cpu didn't come up again during resume. In summary not only the complexity can be reduced, but the failure tolerance will be even better with this series: With a dedicated hook for failing cpus when resuming it is now possible to survive e.g. a cpupool being left without any cpu after resume by moving its domains to cpupool0. Juergen Gross (6): xen/sched: call cpu_disable_scheduler() via cpu notifier xen: add helper for calling notifier_call_chain() to common/cpu.c xen: add new cpu notifier action CPU_RESUME_FAILED xen: don't free percpu areas during suspend xen/cpupool: simplify suspend/resume handling xen/sched: don't disable scheduler on cpus during suspend xen/arch/arm/smpboot.c | 4 - xen/arch/x86/percpu.c | 3 +- xen/arch/x86/smpboot.c | 3 - xen/common/cpu.c | 61 +++--- xen/common/cpupool.c | 131 - xen/common/schedule.c | 203 +++-- xen/include/xen/cpu.h | 29 --- xen/include/xen/sched-if.h | 1 - 8 files changed, 190 insertions(+), 245 deletions(-) -- 2.16.4 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 3/6] xen: add new cpu notifier action CPU_RESUME_FAILED
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 in order to know which resources are available then. Signed-off-by: Juergen Gross Reviewed-by: Dario Faggioli Reviewed-by: George Dunlap --- V2: - added comment in xen/include/xen/cpu.h (Dario Faggioli) --- xen/common/cpu.c | 5 + xen/include/xen/cpu.h | 29 ++--- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/xen/common/cpu.c b/xen/common/cpu.c index 8bf69600a6..a6efc5e604 100644 --- a/xen/common/cpu.c +++ b/xen/common/cpu.c @@ -218,7 +218,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 ) +cpu_notifier_call_chain(cpu, CPU_RESUME_FAILED, NULL, true); + cpumask_clear(&frozen_cpus); } diff --git a/xen/include/xen/cpu.h b/xen/include/xen/cpu.h index 2fe3ec05d8..4638c509e2 100644 --- a/xen/include/xen/cpu.h +++ b/xen/include/xen/cpu.h @@ -22,33 +22,40 @@ void register_cpu_notifier(struct notifier_block *nb); * CPU_UP_PREPARE -> CPU_STARTING -> CPU_ONLINE -- successful CPU up * CPU_DOWN_PREPARE -> CPU_DOWN_FAILED -- failed CPU down * CPU_DOWN_PREPARE -> CPU_DYING -> CPU_DEAD-- successful CPU down - * + * in the resume case we have additionally: + * CPU_UP_PREPARE -> CPU_UP_CANCELLED -> CPU_RESUME_FAILED -- CPU not resumed + * with the CPU_RESUME_FAILED handler called only after all CPUs have been + * tried to put online again in order to know which CPUs did restart + * successfully. + * * Hence note that only CPU_*_PREPARE handlers are allowed to fail. Also note * that once CPU_DYING is delivered, an offline action can no longer fail. - * + * * Notifiers are called highest-priority-first when: * (a) A CPU is coming up; or (b) CPU_DOWN_FAILED * Notifiers are called lowest-priority-first when: * (a) A CPU is going down; or (b) CPU_UP_CANCELED */ /* CPU_UP_PREPARE: Preparing to bring CPU online. */ -#define CPU_UP_PREPARE (0x0001 | NOTIFY_FORWARD) +#define CPU_UP_PREPARE(0x0001 | NOTIFY_FORWARD) /* CPU_UP_CANCELED: CPU is no longer being brought online. */ -#define CPU_UP_CANCELED (0x0002 | NOTIFY_REVERSE) +#define CPU_UP_CANCELED (0x0002 | NOTIFY_REVERSE) /* CPU_STARTING: CPU nearly online. Runs on new CPU, irqs still disabled. */ -#define CPU_STARTING (0x0003 | NOTIFY_FORWARD) +#define CPU_STARTING (0x0003 | NOTIFY_FORWARD) /* CPU_ONLINE: CPU is up. */ -#define CPU_ONLINE (0x0004 | NOTIFY_FORWARD) +#define CPU_ONLINE(0x0004 | NOTIFY_FORWARD) /* CPU_DOWN_PREPARE: CPU is going down. */ -#define CPU_DOWN_PREPARE (0x0005 | NOTIFY_REVERSE) +#define CPU_DOWN_PREPARE (0x0005 | NOTIFY_REVERSE) /* CPU_DOWN_FAILED: CPU is no longer going down. */ -#define CPU_DOWN_FAILED (0x0006 | NOTIFY_FORWARD) +#define CPU_DOWN_FAILED (0x0006 | NOTIFY_FORWARD) /* CPU_DYING: CPU is nearly dead (in stop_machine context). */ -#define CPU_DYING(0x0007 | NOTIFY_REVERSE) +#define CPU_DYING (0x0007 | NOTIFY_REVERSE) /* CPU_DEAD: CPU is dead. */ -#define CPU_DEAD (0x0008 | NOTIFY_REVERSE) +#define CPU_DEAD (0x0008 | NOTIFY_REVERSE) /* 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) /* Perform CPU hotplug. May return -EAGAIN. */ int cpu_down(unsigned int cpu); -- 2.16.4 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH RFC 00/55] x86: use domheap page for xen page tables
On Thu, 2019-02-07 at 16:44 +, Wei Liu wrote: > This series switches xen page tables from xenheap page to domheap > page. > > This is required so that when we implement xenheap on top of vmap > there won't > be a loop. > > It is done in roughly three steps: > > 1. Introduce a new set of APIs, implement the old APIs on top of the > new ones. > New APIs still use xenheap pages. > 2. Switch each site which manipulate page tables to use the new APIs. > 3. Switch new APIs to use domheap page. > > You can find the series at: > > https://xenbits.xen.org/git-http/people/liuw/xen.git xen-pt- > allocation-1 > > Wei. Tested-by: Stefan Nuernberger I backported the latest series to an internal version based on the 4.11.1 release. I gave it a good spin on a variety of hardware generations. It's stable with no regressions found. I did not do any performance measurements, though. I did not test PV guests, only HVM domU with PV dom0. - Stefan Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich Ust-ID: DE 289 237 879 Eingetragen am Amtsgericht Charlottenburg HRB 149173 B ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-4.4 test] 134134: trouble: blocked/broken/fail/pass
flight 134134 linux-4.4 real [real] http://logs.test-lab.xenproject.org/osstest/logs/134134/ Failures and problems with tests :-( 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. 133468 build-arm64-pvops 4 host-install(4)broken REGR. vs. 133468 build-arm64 4 host-install(4)broken REGR. vs. 133468 Tests which are failing intermittently (not blocking): test-amd64-i386-examine 8 reboot fail in 134102 pass in 134134 test-amd64-i386-freebsd10-amd64 7 xen-boot fail in 134102 pass in 134134 test-amd64-amd64-xl-qemuu-ovmf-amd64 16 guest-localmigrate/x10 fail pass in 134102 test-armhf-armhf-libvirt-raw 10 debian-di-install fail pass in 134102 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-xsm 1 build-check(1) blocked n/a test-arm64-arm64-examine 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-credit2 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 12 migrate-support-check fail in 134102 never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-check fail in 134102 never pass 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-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-armhf-armhf-libvirt 13 migrate-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-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-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-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 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-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail 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-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 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 ne
Re: [Xen-devel] [PATCH v2 0/6] xen: simplify suspend/resume handling
Hello Juergen, On Thu, 28 Mar 2019 at 14:09, Juergen Gross wrote: > > Especially in the scheduler area (schedule.c, cpupool.c) there is a > rather complex handling involved when doing suspend and resume. > > This can be simplified a lot by not performing a complete cpu down and > up cycle for the non-boot cpus, but keeping the pure software related > state and freeing it only in case a cpu didn't come up again during > resume. > > In summary not only the complexity can be reduced, but the failure > tolerance will be even better with this series: With a dedicated hook > for failing cpus when resuming it is now possible to survive e.g. a > cpupool being left without any cpu after resume by moving its domains > to cpupool0. > > Juergen Gross (6): > xen/sched: call cpu_disable_scheduler() via cpu notifier > xen: add helper for calling notifier_call_chain() to common/cpu.c > xen: add new cpu notifier action CPU_RESUME_FAILED > xen: don't free percpu areas during suspend > xen/cpupool: simplify suspend/resume handling > xen/sched: don't disable scheduler on cpus during suspend > > xen/arch/arm/smpboot.c | 4 - > xen/arch/x86/percpu.c | 3 +- > xen/arch/x86/smpboot.c | 3 - > xen/common/cpu.c | 61 +++--- > xen/common/cpupool.c | 131 - > xen/common/schedule.c | 203 > +++-- > xen/include/xen/cpu.h | 29 --- > xen/include/xen/sched-if.h | 1 - > 8 files changed, 190 insertions(+), 245 deletions(-) > I tested your patch series on ARM64 platform. We had issue with hard affinity - there was assertion failure in sched_credit2 code during suspension if one of the vCPUs is pinned to non-0 pCPU. Seems, your patch series fixes the issue during suspend. But now I'm seeing crash during resume: (XEN) suspend.c:198: Resume (XEN) Enabling non-boot CPUs ... (XEN) Bringing up CPU1 (XEN) CPU1 will call ARM_SMCCC_ARCH_WORKAROUND_1 on exception entry (XEN) CPU 1 booted. (XEN) Bringing up CPU2 (XEN) Data Abort Trap. Syndrome=0x6 (XEN) Walking Hypervisor VA 0x0 on CPU1 via TTBR 0x781a8000 (XEN) 0TH[0x0] = 0x781b0f7f (XEN) 1ST[0x0] = 0x781aaf7f (XEN) 2ND[0x0] = 0x (XEN) CPU1: Unexpected Trap: Data Abort (XEN) [ Xen-4.13-unstable arm64 debug=y Not tainted ] (XEN) CPU:1 (XEN) PC: 00233660 _spin_lock+0x1c/0x88 (XEN) LR: 0023365c (XEN) SP: 80037ffc7d50 (XEN) CPSR: 62c9 MODE:64-bit EL2h (Hypervisor, handler) (XEN) X0: 0006 X1: X2: (XEN) X3: 0002 X4: 80037fc94480 X5: (XEN) X6: 0080 X7: 80037ffb X8: 002a1000 (XEN) X9: 000a X10: 80037ffc7bf8 X11: 0031 (XEN) X12: 0001 X13: 0027fff0 X14: 0020 (XEN) X15: X16: X17: (XEN) X18: X19: X20: (XEN) X21: 80037ffd0108 X22: 0001 X23: 0033bc88 (XEN) X24: 00336020 X25: X26: 0001 (XEN) X27: 00336000 X28: FP: 80037ffc7d50 (XEN) (XEN) VTCR_EL2: 80023558 (XEN) VTTBR_EL2: (XEN) (XEN) SCTLR_EL2: 30cd183d (XEN)HCR_EL2: 0038 (XEN) TTBR0_EL2: 781a8000 (XEN) (XEN)ESR_EL2: 9606 (XEN) HPFAR_EL2: (XEN)FAR_EL2: (XEN) (XEN) Xen stack trace from sp=80037ffc7d50: (XEN)80037ffc7d70 002336e8 80037ffd2000 0023e00c (XEN)80037ffc7d80 0022e90c 80037ffc7e10 00232af8 (XEN)0001 002fbb00 0033cf20 (XEN)002a0680 0001 0001 0001 (XEN) 80037ffc7e90 80037ffc7e50 ffc8 (XEN)0029f008 002ffc41 80037ffc7e90 00263c68 (XEN)80037ffc7e50 00232b6c 0001 0002 (XEN)0001 002fbb80 00336448 002fbb00 (XEN)80037ffc7e60 00257230 80037ffc7e90 00263c6c (XEN)0001 77e8 0001 (XEN) 0001 0001 (XEN) (XEN) (XEN) (XEN) (XEN) (XEN) (XEN) 000
[Xen-devel] [xen-unstable-smoke test] 134154: trouble: blocked/broken/pass
flight 134154 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/134154/ 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 Z3 days 14 attempts Testing same since 134136 2019-03-27 18:00:28 Z0 days4 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
Re: [Xen-devel] [PATCH v2 0/6] xen: simplify suspend/resume handling
On 28/03/2019 14:01, Volodymyr Babchuk wrote: > Hello Juergen, > > On Thu, 28 Mar 2019 at 14:09, Juergen Gross wrote: >> >> Especially in the scheduler area (schedule.c, cpupool.c) there is a >> rather complex handling involved when doing suspend and resume. >> >> This can be simplified a lot by not performing a complete cpu down and >> up cycle for the non-boot cpus, but keeping the pure software related >> state and freeing it only in case a cpu didn't come up again during >> resume. >> >> In summary not only the complexity can be reduced, but the failure >> tolerance will be even better with this series: With a dedicated hook >> for failing cpus when resuming it is now possible to survive e.g. a >> cpupool being left without any cpu after resume by moving its domains >> to cpupool0. >> >> Juergen Gross (6): >> xen/sched: call cpu_disable_scheduler() via cpu notifier >> xen: add helper for calling notifier_call_chain() to common/cpu.c >> xen: add new cpu notifier action CPU_RESUME_FAILED >> xen: don't free percpu areas during suspend >> xen/cpupool: simplify suspend/resume handling >> xen/sched: don't disable scheduler on cpus during suspend >> >> xen/arch/arm/smpboot.c | 4 - >> xen/arch/x86/percpu.c | 3 +- >> xen/arch/x86/smpboot.c | 3 - >> xen/common/cpu.c | 61 +++--- >> xen/common/cpupool.c | 131 - >> xen/common/schedule.c | 203 >> +++-- >> xen/include/xen/cpu.h | 29 --- >> xen/include/xen/sched-if.h | 1 - >> 8 files changed, 190 insertions(+), 245 deletions(-) >> > > I tested your patch series on ARM64 platform. We had issue with hard > affinity - there was assertion failure in sched_credit2 code during > suspension if one of the vCPUs is pinned to non-0 pCPU. > > Seems, your patch series fixes the issue during suspend. But now I'm > seeing crash during resume: Oh, I wasn't aware the suspend/resume is possible on ARM, too. I need to modify arch/arm/percpu.c like on x86. Could you test the attached patch if it is working? Juergen diff --git a/xen/arch/arm/percpu.c b/xen/arch/arm/percpu.c index 25442c48fe..597027c6c9 100644 --- a/xen/arch/arm/percpu.c +++ b/xen/arch/arm/percpu.c @@ -58,11 +58,14 @@ static int cpu_percpu_callback( switch ( action ) { case CPU_UP_PREPARE: -rc = init_percpu_area(cpu); +if ( system_state != SYS_STATE_resume ) +rc = init_percpu_area(cpu); break; case CPU_UP_CANCELED: case CPU_DEAD: -free_percpu_area(cpu); +case CPU_RESUME_FAILED: +if ( system_state != SYS_STATE_suspend ) +free_percpu_area(cpu); break; default: break; ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v6 1/1] cameraif: add ABI for para-virtual camera
On 3/22/19 10:22 AM, Hans Verkuil wrote: On 3/22/19 8:37 AM, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko This is the ABI for the two halves of a para-virtualized camera driver which extends Xen's reach multimedia capabilities even farther enabling it for video conferencing, In-Vehicle Infotainment, high definition maps etc. The initial goal is to support most needed functionality with the final idea to make it possible to extend the protocol if need be: 1. Provide means for base virtual device configuration: - pixel formats - resolutions - frame rates 2. Support basic camera controls: - contrast - brightness - hue - saturation 3. Support streaming control Signed-off-by: Oleksandr Andrushchenko Looks good! Reviewed-by: Hans Verkuil Juergen, could you please take a look? Thank you for all your work on this. Regards, Hans --- xen/include/public/io/cameraif.h | 1374 ++ 1 file changed, 1374 insertions(+) create mode 100644 xen/include/public/io/cameraif.h diff --git a/xen/include/public/io/cameraif.h b/xen/include/public/io/cameraif.h new file mode 100644 index ..acbcbf3bd411 --- /dev/null +++ b/xen/include/public/io/cameraif.h @@ -0,0 +1,1374 @@ +/** + * cameraif.h + * + * Unified camera device I/O interface for Xen guest OSes. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to + * deal in the Software without restriction, including without limitation the + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or + * sell copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + * + * Copyright (C) 2018-2019 EPAM Systems Inc. + * + * Author: Oleksandr Andrushchenko + */ + +#ifndef __XEN_PUBLIC_IO_CAMERAIF_H__ +#define __XEN_PUBLIC_IO_CAMERAIF_H__ + +#include "ring.h" +#include "../grant_table.h" + +/* + ** + * Protocol version + ** + */ +#define XENCAMERA_PROTOCOL_VERSION "1" + +/* + ** + * Feature and Parameter Negotiation + ** + * + * Front->back notifications: when enqueuing a new request, sending a + * notification can be made conditional on xencamera_req (i.e., the generic + * hold-off mechanism provided by the ring macros). Backends must set + * xencamera_req appropriately (e.g., using RING_FINAL_CHECK_FOR_REQUESTS()). + * + * Back->front notifications: when enqueuing a new response, sending a + * notification can be made conditional on xencamera_resp (i.e., the generic + * hold-off mechanism provided by the ring macros). Frontends must set + * xencamera_resp appropriately (e.g., using RING_FINAL_CHECK_FOR_RESPONSES()). + * + * The two halves of a para-virtual camera driver utilize nodes within + * XenStore to communicate capabilities and to negotiate operating parameters. + * This section enumerates these nodes which reside in the respective front and + * backend portions of XenStore, following the XenBus convention. + * + * All data in XenStore is stored as strings. Nodes specifying numeric + * values are encoded in decimal. Integer value ranges listed below are + * expressed as fixed sized integer types capable of storing the conversion + * of a properly formatted node string, without loss of information. + * + ** + *Example configuration + ** + * + * This is an example of backend and frontend configuration: + * + *- Backend --- + * + * /local/domain/0/backend/vcamera/1/0/frontend-id = "1" + * /local/domain/0/backend/vcamera/1/0/frontend = "/local/domain/1/device/vcamera/0" + * /local/domain/0/backend/vcamera/1/0/state
Re: [Xen-devel] [PATCH v2 0/6] xen: simplify suspend/resume handling
On 3/28/19 1:21 PM, Juergen Gross wrote: On 28/03/2019 14:01, Volodymyr Babchuk wrote: Hello Juergen, On Thu, 28 Mar 2019 at 14:09, Juergen Gross wrote: Especially in the scheduler area (schedule.c, cpupool.c) there is a rather complex handling involved when doing suspend and resume. This can be simplified a lot by not performing a complete cpu down and up cycle for the non-boot cpus, but keeping the pure software related state and freeing it only in case a cpu didn't come up again during resume. In summary not only the complexity can be reduced, but the failure tolerance will be even better with this series: With a dedicated hook for failing cpus when resuming it is now possible to survive e.g. a cpupool being left without any cpu after resume by moving its domains to cpupool0. Juergen Gross (6): xen/sched: call cpu_disable_scheduler() via cpu notifier xen: add helper for calling notifier_call_chain() to common/cpu.c xen: add new cpu notifier action CPU_RESUME_FAILED xen: don't free percpu areas during suspend xen/cpupool: simplify suspend/resume handling xen/sched: don't disable scheduler on cpus during suspend xen/arch/arm/smpboot.c | 4 - xen/arch/x86/percpu.c | 3 +- xen/arch/x86/smpboot.c | 3 - xen/common/cpu.c | 61 +++--- xen/common/cpupool.c | 131 - xen/common/schedule.c | 203 +++-- xen/include/xen/cpu.h | 29 --- xen/include/xen/sched-if.h | 1 - 8 files changed, 190 insertions(+), 245 deletions(-) I tested your patch series on ARM64 platform. We had issue with hard affinity - there was assertion failure in sched_credit2 code during suspension if one of the vCPUs is pinned to non-0 pCPU. Seems, your patch series fixes the issue during suspend. But now I'm seeing crash during resume: Oh, I wasn't aware the suspend/resume is possible on ARM, too. We don't have suspend/resume on Arm... There are series on the ML but I would not consider then bug free (there was the first posting). 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/6] xen: simplify suspend/resume handling
Hi, On 3/28/19 1:01 PM, Volodymyr Babchuk wrote: Hello Juergen, On Thu, 28 Mar 2019 at 14:09, Juergen Gross wrote: Especially in the scheduler area (schedule.c, cpupool.c) there is a rather complex handling involved when doing suspend and resume. This can be simplified a lot by not performing a complete cpu down and up cycle for the non-boot cpus, but keeping the pure software related state and freeing it only in case a cpu didn't come up again during resume. In summary not only the complexity can be reduced, but the failure tolerance will be even better with this series: With a dedicated hook for failing cpus when resuming it is now possible to survive e.g. a cpupool being left without any cpu after resume by moving its domains to cpupool0. Juergen Gross (6): xen/sched: call cpu_disable_scheduler() via cpu notifier xen: add helper for calling notifier_call_chain() to common/cpu.c xen: add new cpu notifier action CPU_RESUME_FAILED xen: don't free percpu areas during suspend xen/cpupool: simplify suspend/resume handling xen/sched: don't disable scheduler on cpus during suspend xen/arch/arm/smpboot.c | 4 - xen/arch/x86/percpu.c | 3 +- xen/arch/x86/smpboot.c | 3 - xen/common/cpu.c | 61 +++--- xen/common/cpupool.c | 131 - xen/common/schedule.c | 203 +++-- xen/include/xen/cpu.h | 29 --- xen/include/xen/sched-if.h | 1 - 8 files changed, 190 insertions(+), 245 deletions(-) I tested your patch series on ARM64 platform. We had issue with hard affinity - there was assertion failure in sched_credit2 code during suspension if one of the vCPUs is pinned to non-0 pCPU. When you report an error, please make clear what commit you are using and whether you have patches applied on top. In this case, we have no support of suspend/resume on Arm today. So bug report around suspend/resume is a bit confusing to have. It is also more difficult to help when you don't have the full picture as a bug may be in your code and upstream Xen. I saw Juergen suggested a fix, please carry it in whatever series you have. (XEN) (XEN) Panic on CPU 0: (XEN) PSCI cpu off failed for CPU0 err=-3 (XEN) PSCI CPU off failing is never a good news. Here, the command has been denied by PSCI monitor. But... why does CPU off is actually called on CPU0? Shouldn't we have turned off the platform instead? (XEN) (XEN) Reboot in five seconds... Are the logs below actually a mistaken paste? (XEN) CPU2 will call ARM_SMCCC_ARCH_WORKAROUND_1 on exception entry (XEN) CPU 2 booted. (XEN) Data Abort Trap. Syndrome=0x6 (XEN) Walking Hypervisor VA 0x0 on CPU2 via TTBR 0x781a8000 (XEN) 0TH[0x0] = 0x781b0f7f (XEN) 1ST[0x0] = 0x781aaf7f (XEN) 2ND[0x0] = 0x (XEN) CPU2: Unexpected Trap: Data Abort (XEN) [ Xen-4.13-unstable arm64 debug=y Not tainted ] (XEN) CPU:2 (XEN) PC: 00233660 _spin_lock+0x1c/0x88 (XEN) LR: 0023365c (XEN) SP: 80037ff77d50 (XEN) CPSR: a2c9 MODE:64-bit EL2h (Hypervisor, handler) (XEN) X0: 0006 X1: fffe X2: (XEN) X3: 0002 X4: 80037fc42480 X5: (XEN) X6: 0080 X7: 80037ffb X8: 002a1000 (XEN) X9: 000a X10: 80037ff77bf8 X11: 0032 (XEN) X12: 0001 X13: 0027fff0 X14: 0020 (XEN) X15: X16: X17: (XEN) X18: X19: X20: (XEN) X21: 80037ff7e108 X22: 0002 X23: 0033bc88 (XEN) X24: 00336020 X25: X26: 0002 (XEN) X27: 00336000 X28: FP: 80037ff77d50 (XEN) (XEN) VTCR_EL2: 80023558 (XEN) VTTBR_EL2: (XEN) (XEN) SCTLR_EL2: 30cd183d (XEN)HCR_EL2: 0038 (XEN) TTBR0_EL2: 781a8000 (XEN) (XEN)ESR_EL2: 9606 (XEN) HPFAR_EL2: (XEN)FAR_EL2: (XEN) (XEN) Xen stack trace from sp=80037ff77d50: (XEN)80037ff77d70 002336e8 80037ff7d000 0023e00c (XEN)80037ff77d80 0022e90c 80037ff77e10 00232af8 (XEN)0002 002fbb00 0033cf20 (XEN)002a0680 0001 0001 0001 (XEN) 80037ff77e90 80037ff77e50 ffc8 (XEN)0029f008 002ffc41 80037ff77e90 00263c68 (XEN)80037ff77e50 00232b6c 0002 0004 (XEN)0002 002fbc00 00336448 002fbb00 (XEN)80037ff77e60 0025
Re: [Xen-devel] [PATCH v2 0/6] xen: simplify suspend/resume handling
Hi, On Thu, 28 Mar 2019 at 15:21, Juergen Gross wrote: > > On 28/03/2019 14:01, Volodymyr Babchuk wrote: > > Hello Juergen, > > > > On Thu, 28 Mar 2019 at 14:09, Juergen Gross wrote: > >> > >> Especially in the scheduler area (schedule.c, cpupool.c) there is a > >> rather complex handling involved when doing suspend and resume. > >> > >> This can be simplified a lot by not performing a complete cpu down and > >> up cycle for the non-boot cpus, but keeping the pure software related > >> state and freeing it only in case a cpu didn't come up again during > >> resume. > >> > >> In summary not only the complexity can be reduced, but the failure > >> tolerance will be even better with this series: With a dedicated hook > >> for failing cpus when resuming it is now possible to survive e.g. a > >> cpupool being left without any cpu after resume by moving its domains > >> to cpupool0. > >> > >> Juergen Gross (6): > >> xen/sched: call cpu_disable_scheduler() via cpu notifier > >> xen: add helper for calling notifier_call_chain() to common/cpu.c > >> xen: add new cpu notifier action CPU_RESUME_FAILED > >> xen: don't free percpu areas during suspend > >> xen/cpupool: simplify suspend/resume handling > >> xen/sched: don't disable scheduler on cpus during suspend > >> > >> xen/arch/arm/smpboot.c | 4 - > >> xen/arch/x86/percpu.c | 3 +- > >> xen/arch/x86/smpboot.c | 3 - > >> xen/common/cpu.c | 61 +++--- > >> xen/common/cpupool.c | 131 - > >> xen/common/schedule.c | 203 > >> +++-- > >> xen/include/xen/cpu.h | 29 --- > >> xen/include/xen/sched-if.h | 1 - > >> 8 files changed, 190 insertions(+), 245 deletions(-) > >> > > > > I tested your patch series on ARM64 platform. We had issue with hard > > affinity - there was assertion failure in sched_credit2 code during > > suspension if one of the vCPUs is pinned to non-0 pCPU. > > > > Seems, your patch series fixes the issue during suspend. But now I'm > > seeing crash during resume: > > Oh, I wasn't aware the suspend/resume is possible on ARM, too. It is not upstreamed yet, but yes, it is possible. > I need to modify arch/arm/percpu.c like on x86. > Could you test the attached patch if it is working? Yes, it is fixes the issue. Thank you. -- WBR Volodymyr Babchuk aka lorc [+380976646013] mailto: vlad.babc...@gmail.com ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 0/6] xen: simplify suspend/resume handling
On 28/03/2019 14:33, Julien Grall wrote: > Hi, > > On 3/28/19 1:01 PM, Volodymyr Babchuk wrote: >> Hello Juergen, >> >> On Thu, 28 Mar 2019 at 14:09, Juergen Gross wrote: >>> >>> Especially in the scheduler area (schedule.c, cpupool.c) there is a >>> rather complex handling involved when doing suspend and resume. >>> >>> This can be simplified a lot by not performing a complete cpu down and >>> up cycle for the non-boot cpus, but keeping the pure software related >>> state and freeing it only in case a cpu didn't come up again during >>> resume. >>> >>> In summary not only the complexity can be reduced, but the failure >>> tolerance will be even better with this series: With a dedicated hook >>> for failing cpus when resuming it is now possible to survive e.g. a >>> cpupool being left without any cpu after resume by moving its domains >>> to cpupool0. >>> >>> Juergen Gross (6): >>> xen/sched: call cpu_disable_scheduler() via cpu notifier >>> xen: add helper for calling notifier_call_chain() to common/cpu.c >>> xen: add new cpu notifier action CPU_RESUME_FAILED >>> xen: don't free percpu areas during suspend >>> xen/cpupool: simplify suspend/resume handling >>> xen/sched: don't disable scheduler on cpus during suspend >>> >>> xen/arch/arm/smpboot.c | 4 - >>> xen/arch/x86/percpu.c | 3 +- >>> xen/arch/x86/smpboot.c | 3 - >>> xen/common/cpu.c | 61 +++--- >>> xen/common/cpupool.c | 131 - >>> xen/common/schedule.c | 203 >>> +++-- >>> xen/include/xen/cpu.h | 29 --- >>> xen/include/xen/sched-if.h | 1 - >>> 8 files changed, 190 insertions(+), 245 deletions(-) >>> >> >> I tested your patch series on ARM64 platform. We had issue with hard >> affinity - there was assertion failure in sched_credit2 code during >> suspension if one of the vCPUs is pinned to non-0 pCPU. > When you report an error, please make clear what commit you are using > and whether you have patches applied on top. > > In this case, we have no support of suspend/resume on Arm today. So bug > report around suspend/resume is a bit confusing to have. It is also more > difficult to help when you don't have the full picture as a bug may be > in your code and upstream Xen. > > I saw Juergen suggested a fix, please carry it in whatever series you have. > >> (XEN) >> (XEN) Panic on CPU 0: >> (XEN) PSCI cpu off failed for CPU0 err=-3 >> (XEN) > > PSCI CPU off failing is never a good news. Here, the command has been > denied by PSCI monitor. But... why does CPU off is actually called on > CPU0? Shouldn't we have turned off the platform instead? Could it be that a scheduler lock is no longer reachable as the percpu memory of another cpu has been released and allocated again? That would be one of the possible results of my series. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 4/6] xen: don't free percpu areas during suspend
>>> On 28.03.19 at 13:06, 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. > > It should be noted that there is a potential change in behaviour as > the percpu areas are no longer zeroed out during suspend/resume. While > I have checked the called cpu notifier hooks to cope with that there > might be some well hidden dependency on the previous behaviour. OTOH > a component not registering itself for cpu down/up and expecting to > see a zeroed percpu variable after suspend/resume is kind of broken > already. Back at the time it was intentional to have this behavior, and code was in fact written to rely on it. What I can't immediately tell is whether all such code has gone away. > And the opposite case, where a component is not registered > to be called for cpu down/up and is not expecting a percpu variable > suddenly to be zero due to suspend/resume is much more probable, > especially as the suspend/resume functionality seems not to be tested > that often. Right, but CPU offline/online is pretty easy to test (and independent of firmware support), and it would continue to have the observed effect. I agree though that for suspend/ resume it is more likely to be wanted to have the data retained. But don't forget that it was always implied that APs would go fully down and then come back up during a suspend/ resume cycle. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 0/6] xen: simplify suspend/resume handling
On 28/03/2019 13:37, Juergen Gross wrote: > On 28/03/2019 14:33, Julien Grall wrote: >> Hi, >> >> On 3/28/19 1:01 PM, Volodymyr Babchuk wrote: >>> Hello Juergen, >>> >>> On Thu, 28 Mar 2019 at 14:09, Juergen Gross wrote: Especially in the scheduler area (schedule.c, cpupool.c) there is a rather complex handling involved when doing suspend and resume. This can be simplified a lot by not performing a complete cpu down and up cycle for the non-boot cpus, but keeping the pure software related state and freeing it only in case a cpu didn't come up again during resume. In summary not only the complexity can be reduced, but the failure tolerance will be even better with this series: With a dedicated hook for failing cpus when resuming it is now possible to survive e.g. a cpupool being left without any cpu after resume by moving its domains to cpupool0. Juergen Gross (6): xen/sched: call cpu_disable_scheduler() via cpu notifier xen: add helper for calling notifier_call_chain() to common/cpu.c xen: add new cpu notifier action CPU_RESUME_FAILED xen: don't free percpu areas during suspend xen/cpupool: simplify suspend/resume handling xen/sched: don't disable scheduler on cpus during suspend xen/arch/arm/smpboot.c | 4 - xen/arch/x86/percpu.c | 3 +- xen/arch/x86/smpboot.c | 3 - xen/common/cpu.c | 61 +++--- xen/common/cpupool.c | 131 - xen/common/schedule.c | 203 +++-- xen/include/xen/cpu.h | 29 --- xen/include/xen/sched-if.h | 1 - 8 files changed, 190 insertions(+), 245 deletions(-) >>> >>> I tested your patch series on ARM64 platform. We had issue with hard >>> affinity - there was assertion failure in sched_credit2 code during >>> suspension if one of the vCPUs is pinned to non-0 pCPU. >> When you report an error, please make clear what commit you are using >> and whether you have patches applied on top. >> >> In this case, we have no support of suspend/resume on Arm today. So bug >> report around suspend/resume is a bit confusing to have. It is also more >> difficult to help when you don't have the full picture as a bug may be >> in your code and upstream Xen. >> >> I saw Juergen suggested a fix, please carry it in whatever series you have. >> >>> (XEN) >>> (XEN) Panic on CPU 0: >>> (XEN) PSCI cpu off failed for CPU0 err=-3 >>> (XEN) >> >> PSCI CPU off failing is never a good news. Here, the command has been >> denied by PSCI monitor. But... why does CPU off is actually called on >> CPU0? Shouldn't we have turned off the platform instead? > > Could it be that a scheduler lock is no longer reachable as the percpu > memory of another cpu has been released and allocated again? That would > be one of the possible results of my series. The data abort shown before the panic is potentially the percpu issue. But I don't think it will have the effect to try to turn off CPU0. This looks more an issue in the machine_halt/machine_restart path. Indeed CPU off may rightfully return -3 (DENIED) if the Trusted-OS reside on this CPU. We technically should have checked before that the CPU could be turned off. But it looks like we are missing this code. I vaguely remember to already have pointed out that issue in the past. Cheers, > > > Juergen > -- 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/6] xen: simplify suspend/resume handling
Hello Julien, On Thu, 28 Mar 2019 at 15:33, Julien Grall wrote: > > Hi, > > On 3/28/19 1:01 PM, Volodymyr Babchuk wrote: > > Hello Juergen, > > > > On Thu, 28 Mar 2019 at 14:09, Juergen Gross wrote: > >> > >> Especially in the scheduler area (schedule.c, cpupool.c) there is a > >> rather complex handling involved when doing suspend and resume. > >> > >> This can be simplified a lot by not performing a complete cpu down and > >> up cycle for the non-boot cpus, but keeping the pure software related > >> state and freeing it only in case a cpu didn't come up again during > >> resume. > >> > >> In summary not only the complexity can be reduced, but the failure > >> tolerance will be even better with this series: With a dedicated hook > >> for failing cpus when resuming it is now possible to survive e.g. a > >> cpupool being left without any cpu after resume by moving its domains > >> to cpupool0. > >> > >> Juergen Gross (6): > >>xen/sched: call cpu_disable_scheduler() via cpu notifier > >>xen: add helper for calling notifier_call_chain() to common/cpu.c > >>xen: add new cpu notifier action CPU_RESUME_FAILED > >>xen: don't free percpu areas during suspend > >>xen/cpupool: simplify suspend/resume handling > >>xen/sched: don't disable scheduler on cpus during suspend > >> > >> xen/arch/arm/smpboot.c | 4 - > >> xen/arch/x86/percpu.c | 3 +- > >> xen/arch/x86/smpboot.c | 3 - > >> xen/common/cpu.c | 61 +++--- > >> xen/common/cpupool.c | 131 - > >> xen/common/schedule.c | 203 > >> +++-- > >> xen/include/xen/cpu.h | 29 --- > >> xen/include/xen/sched-if.h | 1 - > >> 8 files changed, 190 insertions(+), 245 deletions(-) > >> > > > > I tested your patch series on ARM64 platform. We had issue with hard > > affinity - there was assertion failure in sched_credit2 code during > > suspension if one of the vCPUs is pinned to non-0 pCPU. > When you report an error, please make clear what commit you are using > and whether you have patches applied on top. Sure. > In this case, we have no support of suspend/resume on Arm today. So bug > report around suspend/resume is a bit confusing to have. It is also more > difficult to help when you don't have the full picture as a bug may be > in your code and upstream Xen. Agree. But in this case, changes were done to the common code mostly. I assumed that it would be good to check and report it for Arm, even if Arm suspend/resume is not upstreamed yet. Besides, this patch series fixed another issue in the common suspend/resume code. > I saw Juergen suggested a fix, please carry it in whatever series you have. Yes, this patch fixes the issue. We are using patch series by Mirela, that you mentioned earlier, by the way. > > (XEN) > > (XEN) Panic on CPU 0: > > (XEN) PSCI cpu off failed for CPU0 err=-3 > > (XEN) > > PSCI CPU off failing is never a good news. Here, the command has been > denied by PSCI monitor. But... why does CPU off is actually called on > CPU0? Shouldn't we have turned off the platform instead? I think, this is because CPU1 is performing machine_restart(), so it asked CPU0 to halt itself. > > > (XEN) > > (XEN) Reboot in five seconds... > > Are the logs below actually a mistaken paste? No, this is what I'm seeing in my serial console. > > (XEN) CPU2 will call ARM_SMCCC_ARCH_WORKAROUND_1 on exception entry > > (XEN) CPU 2 booted. > > (XEN) Data Abort Trap. Syndrome=0x6 > > (XEN) Walking Hypervisor VA 0x0 on CPU2 via TTBR 0x781a8000 > > (XEN) 0TH[0x0] = 0x781b0f7f > > (XEN) 1ST[0x0] = 0x781aaf7f > > (XEN) 2ND[0x0] = 0x > > (XEN) CPU2: Unexpected Trap: Data Abort > > (XEN) [ Xen-4.13-unstable arm64 debug=y Not tainted ] > > (XEN) CPU:2 > > (XEN) PC: 00233660 _spin_lock+0x1c/0x88 > > (XEN) LR: 0023365c > > (XEN) SP: 80037ff77d50 > > (XEN) CPSR: a2c9 MODE:64-bit EL2h (Hypervisor, handler) > > (XEN) X0: 0006 X1: fffe X2: > > (XEN) X3: 0002 X4: 80037fc42480 X5: > > (XEN) X6: 0080 X7: 80037ffb X8: 002a1000 > > (XEN) X9: 000a X10: 80037ff77bf8 X11: 0032 > > (XEN) X12: 0001 X13: 0027fff0 X14: 0020 > > (XEN) X15: X16: X17: > > (XEN) X18: X19: X20: > > (XEN) X21: 80037ff7e108 X22: 0002 X23: 0033bc88 > > (XEN) X24: 00336020 X25: X26: 0002 > > (XEN) X27: 00336000 X28: FP: 80037ff77d50 > > (XEN) > > (XEN) VTCR_EL2: 80023558 > > (XEN) VTTBR_E
[Xen-devel] [PATCH v2 1/1] 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. Signed-off-by: Xin Li --- CC: Jan Beulich CC: Igor Druzhinin CC: Sergey Dyasli CC: Andrew Cooper v2 1: write the struct if any of the strings is provided 2: add contained_handles as flexible array member 3: update commit message and fix style issue --- tools/firmware/hvmloader/smbios.c | 69 - tools/firmware/hvmloader/smbios_types.h | 7 +++ xen/include/public/hvm/hvm_xs_strings.h | 6 +++ 3 files changed, 80 insertions(+), 2 deletions(-) diff --git a/tools/firmware/hvmloader/smbios.c b/tools/firmware/hvmloader/smbios.c index 40d8399be1..7815f1dbd3 100644 --- a/tools/firmware/hvmloader/smbios.c +++ b/tools/firmware/hvmloader/smbios.c @@ -497,9 +497,11 @@ static void * smbios_type_2_init(void *start) { struct smbios_type_2 *p = (struct smbios_type_2 *)start; +const char *s; uint8_t *ptr; void *pts; uint32_t length; +unsigned int counter = 0; pts = get_smbios_pt_struct(2, &length); if ( (pts != NULL)&&(length > 0) ) @@ -518,8 +520,71 @@ smbios_type_2_init(void *start) return (start + length); } -/* Only present when passed in */ -return start; +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 */ +p->chassis_handle = SMBIOS_HANDLE_TYPE3; +p->board_type = 0x0a; /* Motherboard */ +start += sizeof(*p); + +s = xenstore_read(HVM_XS_BASEBOARD_MANUFACTURER, NULL); +if ( (s != NULL) && (*s != '\0') ) +{ +strcpy(start, s); +start += strlen(s) + 1; +p->manufacturer_str = ++counter; +} + +s = xenstore_read(HVM_XS_BASEBOARD_PRODUCT_NAME, NULL); +if ( (s != NULL) && (*s != '\0') ) +{ +strcpy(start, s); +start += strlen(s) + 1; +p->product_name_str = ++counter; +} + +s = xenstore_read(HVM_XS_BASEBOARD_VERSION, NULL); +if ( (s != NULL) && (*s != '\0') ) +{ +strcpy(start, s); +start += strlen(s) + 1; +p->version_str = ++counter; +} + +s = xenstore_read(HVM_XS_BASEBOARD_SERIAL_NUMBER, NULL); +if ( (s != NULL) && (*s != '\0') ) +{ +strcpy(start, s); +start += strlen(s) + 1; +p->serial_number_str = ++counter; +} + +s = xenstore_read(HVM_XS_BASEBOARD_ASSET_TAG, NULL); +if ( (s != NULL) && (*s != '\0') ) +{ +strcpy(start, s); +start += strlen(s) + 1; +p->asset_tag_str = ++counter; +} + +s = xenstore_read(HVM_XS_BASEBOARD_LOCATION_IN_CHASSIS, NULL); +if ( (s != NULL) && (*s != '\0') ) +{ +strcpy(start, s); +start += strlen(s) + 1; +p->location_in_chassis_str = ++counter; +} + +if ( counter ) +{ +*((uint8_t *)start) = 0; +return start + 1; +} + +/* Only present when passed in or with customized string */ +return start - sizeof(*p); } /* Type 3 -- System Enclosure */ diff --git a/tools/firmware/hvmloader/smbios_types.h b/tools/firmware/hvmloader/smbios_types.h index acb63e2fe9..7c648ece71 100644 --- a/tools/firmware/hvmloader/smbios_types.h +++ b/tools/firmware/hvmloader/smbios_types.h @@ -90,6 +90,13 @@ 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[]; } __attribute__ ((packed)); /* System Enclosure - Contained Elements */ diff --git a/xen/include/public/hvm/hvm_xs_strings.h b/xen/include/public/hvm/hvm_xs_strings.h index fea1dd4407..fba2546424 100644 --- a/xen/include/public/hvm/hvm_xs_strings.h +++ b/xen/include/public/hvm/hvm_xs_strings.h @@ -69,6 +69,12 @@ #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
Re: [Xen-devel] Maintainers, please tell us how to boot your machines!
> -Original Messages- > From: "Markus Armbruster" > Sent Time: 2019-03-20 02:34:45 (Wednesday) > To: qemu-de...@nongnu.org > Cc: "Edgar E. Iglesias" , "Hervé Poussineau" > , "Aleksandar Markovic" , > "Aleksandar Rikalo" , "Alexander Graf" , > "Andrey Smirnov" , "Andrzej Zaborowski" > , "Anthony Green" , "Anthony Perard" > , "Aurelien Jarno" , > "Bastian Koppelmann" , "Chris Wulff" > , "Guan Xuetao" , "Igor Mitsyanko" > , "Jan Kiszka" , "Jean-Christophe > Dubois" , "Marek Vasut" , "Paul Durrant" > , "Peter Maydell" , > "Stefano Stabellini" , qemu-...@nongnu.org, > qemu-...@nongnu.org, xen-devel@lists.xenproject.org > Subject: Re: Maintainers, please tell us how to boot your machines! > > [...] > > Machines with no supporter, but at least one maintainer: > > [...] > > = hw/unicore32/puv3.c = > Guan Xuetao (maintainer:UniCore32) > > > Targets where we have received information for *no* machine so far: > > [...] > unicore I'm here. Now I can finally deal with my email account after several years chaos. Anyway, sorry for so late. To boot unicore32 machine, following commands are tested in branch stable-2.7: To configure unicore32 target: ./configure --target-list=unicore32-softmmu \ --enable-debug \ --disable-sdl \ --enable-curses \ --extra-cflags="-D restrict=restricT" \ To run a simple busybox image on qemu-defconfig kernel: ./qemu-system-unicore32 -curses -M puv3 -m 512 -icount 0 -kernel somedir/zImage Please check github for more details: https://github.com/gxt/UniCore32 Thanks, Guan Xuetao ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [distros-debian-wheezy test] 83836: trouble: blocked/broken
flight 83836 distros-debian-wheezy real [real] http://osstest.xensource.com/osstest/logs/83836/ 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-wheezy-netboot-pvgrub 1 build-check(1) blocked n/a test-amd64-amd64-i386-wheezy-netboot-pygrub 1 build-check(1) blocked n/a test-amd64-amd64-amd64-wheezy-netboot-pvgrub 1 build-check(1) blocked n/a test-amd64-i386-amd64-wheezy-netboot-pygrub 1 build-check(1) blocked n/a build-armhf-pvops 4 host-install(4) broken like 83763 build-armhf 4 host-install(4) broken like 83763 build-amd64 4 host-install(4) broken like 83763 build-i386-pvops 4 host-install(4) broken like 83763 build-i3864 host-install(4) broken like 83763 build-amd64-pvops 4 host-install(4) broken like 83763 build-armhf-pvops 5 capture-logs broken like 83763 build-armhf 5 capture-logs broken like 83763 baseline version: flight 83763 jobs: build-amd64 broken build-armhf broken build-i386 broken build-amd64-pvopsbroken build-armhf-pvopsbroken build-i386-pvops broken test-amd64-amd64-amd64-wheezy-netboot-pvgrub blocked test-amd64-i386-i386-wheezy-netboot-pvgrub blocked test-amd64-i386-amd64-wheezy-netboot-pygrub blocked test-amd64-amd64-i386-wheezy-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 v2 1/1] hvmloader: add SMBIOS type 2 info for customized string
>>> On 28.03.19 at 14:56, wrote: > Extend smbios type 2 struct to match specification, add support to > write it when customized string provided and no smbios passed in. > > Signed-off-by: Xin Li Reviewed-by: Jan Beulich with one minor remaining remark: > @@ -518,8 +520,71 @@ smbios_type_2_init(void *start) > return (start + length); > } > > -/* Only present when passed in */ > -return start; > +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 */ > +p->chassis_handle = SMBIOS_HANDLE_TYPE3; > +p->board_type = 0x0a; /* Motherboard */ > +start += sizeof(*p); > + > +s = xenstore_read(HVM_XS_BASEBOARD_MANUFACTURER, NULL); > +if ( (s != NULL) && (*s != '\0') ) > +{ > +strcpy(start, s); > +start += strlen(s) + 1; > +p->manufacturer_str = ++counter; > +} > + > +s = xenstore_read(HVM_XS_BASEBOARD_PRODUCT_NAME, NULL); > +if ( (s != NULL) && (*s != '\0') ) > +{ > +strcpy(start, s); > +start += strlen(s) + 1; > +p->product_name_str = ++counter; > +} > + > +s = xenstore_read(HVM_XS_BASEBOARD_VERSION, NULL); > +if ( (s != NULL) && (*s != '\0') ) > +{ > +strcpy(start, s); > +start += strlen(s) + 1; > +p->version_str = ++counter; > +} > + > +s = xenstore_read(HVM_XS_BASEBOARD_SERIAL_NUMBER, NULL); > +if ( (s != NULL) && (*s != '\0') ) > +{ > +strcpy(start, s); > +start += strlen(s) + 1; > +p->serial_number_str = ++counter; > +} > + > +s = xenstore_read(HVM_XS_BASEBOARD_ASSET_TAG, NULL); > +if ( (s != NULL) && (*s != '\0') ) > +{ > +strcpy(start, s); > +start += strlen(s) + 1; > +p->asset_tag_str = ++counter; > +} > + > +s = xenstore_read(HVM_XS_BASEBOARD_LOCATION_IN_CHASSIS, NULL); > +if ( (s != NULL) && (*s != '\0') ) > +{ > +strcpy(start, s); > +start += strlen(s) + 1; > +p->location_in_chassis_str = ++counter; > +} > + > +if ( counter ) > +{ > +*((uint8_t *)start) = 0; There's another pair of unnecessary parentheses here. If I end up committing this I may take the liberty to drop them. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 0/7] x86: eliminate Intel-isms from x2APIC setup
This is a first preparatory step for enabling x2APIC support also for AMD (plus some misc cleanup). 1: entry: drop unused header inclusions 2: APIC: suppress redundant "Switched to ..." messages 3: ACPI: also parse AMD IOMMU tables early 4: IOMMU: introduce init-ops structure 5: IOMMU: abstract Intel-specific iommu_supports_eim() 6: IOMMU: abstract Intel-specific iommu_{en,dis}able_x2apic_IR() 7: IOMMU: initialize iommu_ops in vendor-independent code Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 0/6] xen: simplify suspend/resume handling
Hi, On 3/28/19 1:56 PM, Volodymyr Babchuk wrote: On Thu, 28 Mar 2019 at 15:33, Julien Grall wrote: On 3/28/19 1:01 PM, Volodymyr Babchuk wrote: On Thu, 28 Mar 2019 at 14:09, Juergen Gross wrote: (XEN) (XEN) Panic on CPU 0: (XEN) PSCI cpu off failed for CPU0 err=-3 (XEN) PSCI CPU off failing is never a good news. Here, the command has been denied by PSCI monitor. But... why does CPU off is actually called on CPU0? Shouldn't we have turned off the platform instead? I think, this is because CPU1 is performing machine_restart(), so it asked CPU0 to halt itself. The PSCI call SYSTEM_OFF/SYSTEM_RESET requires all the CPUs to be in a known state. The PSCI spec actually suggest to turn all the CPUs but one off. Another alternative is to put them in a quiescent state. CPU_OFF will return -3 (i.e DENIED) if the Trusted OS is Uniprocessor and resides on the core that you are about to turn off. I assume your platform have a TOS UP and currently resides on CPU0. SYSTEM_OFF/SYSTEM_RESET call can be done from any CPUs. If we want to avoid the problem with CPU_OFF, then the best options is to put the all CPUs but one in a quiescent state (something like while (1) cpu_relax/wfi). On a side node, we should probably want to remove the panic in call_psci_cpu_off as this will be used by the suspend code. Indeed, the trusted OS may not reside on the boot CPU, so you may hit the panic as well. (XEN) (XEN) Reboot in five seconds... Are the logs below actually a mistaken paste? No, this is what I'm seeing in my serial console. I guess this is happening because we recurse in machine_halt(). We probably want to drop any panic in that code path. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 1/7] x86/entry: drop unused header inclusions
I'm in particular after getting rid of asm/apicdef.h, but there are more no longer (or perhaps never having been) used ones. Signed-off-by: Jan Beulich --- a/xen/arch/x86/hvm/svm/entry.S +++ b/xen/arch/x86/hvm/svm/entry.S @@ -19,13 +19,8 @@ .file "svm/entry.S" -#include -#include -#include #include -#include #include -#include #define VMRUN .byte 0x0F,0x01,0xD8 #define STGI .byte 0x0F,0x01,0xDC --- a/xen/arch/x86/hvm/vmx/entry.S +++ b/xen/arch/x86/hvm/vmx/entry.S @@ -18,13 +18,8 @@ .file "vmx/entry.S" -#include -#include -#include #include -#include #include -#include #define VMRESUME .byte 0x0f,0x01,0xc3 #define VMLAUNCH .byte 0x0f,0x01,0xc2 --- a/xen/arch/x86/x86_64/compat/entry.S +++ b/xen/arch/x86/x86_64/compat/entry.S @@ -4,10 +4,7 @@ .file "x86_64/compat/entry.S" -#include -#include #include -#include #include #include #include --- a/xen/arch/x86/x86_64/entry.S +++ b/xen/arch/x86/x86_64/entry.S @@ -6,10 +6,7 @@ .file "x86_64/entry.S" -#include -#include #include -#include #include #include #include ___ 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/27/19 2:38 PM, Andrew Cooper wrote: > 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. Andy, you're not being reasonable here. Just because *you* can't think of how disabling the DRHD could be useful behavior doesn't mean there isn't one. The original patch took time and effort to write; so one of two things is true: 1. The authors were attempting to address a theoretical concern; the behavior in question didn't fix a real problem they had, or 2. The authors were attempting to address a real problem they had, and the patch in question fixed it (for some value of "fixed"). #1 does happen, but on the whole, #2 is more likely; so it's much better to assume that there was a problem that the patch fixed, even if it might not have been the best *way* to fix it. And in fact, if you go back and look at the original discussion [1] (which involved Intel, Fujitsu, HP, and others), #2 turns out to to be the case. Lots of BIOSes had issues with misreporting RMRRs and DRHDs, and on at least one of those, enabing a DRHD which had invalid RMRRs and things behind it caused the box not to boot [2]. Recall that at the time, VT-d was very new, and didn't have wide support. So, Keir, seeing all these reports, said: "If we want to keep iommu=1 as default, then it is unacceptable to fail to boot on a fairly wide range of modern systems. We have to warn-and disable, partially or completely, unless iommu=force is specified. Or we need to revert to iommu=0 as the default." [3] I think that was a very sensible approach, given the circumstances. Now, as it happens, while there were lots of reports of invalid RMRR / DRHD information from BIOSes, the only report I could find of something actually failing to boot was a Fujitsu private platform [4]. So it might actually be the case that, while BIOS bugs were common, failing to boot when enabling "invalid" DRHDs was pretty rare. Or it may have been common. We don't really have any way of knowing. I continue to think that given that none of this was captured in the commit message or code comments, and that we had two releases where this behavior was disabled with no bug reports, that removing the code was a reasonable thing to do. But asserting that there is no conceivable reason for the code to ever have existed is not -- even without doing the archaeology. Regarding what to do in light of this further background: Given that VT-d is now a more mature and widespread technology, given that it's required on many systems, given that we've had two release cycles with no reported problems, given XenRT's extensive testing, and finally given the fact that the only known situation where disabling the DRHD was necessary was on a "private" platform, think that removing the code and seeing what happens is the best approach. -George [1] https://lists.xenproject.org/archives/html/xen-devel/2010-01/msg00665.html [2] https://lists.xenproject.org/archives/html/xen-devel/2010-01/msg00691.html [3] https://lists.xenproject.org/archives/html/xen-devel/2010-01/msg00731.html [4] https://lists.xenproject.org/arch
[Xen-devel] [PATCH 2/7] x86/APIC: suppress redundant "Switched to ..." messages
There's no need to log anything when what we "switch to" is what is in use already. Signed-off-by: Jan Beulich --- a/xen/arch/x86/apic.c +++ b/xen/arch/x86/apic.c @@ -884,6 +884,7 @@ void x2apic_ap_setup(void) void __init x2apic_bsp_setup(void) { struct IO_APIC_route_entry **ioapic_entries = NULL; +const char *orig_name; if ( !cpu_has_x2apic ) return; @@ -946,8 +947,10 @@ void __init x2apic_bsp_setup(void) force_iommu = 1; +orig_name = genapic.name; genapic = *apic_x2apic_probe(); -printk("Switched to APIC driver %s.\n", genapic.name); +if ( genapic.name != orig_name ) +printk("Switched to APIC driver %s\n", genapic.name); if ( !x2apic_enabled ) { --- a/xen/arch/x86/genapic/probe.c +++ b/xen/arch/x86/genapic/probe.c @@ -85,7 +85,8 @@ int __init mps_oem_check(struct mp_confi int i; for (i = 0; apic_probe[i]; ++i) { if (apic_probe[i]->mps_oem_check(mpc,oem,productid)) { - if (!cmdline_apic) { + if (!cmdline_apic && +genapic.name != apic_probe[i]->name) { genapic = *apic_probe[i]; printk(KERN_INFO "Switched to APIC driver `%s'.\n", genapic.name); @@ -101,7 +102,8 @@ int __init acpi_madt_oem_check(char *oem int i; for (i = 0; apic_probe[i]; ++i) { if (apic_probe[i]->acpi_madt_oem_check(oem_id, oem_table_id)) { - if (!cmdline_apic) { + if (!cmdline_apic && +genapic.name != apic_probe[i]->name) { genapic = *apic_probe[i]; printk(KERN_INFO "Switched to APIC driver `%s'.\n", genapic.name); ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 3/7] x86/ACPI: also parse AMD IOMMU tables early
In order to be able to initialize x2APIC mode we need to parse respective ACPI tables early. Split amd_iov_detect() into two parts for this purpose, and call the initial part earlier on. Signed-off-by: Jan Beulich --- a/xen/arch/x86/acpi/boot.c +++ b/xen/arch/x86/acpi/boot.c @@ -733,7 +733,7 @@ int __init acpi_boot_init(void) acpi_mmcfg_init(); - acpi_dmar_init(); + acpi_iommu_init(); erst_init(); --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include "../ats.h" @@ -170,7 +171,7 @@ static void amd_iommu_setup_domain_devic } } -int __init amd_iov_detect(void) +int __init acpi_ivrs_init(void) { INIT_LIST_HEAD(&amd_iommu_head); @@ -184,6 +185,14 @@ int __init amd_iov_detect(void) return -ENODEV; } +return 0; +} + +int __init amd_iov_detect(void) +{ +if ( !iommu_enable && !iommu_intremap ) +return 0; + iommu_ops = amd_iommu_ops; if ( amd_iommu_init() != 0 ) --- a/xen/include/asm-x86/acpi.h +++ b/xen/include/asm-x86/acpi.h @@ -26,6 +26,7 @@ #include #include #include +#include #define COMPILER_DEPENDENT_INT64 long long #define COMPILER_DEPENDENT_UINT64 unsigned long long @@ -145,6 +146,15 @@ extern u32 pmtmr_ioport; extern unsigned int pmtmr_width; int acpi_dmar_init(void); +int acpi_ivrs_init(void); + +static inline int acpi_iommu_init(void) +{ +int ret = acpi_dmar_init(); + +return ret == -ENODEV ? acpi_ivrs_init() : ret; +} + void acpi_mmcfg_init(void); /* Incremented whenever we transition through S3. Value is 1 during boot. */ ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 4/7] x86/IOMMU: introduce init-ops structure
Do away with the CPU vendor dependency, and set the init ops pointer based on what ACPI tables have been found. Also take the opportunity and add __read_mostly to iommu_ops. Signed-off-by: Jan Beulich --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c @@ -30,6 +30,7 @@ static bool_t __read_mostly init_done; +static const struct iommu_init_ops _iommu_init_ops; static const struct iommu_ops amd_iommu_ops; struct amd_iommu *find_iommu_for_device(int seg, int bdf) @@ -185,10 +186,12 @@ int __init acpi_ivrs_init(void) return -ENODEV; } +iommu_init_ops = &_iommu_init_ops; + return 0; } -int __init amd_iov_detect(void) +static int __init iov_detect(void) { if ( !iommu_enable && !iommu_intremap ) return 0; @@ -604,3 +607,7 @@ static const struct iommu_ops __initcons .crash_shutdown = amd_iommu_crash_shutdown, .dump_p2m_table = amd_dump_p2m_table, }; + +static const struct iommu_init_ops __initconstrel _iommu_init_ops = { +.setup = iov_detect, +}; --- a/xen/drivers/passthrough/vtd/dmar.c +++ b/xen/drivers/passthrough/vtd/dmar.c @@ -993,7 +993,11 @@ int __init acpi_dmar_init(void) ret = parse_dmar_table(acpi_parse_dmar); if ( !ret ) +{ +iommu_init_ops = &intel_iommu_init_ops; + return add_user_rmrr(); +} return ret; } --- a/xen/drivers/passthrough/vtd/extern.h +++ b/xen/drivers/passthrough/vtd/extern.h @@ -27,6 +27,7 @@ struct pci_ats_dev; extern bool_t rwbf_quirk; +extern const struct iommu_init_ops intel_iommu_init_ops; extern const struct iommu_ops intel_iommu_ops; void print_iommu_regs(struct acpi_drhd_unit *drhd); --- a/xen/drivers/passthrough/vtd/iommu.c +++ b/xen/drivers/passthrough/vtd/iommu.c @@ -2280,7 +2280,7 @@ static void __hwdom_init setup_hwdom_rmr pcidevs_unlock(); } -int __init intel_vtd_setup(void) +static int __init vtd_setup(void) { struct acpi_drhd_unit *drhd; struct iommu *iommu; @@ -2735,6 +2735,10 @@ const struct iommu_ops __initconstrel in .dump_p2m_table = vtd_dump_p2m_table, }; +const struct iommu_init_ops __initconstrel intel_iommu_init_ops = { +.setup = vtd_setup, +}; + /* * Local variables: * mode: C --- a/xen/drivers/passthrough/x86/iommu.c +++ b/xen/drivers/passthrough/x86/iommu.c @@ -23,7 +23,8 @@ #include #include -struct iommu_ops iommu_ops; +const struct iommu_init_ops *__initdata iommu_init_ops; +struct iommu_ops __read_mostly iommu_ops; void iommu_update_ire_from_apic( unsigned int apic, unsigned int reg, unsigned int value) --- a/xen/include/asm-x86/iommu.h +++ b/xen/include/asm-x86/iommu.h @@ -56,9 +56,6 @@ struct arch_iommu struct guest_iommu *g_iommu; }; -int intel_vtd_setup(void); -int amd_iov_detect(void); - extern struct iommu_ops iommu_ops; static inline const struct iommu_ops *iommu_get_ops(void) @@ -67,17 +64,15 @@ static inline const struct iommu_ops *io return &iommu_ops; } +struct iommu_init_ops { +int (*setup)(void); +}; + +extern const struct iommu_init_ops *iommu_init_ops; + static inline int iommu_hardware_setup(void) { -switch ( boot_cpu_data.x86_vendor ) -{ -case X86_VENDOR_INTEL: -return intel_vtd_setup(); -case X86_VENDOR_AMD: -return amd_iov_detect(); -} - -return -ENODEV; +return iommu_init_ops ? iommu_init_ops->setup() : -ENODEV; } /* Are we using the domain P2M table as its IOMMU pagetable? */ ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 5/7] x86/IOMMU: abstract Intel-specific iommu_supports_eim()
Introduce a respective element in struct iommu_init_ops. Take the liberty and also switch intel_iommu_supports_eim() to bool/ true/false, to fully match the hook's type. Signed-off-by: Jan Beulich --- a/xen/arch/x86/apic.c +++ b/xen/arch/x86/apic.c @@ -899,14 +899,14 @@ void __init x2apic_bsp_setup(void) printk("x2APIC: Already enabled by BIOS: Ignoring cmdline disable.\n"); } -if ( !iommu_supports_eim() ) +if ( !iommu_supports_x2apic() ) { if ( !x2apic_enabled ) { -printk("Not enabling x2APIC: depends on iommu_supports_eim.\n"); +printk("Not enabling x2APIC: depends on IOMMU support\n"); return; } -panic("x2APIC: already enabled by BIOS, but iommu_supports_eim failed\n"); +panic("x2APIC: already enabled by BIOS, but no IOMMU support\n"); } if ( (ioapic_entries = alloc_ioapic_entries()) == NULL ) --- a/xen/drivers/passthrough/vtd/extern.h +++ b/xen/drivers/passthrough/vtd/extern.h @@ -34,6 +34,8 @@ void print_iommu_regs(struct acpi_drhd_u void print_vtd_entries(struct iommu *iommu, int bus, int devfn, u64 gmfn); keyhandler_fn_t vtd_dump_iommu_info; +bool intel_iommu_supports_eim(void); + int enable_qinval(struct iommu *iommu); void disable_qinval(struct iommu *iommu); int enable_intremap(struct iommu *iommu, int eim); --- a/xen/drivers/passthrough/vtd/intremap.c +++ b/xen/drivers/passthrough/vtd/intremap.c @@ -142,13 +142,13 @@ static void set_hpet_source_id(unsigned set_ire_sid(ire, SVT_VERIFY_SID_SQ, SQ_13_IGNORE_3, hpetid_to_bdf(id)); } -bool_t __init iommu_supports_eim(void) +bool __init intel_iommu_supports_eim(void) { struct acpi_drhd_unit *drhd; unsigned int apic; if ( !iommu_qinval || !iommu_intremap || list_empty(&acpi_drhd_units) ) -return 0; +return false; /* We MUST have a DRHD unit for each IOAPIC. */ for ( apic = 0; apic < nr_ioapics; apic++ ) @@ -157,16 +157,16 @@ bool_t __init iommu_supports_eim(void) dprintk(XENLOG_WARNING VTDPREFIX, "There is not a DRHD for IOAPIC %#x (id: %#x)!\n", apic, IO_APIC_ID(apic)); -return 0; +return false; } for_each_drhd_unit ( drhd ) if ( !ecap_queued_inval(drhd->iommu->ecap) || !ecap_intr_remap(drhd->iommu->ecap) || !ecap_eim(drhd->iommu->ecap) ) -return 0; +return false; -return 1; +return true; } /* @@ -889,7 +889,7 @@ int iommu_enable_x2apic_IR(void) if ( system_state < SYS_STATE_active ) { -if ( !iommu_supports_eim() ) +if ( !intel_iommu_supports_eim() ) return -EOPNOTSUPP; if ( !platform_supports_x2apic() ) --- a/xen/drivers/passthrough/vtd/iommu.c +++ b/xen/drivers/passthrough/vtd/iommu.c @@ -2737,6 +2737,7 @@ const struct iommu_ops __initconstrel in const struct iommu_init_ops __initconstrel intel_iommu_init_ops = { .setup = vtd_setup, +.supports_x2apic = intel_iommu_supports_eim, }; /* --- a/xen/include/asm-x86/iommu.h +++ b/xen/include/asm-x86/iommu.h @@ -66,6 +66,7 @@ static inline const struct iommu_ops *io struct iommu_init_ops { int (*setup)(void); +bool (*supports_x2apic)(void); }; extern const struct iommu_init_ops *iommu_init_ops; @@ -87,7 +88,14 @@ int iommu_setup_hpet_msi(struct msi_desc int adjust_vtd_irq_affinities(void); int __must_check iommu_pte_flush(struct domain *d, u64 gfn, u64 *pte, int order, int present); -bool_t iommu_supports_eim(void); + +static inline bool iommu_supports_x2apic(void) +{ +return iommu_init_ops && iommu_init_ops->supports_x2apic + ? iommu_init_ops->supports_x2apic() + : false; +} + int iommu_enable_x2apic_IR(void); void iommu_disable_x2apic_IR(void); ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 6/7] x86/IOMMU: abstract Intel-specific iommu_{en, dis}able_x2apic_IR()
Introduce respective elements in struct iommu_init_ops as well as a pointer to the main ops structure. Signed-off-by: Jan Beulich --- a/xen/drivers/passthrough/vtd/extern.h +++ b/xen/drivers/passthrough/vtd/extern.h @@ -35,6 +35,8 @@ void print_vtd_entries(struct iommu *iom keyhandler_fn_t vtd_dump_iommu_info; bool intel_iommu_supports_eim(void); +int intel_iommu_enable_x2apic_IR(void); +void intel_iommu_disable_x2apic_IR(void); int enable_qinval(struct iommu *iommu); void disable_qinval(struct iommu *iommu); --- a/xen/drivers/passthrough/vtd/intremap.c +++ b/xen/drivers/passthrough/vtd/intremap.c @@ -882,23 +882,13 @@ out: * This function is used to enable Interrupt remapping when * enable x2apic */ -int iommu_enable_x2apic_IR(void) +int intel_iommu_enable_x2apic_IR(void) { struct acpi_drhd_unit *drhd; struct iommu *iommu; -if ( system_state < SYS_STATE_active ) -{ -if ( !intel_iommu_supports_eim() ) -return -EOPNOTSUPP; - -if ( !platform_supports_x2apic() ) -return -ENXIO; - -iommu_ops = intel_iommu_ops; -} -else if ( !x2apic_enabled ) -return -EOPNOTSUPP; +if ( system_state < SYS_STATE_active && !platform_supports_x2apic() ) +return -ENXIO; for_each_drhd_unit ( drhd ) { @@ -946,14 +936,10 @@ int iommu_enable_x2apic_IR(void) * This function is used to disable Interrutp remapping when * suspend local apic */ -void iommu_disable_x2apic_IR(void) +void intel_iommu_disable_x2apic_IR(void) { struct acpi_drhd_unit *drhd; -/* x2apic_enabled implies iommu_supports_eim(). */ -if ( !x2apic_enabled ) -return; - for_each_drhd_unit ( drhd ) disable_intremap(drhd->iommu); --- a/xen/drivers/passthrough/vtd/iommu.c +++ b/xen/drivers/passthrough/vtd/iommu.c @@ -2720,6 +2720,8 @@ const struct iommu_ops __initconstrel in .free_page_table = iommu_free_page_table, .reassign_device = reassign_device_ownership, .get_device_group_id = intel_iommu_group_id, +.enable_x2apic_IR = intel_iommu_enable_x2apic_IR, +.disable_x2apic_IR = intel_iommu_disable_x2apic_IR, .update_ire_from_apic = io_apic_write_remap_rte, .update_ire_from_msi = msi_msg_write_remap_rte, .read_apic_from_ire = io_apic_read_remap_rte, @@ -2736,6 +2738,7 @@ const struct iommu_ops __initconstrel in }; const struct iommu_init_ops __initconstrel intel_iommu_init_ops = { +.ops = &intel_iommu_ops, .setup = vtd_setup, .supports_x2apic = intel_iommu_supports_eim, }; --- a/xen/drivers/passthrough/x86/iommu.c +++ b/xen/drivers/passthrough/x86/iommu.c @@ -26,6 +26,24 @@ const struct iommu_init_ops *__initdata iommu_init_ops; struct iommu_ops __read_mostly iommu_ops; +int iommu_enable_x2apic_IR(void) +{ +if ( system_state < SYS_STATE_active ) +{ +if ( !iommu_supports_x2apic() ) +return -EOPNOTSUPP; + +iommu_ops = *iommu_init_ops->ops; +} +else if ( !x2apic_enabled ) +return -EOPNOTSUPP; + +if ( !iommu_ops.enable_x2apic_IR ) +return -EOPNOTSUPP; + +return iommu_ops.enable_x2apic_IR(); +} + void iommu_update_ire_from_apic( unsigned int apic, unsigned int reg, unsigned int value) { --- a/xen/include/asm-x86/apic.h +++ b/xen/include/asm-x86/apic.h @@ -29,7 +29,6 @@ enum apic_mode { }; extern u8 apic_verbosity; -extern bool x2apic_enabled; extern bool directed_eoi_enabled; void check_x2apic_preenabled(void); --- a/xen/include/asm-x86/apicdef.h +++ b/xen/include/asm-x86/apicdef.h @@ -126,4 +126,6 @@ #define MAX_IO_APICS 128 +extern bool x2apic_enabled; + #endif --- a/xen/include/asm-x86/iommu.h +++ b/xen/include/asm-x86/iommu.h @@ -17,6 +17,7 @@ #include #include #include +#include #include #include @@ -65,6 +66,7 @@ static inline const struct iommu_ops *io } struct iommu_init_ops { +const struct iommu_ops *ops; int (*setup)(void); bool (*supports_x2apic)(void); }; @@ -97,7 +99,12 @@ static inline bool iommu_supports_x2apic } int iommu_enable_x2apic_IR(void); -void iommu_disable_x2apic_IR(void); + +static inline void iommu_disable_x2apic_IR(void) +{ +if ( x2apic_enabled && iommu_ops.disable_x2apic_IR ) +iommu_ops.disable_x2apic_IR(); +} extern bool untrusted_msi; --- a/xen/include/xen/iommu.h +++ b/xen/include/xen/iommu.h @@ -216,11 +216,16 @@ struct iommu_ops { unsigned int *flags); void (*free_page_table)(struct page_info *); + #ifdef CONFIG_X86 +int (*enable_x2apic_IR)(void); +void (*disable_x2apic_IR)(void); + void (*update_ire_from_apic)(unsigned int apic, unsigned int reg, unsigned int value); unsigned int (*read_apic_from_ire)(unsigned int apic, unsigned int reg); int (*setup_hpet_msi)(struct msi_desc *); #endif /* CONFIG_X86 */ + int __must_check (*suspend)(void); void (*resume)(void); void (*share_p2m)(struct domain *d);
Re: [Xen-devel] [PATCH v2 0/6] xen: simplify suspend/resume handling
On Thu, 2019-03-28 at 15:56 +0200, Volodymyr Babchuk wrote: > On Thu, 28 Mar 2019 at 15:33, Julien Grall > wrote: > > > Are the logs below actually a mistaken paste? > No, this is what I'm seeing in my serial console. > Err, sorry, I'm a bit confused now... So, if you use the ARM suspend/resume patches + Juergen series + Juergen fixup patch in this subthread, do you still have issues or is everything allright? If you still have issues, which splat are you seeing? 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] [PATCH 7/7] x86/IOMMU: initialize iommu_ops in vendor-independent code
Move this into iommu_hardware_setup() and make that function non- inline. Move its declaration into common code. Signed-off-by: Jan Beulich --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c @@ -31,7 +31,6 @@ static bool_t __read_mostly init_done; static const struct iommu_init_ops _iommu_init_ops; -static const struct iommu_ops amd_iommu_ops; struct amd_iommu *find_iommu_for_device(int seg, int bdf) { @@ -196,8 +195,6 @@ static int __init iov_detect(void) if ( !iommu_enable && !iommu_intremap ) return 0; -iommu_ops = amd_iommu_ops; - if ( amd_iommu_init() != 0 ) { printk("AMD-Vi: Error initialization\n"); @@ -582,7 +579,7 @@ static void amd_dump_p2m_table(struct do amd_dump_p2m_table_level(hd->arch.root_table, hd->arch.paging_mode, 0, 0); } -static const struct iommu_ops __initconstrel amd_iommu_ops = { +static const struct iommu_ops __initconstrel _iommu_ops = { .init = amd_iommu_domain_init, .hwdom_init = amd_iommu_hwdom_init, .add_device = amd_iommu_add_device, @@ -609,5 +606,6 @@ static const struct iommu_ops __initcons }; static const struct iommu_init_ops __initconstrel _iommu_init_ops = { +.ops = &_iommu_ops, .setup = iov_detect, }; --- a/xen/drivers/passthrough/vtd/iommu.c +++ b/xen/drivers/passthrough/vtd/iommu.c @@ -2305,8 +2305,6 @@ static int __init vtd_setup(void) goto error; } -iommu_ops = intel_iommu_ops; - /* We enable the following features only if they are supported by all VT-d * engines: Snoop Control, DMA passthrough, Queued Invalidation, Interrupt * Remapping, and Posted Interrupt --- a/xen/drivers/passthrough/x86/iommu.c +++ b/xen/drivers/passthrough/x86/iommu.c @@ -26,6 +26,19 @@ const struct iommu_init_ops *__initdata iommu_init_ops; struct iommu_ops __read_mostly iommu_ops; +int __init iommu_hardware_setup(void) +{ +if ( !iommu_init_ops ) +return -ENODEV; + +if ( !iommu_ops.init ) +iommu_ops = *iommu_init_ops->ops; +else +ASSERT(iommu_ops.init == iommu_init_ops->ops->init); + +return iommu_init_ops->setup(); +} + int iommu_enable_x2apic_IR(void) { if ( system_state < SYS_STATE_active ) --- a/xen/include/asm-arm/iommu.h +++ b/xen/include/asm-arm/iommu.h @@ -26,8 +26,6 @@ struct arch_iommu const struct iommu_ops *iommu_get_ops(void); void iommu_set_ops(const struct iommu_ops *ops); -int iommu_hardware_setup(void); - #endif /* __ARCH_ARM_IOMMU_H__ */ /* --- a/xen/include/asm-x86/iommu.h +++ b/xen/include/asm-x86/iommu.h @@ -73,11 +73,6 @@ struct iommu_init_ops { extern const struct iommu_init_ops *iommu_init_ops; -static inline int iommu_hardware_setup(void) -{ -return iommu_init_ops ? iommu_init_ops->setup() : -ENODEV; -} - /* Are we using the domain P2M table as its IOMMU pagetable? */ #define iommu_use_hap_pt(d) \ (hap_enabled(d) && has_iommu_pt(d) && iommu_hap_pt_share) --- a/xen/include/xen/iommu.h +++ b/xen/include/xen/iommu.h @@ -65,6 +65,7 @@ extern int8_t iommu_hwdom_reserved; extern unsigned int iommu_dev_iotlb_timeout; int iommu_setup(void); +int iommu_hardware_setup(void); int iommu_domain_init(struct domain *d); void iommu_hwdom_init(struct domain *d); ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 0/6] xen: simplify suspend/resume handling
Hello Dario, On Thu, 28 Mar 2019 at 16:54, Dario Faggioli wrote: > > On Thu, 2019-03-28 at 15:56 +0200, Volodymyr Babchuk wrote: > > On Thu, 28 Mar 2019 at 15:33, Julien Grall > > wrote: > > > > > Are the logs below actually a mistaken paste? > > No, this is what I'm seeing in my serial console. > > > Err, sorry, I'm a bit confused now... > > So, if you use the ARM suspend/resume patches + Juergen series + > Juergen fixup patch in this subthread, do you still have issues or is > everything allright? No, Juergen's fixup fixed the issue. So, everything is perfectly fine. That was discussion of unrelated issue in ARM platform code. I think, we need to drop off x86 folks from CC. Sorry for the noise. -- WBR Volodymyr Babchuk aka lorc [+380976646013] mailto: vlad.babc...@gmail.com ___ 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/28/19 3:26 AM, Jan Beulich wrote: On 27.03.19 at 18:28, wrote: >> This also lacks some of the features of mwait-idle has and duplicates >> the limited functionality. > > Would you mind clarifying the lack-of-features aspect? The > only difference to your patches that I can spot is that you set > .target_residency in the static tables. If the value wanted > for CC6 is really 1000 instead of the 800 the default > calculation would produce, then this would be easy enough > to arrange for in my variant of the patch as well. > > The mwait-idle driver would not have been needed at all if all > BIOSes surfaced complete data via ACPI. Therefore, by > suitably populating tables, it ought to be possible in theory to > use just that one driver. It's just that for Intel CPUs we've > decided to follow what Linux does, hence the separate > driver. There's no Linux precedent for AMD (afaict). target_residency and some of the checks IIRC. Yes, but that's Linux and this is Xen. Linux has an AML interpreter and Xen does not. That's an apple to oranges comparison. You can't compare Xen to Linux for this because the features they have and how they work are different. >> There's also a lack of comments which may or >> may not be needed. So that would add to the line change count if you >> care about that. >> >> I'm not sure why you're so adverse to the mwait-idle patches. We're >> hard coding values in and using mwait (just like Intel is), but the only >> real change we need is using halt for one c-state. > > But that's precisely what I dislike, as getting us further away > from the Linux driver. And btw, if we were to go that route, > then I think we'd better call acpi_idle_do_entry() than to > duplicate further parts of it. But that would also remove some > of that other part of the benefits of mwait_idle() over > acpi_processor_idle(): The former is much more streamlined, > due to not having to care about anything other than MWAIT. > > As an aside, despite having followed the HLT approach in my > draft patch, I continue to be unconvinced that this is what we > actually want. There's a respective TBD remark there. > > Jan > > The changes needed are small though... most of the changes are non-intrusive. It's just a couple of lines here and then and then something where it calls what entry_method. Although, I think using acpi_idle_do_entry() is perfectly fine. With the acpi_idle_do_entry change, the line change count of the mwait-idle patches is 65 lines. A lot of that is the structures (28 lines), which isn't any _real_ code change. One function call and one switch statement isn't going to change the performance or how streamlined it is. The only other change is when it's initialized which is only at start up. Functionally, it should go in mwait-idle. The changes are small, the functionally the same, there's no duplication of functionality or code. Brian ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 0/3] mwait support for AMD processors
This patch series add support and enablement for mwait on AMD Naples and Rome processors. Newer AMD processors support mwait, but only for c1, and for c2 halt is used. The mwait-idle driver is modified to be able to use both mwait and halt for idling. Brian Woods (3): mwait-idle: add support for using halt mwait-idle: add support for AMD processors mwait-idle: add enablement for AMD Naples and Rome xen/arch/x86/acpi/cpu_idle.c | 2 +- xen/arch/x86/cpu/mwait-idle.c | 62 ++- xen/include/asm-x86/cpuidle.h | 1 + 3 files changed, 57 insertions(+), 8 deletions(-) -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 2/3] mwait-idle: add support for AMD processors
From: Brian Woods Newer AMD processors (F17h) have mwait support which is compatible with Intel. Add some checks to make sure vendor specific code is run correctly and some infrastructure to facilitate adding AMD processors. This is done so that Xen will not be reliant on dom0 passing the parsed ACPI tables back since Xen doesn't have an AML interpreter. This can be unreliable or broken in some cases. Signed-off-by: Brian Woods --- xen/arch/x86/cpu/mwait-idle.c | 21 - 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/xen/arch/x86/cpu/mwait-idle.c b/xen/arch/x86/cpu/mwait-idle.c index b9c7f75882..58629f1c29 100644 --- a/xen/arch/x86/cpu/mwait-idle.c +++ b/xen/arch/x86/cpu/mwait-idle.c @@ -964,6 +964,13 @@ static const struct x86_cpu_id intel_idle_ids[] __initconstrel = { {} }; +#define ACPU(family, model, cpu) \ + { X86_VENDOR_AMD, family, model, X86_FEATURE_ALWAYS, &idle_cpu_##cpu} + +static const struct x86_cpu_id amd_idle_ids[] __initconstrel = { + {} +}; + /* * ivt_idle_state_table_update(void) * @@ -1100,6 +1107,9 @@ static void __init sklh_idle_state_table_update(void) */ static void __init mwait_idle_state_table_update(void) { + if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL) + return; + switch (boot_cpu_data.x86_model) { case 0x3e: /* IVT */ ivt_idle_state_table_update(); @@ -1117,7 +1127,16 @@ static void __init mwait_idle_state_table_update(void) static int __init mwait_idle_probe(void) { unsigned int eax, ebx, ecx; - const struct x86_cpu_id *id = x86_match_cpu(intel_idle_ids); + const struct x86_cpu_id *id = NULL; + + switch (boot_cpu_data.x86_vendor) { + case X86_VENDOR_INTEL: + id = x86_match_cpu(intel_idle_ids); + break; + case X86_VENDOR_AMD: + id = x86_match_cpu(amd_idle_ids); + break; + } if (!id) { pr_debug(PREFIX "does not run on family %d model %d\n", -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 1/3] mwait-idle: add support for using halt
From: Brian Woods Some AMD processors can use a mixture of mwait and halt for accessing various c-states. In preparation for adding support for AMD processors, update the mwait-idle driver to optionally use halt. Signed-off-by: Brian Woods --- xen/arch/x86/acpi/cpu_idle.c | 2 +- xen/arch/x86/cpu/mwait-idle.c | 19 +-- xen/include/asm-x86/cpuidle.h | 1 + 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/xen/arch/x86/acpi/cpu_idle.c b/xen/arch/x86/acpi/cpu_idle.c index 654de24f40..b45824d343 100644 --- a/xen/arch/x86/acpi/cpu_idle.c +++ b/xen/arch/x86/acpi/cpu_idle.c @@ -439,7 +439,7 @@ static void acpi_processor_ffh_cstate_enter(struct acpi_processor_cx *cx) mwait_idle_with_hints(cx->address, MWAIT_ECX_INTERRUPT_BREAK); } -static void acpi_idle_do_entry(struct acpi_processor_cx *cx) +void acpi_idle_do_entry(struct acpi_processor_cx *cx) { struct cpu_info *info = get_cpu_info(); diff --git a/xen/arch/x86/cpu/mwait-idle.c b/xen/arch/x86/cpu/mwait-idle.c index f89c52f256..b9c7f75882 100644 --- a/xen/arch/x86/cpu/mwait-idle.c +++ b/xen/arch/x86/cpu/mwait-idle.c @@ -103,6 +103,11 @@ static const struct cpuidle_state { #define CPUIDLE_FLAG_DISABLED 0x1 /* + * On certain AMD families that support mwait, only c1 can be reached by + * mwait and to reach c2, halt has to be used. + */ +#define CPUIDLE_FLAG_USE_HALT 0x2 +/* * Set this flag for states where the HW flushes the TLB for us * and so we don't need cross-calls to keep it consistent. * If this flag is set, SW flushes the TLB, so even if the @@ -784,7 +789,7 @@ static void mwait_idle(void) update_last_cx_stat(power, cx, before); if (cpu_is_haltable(cpu)) - mwait_idle_with_hints(eax, MWAIT_ECX_INTERRUPT_BREAK); + acpi_idle_do_entry(cx); after = cpuidle_get_tick(); @@ -1184,8 +1189,9 @@ static int mwait_idle_cpu_init(struct notifier_block *nfb, for (cstate = 0; cpuidle_state_table[cstate].target_residency; ++cstate) { unsigned int num_substates, hint, state; struct acpi_processor_cx *cx; + const unsigned int cflags = cpuidle_state_table[cstate].flags; - hint = flg2MWAIT(cpuidle_state_table[cstate].flags); + hint = flg2MWAIT(cflags); state = MWAIT_HINT2CSTATE(hint) + 1; if (state > max_cstate) { @@ -1196,13 +1202,13 @@ static int mwait_idle_cpu_init(struct notifier_block *nfb, /* Number of sub-states for this state in CPUID.MWAIT. */ num_substates = (mwait_substates >> (state * 4)) & MWAIT_SUBSTATE_MASK; + /* If NO sub-states for this state in CPUID, skip it. */ - if (num_substates == 0) + if (num_substates == 0 && !(cflags & CPUIDLE_FLAG_USE_HALT)) continue; /* if state marked as disabled, skip it */ - if (cpuidle_state_table[cstate].flags & - CPUIDLE_FLAG_DISABLED) { + if (cflags & CPUIDLE_FLAG_DISABLED) { printk(XENLOG_DEBUG PREFIX "state %s is disabled", cpuidle_state_table[cstate].name); continue; @@ -1221,7 +1227,8 @@ static int mwait_idle_cpu_init(struct notifier_block *nfb, cx = dev->states + dev->count; cx->type = state; cx->address = hint; - cx->entry_method = ACPI_CSTATE_EM_FFH; + cx->entry_method = cflags & CPUIDLE_FLAG_USE_HALT ? + ACPI_CSTATE_EM_HALT : ACPI_CSTATE_EM_FFH; cx->latency = cpuidle_state_table[cstate].exit_latency; cx->target_residency = cpuidle_state_table[cstate].target_residency; diff --git a/xen/include/asm-x86/cpuidle.h b/xen/include/asm-x86/cpuidle.h index 08da01803f..33c8cf1593 100644 --- a/xen/include/asm-x86/cpuidle.h +++ b/xen/include/asm-x86/cpuidle.h @@ -18,6 +18,7 @@ extern uint64_t (*cpuidle_get_tick)(void); int mwait_idle_init(struct notifier_block *); int cpuidle_init_cpu(unsigned int cpu); +void acpi_idle_do_entry(struct acpi_processor_cx *cx); void default_dead_idle(void); void acpi_dead_idle(void); void trace_exit_reason(u32 *irq_traced); -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 3/3] mwait-idle: add enablement for AMD Naples and Rome
From: Brian Woods Add the needed data structures for enabling Naples (F17h M01h). Since Rome (F17h M31h) has the same c-state latencies and entry methods, the c-state information can be used for Rome as well. For both Naples and Rome, mwait is used for c1 (cc1) and halt is functionally the same as c2 (cc6). If c2 (cc6) is disabled in BIOS, then halt functions similar to c1 (cc1). Signed-off-by: Brian Woods --- xen/arch/x86/cpu/mwait-idle.c | 22 ++ 1 file changed, 22 insertions(+) diff --git a/xen/arch/x86/cpu/mwait-idle.c b/xen/arch/x86/cpu/mwait-idle.c index 58629f1c29..0d5d4caa4d 100644 --- a/xen/arch/x86/cpu/mwait-idle.c +++ b/xen/arch/x86/cpu/mwait-idle.c @@ -720,6 +720,22 @@ static const struct cpuidle_state dnv_cstates[] = { {} }; +static const struct cpuidle_state naples_cstates[] = { + { + .name = "CC1", + .flags = MWAIT2flg(0x00), + .exit_latency = 1, + .target_residency = 2, + }, + { + .name = "CC6", + .flags = MWAIT2flg(0x10) | CPUIDLE_FLAG_USE_HALT, + .exit_latency = 400, + .target_residency = 1000, + }, + {} +}; + static void mwait_idle(void) { unsigned int cpu = smp_processor_id(); @@ -964,10 +980,16 @@ static const struct x86_cpu_id intel_idle_ids[] __initconstrel = { {} }; +static const struct idle_cpu idle_cpu_naples = { + .state_table = naples_cstates, +}; + #define ACPU(family, model, cpu) \ { X86_VENDOR_AMD, family, model, X86_FEATURE_ALWAYS, &idle_cpu_##cpu} static const struct x86_cpu_id amd_idle_ids[] __initconstrel = { + ACPU(0x17, 0x01, naples), + ACPU(0x17, 0x31, naples), /* Rome shares the same c-state config */ {} }; -- 2.11.0 ___ 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/26/19 1:39 PM, Jan Beulich wrote: >> 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, there was an earlier regression, which - if it > had been noticed in time - would have made all versions from 4.2 > to 4.11 behave like 4.12 without your change. This behavior was > intended by the original author. Ripping the code out by convincing > people to bypass normal review flow is, well, not very nice to put it > mildly. I would like to say, I thought Andy tried to be very scrupulous here. He had two different R-b's from people familiar with the code, and an Ack from the release coordinator. I think he would have been justified in checking the patch in on that basis, but just to make sure he was doing the right thing, he asked someone else from "The Rest" (me) to double-check to make sure. Given the difference between how the two of you approach this sort of thing, I understand why you might interpret that in a negative light. But I don't think it's in the best interest of the project for people to be completely risk-averse in this sort of situation. We have source control for a reason: If the change turns out to have been wrong, we can revert it. -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2] xen/sched: fix credit2 smt idle handling
Credit2's smt_idle_mask_set() and smt_idle_mask_clear() are used to identify idle cores where vcpus can be moved to. A core is thought to be idle when all siblings are known to have the idle vcpu running on them. Unfortunately the information of a vcpu running on a cpu is per runqueue. So in case not all siblings are in the same runqueue a core will never be regarded to be idle, as the sibling not in the runqueue is never known to run the idle vcpu. Use a credit2 specific cpumask of siblings with only those cpus being marked which are in the same runqueue as the cpu in question. Signed-off-by: Juergen Gross --- V2: - use credit2 per-cpu specific sibling mask --- xen/common/sched_credit2.c | 25 + 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c index 543dc3664d..6958b265fc 100644 --- a/xen/common/sched_credit2.c +++ b/xen/common/sched_credit2.c @@ -504,6 +504,7 @@ struct csched2_private { * Physical CPU */ struct csched2_pcpu { +cpumask_t sibling_mask;/* Siblings in the same runqueue */ int runq_id; }; @@ -656,7 +657,7 @@ static inline void smt_idle_mask_set(unsigned int cpu, const cpumask_t *idlers, cpumask_t *mask) { -const cpumask_t *cpu_siblings = per_cpu(cpu_sibling_mask, cpu); +const cpumask_t *cpu_siblings = &csched2_pcpu(cpu)->sibling_mask; if ( cpumask_subset(cpu_siblings, idlers) ) cpumask_or(mask, mask, cpu_siblings); @@ -668,10 +669,10 @@ void smt_idle_mask_set(unsigned int cpu, const cpumask_t *idlers, static inline void smt_idle_mask_clear(unsigned int cpu, cpumask_t *mask) { -const cpumask_t *cpu_siblings = per_cpu(cpu_sibling_mask, cpu); +const cpumask_t *cpu_siblings = &csched2_pcpu(cpu)->sibling_mask; if ( cpumask_subset(cpu_siblings, mask) ) -cpumask_andnot(mask, mask, per_cpu(cpu_sibling_mask, cpu)); +cpumask_andnot(mask, mask, cpu_siblings); } /* @@ -3793,6 +3794,7 @@ init_pdata(struct csched2_private *prv, struct csched2_pcpu *spc, unsigned int cpu) { struct csched2_runqueue_data *rqd; +unsigned int rcpu; ASSERT(rw_is_write_locked(&prv->lock)); ASSERT(!cpumask_test_cpu(cpu, &prv->initialized)); @@ -3810,12 +3812,23 @@ init_pdata(struct csched2_private *prv, struct csched2_pcpu *spc, printk(XENLOG_INFO " First cpu on runqueue, activating\n"); activate_runqueue(prv, spc->runq_id); } - + __cpumask_set_cpu(cpu, &rqd->idle); __cpumask_set_cpu(cpu, &rqd->active); __cpumask_set_cpu(cpu, &prv->initialized); __cpumask_set_cpu(cpu, &rqd->smt_idle); +/* On the boot cpu we are called before cpu_sibling_mask has been set up. */ +if ( cpu == 0 && system_state < SYS_STATE_active ) +__cpumask_set_cpu(cpu, &csched2_pcpu(cpu)->sibling_mask); +else +for_each_cpu ( rcpu, per_cpu(cpu_sibling_mask, cpu) ) +if ( cpumask_test_cpu(rcpu, &rqd->active) ) +{ +__cpumask_set_cpu(cpu, &csched2_pcpu(rcpu)->sibling_mask); +__cpumask_set_cpu(rcpu, &csched2_pcpu(cpu)->sibling_mask); +} + if ( cpumask_weight(&rqd->active) == 1 ) rqd->pick_bias = cpu; @@ -3897,6 +3910,7 @@ csched2_deinit_pdata(const struct scheduler *ops, void *pcpu, int cpu) struct csched2_private *prv = csched2_priv(ops); struct csched2_runqueue_data *rqd; struct csched2_pcpu *spc = pcpu; +unsigned int rcpu; write_lock_irqsave(&prv->lock, flags); @@ -3923,6 +3937,9 @@ csched2_deinit_pdata(const struct scheduler *ops, void *pcpu, int cpu) printk(XENLOG_INFO "Removing cpu %d from runqueue %d\n", cpu, spc->runq_id); +for_each_cpu ( rcpu, &rqd->active ) +__cpumask_clear_cpu(cpu, &csched2_pcpu(rcpu)->sibling_mask); + __cpumask_clear_cpu(cpu, &rqd->idle); __cpumask_clear_cpu(cpu, &rqd->smt_idle); __cpumask_clear_cpu(cpu, &rqd->active); -- 2.16.4 ___ 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 28.03.19 at 16:02, wrote: > On 3/28/19 3:26 AM, Jan Beulich wrote: > On 27.03.19 at 18:28, wrote: >>> This also lacks some of the features of mwait-idle has and duplicates >>> the limited functionality. >> >> Would you mind clarifying the lack-of-features aspect? The >> only difference to your patches that I can spot is that you set >> .target_residency in the static tables. If the value wanted >> for CC6 is really 1000 instead of the 800 the default >> calculation would produce, then this would be easy enough >> to arrange for in my variant of the patch as well. >> >> The mwait-idle driver would not have been needed at all if all >> BIOSes surfaced complete data via ACPI. Therefore, by >> suitably populating tables, it ought to be possible in theory to >> use just that one driver. It's just that for Intel CPUs we've >> decided to follow what Linux does, hence the separate >> driver. There's no Linux precedent for AMD (afaict). > > target_residency and some of the checks IIRC. Could you be more specific what checks you mean? > Yes, but that's Linux and this is Xen. Linux has an AML interpreter and > Xen does not. That's an apple to oranges comparison. You can't compare > Xen to Linux for this because the features they have and how they work > are different. It's not a direct comparison, sure. But lack of suitable ACPI data (known to happen in practice) would put Linux into exactly the same position. If Linux accepted changes to the driver to use entry methods other than MWAIT, I'd not be as opposed (but I'd still question their reasoning then). > Functionally, it should go in mwait-idle. That's what I continue to question, seeing the HLT additions you're making. Plus older families (which you didn't cover at all so far) apparently would want HLT alone spelled out, which is even less suitable for a driver with this name. Jan ___ 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 28.03.19 at 16:06, wrote: > On 3/26/19 1:39 PM, Jan Beulich wrote: >>> 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, there was an earlier regression, which - if it >> had been noticed in time - would have made all versions from 4.2 >> to 4.11 behave like 4.12 without your change. This behavior was >> intended by the original author. Ripping the code out by convincing >> people to bypass normal review flow is, well, not very nice to put it >> mildly. > > I would like to say, I thought Andy tried to be very scrupulous here. > He had two different R-b's from people familiar with the code, and an > Ack from the release coordinator. I think he would have been justified > in checking the patch in on that basis, Hmm, a very interesting position - maintainer acks are then not necessary anymore. I think in that case quite a few of my patches could have been committed long ago. In fact, had I been fast enough (just as was the case here) I could then have committed "x86/mtrr: fix build with gcc9" as well, since Andrew's sort of objecting response arrived only about a week after I had received Wei's and Roger's R-b. While taking that position would eliminate some of the gigantic stalls I'm observing for some of my patches, I think a more formal weakening of the need for maintainer acks should then first be put in place. > but just to make sure he was > doing the right thing, he asked someone else from "The Rest" (me) to > double-check to make sure. But there was no real urgency: The supposedly regressing patch was put in back in August. So there was an entire half year to notice and work around the issue. (I was made vaguely aware of it by Andrew a few days before this patch arrived.) > Given the difference between how the two of you approach this sort of > thing, I understand why you might interpret that in a negative light. > But I don't think it's in the best interest of the project for people to > be completely risk-averse in this sort of situation. We have source > control for a reason: If the change turns out to have been wrong, we can > revert it. Sure, I can accept taking this position in the middle of a dev cycle. I don't think it's appropriate immediately prior to a release. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/7] x86/entry: drop unused header inclusions
On 28/03/2019 14:48, Jan Beulich wrote: > I'm in particular after getting rid of asm/apicdef.h, but there are more > no longer (or perhaps never having been) used ones. > > Signed-off-by: Jan Beulich Reviewed-by: Andrew Cooper ___ 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/28/19 10:50 AM, Jan Beulich wrote: On 28.03.19 at 16:02, wrote: >> On 3/28/19 3:26 AM, Jan Beulich wrote: >> On 27.03.19 at 18:28, wrote: This also lacks some of the features of mwait-idle has and duplicates the limited functionality. >>> >>> Would you mind clarifying the lack-of-features aspect? The >>> only difference to your patches that I can spot is that you set >>> .target_residency in the static tables. If the value wanted >>> for CC6 is really 1000 instead of the 800 the default >>> calculation would produce, then this would be easy enough >>> to arrange for in my variant of the patch as well. >>> >>> The mwait-idle driver would not have been needed at all if all >>> BIOSes surfaced complete data via ACPI. Therefore, by >>> suitably populating tables, it ought to be possible in theory to >>> use just that one driver. It's just that for Intel CPUs we've >>> decided to follow what Linux does, hence the separate >>> driver. There's no Linux precedent for AMD (afaict). >> >> target_residency and some of the checks IIRC. > > Could you be more specific what checks you mean? > >> Yes, but that's Linux and this is Xen. Linux has an AML interpreter and >> Xen does not. That's an apple to oranges comparison. You can't compare >> Xen to Linux for this because the features they have and how they work >> are different. > > It's not a direct comparison, sure. But lack of suitable ACPI data > (known to happen in practice) would put Linux into exactly the > same position. If Linux accepted changes to the driver to use > entry methods other than MWAIT, I'd not be as opposed (but I'd > still question their reasoning then). Xen doesn't have an AML interpreter though and you can't reliably get the ACPI data from Dom0 in my experience. You can't dictate what happens in Xen by what happens in Linux when the systems function completely different. It's an apples and oranges situation. >> Functionally, it should go in mwait-idle. > > That's what I continue to question, seeing the HLT additions you're > making. Plus older families (which you didn't cover at all so far) > apparently would want HLT alone spelled out, which is even less > suitable for a driver with this name. > > Jan Older families aren't compatible with Intel's mwait and we don't have any interest in enabling mwait on older families. Older families won't be using this driver (mwait-idle) at all, but rather the acpi cpu_idle driver. That's why there's only talk of F17h in the commits and code. Brian ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [libvirt test] 134142: trouble: blocked/broken/pass
flight 134142 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/134142/ 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 broken build-arm64-pvopsbroken build-arm64-pvops 4 host-install(4)broken REGR. vs. 133846 build-arm64-xsm 4 host-install(4)broken REGR. vs. 133846 build-arm64 4 host-install(4)broken REGR. vs. 133846 Tests which did not succeed, but are not blocking: test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-libvirt 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-qcow2 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-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 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-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 598641f460fce981405bd9cf07e7d6a34deb37bd baseline version: libvirt 25e2e4e04f13901b3db903b2301bd11381bdf128 Last test of basis 133846 2019-03-16 02:09:09 Z 12 days Failing since133876 2019-03-17 11:33:04 Z 11 days8 attempts Testing same since 134142 2019-03-27 22:36:35 Z0 days1 attempts People who touched revisions under test: Andrea Bolognani Cole Robinson Daniel Henrique Barboza Daniel P. Berrangé Eric Blake Jason Dillaman Jiri Denemark Laine Stump 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 pass 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-xsmpass test-amd64-amd64-libvirt-xsm pass test-arm64-arm64-libvirt-xsm blocked test-amd64-i386-libvirt-xsm pass test-amd64-amd64-libvirt pass test-arm64-arm64-libvirt blocked test-armhf-armhf-libvirt pass test-amd64-i386-libvirt pass test-amd64-amd64-libvirt-pairpass test-amd64-i386-libvirt-pair pass 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 of osstest in general, is at http://xenbits.xen
Re: [Xen-devel] [PATCH 2/7] x86/APIC: suppress redundant "Switched to ..." messages
On 28/03/2019 14:49, Jan Beulich wrote: > There's no need to log anything when what we "switch to" is what is in > use already. > > Signed-off-by: Jan Beulich Reviewed-by: Andrew Cooper ___ 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/28/19 5:03 AM, Jan Beulich wrote: On 27.03.19 at 18:07, wrote: >> 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. > I don't think it's needed before my change either, so I'd call > removing this an orthogonal change. As said in the commit > message, the goal was to leave this function alone as far as > possible. > > Before my patch, xen_filter_cpu_maps() gets called long after > prefill_possible_map(), and by the purpose of the latter function > the possible map shouldn't be altered anymore once that has > run. Adding bits to it is surely not going to have the intended > effect (setup_per_cpu_areas() has already run), while removing > bits may have some effect, but comes too late at least for > setup_per_cpu_areas(). OK. Then it looks like even though your patch changes behavior slightly (so technically I guess it's not purely a cleanup) this shouldn't makes any difference at least as far as possible cpu mask is concerned: if we don't have percpu areas set up we can't do much for that vcpu since it seems to me xen_vcpu_setup(), for example, won't do well. > > And if we were to remove this, I think the CONFIG_HOTPLUG_CPU > section should go away as well. After all prefill_possible_map() > also sets nr_cpu_ids. To be honest, it was largely this code > fragment which made me want not touch the function more than > necessary: The comment there makes not clear to me at all why > all of this needs to be in an #ifdef in the first place. This was introduced by cf405ae612b0 ("xen/smp: Fix crash when booting with ACPI hotplug CPUs."). I am not sure this is still relevant. The ACPI hotplug code had changed, not significantly but sufficiently enough to alter behavior. acpi_processor_hotadd_init() now fails before it gets a chance to call arch_register_cpu() for vcpu>dom0_max_vcpus. > > Let me know whether you really want me to fold this extra > cleanup into this patch. If so, I'd then wonder whether the > set_cpu_present() from xen_pv_smp_prepare_cpus() shouldn't > be moved here, too. And the fiddling with the possible map > there looks bogus as well: Bring-up of CPUs past the command > line option should be avoided at boot time, but they shouldn't > be excluded from getting brought up later on. Note how > native_smp_prepare_cpus() ignores its parameter altogether. Yes, that does look strange. Given especially xen_pv_smp_prepare_cpus(), I think re-working proper setting of present/possible masks is well beyond the scope of your original patch. -boris ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/7] x86/ACPI: also parse AMD IOMMU tables early
On 28/03/2019 14:49, Jan Beulich wrote: > In order to be able to initialize x2APIC mode we need to parse > respective ACPI tables early. Split amd_iov_detect() into two parts for > this purpose, and call the initial part earlier on. > > Signed-off-by: Jan Beulich Reviewed-by: Andrew Cooper > --- a/xen/include/asm-x86/acpi.h > +++ b/xen/include/asm-x86/acpi.h > @@ -26,6 +26,7 @@ > #include > #include > #include > +#include > > #define COMPILER_DEPENDENT_INT64 long long > #define COMPILER_DEPENDENT_UINT64 unsigned long long > @@ -145,6 +146,15 @@ extern u32 pmtmr_ioport; > extern unsigned int pmtmr_width; > > int acpi_dmar_init(void); > +int acpi_ivrs_init(void); > + > +static inline int acpi_iommu_init(void) > +{ > +int ret = acpi_dmar_init(); > + > +return ret == -ENODEV ? acpi_ivrs_init() : ret; > +} > + I suppose we really don't have any better option here than to try for all APCI tables we may be interested in. I certainly can't think of any identifying information we could usefully switch() on instead. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] x86emul: don't read mask register on AVX512F-incapable platforms
Reported-by: George Dunlap Signed-off-by: Jan Beulich --- This is surely a stable tree candidate, unless it could still make it into 4.12 before the release. --- a/xen/arch/x86/x86_emulate/x86_emulate.c +++ b/xen/arch/x86/x86_emulate/x86_emulate.c @@ -3511,7 +3511,7 @@ x86_emulate( } /* With a memory operand, fetch the mask register in use (if any). */ -if ( ea.type == OP_MEM && evex.opmsk ) +if ( ea.type == OP_MEM && cpu_has_avx512f && evex.opmsk ) { uint8_t *stb = get_stub(stub); ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 4/7] x86/IOMMU: introduce init-ops structure
On 28/03/2019 14:51, Jan Beulich wrote: > Do away with the CPU vendor dependency, and set the init ops pointer > based on what ACPI tables have been found. "based on which APCI tables..." > > Also take the opportunity and add __read_mostly to iommu_ops. > > Signed-off-by: Jan Beulich Reviewed-by: Andrew Cooper ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 5/7] x86/IOMMU: abstract Intel-specific iommu_supports_eim()
On 28/03/2019 14:52, Jan Beulich wrote: > Introduce a respective element in struct iommu_init_ops. > > Take the liberty and also switch intel_iommu_supports_eim() to bool/ > true/false, to fully match the hook's type. > > Signed-off-by: Jan Beulich Reviewed-by: Andrew Cooper ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [ovmf test] 134143: all pass - PUSHED
flight 134143 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/134143/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 8028f03032182f2c72e7699e1d14322bb5586581 baseline version: ovmf c455bc8c8d78ad51c24426a500914ea32504bf06 Last test of basis 134113 2019-03-27 05:12:07 Z1 days Testing same since 134143 2019-03-27 22:41:34 Z0 days1 attempts People who touched revisions under test: Anthony PERARD Ard Biesheuvel Hao Wu Laszlo Ersek Shenglei Zhang 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 c455bc8c8d..8028f03032 8028f03032182f2c72e7699e1d14322bb5586581 -> xen-tested-master ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 6/7] x86/IOMMU: abstract Intel-specific iommu_{en, dis}able_x2apic_IR()
On 28/03/2019 14:53, Jan Beulich wrote: > @@ -35,6 +35,8 @@ void print_vtd_entries(struct iommu *iom > keyhandler_fn_t vtd_dump_iommu_info; > > bool intel_iommu_supports_eim(void); > +int intel_iommu_enable_x2apic_IR(void); > +void intel_iommu_disable_x2apic_IR(void); Is there any particular reason why these retain their _IR suffix? I'd suggest going with intel_iommu_{en,dis}able_eim() to match the supports name here, whereas... > --- a/xen/drivers/passthrough/vtd/iommu.c > +++ b/xen/drivers/passthrough/vtd/iommu.c > @@ -2720,6 +2720,8 @@ const struct iommu_ops __initconstrel in > .free_page_table = iommu_free_page_table, > .reassign_device = reassign_device_ownership, > .get_device_group_id = intel_iommu_group_id, > +.enable_x2apic_IR = intel_iommu_enable_x2apic_IR, > +.disable_x2apic_IR = intel_iommu_disable_x2apic_IR, > .update_ire_from_apic = io_apic_write_remap_rte, > .update_ire_from_msi = msi_msg_write_remap_rte, > .read_apic_from_ire = io_apic_read_remap_rte, > @@ -2736,6 +2738,7 @@ const struct iommu_ops __initconstrel in > }; > > const struct iommu_init_ops __initconstrel intel_iommu_init_ops = { > +.ops = &intel_iommu_ops, > .setup = vtd_setup, > .supports_x2apic = intel_iommu_supports_eim, > }; > --- a/xen/drivers/passthrough/x86/iommu.c > +++ b/xen/drivers/passthrough/x86/iommu.c > @@ -26,6 +26,24 @@ > const struct iommu_init_ops *__initdata iommu_init_ops; > struct iommu_ops __read_mostly iommu_ops; > > +int iommu_enable_x2apic_IR(void) ... using iommu_{en,dis}able_x2apic() here to match the supports_x2apic() init hook. I don't think these shorter names are any more ambiguous, and loosing the _IR suffix does make them more consistent with the rest of Xen's function naming conventions. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] Boot Linux on PVH guest via OVMF/UEFI issues
Hi, I've been working on adding PVH support to OVMF and boot Linux with it. The last issue I had was a guest crash without any error messages. Tests done with Linux 4.20.12-arch1-1-ARCH. In this mail, I'll discuss about two issues: - Linux oops/panic but don't print anything in the console. - Linux requires a VGA region or crash. # About the guest crash without error messages I had a guest crash just after systemd started to print messages on the console. But neither Linux nor Xen had printed anything about why the guest crash. No amount of command line options helped. I've managed to have Linux print the error message on panic, but I had to make a modification so that Linux would not tell Xen when a crash happened. This patch: diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c --- a/arch/x86/xen/enlighten.c +++ b/arch/x86/xen/enlighten.c @@ -277,8 +277,6 @@ void xen_emergency_restart(void) static int xen_panic_event(struct notifier_block *this, unsigned long event, void *ptr) { - if (!kexec_crash_loaded()) - xen_reboot(SHUTDOWN_crash); return NOTIFY_DONE; } Is it possible in the future for the kernel to actually write panic message before Xen destroy the domain? # PVH & EFI & VGA regions. The issue I had was when systemd was trying to start the virtual console. In Linux, one of visual_init/vc_allocate was called and it would crash because the pointer `conswitchp` was NULL or never initialized properly. The issue is here: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/arch/x86/kernel/setup.c?h=v4.20.17#n1259 'conswitchp' is only set conditionally on EFI based on the type of memory at the VGA region. So if OVMF says that it's just RAM, then Linux crash. So should I lie about the memory layout and tell the kernel that there is no RAM around the VGA region? Or is maybe Linux can be changed and allow it to use that region as RAM? (And set conswitchp=&dummy_con when the VGA region is absent.) That might be related to what's done when Linux is booted via the PVH entry point, here: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/arch/x86/xen/enlighten_pvh.c?h=v4.20.17#n51 Do you know why that ISA region (VGA + other bits) is marked as reserved? Thanks, -- Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86emul: don't read mask register on AVX512F-incapable platforms
On 3/28/19 5:03 PM, Jan Beulich wrote: > Reported-by: George Dunlap > Signed-off-by: Jan Beulich That seems to fix all the ones related to the crashes I was seeing: Tested-by: George Dunlap > --- > This is surely a stable tree candidate, unless it could still make it > into 4.12 before the release. > > --- a/xen/arch/x86/x86_emulate/x86_emulate.c > +++ b/xen/arch/x86/x86_emulate/x86_emulate.c > @@ -3511,7 +3511,7 @@ x86_emulate( > } > > /* With a memory operand, fetch the mask register in use (if any). */ > -if ( ea.type == OP_MEM && evex.opmsk ) > +if ( ea.type == OP_MEM && cpu_has_avx512f && evex.opmsk ) > { > uint8_t *stb = get_stub(stub); > > > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 7/7] x86/IOMMU: initialize iommu_ops in vendor-independent code
On 28/03/2019 14:54, Jan Beulich wrote: > --- a/xen/drivers/passthrough/x86/iommu.c > +++ b/xen/drivers/passthrough/x86/iommu.c > @@ -26,6 +26,19 @@ > const struct iommu_init_ops *__initdata iommu_init_ops; > struct iommu_ops __read_mostly iommu_ops; > > +int __init iommu_hardware_setup(void) > +{ > +if ( !iommu_init_ops ) > +return -ENODEV; > + > +if ( !iommu_ops.init ) > +iommu_ops = *iommu_init_ops->ops; > +else > +ASSERT(iommu_ops.init == iommu_init_ops->ops->init); What is this ASSERT() intended to catch? We pass through this function exactly once, making the else path dead. Do you have some plans in future series which make this a non-init function? ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86emul: don't read mask register on AVX512F-incapable platforms
On 28/03/2019 17:44, George Dunlap wrote: > On 3/28/19 5:03 PM, Jan Beulich wrote: >> Reported-by: George Dunlap >> Signed-off-by: Jan Beulich > That seems to fix all the ones related to the crashes I was seeing: > > Tested-by: George Dunlap > >> --- >> This is surely a stable tree candidate, unless it could still make it >> into 4.12 before the release. >> >> --- a/xen/arch/x86/x86_emulate/x86_emulate.c >> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c >> @@ -3511,7 +3511,7 @@ x86_emulate( >> } >> >> /* With a memory operand, fetch the mask register in use (if any). */ >> -if ( ea.type == OP_MEM && evex.opmsk ) >> +if ( ea.type == OP_MEM && cpu_has_avx512f && evex.opmsk ) So, this certainly will fix the crash as reported, you need an %xcr0 check, to prevent other ways of this stub faulting. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-4.19 test] 134139: regressions - trouble: blocked/broken/fail/pass
flight 134139 linux-4.19 real [real] http://logs.test-lab.xenproject.org/osstest/logs/134139/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-pvopsbroken build-arm64-xsm broken 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-armhf-armhf-xl-vhd 6 xen-install fail REGR. vs. 129313 test-armhf-armhf-libvirt-raw 10 debian-di-installfail REGR. vs. 129313 Tests which did not succeed, but are not blocking: 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-credit1 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl 1 build-check(1) blocked n/a test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 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-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-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-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-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-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-amd64-i386-xl-qemuu-ws16-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-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: linux3a2156c839c75c24691e3c672a6d607b24b0c210 baseline version: linux84df9525b0c27f3ebc2ebb1864fa62a97fdedb7d Last test of basis 129313 2018-11-02 05:39:08 Z 146 days Failing since129412 2018-11-04 14:10:15 Z 144 days 99 attempts Testing same since 134139 2019-03-27 19:54:26 Z0 days1 attempts 1679 people touched revisions under test, not listing them all jobs: build-amd64-xsm pass build-arm64-xsm broken buil
[Xen-devel] Xendbg: a full-featured debugger for both PV & HVM Xen guests
Hello everyone, I'm Spencer Michaels, creator of Xendbg, a recently-released full-featured debugger for both HVM and PV Xen guests. I developed Xendbg under the auspices of my company, NCC Group, and released it via a post on their blog about two months ago. Andrew Cooper kindly pointed out to me today that I neglected to cross-post to xen-devel, which in retrospect is obviously the best place for a Xen debugger release announcement! So, I thought I should do so now; my apologies for not thinking of this sooner. The original release blog post can be found here: https://www.nccgroup.trust/us/about-us/newsroom-and-events/blog/2019/january/xendbg-a-full-featured-debugger-for-the-xen-hypervisor/ The source is on Github (MIT licensed): https://github.com/nccgroup/xendbg A talk on unikernel security I gave late last year also has a bit about Xendbg, with a demo of an earlier prototype: https://youtu.be/b68VFuB_y5M?t=1933 The blog post explains why and how I created Xendbg, and some of the difficulties I faced in doing so. Basically, I was researching Xen-based unikernels and ended up needing a full-featured debugger (gdbsx wasn't sufficient); when I failed to find any, I ended up writing one myself. As Andrew pointed out to me today, I had a few theoretical misconceptions about Xen's architecture when I wrote the original post; in particular, I didn't describe dom0 as being a Xen VM itself. On the whole, though, I think the post is a decent intro to what it's like to develop tooling based on the Xen VMI APIs, and I hope Xendbg can serve as a reference implementation for others who want to do so as well. In the near future, I will likely be contributing documentation on the VMI APIs I figured out how to use while developing Xendbg (most of which are totally undocumented). I think this would be a big help for people like myself who want to write Xen debugging tools but don't know where to start. Finally, I should note that while writing Xendbg, some of my questions about the VMI APIs were answered on this very list by Andrew Cooper and Tamas Lengyel, so special thanks to the both of you! Much of the page table reading/writing functionality was built with their help. If you have any questions about Xendbg or my experience with the Xen VMI APIs, I'd be happy to answer them. Thanks, Spencer P.S. For those interested, the unikernel security whitepaper on which the talk I mentioned above is based is going to be published this coming Monday on our blog (link below). It's not about Xen specifically, but it is relatively heavily Xen-related because the unikernels I looked at were both Xen-based, so some people here may be interested in it. Some of the test results were obtained using Xendbg. https://www.nccgroup.trust/us/about-us/newsroom-and-events/blog/ ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 134160: trouble: blocked/broken/pass
flight 134160 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/134160/ 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 Z6 days Failing since134068 2019-03-25 12:00:51 Z3 days 15 attempts Testing same since 134136 2019-03-27 18:00:28 Z1 days5 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] 83838: trouble: blocked/broken
This run is configured for baseline tests only. flight 83838 ovmf real [real] http://osstest.xensource.com/osstest/logs/83838/ 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 8028f03032182f2c72e7699e1d14322bb5586581 baseline version: ovmf c455bc8c8d78ad51c24426a500914ea32504bf06 Last test of basis83833 2019-03-27 22:25:02 Z0 days Testing same since83838 2019-03-28 17:29:34 Z0 days1 attempts People who touched revisions under test: Anthony PERARD Ard Biesheuvel Hao Wu Laszlo Ersek Shenglei Zhang 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 8028f03032182f2c72e7699e1d14322bb5586581 Author: Shenglei Zhang Date: Mon Feb 25 13:55:54 2019 +0800 MdePkg/BaseUefiDecompressLib: Improve performance of boundary validation The boundary validation checking in MakeTable() performs on every loop iteration. This could be improved by checking just once before the loop. https://bugzilla.tianocore.org/show_bug.cgi?id=1329 Cc: Michael D Kinney Cc: Liming Gao Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Shenglei Zhang Reviewed-by: Liming Gao commit 55756c88aec9d3269d8544746907838ba921e90b Author: Shenglei Zhang Date: Mon Feb 25 13:53:37 2019 +0800 BaseTools/C/Common: Improve performance of boundary validation The boundary validation checking in MakeTable() performs on every loop iteration. This could be improved by checking just once before the loop. https://bugzilla.tianocore.org/show_bug.cgi?id=1329 Cc: Bob Feng Cc: Liming Gao Cc: Yonghong Zhu Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Shenglei Zhang Reviewed-by: Liming Gao commit f67786e381717ee02570d33445ea8c326986b2ac Author: Shenglei Zhang Date: Mon Feb 25 13:37:56 2019 +0800 BaseTools/TianoCompress: Improve performance of boundary validation The boundary validation checking in MakeTable() performs on every loop iteration. This could be im
[Xen-devel] [linux-4.9 test] 134149: regressions - trouble: blocked/broken/fail/pass
flight 134149 linux-4.9 real [real] http://logs.test-lab.xenproject.org/osstest/logs/134149/ 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-pvops 4 host-install(4)broken REGR. vs. 134015 build-arm64 4 host-install(4)broken REGR. vs. 134015 build-arm64-xsm 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-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-examine 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-arm64-arm64-libvirt-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-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-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-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 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-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-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-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 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-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-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-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 Z5 days Tes
Re: [Xen-devel] [PATCH] xen/netfront: Remove unneeded .resume callback
On Wed, Mar 27, 2019 at 08:40:20AM +0200, Oleksandr Andrushchenko wrote: > On 3/25/19 7:30 PM, Anchal Agarwal wrote: > >On Fri, Mar 22, 2019 at 10:44:33AM +, Oleksandr Andrushchenko wrote: > >>On 3/20/19 5:50 AM, Munehisa Kamata wrote: > >>>On 3/18/2019 3:02 AM, Oleksandr Andrushchenko wrote: > +Amazon > pls see inline > >>>Hi Oleksandr, > >>> > >>>Let me add some comments as the original author of the series. > >>Thank you for your work! > >Hi Oleksandr, > On 3/14/19 9:00 PM, Julien Grall wrote: > >Hi, > > > >On 3/14/19 3:40 PM, Boris Ostrovsky wrote: > >>On 3/14/19 11:10 AM, Oleksandr Andrushchenko wrote: > >>>On 3/14/19 5:02 PM, Boris Ostrovsky wrote: > On 3/14/19 10:52 AM, Oleksandr Andrushchenko wrote: > >On 3/14/19 4:47 PM, Boris Ostrovsky wrote: > >>On 3/14/19 9:17 AM, Oleksandr Andrushchenko wrote: > >>>From: Oleksandr Andrushchenko > >>> > >>>Currently on driver resume we remove all the network queues and > >>>destroy shared Tx/Rx rings leaving the driver in its current state > >>>and never signaling the backend of this frontend's state change. > >>>This leads to the number of consequences: > >>>- when frontend withdraws granted references to the rings etc. it > >>>cannot > >>> be cleanly done as the backend still holds those (it > >>> was not > >>>told to > >>> free the resources) > >>>- it is not possible to resume driver operation as all the > >>>communication > >>> means with the backned were destroyed by the frontend, > >>> thus > >>> making the frontend appear to the guest OS as > >>> functional, but > >>> not really. > >>What do you mean? Are you saying that after resume you lose > >>connectivity? > >Exactly, if you take a look at the .resume callback as it is now > >what it does it destroys the rings etc. and never notifies the > >backend > >of that, e.g. it stays in, say, connected state with communication > >channels destroyed. It never goes into any other Xen bus state, so > >there is > >no way its state machine can help recovering. > My tree is about a month old so perhaps there is some sort of > regression > but this certainly works for me. After resume netfront gets > XenbusStateInitWait from backend which causes xennet_connect(). > >>>Ah, the difference can be of the way we get the guest enter > >>>the suspend state. I am making my guest to suspend with: > >>>echo mem > /sys/power/state > >>>And then I use an interrupt to the guest (this is a test code) > >>>to wake it up. > >>>Could you please share your exact use-case when the guest enters > >>>suspend > >>>and what you do to resume it? > >>xl save / xl restore > >> > >>>I can see no way backend may want enter XenbusStateInitWait in my > >>>use-case > >>>as it simply doesn't know we want him to. > >>Yours looks like ACPI path, I don't know how well it was tested TBH. > >I remember a series from amazon [1] that plays around suspend and > >hibernation. The patch [2] leads me to think that guest triggered > >suspend/resume does not work properly. It looks like the series has > >never been fully reviewed. Not sure why... > Julien, thanks a lot for bringing these patches to our attention which we > obviously missed. > >Anyway, from my understanding this series may solve Oleksandr issue. > >However, this would only address the common code side. AFAIK Oleksandr > >is targeting Arm platform. If so, I think this would require more work > >than this series. Arm code still miss few bits properly suspend/resume > >arch specific code (see [2]). > > > >I have a branch on my git to track the series. However, they never have > >been resent after Ian Campbell left Citrix. I would be happy to review > >them if someone wants to pick them up and repost them. > > > First of all, let me make it clear that we are interested in hibernation > long term, so it would be > desirable to re-use as much work form resume/suspend as we can. But, we > see it as a step by > step work, e.g. first S2RAM and later on hibernation. > Let me clarify the immediate use-case that we have, so it is easier to > understand what we want > and what we don't at the moment. We are about to continue work started by > Mirela/Xilinx on > Suspend-to-RAM for ARM [3] and we made number of assumptions: > 1. We are talking about *system* suspend, e.g. the goal is to suspend all > the components > of the system and Xen itself at once. Think about this as fast-boot > and/or energy saving > feature if you will. > 2. With suspend/
[Xen-devel] [xen-unstable test] 134048: trouble: blocked/broken/fail/pass
flight 134048 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/134048/ Failures and problems with tests :-( 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 4 host-install(4)broken REGR. vs. 134007 build-arm64-xsm 4 host-install(4)broken REGR. vs. 134007 build-arm64-pvops 4 host-install(4)broken REGR. vs. 134007 Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a build-arm64-libvirt 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-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 134007 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 134007 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 134007 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 134007 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 134007 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 134007 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 134007 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 134007 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 134007 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-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-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-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 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-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-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-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-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail 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-amd64-xl-qemut-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass version targeted for testing: xen cb70a26f78848fe45f593f7ebc9cfaac760a791b baseline version: xen cb70a26f78848fe45f593f7ebc9cfaac760a791b Last test of basis 134048 2019-03-24 11:40:25 Z4 days Testing same since (not found) 0 attempts jobs: build-amd64-xsm pass build-arm64-xsm broken build-i386-xsm
[Xen-devel] [qemu-mainline bisection] complete test-amd64-i386-freebsd10-amd64
branch xen-unstable xenbranch xen-unstable job test-amd64-i386-freebsd10-amd64 testid guest-saverestore Tree: linux git://xenbits.xen.org/linux-pvops.git Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://git.qemu.org/qemu.git Tree: xen git://xenbits.xen.org/xen.git *** Found and reproduced problem changeset *** Bug is in tree: qemuu git://git.qemu.org/qemu.git Bug introduced: e60483f2f8498ae08ae79ca4c6fb03a3317f5e1e Bug not present: 9cd97956cfdde85d5887f2ea54ff598f615ee1b1 Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/134170/ commit e60483f2f8498ae08ae79ca4c6fb03a3317f5e1e Author: Markus Armbruster Date: Wed Mar 13 09:43:30 2019 +0100 vl: Fix to create migration object before block backends again Recent commit cda4aa9a5a0 moved block backend creation before machine property evaluation. This broke qemu-iotests 055. Turns out we need to create the migration object before block backends, so block backends can add migration blockers. Fix by calling migration_object_init() earlier, right before configure_blockdev(). Fixes: cda4aa9a5a08777cf13e164c0543bd4888b8adce Reported-by: Kevin Wolf Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf For bisection revision-tuple graph see: http://logs.test-lab.xenproject.org/osstest/results/bisect/qemu-mainline/test-amd64-i386-freebsd10-amd64.guest-saverestore.html Revision IDs in each graph node refer, respectively, to the Trees above. Running cs-bisection-step --graph-out=/home/logs/results/bisect/qemu-mainline/test-amd64-i386-freebsd10-amd64.guest-saverestore --summary-out=tmp/134170.bisection-summary --basis-template=133909 --blessings=real,real-bisect qemu-mainline test-amd64-i386-freebsd10-amd64 guest-saverestore Searching for failure / basis pass: 133997 fail [host=pinot1] / 133909 [host=baroque1] 133872 [host=pinot0] 133844 [host=debina1] 133791 [host=elbling1] 133750 [host=elbling0] 133703 [host=fiano1] 133677 [host=fiano0] 133650 [host=baroque0] 133613 [host=chardonnay1] 133589 [host=italia1] 133576 ok. Failure / basis pass flights: 133997 / 133576 (tree with no url: minios) (tree with no url: ovmf) (tree with no url: seabios) Tree: linux git://xenbits.xen.org/linux-pvops.git Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://git.qemu.org/qemu.git Tree: xen git://xenbits.xen.org/xen.git Latest 5726a8d0f1958af80ad8e514bc2c18d213e739b7 c530a75c1e6a472b0eb9558310b518f0dfcd8860 d0d8ad39ecb51cd7497cd524484fe09f50876798 d97a39d903fe33c45be83ac6943a2f82a3649a11 59e9783ddf18e650622e0573cad4f08db65592e4 Basis pass 30921fc1e5fcf904f9afddeece1288f5b16ba017 c530a75c1e6a472b0eb9558310b518f0dfcd8860 d0d8ad39ecb51cd7497cd524484fe09f50876798 1d31f1872b337e4acac5bf6b3c2a45b66e43b494 f393b82fe5ba3ed9cfe2b306ffa53368e55b75af Generating revisions with ./adhoc-revtuple-generator git://xenbits.xen.org/linux-pvops.git#30921fc1e5fcf904f9afddeece1288f5b16ba017-5726a8d0f1958af80ad8e514bc2c18d213e739b7 git://xenbits.xen.org/osstest/linux-firmware.git#c530a75c1e6a472b0eb9558310b518f0dfcd8860-c530a75c1e6a472b0eb9558310b518f0dfcd8860 git://xenbits.xen.org/qemu-xen-traditional.git#d0d8ad39ecb51cd7497cd524484fe09f50876798-d0d8ad39ecb51cd7497cd524484fe09f50876798 git://git.qemu.org/qemu.git#1d31f1872b337e4acac5bf6b3c2a45b66e43b4\ 94-d97a39d903fe33c45be83ac6943a2f82a3649a11 git://xenbits.xen.org/xen.git#f393b82fe5ba3ed9cfe2b306ffa53368e55b75af-59e9783ddf18e650622e0573cad4f08db65592e4 Loaded 16280 nodes in revision graph Searching for test results: 133576 pass 30921fc1e5fcf904f9afddeece1288f5b16ba017 c530a75c1e6a472b0eb9558310b518f0dfcd8860 d0d8ad39ecb51cd7497cd524484fe09f50876798 1d31f1872b337e4acac5bf6b3c2a45b66e43b494 f393b82fe5ba3ed9cfe2b306ffa53368e55b75af 133589 [host=italia1] 133613 [host=chardonnay1] 133677 [host=fiano0] 133650 [host=baroque0] 133791 [host=elbling1] 133703 [host=fiano1] 133750 [host=elbling0] 133844 [host=debina1] 133872 [host=pinot0] 133909 [host=baroque1] 133939 fail irrelevant 133975 fail irrelevant 133997 fail 5726a8d0f1958af80ad8e514bc2c18d213e739b7 c530a75c1e6a472b0eb9558310b518f0dfcd8860 d0d8ad39ecb51cd7497cd524484fe09f50876798 d97a39d903fe33c45be83ac6943a2f82a3649a11 59e9783ddf18e650622e0573cad4f08db65592e4 134106 fail 5726a8d0f1958af80ad8e514bc2c18d213e739b7 c530a75c1e6a472b0eb9558310b518f0dfcd8860 d0d8ad39ecb51cd7497cd524484fe09f50876798 dd154c4d9f48a44ad24e13f46033d0f10a05c923 59e9783ddf18e650622e0573cad4f08db65592e4 134138 pass 99403097be0cbe12042775d9ca3a66f2018adc3e c530a75c1e6a472b0eb9558310b518f0dfcd8860 d0d8ad39ecb51cd7497cd524484fe09f50876798 62cfabb52210139843e26c95434356f73a0631b9 4deeaf2a3ee50b096426eea41a4c9b96ded0f029 134128 pass
[Xen-devel] [xen-unstable-smoke test] 134169: trouble: blocked/broken/pass
flight 134169 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/134169/ 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 Z6 days Failing since134068 2019-03-25 12:00:51 Z3 days 16 attempts Testing same since 134136 2019-03-27 18:00:28 Z1 days6 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] [qemu-mainline test] 134156: regressions - trouble: blocked/broken/fail/pass
flight 134156 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/134156/ 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-xsm 4 host-install(4)broken REGR. vs. 133909 build-arm64 4 host-install(4)broken REGR. vs. 133909 test-amd64-i386-freebsd10-amd64 14 guest-saverestore fail REGR. vs. 133909 test-amd64-i386-freebsd10-i386 14 guest-saverestore fail REGR. vs. 133909 test-amd64-amd64-xl-qemuu-ovmf-amd64 13 guest-saverestore fail REGR. vs. 133909 test-amd64-amd64-xl-qemuu-win7-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-amd64-xl-qemuu-debianhvm-amd64-shadow 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-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-xl 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 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 build-arm64-libvirt 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-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 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-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-rtds 13 migrate-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
[Xen-devel] [PATCH v2 1/1] 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. Signed-off-by: Xin Li Reviewed-by: Jan Beulich --- CC: Jan Beulich CC: Igor Druzhinin CC: Sergey Dyasli CC: Andrew Cooper v2 1: write the struct if any of the strings is provided 2: add contained_handles as flexible array member 3: update commit message and fix style issue --- tools/firmware/hvmloader/smbios.c | 69 - tools/firmware/hvmloader/smbios_types.h | 7 +++ xen/include/public/hvm/hvm_xs_strings.h | 6 +++ 3 files changed, 80 insertions(+), 2 deletions(-) diff --git a/tools/firmware/hvmloader/smbios.c b/tools/firmware/hvmloader/smbios.c index 40d8399be1..97a054e9e3 100644 --- a/tools/firmware/hvmloader/smbios.c +++ b/tools/firmware/hvmloader/smbios.c @@ -497,9 +497,11 @@ static void * smbios_type_2_init(void *start) { struct smbios_type_2 *p = (struct smbios_type_2 *)start; +const char *s; uint8_t *ptr; void *pts; uint32_t length; +unsigned int counter = 0; pts = get_smbios_pt_struct(2, &length); if ( (pts != NULL)&&(length > 0) ) @@ -518,8 +520,71 @@ smbios_type_2_init(void *start) return (start + length); } -/* Only present when passed in */ -return start; +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 */ +p->chassis_handle = SMBIOS_HANDLE_TYPE3; +p->board_type = 0x0a; /* Motherboard */ +start += sizeof(*p); + +s = xenstore_read(HVM_XS_BASEBOARD_MANUFACTURER, NULL); +if ( (s != NULL) && (*s != '\0') ) +{ +strcpy(start, s); +start += strlen(s) + 1; +p->manufacturer_str = ++counter; +} + +s = xenstore_read(HVM_XS_BASEBOARD_PRODUCT_NAME, NULL); +if ( (s != NULL) && (*s != '\0') ) +{ +strcpy(start, s); +start += strlen(s) + 1; +p->product_name_str = ++counter; +} + +s = xenstore_read(HVM_XS_BASEBOARD_VERSION, NULL); +if ( (s != NULL) && (*s != '\0') ) +{ +strcpy(start, s); +start += strlen(s) + 1; +p->version_str = ++counter; +} + +s = xenstore_read(HVM_XS_BASEBOARD_SERIAL_NUMBER, NULL); +if ( (s != NULL) && (*s != '\0') ) +{ +strcpy(start, s); +start += strlen(s) + 1; +p->serial_number_str = ++counter; +} + +s = xenstore_read(HVM_XS_BASEBOARD_ASSET_TAG, NULL); +if ( (s != NULL) && (*s != '\0') ) +{ +strcpy(start, s); +start += strlen(s) + 1; +p->asset_tag_str = ++counter; +} + +s = xenstore_read(HVM_XS_BASEBOARD_LOCATION_IN_CHASSIS, NULL); +if ( (s != NULL) && (*s != '\0') ) +{ +strcpy(start, s); +start += strlen(s) + 1; +p->location_in_chassis_str = ++counter; +} + +if ( counter ) +{ +*(uint8_t *)start = 0; +return start + 1; +} + +/* Only present when passed in or with customized string */ +return start - sizeof(*p); } /* Type 3 -- System Enclosure */ diff --git a/tools/firmware/hvmloader/smbios_types.h b/tools/firmware/hvmloader/smbios_types.h index acb63e2fe9..7c648ece71 100644 --- a/tools/firmware/hvmloader/smbios_types.h +++ b/tools/firmware/hvmloader/smbios_types.h @@ -90,6 +90,13 @@ 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[]; } __attribute__ ((packed)); /* System Enclosure - Contained Elements */ diff --git a/xen/include/public/hvm/hvm_xs_strings.h b/xen/include/public/hvm/hvm_xs_strings.h index fea1dd4407..fba2546424 100644 --- a/xen/include/public/hvm/hvm_xs_strings.h +++ b/xen/include/public/hvm/hvm_xs_strings.h @@ -69,6 +69,12 @@ #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_ASS
[Xen-devel] [linux-4.4 test] 134158: trouble: blocked/broken/fail/pass
flight 134158 linux-4.4 real [real] http://logs.test-lab.xenproject.org/osstest/logs/134158/ 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 broken build-arm64-pvopsbroken build-arm64-xsm 4 host-install(4)broken REGR. vs. 133468 build-arm64-pvops 4 host-install(4)broken REGR. vs. 133468 build-arm64 4 host-install(4)broken REGR. vs. 133468 Tests which are failing intermittently (not blocking): test-amd64-i386-examine 8 reboot fail in 134102 pass in 134158 test-amd64-i386-freebsd10-amd64 7 xen-boot fail in 134102 pass in 134158 test-armhf-armhf-libvirt-raw 10 debian-di-install fail pass in 134102 Tests which did not succeed, but are not blocking: build-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-examine 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-libvirt-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-raw 12 migrate-support-check fail in 134102 never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-check fail in 134102 never pass 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-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 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-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-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-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 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-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 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-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-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-amd64-amd64-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-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 p
[Xen-devel] [libvirt test] 134162: regressions - trouble: blocked/broken/fail/pass
flight 134162 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/134162/ 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-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-arm64-arm64-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-xsm 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-qcow2 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-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 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 83b1808ca2605b631828038094e96dcf8cc5e442 baseline version: libvirt 25e2e4e04f13901b3db903b2301bd11381bdf128 Last test of basis 133846 2019-03-16 02:09:09 Z 13 days Failing since133876 2019-03-17 11:33:04 Z 11 days9 attempts Testing same since 134162 2019-03-28 16:39:11 Z0 days1 attempts People who touched revisions under test: Andrea Bolognani Cole Robinson Daniel Henrique Barboza Daniel P. Berrangé Eric Blake Jason Dillaman Jiri Denemark Laine Stump 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.xen