Re: [Xen-devel] [PATCH 00/12] add per-domain and per-cpupool generic parameters
On 21/09/18 10:52, Wei Liu wrote: > On Fri, Sep 21, 2018 at 07:23:23AM +0200, Juergen Gross wrote: >> On 20/09/18 18:06, Wei Liu wrote: >>> On Wed, Sep 19, 2018 at 07:58:50PM +0200, Juergen Gross wrote: Did you look into the patches, especially patch 10? The parameters set are all stored in domain config via libxl__arch_domain_save_config(). >>> >>> No, I didn't. >>> >>> I think the general idea of what you do in patch 10 should work. However >>> I want to comment on the implementation. >>> >>> It appears that the implementation in patch 10 concatenates the new >>> settings to the old ones. It is not very nice imo. >>> >>> If for the life time of the domain you set X times the same parameter >>> you get a string of foo=bar1 foo=bar2 in the saved config file. >>> >>> There is probably a simple solution: make the parameter list in IDL a >>> key value list. You then update the list accordingly. >> >> The problem with that approach are parameters with sub-parameters: >> >> par=sub1=no,sub2=yes >> par=sub2=yes > > That means the value type of the top level key value list should ideally > be another key value list. I do notice the limitation in the key value > list type: the value can only be string. > > There is another way to solve this: further parse the sub-parameters. > This doesn't require any parameter specific knowledge and there are > already functions to split strings. I don't think this will work for the general case. It might be that par=no will switch off all sub-parameters. How would you handle that? I'm looking into a way to report the current parameter settings. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] Xen PV: Sample new PV driver for buffer sharing between domains
Hi, I am using Debian as Domain-0 and Debian as Domain-U on Hikey960 platform(ARMv8) and using Xen-4.8 stable release. Here I want to create a PV frontend and backend to share memory between Domain-0 and Domain-U. I used below link to create frontend and backend, https://fnordig.de/2016/12/02/xen-a-backend-frontend-driver-example/ But I am facing below errors while adding device vdevb to xenstore. Below errors I am getting from xenbus_switch_state(). vdevb vdevb-0: failed to write error node for device/vdevb/0 (13 writing new state) Please suggest me, How to create PV drivers. Thanks, Omkar B -- This message contains confidential information and is intended only for the individual(s) named. If you are not the intended recipient, you are notified that disclosing, copying, distributing or taking any action in reliance on the contents of this mail and attached file/s is strictly prohibited. Please notify the sender immediately and delete this e-mail from your system. E-mail transmission cannot be guaranteed to be secured or error-free as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete, or contain viruses. The sender therefore does not accept liability for any errors or omissions in the contents of this message, which arise as a result of e-mail transmission. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-3.18 test] 128096: regressions - FAIL
flight 128096 linux-3.18 real [real] http://logs.test-lab.xenproject.org/osstest/logs/128096/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-armhf-armhf-xl-vhd 10 debian-di-installfail REGR. vs. 127486 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-xl 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-armhf-armhf-libvirt 14 saverestore-support-checkfail like 127486 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 127486 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 127486 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 127486 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 127486 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 127486 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass build-arm64-pvops 6 kernel-build fail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail 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-xl-pvshim12 guest-start fail 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-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 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-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 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 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-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-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail 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-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail 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 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass version targeted for testing: linux921b2fed6a79439ef1609ef4af0ada5cccb3555c baseline version: linuxc0305995d3676c8f7764eb79a7f99de8d18c591a Last test of basis 127486 2018-09-10 23:39:53 Z 16 days Testing same since 128096 2018-09-26 06:42:33 Z0 days1 attempts People who touched revisions under test: Aaron Knister Adam Radford Alan Stern Aleh Filipovich Aleh Filipovich Alexandre Belloni Amit Pundir Anatoly Trosinenko Andreas Gruenbacher Andrew Morton Andrey Ryabinin Andy Shevchenko Anton Vasilyev Arnaldo Carvalho de Melo Arnd Bergmann Bartlomiej Zolnierkiewicz Ben Hutchings Bin Yang BingJing Chang Boris Brezillon Breno Leitao Chao Yu Charles Keepax Chas Williams Dan Carpenter
[Xen-devel] [ovmf baseline-only test] 75299: trouble: blocked/broken
This run is configured for baseline tests only. flight 75299 ovmf real [real] http://osstest.xensource.com/osstest/logs/75299/ 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-i386-pvops 4 host-install(4) broken baseline untested build-i3864 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 version targeted for testing: ovmf 447b08b3d2a3e04a9fccda68c72a2ff62d8197e9 baseline version: ovmf 2939283f2df3b8a0871e9bc7b2dd3718146318f4 Last test of basis75295 2018-09-26 08:32:21 Z0 days Testing same since75299 2018-09-27 00:50:12 Z0 days1 attempts People who touched revisions under test: Ruiyu Ni 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-i386-pvops host-install(4) broken-step build-i386 host-install(4) broken-step build-amd64-pvops host-install(4) broken-step build-i386-xsm host-install(4) Push not applicable. commit 447b08b3d2a3e04a9fccda68c72a2ff62d8197e9 Author: Ruiyu Ni Date: Tue Sep 25 13:21:40 2018 +0800 UefiCpuPkg/MtrrLib: Revert "Skip MSR access when the pair is invalid" REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1187 The patch reverts 9c8c4478cfcacaf5fd60b75ff78d26732d93a5b8 "UefiCpuPkg/MtrrLib: Skip Base MSR access when the pair is invalid". Microsoft Windows will report an error in event manager if MTRR usage is different across hibernate even when the difference is in an non valid MTRR pair. This seems like a bug in Windows but for compatibility and servicing reasons we think a change in UEFI would wise. A Windows change has already been submitted for the next iteration (2019 time frame). Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ruiyu Ni Cc: Michael D Kinney Reviewed-by: Eric Dong Cc: Sean Brogan commit 69b40465048e4289854d881e90007811c09d42d8 Author: Ruiyu Ni Date: Tue Sep 25 10:58:56 2018 +0800 MdeModulePkg/PciHostBridge: Move declaration of mIoMmu to header file The change doesn't have functionality impact. It just renames the mIoMmuProtocol to mIoMmu and moves the declaration from PciRootBridgeIo.c to PciHostBridge.h. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ruiyu Ni Suggested-by: Star Zeng Reviewed-by: Star Zeng Reviewed-by: Laszlo Ersek commit
Re: [Xen-devel] [PATCH 09/12] tools/xl: add support for setting generic per-cpupool parameters
On 26/09/18 19:17, Dario Faggioli wrote:
> On Tue, 2018-09-18 at 08:03 +0200, Juergen Gross wrote:
>> Add a new xl command "cpupool-set-parameters" and cpupool config file
>> support for setting per-cpupool generic parameters.
>>
>> Signed-off-by: Juergen Gross
>>
> Seems good to me. Couple of questions.
>
> Question one: what about getting (and displaying, I guess in
> cpupoolinfo) the cpupool parameters?
>
>> --- a/tools/xl/xl_cpupool.c
>> +++ b/tools/xl/xl_cpupool.c
>> @@ -615,6 +625,35 @@ out:
>> return rc;
>> }
>>
>> +int main_cpupoolsetparameters(int argc, char **argv)
>> +{
>> +int opt;
>> +const char *pool;
>> +char *params;
>> +uint32_t poolid;
>> +
>> +SWITCH_FOREACH_OPT(opt, "", NULL, "cpupool-set-parameters", 2) {
>> +/* No options */
>> +}
>> +
>> +pool = argv[optind++];
>> +if (libxl_cpupool_qualifier_to_cpupoolid(ctx, pool, ,
>> NULL) ||
>> +!libxl_cpupoolid_is_valid(ctx, poolid)) {
>> +fprintf(stderr, "unknown cpupool '%s'\n", pool);
>> +return EXIT_FAILURE;
>> +}
>> +
> Since we know that we, AFAIUI, never allow changing the parameters for
> a cpupool with domains in it already, shall we test this here, and bail
> early, with a specific error message, without even trying going down in
> Xen?
>
> I know it's sort-of duplicating checks with what's in the hypervisor,
> but I think it would be a common mistake, that it's worth trying to
> prevent/address specifically.
That's exactly what the PARAM_FLAG_RUNTIME is meant for. I could think
of parameters which might be changeable even with active domains in the
cpupool. So I wouldn't like to test that in the tools as we would need
to add the knowledge of each parameter to the tools.
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH trivial] xen/balloon: Grammar s/Is it/It is/
Signed-off-by: Geert Uytterhoeven --- drivers/xen/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig index 90d387b50ab747f5..7f42d41f66ee98e3 100644 --- a/drivers/xen/Kconfig +++ b/drivers/xen/Kconfig @@ -86,7 +86,7 @@ config XEN_SCRUB_PAGES_DEFAULT help Scrub pages before returning them to the system for reuse by other domains. This makes sure that any confidential data - is not accidentally visible to other domains. Is it more + is not accidentally visible to other domains. It is more secure, but slightly less efficient. This can be controlled with xen_scrub_pages=0 parameter and /sys/devices/system/xen_memory/xen_memory0/scrub_pages. -- 2.17.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [ovmf test] 128098: all pass - PUSHED
flight 128098 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/128098/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 447b08b3d2a3e04a9fccda68c72a2ff62d8197e9 baseline version: ovmf 2939283f2df3b8a0871e9bc7b2dd3718146318f4 Last test of basis 128086 2018-09-26 02:37:03 Z0 days Testing same since 128098 2018-09-26 07:11:44 Z0 days1 attempts People who touched revisions under test: Ruiyu Ni 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 2939283f2d..447b08b3d2 447b08b3d2a3e04a9fccda68c72a2ff62d8197e9 -> xen-tested-master ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable test] 128084: tolerable FAIL
flight 128084 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/128084/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-armhf-armhf-examine 6 xen-installfail pass in 128033 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 128033 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 128033 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 128033 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 128033 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 128033 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 128033 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 128033 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 128033 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 128033 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-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 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-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 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 13 migrate-support-checkfail never pass test-armhf-armhf-xl 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 13 migrate-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-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-amd64-i386-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-armhf-armhf-libvirt-raw 12 migrate-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 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 940185b2f6f343251c2b83bd96e599398cea51ec baseline version: xen 940185b2f6f343251c2b83bd96e599398cea51ec Last test of basis 128084 2018-09-26 01:51:53 Z0 days Testing same since (not found) 0 attempts jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64-xtf pass build-amd64 pass build-arm64 pass build-armhf pass build-amd64-xen-freebsd pass
Re: [Xen-devel] IOREQ server on Arm
Hi Paul,
On 09/26/2018 01:01 PM, Paul Durrant wrote:
-Original Message-
From: Jan Beulich [mailto:jbeul...@suse.com]
Sent: 26 September 2018 12:57
To: Paul Durrant
Cc: Julien Grall ; Andrew Cooper
; Roger Pau Monne ;
Stefano Stabellini ; xen-devel
Subject: RE: IOREQ server on Arm
On 26.09.18 at 13:02, wrote:
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -1105,8 +1105,11 @@ static int acquire_resource(
for ( i = 0; !rc && i < xmar.nr_frames; i++ )
{
-rc = set_foreign_p2m_entry(currd, gfn_list[i],
- _mfn(mfn_list[i]));
+rc = (xmar.flags & XENMEM_rsrc_acq_caller_owned) ?
+guest_physmap_add_entry(currd, gfn_list[i],
+_mfn(mfn_list[i]), 0,
p2m_ram_rw) :
+set_foreign_p2m_entry(currd, gfn_list[i],
+ _mfn(mfn_list[i]));
/* rc should be -EIO for any iteration other than the first
*/
if ( rc && i )
rc = -EIO;
But the guest_physmap_add_entry() is problematic as it will IOMMU map
pages
as well, which is probably not wanted.
Yeah, I'd prefer if we avoided establishing IOMMU mappings here.
How about transforming set_foreign_p2m_entry() into
set_special_p2m_entry(), with a type passed in?
That sounds like it might work.
Julien, do you want page types to distinguish caller-owned resources from
normal RAM are you ok with p2m_ram_rw even though it could be subject of
another domain's foreign map?
Based on your previous e-mail, I would be fine with that on Arm.
This brings me to the next question. Do you expect set_special_p2m_entry
to take a reference on the page?
If not, we may run into some troubles because AFAICT you can map twice
the ioreq page in a guest but reference will only be taken on the
allocation.
However, the unmap path will always drop a reference when removing the
page. This is because Xen at the moment, reference will not be taken on
mapping but allocation (we assume a page could not be mapped twice in a
guest).
Foreign mapping on Arm are a bit special because we get a reference on
mapping them and will drop it when the mapping disappear. So we would
not have any problem there.
Any thoughts?
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 2/3] xen/arm: vgic-v3: Don't create empty re-distributor regions
Hi Stefano, On 09/25/2018 09:38 PM, Stefano Stabellini wrote: On Tue, 4 Sep 2018, Julien Grall wrote: At the moment, Xen is assuming the hardware domain will have the same number of re-distributor regions as the host. However, as the number of CPUs or the stride (e.g on GICv4) may be different we end up exposing regions which does not contain any re-distributors. When booting, Linux will go through all the re-distributor region to check whether a property (e.g vPLIs) is available accross all the re-distributors. This will result to a data abort on empty regions because there are no underlying re-distributor. So we need to limit the number of regions exposed to the hardware domain. The code reworked to only expose the minimun number of regions required by the hardware domain. It is assumed the regions will be populated starting from the first one. I have a question: given that it is possible for a rdist_region to cover more than 1 cpu, could we get into troubles if the last rdist_region of the hardware_domain covers 2 cpus, but actually dom0 only has 1 vcpu? get_vcpu_from_rdist would return NULL and vgic_v3_rdistr_mmio_read/write would return 0. This case seems to be handled correctly but I wanted to double check. 0 means a data abort will be injected into the guest. However, the guest should never touch that because the last valid re-distributor of the regions will have GICR_TYPER.Last set. So the guest OS will stop looking for more re-distributors in that region. > I think we also need to fix vgic_v3_rdist_count? Today it just returns vgic_v3_hw.nr_rdist_regions for dom0. It would be bad if we left it unfixed? If we do that, we might be able to get rid of the modifications to vgic_v3_real_domain_init. We don't want to fix vgic_v3_rdist_count. The helper returns the maximum re-distributors regions. This is used to compute the number of IO handlers and allocating the array storing the regions. I am pretty sure you will say we will waste memory. However, at the moment, we need to know the number of IO handlers much before we know the number of vCPUs. For the array, we would need to go through the regions twice (regions may not be the same size so we can't infer easily the number needed). Overall, the amount of memory used is the same as today (so not really a waste per-se). It might be possible to limit that once we reworked the common code to know the number of vCPUs earlier on (see discussion on patch #1). Reported-by: Shameerali Kolothum Thodi Signed-off-by: Julien Grall --- xen/arch/arm/gic-v3.c | 10 -- xen/arch/arm/vgic-v3.c | 11 +++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c index b2ed0f8b55..4a984cfb12 100644 --- a/xen/arch/arm/gic-v3.c +++ b/xen/arch/arm/gic-v3.c @@ -1274,8 +1274,10 @@ static int gicv3_make_hwdom_dt_node(const struct domain *d, * GIC has two memory regions: Distributor + rdist regions * CPU interface and virtual cpu interfaces accessesed as System registers * So cells are created only for Distributor and rdist regions + * The hardware domain may not used all the regions. So only copy + * what is necessary. */ -new_len = new_len * (gicv3.rdist_count + 1); +new_len = new_len * (d->arch.vgic.nr_regions + 1); Do we also need to fix "#redistributor-regions" just above? Hmm I missed that one. Not sure why it didn't show up in my test. diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c index df1bab3a35..9f729862da 100644 --- a/xen/arch/arm/vgic-v3.c +++ b/xen/arch/arm/vgic-v3.c @@ -1695,8 +1695,19 @@ static int vgic_v3_real_domain_init(struct domain *d) d->arch.vgic.rdist_regions[i].first_cpu = first_cpu; first_cpu += size / GICV3_GICR_SIZE; + +if ( first_cpu >= d->max_vcpus ) +break; This is just a matter of code style and preferences, but I would prefer if the termination condition was at the top as part of the for statement. Of course, it works regardless, so the patch would be OK either way. I thought about it when writing this patch. This would look like: for ( i = 0; (i < vgic_v3_hw.nr_dist_regions) && (first_cpu < d->max_vcpus); i++ ) This is IHMO more difficult to understand (long condition) and slightly strange because for is not incrementing directly first_cpu. I will stick with the current implementation unless you have a more readable solution. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [libvirt test] 128090: regressions - FAIL
flight 128090 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/128090/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-libvirt 6 libvirt-buildfail REGR. vs. 123814 build-amd64-libvirt 6 libvirt-buildfail REGR. vs. 123814 build-i386-libvirt6 libvirt-buildfail REGR. vs. 123814 build-armhf-libvirt 6 libvirt-buildfail REGR. vs. 123814 Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-qcow2 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a version targeted for testing: libvirt 9580c091635df276c13747535f8d2b071cac0fe3 baseline version: libvirt 076a2b409667dd9f716a2a2085e1ffea9d58fe8b Last test of basis 123814 2018-06-05 04:19:23 Z 113 days Failing since123840 2018-06-06 04:19:28 Z 112 days 94 attempts Testing same since 128090 2018-09-26 04:18:47 Z0 days1 attempts People who touched revisions under test: Ales Musil Andrea Bolognani Anya Harter Bing Niu Bjoern Walk Bobo Du Boris Fiuczynski Brijesh Singh Changkuo Shi Chen Hanxiao Christian Ehrhardt Clementine Hayat Cole Robinson Dan Kenigsberg Daniel Nicoletti Daniel P. Berrangé Daniel Veillard Eric Blake Erik Skultety Fabiano Fidêncio Fabiano Fidêncio Farhan Ali Filip Alac Han Han intrigeri intrigeri Jamie Strandboge Jie Wang Jim Fehlig Jiri Denemark John Ferlan Julio Faracco Ján Tomko Kashyap Chamarthy Katerina Koukiou Laine Stump Laszlo Ersek Lin Ma Lubomir Rintel Luyao Huang Marc Hartmayer Marc Hartmayer Marcos Paulo de Souza Marek Marczykowski-Górecki Mark Asselstine Martin Kletzander Matthias Bolte Michal Privoznik Michal Prívozník Nikolay Shirokovskiy Pavel Hrdina Peter Krempa Pino Toscano Radostin Stoyanov Ramy Elkest ramyelkest Richard W.M. Jones Roman Bogorodskiy Roman Bolshakov Shi Lei Shi Lei Shichangkuo Shivaprasad G Bhat Simon Kobyda Stefan Bader Stefan Berger Sukrit Bhatnagar Tomáš Golembiovský Vitaly Kuznetsov w00251574 Wang Huaqiang Wang Yechao Weilun Zhu Wu Zongyong xinhua.Cao jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf pass build-i386 pass build-amd64-libvirt fail build-arm64-libvirt fail build-armhf-libvirt fail build-i386-libvirt fail build-amd64-pvopspass build-arm64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm blocked test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmblocked test-amd64-amd64-libvirt-xsm blocked test-arm64-arm64-libvirt-xsm blocked test-amd64-i386-libvirt-xsm blocked test-amd64-amd64-libvirt blocked test-arm64-arm64-libvirt blocked
Re: [Xen-devel] [PATCH 1/3] [not-for-unstable] xen/arm: vgic-v3: Delay the initialization of the domain information
Hi Stefano,
On 09/25/2018 09:45 PM, Stefano Stabellini wrote:
On Tue, 4 Sep 2018, Andrew Cooper wrote:
On 04/09/18 20:35, Julien Grall wrote:
Hi,
On 09/04/2018 08:21 PM, Julien Grall wrote:
A follow-up patch will require to know the number of vCPUs when
initializating the vGICv3 domain structure. However this information is
not available at domain creation. This is only known once
XEN_DOMCTL_max_vpus is called for that domain.
In order to get the max vCPUs around, delay the domain part of the vGIC
v3 initialization until the first vCPU of the domain is initialized.
Signed-off-by: Julien Grall
---
Cc: Andrew Cooper
This is nasty but I can't find a better way for Xen 4.11 and older. This
is not necessary for unstable as the number of vCPUs is known at domain
creation.
Andrew, I have CCed you to know whether you have a better idea where to
place this call on Xen 4.11 and older.
I just noticed that d->max_vcpus is initialized after
arch_domain_create. So without this patch on Xen 4.12, it will not work.
This is getting nastier because arch_domain_init is the one initialize
the value returned by dom0_max_vcpus. So I am not entirely sure what
to do here.
The positioning after arch_domain_create() is unfortunate, but I
couldn’t manage better with ARM's current behaviour and Jan's insistence
that the allocation of d->vcpu was common. I'd prefer if the dependency
could be broken and the allocation moved earlier.
One option might be to have an arch_check_domainconfig() (or similar?)
which is called very early on and can sanity check the values, including
cross-checking the vgic and max_vcpus settings? It could even be
responsible for mutating XEN_DOMCTL_CONFIG_GIC_NATIVE into the correct
real value.
As for your patch here, its a gross hack, but its probably the best
which can be done.
*Sighs*
If that is what we have to do, it is as ugly as hell, but that is what
we'll do.
This is the best we can do with the current code base. I think it would
be worth reworking the code to make it nicer. I will add it in my TODO list.
My only suggestion to marginally improve it would be instead of:
+if ( v->vcpu_id == 0 )
+{
+rc = vgic_v3_real_domain_init(d);
+if ( rc )
+return rc;
+}
to check on d->arch.vgic.rdist_regions instead:
if ( d->arch.vgic.rdist_regions == NULL )
{
// initialize domain
I would prefer to keep v->vcpu_id == 0 just in case we end up to
re-order the allocation in the future.
Cheers,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-linus test] 128059: regressions - FAIL
flight 128059 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/128059/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-i386-freebsd10-i386 11 guest-startfail REGR. vs. 125898 test-amd64-i386-qemuu-rhel6hvm-intel 10 redhat-install fail REGR. vs. 125898 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-i386-xl-qemuu-ws16-amd64 10 windows-install fail REGR. vs. 125898 test-amd64-i386-qemuu-rhel6hvm-amd 10 redhat-install fail REGR. vs. 125898 test-amd64-i386-xl-qemuu-win7-amd64 10 windows-install fail REGR. vs. 125898 test-amd64-amd64-xl-qemuu-win7-amd64 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-multivcpu 7 xen-bootfail REGR. vs. 125898 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-qcow2 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-xl-pvhv2-intel 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-libvirt-vhd 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-libvirt-xsm 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-xl-shadow 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-qemut-rhel6hvm-intel 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-xl7 xen-boot fail REGR. vs. 125898 test-amd64-i386-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-i386-examine 8 reboot fail REGR. vs. 125898 test-amd64-i386-xl-qemut-debianhvm-amd64 7 xen-boot fail REGR. vs. 125898 test-amd64-amd64-examine 8 reboot fail REGR. vs. 125898 test-amd64-amd64-pygrub 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-freebsd10-amd64 11 guest-start fail REGR. vs. 125898 test-amd64-i386-rumprun-i386 7 xen-boot fail REGR. vs. 125898 test-armhf-armhf-libvirt 7 xen-boot fail REGR. vs. 125898 test-amd64-i386-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 125898 test-amd64-i386-xl-qemut-win10-i386 7 xen-boot fail REGR. vs. 125898 build-i386-libvirt6 libvirt-buildfail REGR. vs. 125898 Regressions which are regarded as allowable (not blocking): test-amd64-amd64-xl-rtds 7 xen-boot fail REGR. vs. 125898 Tests which did not succeed, but are not blocking: test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 125898 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 125898 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 125898 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 125898 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 125898 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-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-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-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-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass
Re: [Xen-devel] [PATCH trivial] xen/balloon: Grammar s/Is it/It is/
On 9/26/18 4:43 AM, Geert Uytterhoeven wrote: > Signed-off-by: Geert Uytterhoeven > --- > drivers/xen/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig > index 90d387b50ab747f5..7f42d41f66ee98e3 100644 > --- a/drivers/xen/Kconfig > +++ b/drivers/xen/Kconfig > @@ -86,7 +86,7 @@ config XEN_SCRUB_PAGES_DEFAULT > help > Scrub pages before returning them to the system for reuse by > other domains. This makes sure that any confidential data > - is not accidentally visible to other domains. Is it more > + is not accidentally visible to other domains. It is more > secure, but slightly less efficient. This can be controlled with > xen_scrub_pages=0 parameter and > /sys/devices/system/xen_memory/xen_memory0/scrub_pages. Applied to for-linus-19b. -boris ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [ovmf baseline-only test] 75295: trouble: blocked/broken
This run is configured for baseline tests only. flight 75295 ovmf real [real] http://osstest.xensource.com/osstest/logs/75295/ 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-i386-pvops 4 host-install(4) broken baseline untested build-i3864 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 version targeted for testing: ovmf 2939283f2df3b8a0871e9bc7b2dd3718146318f4 baseline version: ovmf e5cd809087e5710e019d2766fab13c59a2e2ac28 Last test of basis75292 2018-09-26 02:50:19 Z0 days Testing same since75295 2018-09-26 08:32:21 Z0 days1 attempts People who touched revisions under test: Jian J Wang shenglei Zhang, Shenglei 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-i386-pvops host-install(4) broken-step build-i386 host-install(4) broken-step build-amd64-pvops host-install(4) broken-step build-i386-xsm host-install(4) Push not applicable. commit 2939283f2df3b8a0871e9bc7b2dd3718146318f4 Author: Jian J Wang Date: Tue Sep 18 15:17:11 2018 +0800 UefiCpuPkg/CpuMpPei: fix vs2012 build error REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1166 Visual Studio 2012 will complain uninitialized variable, StackBase, in the CpuPaging.c. This patch adds code to init it to zero and ASSERT check against 0. This is enough since uninit case will only happen during retrieving stack memory via gEfiHobMemoryAllocStackGuid. But this HOB will always be created in advance. Cc: Dandan Bi Cc: Hao A Wu Cc: Eric Dong Cc: Laszlo Ersek Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang Reviewed-by: Laszlo Ersek Reviewed-by: Eric Dong commit 5267926134d17e86672b84fd57b438f05ffa68e1 Author: Jian J Wang Date: Tue Sep 25 16:49:19 2018 +0800 MdeModulePkg/DxeIpl: support more NX related PCDs BZ#1116: https://bugzilla.tianocore.org/show_bug.cgi?id=1116 Currently IA32_EFER.NXE is only set against PcdSetNxForStack. This confuses developers because following two other PCDs also need NXE to be set, but actually not. PcdDxeNxMemoryProtectionPolicy PcdImageProtectionPolicy This patch solves this issue by adding logic to enable IA32_EFER.NXE if any of those PCDs have anything enabled. Cc: Star Zeng Cc: Laszlo Ersek Cc: Ard Biesheuvel Cc:
Re: [Xen-devel] [PATCH v2 6/6] xen/arm: Replace call_smc with arm_smccc_smc
Hi Stefano,
On 09/26/2018 12:57 AM, Stefano Stabellini wrote:
On Tue, 25 Sep 2018, Julien Grall wrote:
diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
index 941eec921b..02737e6caa 100644
--- a/xen/arch/arm/psci.c
+++ b/xen/arch/arm/psci.c
@@ -42,42 +42,53 @@ uint32_t smccc_ver;
static uint32_t psci_cpu_on_nr;
+#define PSCI_RET(res) ((int32_t)(res).a0)
+
int call_psci_cpu_on(int cpu)
{
-return call_smc(psci_cpu_on_nr, cpu_logical_map(cpu),
__pa(init_secondary), 0);
+struct arm_smccc_res res;
+
+arm_smccc_smc(psci_cpu_on_nr, cpu_logical_map(cpu), __pa(init_secondary),
+ );
+
+return (int32_t)res.a0;
}
Can't we use PSCI_RET(res) here?
I missed that one. I will update it.
Also in general, should we care about the type mismatch int32_t vs. int
which is the return type of this function?
The only issue I could see is if sizeof(int) < sizeof(int32_t). If that
happen, then psci.c would be our least concern as we always assume int
would at least 32-bit wide.
I would prefer to keep the return of the function int and casting the
result with (int32_t). The latter is helpful to know what is the size of
the result (a0 is 64-bit).
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 4/6] xen/arm: cpufeature: Add helper to check constant caps
Hi Stefano, On 09/26/2018 05:53 PM, Stefano Stabellini wrote: On Tue, 25 Sep 2018, Julien Grall wrote: Some capababilities are set right during boot and will never change afterwards. At the moment, the function cpu_have_caps will check whether the cap is enabled from the memory. It is possible to avoid the load from the memory by using an ALTERNATIVE. With that the check is just reduced to 1 instruction. Signed-off-by: Julien Grall I enjoyed reading the patch :-) But I don't think it is worth going into this extreme level of optimization. test_bit is efficient enough, right? What do you think we need to use alternatives just to check one bit? We already have an helper using test_bit (see cpus_have_cap). However test_bit requires to load the word from the memory. This is an overhead when the decision never change after boot. One load may be insignificant by itself (thought may have a cache miss), but if you are in hotpath this is a win in long term. The mechanism is very similar to static key but for the poor (I don't have much time to implement better for now). We already use similar construction on other places (see CHECK_WORKAROUND_HELPER). 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 3/6] xen/arm: add SMC wrapper that is compatible with SMCCC v1.0
Hi Stefano, On 09/26/2018 12:50 AM, Stefano Stabellini wrote: On Tue, 25 Sep 2018, Julien Grall wrote: From: Volodymyr Babchuk Existing SMC wrapper call_smc() allows only 4 parameters and returns only one value. This is enough for existing use in PSCI code, but TEE mediator will need a call that is fully compatible with ARM SMCCC v1.0. This patch adds a wrapper for both arm32 and arm64. In the case of arm32, the wrapper is just an alias to the ARM SMCCC v1.1 as the convention is the same. CC: "Edgar E. Iglesias" Signed-off-by: Volodymyr Babchuk [julien: Rework the wrapper to make it closer to SMCC 1.1 wrapper] Signed-off-by: Julien Grall I have been struggling to find the old doc for SMCCC v1.0, all the references have been updated to v1.1 online now. Do you have a link? Are you sure? All the references are still to v1.0 (DEN 0028B). See [1]. diff --git a/xen/arch/arm/arm64/smc.S b/xen/arch/arm/arm64/smc.S new file mode 100644 index 00..b0752be57e --- /dev/null +++ b/xen/arch/arm/arm64/smc.S @@ -0,0 +1,32 @@ +/* + * xen/arch/arm/arm64/smc.S + * + * Wrapper for Secure Monitors Calls + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include +#include + +/* + * void __arm_smccc_1_0_smc(register_t a0, register_t a1, register_t a2, + * register_t a3, register_t a4, register_t a5, + * register_t a6, register_t a7, + * struct arm_smccc_res *res) + */ +ENTRY(__arm_smccc_1_0_smc) +smc #0 +ldr x4, [sp] +cbz x4, 1f /* No need to store the result */ +stp x0, x1, [x4, #SMCCC_RES_a0] +stp x2, x3, [x4, #SMCCC_RES_a2] +1: +ret As I mentioned, I couldn't find the doc, but it looks like the Linux implementation always copies back the results (arch/arm64/kernel/smccc-call.S)? Shouldn't we zero x0-x3 at least? Could you provide more details on what looks wrong? The results are copied in the array res using stp instructions. The only difference with Linux implementation is we don't handle quirk. Cheers, [1] https://developer.arm.com/products/architecture/cpu-architecture/a-profile/docs/den0028/latest/smc-calling-convention-system-software-on-arm-platforms -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86: use alternatives for FS/GS base accesses
On 26/09/18 07:43, Jan Beulich wrote: On 25.09.18 at 18:52, wrote: >> On 29/08/18 17:03, Jan Beulich wrote: >>> Eliminates a couple of branches in particular from the context switch >>> path. >>> >>> Signed-off-by: Jan Beulich >> I've already expressed a dis-inclination to this patch, because it looks >> like a micro-optimisation which won't actually affect measureable >> performance. (And as said before, I could be wrong, but I don't think I >> am...) > Iirc you had indicated you first of all don't like the mix of some constructs > using alternatives and some not. Correct. Consistency (one way or the other) is better overall here. > Eliminating conditional branches is always a Good Thing (tm), it seems to me. By this reasoning, we should compile Xen with movfuscator, which really will get rid of every branch. Doing so would be utter nonsense, ergo this claim is false. > And that's not just for > performance (inside Xen we can't assume at all that any code path, > even the context switch one, is hot enough to have any BTB entries > allocated), This is a valid argument, for why the proposed change might plausibly be an improvement. It is by no means a guarantee that making the change will result in improved performance. > but also for ease of looking at the assembly, should there > be a need to do so. Using alternatives actively obfuscates the logic in the disassembly. It is almost impossible to distinguish the individual fragments, and you rejected my suggestion of rectifying this by putting symbols into the .altinstructions section. It also results in harder to read C, and poorer surrounded code generation, as the compiler has to cope with the union of entry/exit requirements for the blocks. So no - this claim is also false. > Overall I think we ought to make much heavier use > of alternatives patching, so I view this only as a first step towards this. > Otherwise, btw, why did you not object to e.g. clac() / stac() using > alternatives patching? As with so many other things, I very much think > we should settle on a fundamental approach, and then write all code > consistently. If we followed what you say, we'd have to limit patching > to cases where conditionals can't (reasonably) express what we want. I never said that we shouldn't patch conditionals. There is a cost to every use of alternative, and the decision to use a alternatives needs to be justified on their merits outweighing their cost. I'm not currently convinced of the merit/cost tradeoff in this case. >> Have you done some perf analysis since you last posted it? > I don't view this as a worthwhile use of my time, to be honest. Even > a non-measurable improvement is an improvement. I'd understand > your objection if there was a fair reason to be afraid of worse > performance as a result of this change. So you're submitting a performance patch (which you admit might have no measurable improvement) based on logic which I've called into question, and furthermore, you expect me to ack it based on your untested opinion that "its an improvement"? Do you think that repeating myself is a worthwhile use of my time? I'm afraid that I'm going to be very blunt now. What matters, performance wise, is net performance in common workloads, and avoiding catastrophic corner cases. This is a macro problem, not a micro problem, and in my opinion, you are demonstrating repeated poor judgement in this regard. In particular, it is simply not true that improving the micro-performance of a block increases the overall performance. To cover some examples so far this year... This patch still hasn't addressed the concerns about sh[lr]d, and the resulting competition for execution resource on AMD Fam15/16h systems. "x86: enable interrupts earlier with XPTI disabled" was objected to by me on the basis of the increased complexity of following the code, rather than any performance consideration. A contributory factor was that I couldn't see any reason why it would make any performance difference. When Juergen eventually measured it, the results said the performance was worse. (It might be interesting to work out why it was worse overall, because its definitely not obvious, but I suspect we all have more important work to do). "x86/xsave: prefer eager clearing of state over eager restoring" is basic statistics. In this case, worrying about the theoretical longterm trend is having a material performance impact (in Intel's case, 8%) on current users, and I do intend to make Xen fully eager (benefiting all hardware) when I've confirmed what I suspect to be true on the AMD side of things. When all the major OS and hypervisors are fully eager, and when most hardware you can buy today is specifically optimised for this configuration, Xen being the different hurts only ourselves. "x86: use PDEP/PEXT for maddr/direct-map-offset conversion when available" neglects the cache bloat of having 255 copies of the stub, and the pipeline stall from
[Xen-devel] [freebsd-master test] 128102: all pass - PUSHED
flight 128102 freebsd-master real [real] http://logs.test-lab.xenproject.org/osstest/logs/128102/ Perfect :-) All tests in this flight passed as required version targeted for testing: freebsd a62d8e729c6db95f0c2e1de618be0b0796c0a97a baseline version: freebsd e3c2d3a906b1063421584e83d3d3968849b04690 Last test of basis 128006 2018-09-24 09:19:07 Z2 days Testing same since 128102 2018-09-26 09:19:09 Z0 days1 attempts People who touched revisions under test: 0mp <0...@freebsd.org> alc andreast brooks cperciva emaste jhb jhibbits kib markj mav mjg mmacy np 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 e3c2d3a906b..a62d8e729c6 a62d8e729c6db95f0c2e1de618be0b0796c0a97a -> tested/master ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 00/12] add per-domain and per-cpupool generic parameters
On Fri, 2018-09-21 at 09:52 +0100, Wei Liu wrote: > On Fri, Sep 21, 2018 at 07:23:23AM +0200, Juergen Gross wrote: > > On 20/09/18 18:06, Wei Liu wrote: > > > > > > It appears that the implementation in patch 10 concatenates the > > > new > > > settings to the old ones. It is not very nice imo. > > > > > > If for the life time of the domain you set X times the same > > > parameter > > > you get a string of foo=bar1 foo=bar2 in the saved config file. > > > > > > There is probably a simple solution: make the parameter list in > > > IDL a > > > key value list. You then update the list accordingly. > > > > The problem with that approach are parameters with sub-parameters: > > > > par=sub1=no,sub2=yes > > par=sub2=yes > > There is another way to solve this: further parse the sub-parameters. > This doesn't require any parameter specific knowledge and there are > already functions to split strings. > I'm not sure whether we're saying the same thing or not, but can't we, when parameter 'foo', which has been set to 'bar1' already, is being set to 'bar2', search d_config.b_info.parameters for the substring containing 'foo=bar1', replace it with 'foo=bar2', and save d_config again? Regards, Dario -- <> (Raistlin Majere) - Dario Faggioli, Ph.D, http://about.me/dario.faggioli Software Engineer @ SUSE https://www.suse.com/ signature.asc Description: This is a digitally signed message part ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH net-next] net: xen-netback: fix return type of ndo_start_xmit function
From: YueHaibing Date: Wed, 26 Sep 2018 17:18:14 +0800 > The method ndo_start_xmit() is defined as returning an 'netdev_tx_t', > which is a typedef for an enum type, so make sure the implementation in > this driver has returns 'netdev_tx_t' value, and change the function > return type to netdev_tx_t. > > Found by coccinelle. > > Signed-off-by: YueHaibing > Acked-by: Wei Liu Applied. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC PATCH 2/2] x86/mm: Add mem access rights to NPT
On 26/09/18 17:47, George Dunlap wrote: > From: Isaila Alexandru > > This patch adds access control for NPT mode. > > There aren’t enough extra bits to store the access rights in the NPT p2m > table, so we add a radix tree to store extra information. I'm sorry to re-open this argument, but why? ISTR there being some argument based on pagetable sharing with the IOMMU, but that doesn't work at the moment and can't reasonably be made to work. For one, attempting to use pt sharing will break as soon as you try and DMA to a mapped grant. I'm disinclined to let a broken vestigial feature get in the way of real improvements. Beyond that, an NPT PTE has basically the same number of software available bits as an EPT PTE. Am I missing anything? > > For efficiency: > - Only allocate this radix tree when we first store "non-default" >extra information > > - Remove entires which match the default extra information rather >than continuing to store them > > - For superpages, only store an entry for the first gfn in the >superpage. Use the order of the p2m entry being read to determine >the proper place to look in the radix table. > > Modify p2m_type_to_flags() to accept and interpret an access value, > parallel to the ept code. > > Add a set_default_access() method to the p2m-pt and p2m-ept versions > of the p2m rather than setting it directly, to deal with different > default permitted access values. > > Signed-off-by: Alexandru Isaila > Signed-off-by: George Dunlap > --- > NB, this is compile-tested only. > > cc'ing Paul because this is functionality he may want at some point in > the future. > > I'm not sure why we only allow 'int' to be stored in the radix tree, > but that throws away 30-some bits we could otherwise use. We might > consider revising this if we run out of bits here. Probably because we have a old fork of the Linux radix tree functionality, rather than any specific reason to waste 30-some bits. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 09/12] tools/xl: add support for setting generic per-cpupool parameters
On Tue, 2018-09-18 at 08:03 +0200, Juergen Gross wrote:
> Add a new xl command "cpupool-set-parameters" and cpupool config file
> support for setting per-cpupool generic parameters.
>
> Signed-off-by: Juergen Gross
>
Seems good to me. Couple of questions.
Question one: what about getting (and displaying, I guess in
cpupoolinfo) the cpupool parameters?
> --- a/tools/xl/xl_cpupool.c
> +++ b/tools/xl/xl_cpupool.c
> @@ -615,6 +625,35 @@ out:
> return rc;
> }
>
> +int main_cpupoolsetparameters(int argc, char **argv)
> +{
> +int opt;
> +const char *pool;
> +char *params;
> +uint32_t poolid;
> +
> +SWITCH_FOREACH_OPT(opt, "", NULL, "cpupool-set-parameters", 2) {
> +/* No options */
> +}
> +
> +pool = argv[optind++];
> +if (libxl_cpupool_qualifier_to_cpupoolid(ctx, pool, ,
> NULL) ||
> +!libxl_cpupoolid_is_valid(ctx, poolid)) {
> +fprintf(stderr, "unknown cpupool '%s'\n", pool);
> +return EXIT_FAILURE;
> +}
> +
Since we know that we, AFAIUI, never allow changing the parameters for
a cpupool with domains in it already, shall we test this here, and bail
early, with a specific error message, without even trying going down in
Xen?
I know it's sort-of duplicating checks with what's in the hypervisor,
but I think it would be a common mistake, that it's worth trying to
prevent/address specifically.
> +params = argv[optind];
> +
> +if (libxl_cpupool_set_parameters(ctx, poolid, params)) {
> +fprintf(stderr, "cannot set parameters: %s\n", params);
> +fprintf(stderr, "Use \"xl dmesg\" to look for possible
> reason.\n");
> +return EXIT_FAILURE;
> +}
> +
> +return EXIT_SUCCESS;
> +}
>
Regards,
Dario
--
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Software Engineer @ SUSE https://www.suse.com/
signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 05/12] xen: add hypercall interfaces for domain and cpupool parameter setting
On Tue, 2018-09-18 at 08:03 +0200, Juergen Gross wrote:
> Add a new domctl for setting domain specific parameters similar to
> XEN_SYSCTL_set_parameter for global hypervisor parameters.
>
> Enhance XEN_SYSCTL_set_parameter to be usable for setting cpupool
> specific parameters, too. For now do only extended parameter
> checking.
> The cpupool parameter setting will be added later.
>
> Signed-off-by: Juergen Gross
>
Reviewed-by: Dario Faggioli
I mean...
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -1055,12 +1055,18 @@ struct xen_sysctl_livepatch_op {
> * Parameters are a single string terminated by a NUL byte of max.
> size
> * characters. Multiple settings can be specified by separating them
> * with blanks.
> + * Scope can be either global (like boot parameters) or cpupool.
> */
>
> struct xen_sysctl_set_parameter {
> XEN_GUEST_HANDLE_64(char) params; /* IN: pointer to
> parameters. */
> uint16_t size; /* IN: size of
> parameters. */
> -uint16_t pad[3];/* IN: MUST be zero. */
> +uint8_t scope; /* IN: scope of
> parameters. */
> +#define XEN_SYSCTL_SETPAR_SCOPE_GLOBAL 0
> +#define XEN_SYSCTL_SETPAR_SCOPE_CPUPOOL 1
> +uint8_t pad; /* IN: MUST be zero. */
> +uint32_t instance; /* IN: scope global:
> must be zero */
> +/* scope cpupool:
> cpupool id */
>
... I don't particularly like the name 'instance', but I've not been
able to come up with anything else which sounds better... :-/
Regards,
Dario
--
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Software Engineer @ SUSE https://www.suse.com/
signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC PATCH 1/2] mem_access: Fix npfec.kind propagation
On 26/09/18 17:47, George Dunlap wrote:
> The name of the "with_gla" flag is confusing; it has nothing to do
> with the existence or lack thereof of a faulting GLA, but rather where
> the fault originated. The npfec.kind value is always valid, and
> should thus be propagated, regardless of whether gla_valid is set or
> not.
>
> In particular, gla_valid will never be set on AMD systems; but
> npfec.kind will still be valid and should still be propagated.
>
> Signed-off-by: Alexandru Isaila
> Signed-off-by: George Dunlap
> ---
> CC: Andrew Cooper
> CC: Jan Beulich
> CC: Tim Deegan
> CC: Tamas K Lengyel
> CC: Razvan Cojocaru
> ---
> xen/arch/x86/mm/mem_access.c | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c
> index d9e64fcbb9..699a315076 100644
> --- a/xen/arch/x86/mm/mem_access.c
> +++ b/xen/arch/x86/mm/mem_access.c
> @@ -232,12 +232,12 @@ bool p2m_mem_access_check(paddr_t gpa, unsigned long
> gla,
> {
> req->u.mem_access.flags |= MEM_ACCESS_GLA_VALID;
> req->u.mem_access.gla = gla;
> -
> -if ( npfec.kind == npfec_kind_with_gla )
> -req->u.mem_access.flags |= MEM_ACCESS_FAULT_WITH_GLA;
> -else if ( npfec.kind == npfec_kind_in_gpt )
> -req->u.mem_access.flags |= MEM_ACCESS_FAULT_IN_GPT;
> }
> +
> +if ( npfec.kind == npfec_kind_with_gla )
> +req->u.mem_access.flags |= MEM_ACCESS_FAULT_WITH_GLA;
> +else if ( npfec.kind == npfec_kind_in_gpt )
> +req->u.mem_access.flags |= MEM_ACCESS_FAULT_IN_GPT;
Nit. Newline here please, as it is not logically related with the block
below.
~Andrew
> req->u.mem_access.flags |= npfec.read_access? MEM_ACCESS_R : 0;
> req->u.mem_access.flags |= npfec.write_access ? MEM_ACCESS_W : 0;
> req->u.mem_access.flags |= npfec.insn_fetch ? MEM_ACCESS_X : 0;
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 07/12] xen: add domain specific parameter support
I think the subject wants to say "cpupool specific" ? With this adjusted... On Tue, 2018-09-18 at 08:03 +0200, Juergen Gross wrote: > Add the framework for being able to define cpupool specific > parameters. > This includes the needed macros for defining a parameter and the > minimal set of functions for doing parameter parsing. > > Signed-off-by: Juergen Gross > Reviewed-by: Dario Faggioli Regards, Dario -- <> (Raistlin Majere) - Dario Faggioli, Ph.D, http://about.me/dario.faggioli Software Engineer @ SUSE https://www.suse.com/ signature.asc Description: This is a digitally signed message part ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 4/6] xen/arm: cpufeature: Add helper to check constant caps
On Tue, 25 Sep 2018, Julien Grall wrote:
> Some capababilities are set right during boot and will never change
> afterwards. At the moment, the function cpu_have_caps will check whether
> the cap is enabled from the memory.
>
> It is possible to avoid the load from the memory by using an
> ALTERNATIVE. With that the check is just reduced to 1 instruction.
>
> Signed-off-by: Julien Grall
I enjoyed reading the patch :-) But I don't think it is worth going
into this extreme level of optimization. test_bit is efficient enough,
right? What do you think we need to use alternatives just to check one
bit?
> ---
>
> This is the static key for the poor. At some point we might want to
> introduce something similar to static key in Xen.
>
> Changes in v2:
> - Use unlikely
> ---
> xen/include/asm-arm/cpufeature.h | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/xen/include/asm-arm/cpufeature.h
> b/xen/include/asm-arm/cpufeature.h
> index 3de6b54301..c6cbc2ec84 100644
> --- a/xen/include/asm-arm/cpufeature.h
> +++ b/xen/include/asm-arm/cpufeature.h
> @@ -63,6 +63,18 @@ static inline bool cpus_have_cap(unsigned int num)
> return test_bit(num, cpu_hwcaps);
> }
>
> +/* System capability check for constant cap */
> +#define cpus_have_const_cap(num) ({ \
> +bool __ret; \
> +\
> +asm volatile (ALTERNATIVE("mov %0, #0", \
> + "mov %0, #1", \
> + num) \
> + : "=r" (__ret)); \
> +\
> +unlikely(__ret);\
> +})
> +
> static inline void cpus_set_cap(unsigned int num)
> {
> if (num >= ARM_NCAPS)
> --
> 2.11.0
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC PATCH 1/2] mem_access: Fix npfec.kind propagation
On Wed, Sep 26, 2018 at 10:49 AM George Dunlap wrote: > > The name of the "with_gla" flag is confusing; it has nothing to do > with the existence or lack thereof of a faulting GLA, but rather where > the fault originated. The npfec.kind value is always valid, and > should thus be propagated, regardless of whether gla_valid is set or > not. > > In particular, gla_valid will never be set on AMD systems; but > npfec.kind will still be valid and should still be propagated. > > Signed-off-by: Alexandru Isaila > Signed-off-by: George Dunlap Acked-by: Tamas K Lengyel ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [RFC PATCH 2/2] x86/mm: Add mem access rights to NPT
From: Isaila Alexandru
This patch adds access control for NPT mode.
There aren’t enough extra bits to store the access rights in the NPT p2m
table, so we add a radix tree to store extra information.
For efficiency:
- Only allocate this radix tree when we first store "non-default"
extra information
- Remove entires which match the default extra information rather
than continuing to store them
- For superpages, only store an entry for the first gfn in the
superpage. Use the order of the p2m entry being read to determine
the proper place to look in the radix table.
Modify p2m_type_to_flags() to accept and interpret an access value,
parallel to the ept code.
Add a set_default_access() method to the p2m-pt and p2m-ept versions
of the p2m rather than setting it directly, to deal with different
default permitted access values.
Signed-off-by: Alexandru Isaila
Signed-off-by: George Dunlap
---
NB, this is compile-tested only.
cc'ing Paul because this is functionality he may want at some point in
the future.
I'm not sure why we only allow 'int' to be stored in the radix tree,
but that throws away 30-some bits we could otherwise use. We might
consider revising this if we run out of bits here.
CC: Andrew Cooper
CC: Jan Beulich
CC: Tim Deegan
CC: Tamas K Lengyel
CC: Paul Durrant
CC: Razvan Cojocaru
---
xen/arch/x86/mm/mem_access.c | 5 +-
xen/arch/x86/mm/p2m-ept.c| 9 ++
xen/arch/x86/mm/p2m-pt.c | 173 ---
xen/arch/x86/mm/p2m.c| 6 ++
xen/arch/x86/monitor.c | 1 +
xen/include/asm-x86/mem_access.h | 2 +-
xen/include/asm-x86/p2m.h| 18
7 files changed, 192 insertions(+), 22 deletions(-)
diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c
index 699a315076..0f96c43f88 100644
--- a/xen/arch/x86/mm/mem_access.c
+++ b/xen/arch/x86/mm/mem_access.c
@@ -389,10 +389,7 @@ long p2m_set_mem_access(struct domain *d, gfn_t gfn,
uint32_t nr,
/* If request to set default access. */
if ( gfn_eq(gfn, INVALID_GFN) )
-{
-p2m->default_access = a;
-return 0;
-}
+return p2m->set_default_access(p2m, a);
p2m_lock(p2m);
if ( ap2m )
diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
index 1ff4f14ae4..37bdbcf186 100644
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -667,6 +667,14 @@ bool_t ept_handle_misconfig(uint64_t gpa)
return spurious ? (rc >= 0) : (rc > 0);
}
+int ept_set_default_access(struct p2m_domain *p2m, p2m_access_t p2ma)
+{
+/* All access is permitted */
+p2m->default_access = p2ma;
+
+return 0;
+}
+
/*
* ept_set_entry() computes 'need_modify_vtd_table' for itself,
* by observing whether any gfn->mfn translations are modified.
@@ -1255,6 +1263,7 @@ int ept_p2m_init(struct p2m_domain *p2m)
p2m->change_entry_type_global = ept_change_entry_type_global;
p2m->change_entry_type_range = ept_change_entry_type_range;
p2m->memory_type_changed = ept_memory_type_changed;
+p2m->set_default_access = ept_set_default_access;
p2m->audit_p2m = NULL;
p2m->tlb_flush = ept_tlb_flush;
diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c
index 40bfc76a6f..cd8b8c9187 100644
--- a/xen/arch/x86/mm/p2m-pt.c
+++ b/xen/arch/x86/mm/p2m-pt.c
@@ -68,7 +68,8 @@
static unsigned long p2m_type_to_flags(const struct p2m_domain *p2m,
p2m_type_t t,
mfn_t mfn,
- unsigned int level)
+ unsigned int level,
+ p2m_access_t access)
{
unsigned long flags;
/*
@@ -87,23 +88,28 @@ static unsigned long p2m_type_to_flags(const struct
p2m_domain *p2m,
case p2m_ram_paged:
case p2m_ram_paging_in:
default:
-return flags | _PAGE_NX_BIT;
+flags |= _PAGE_NX_BIT;
+break;
case p2m_grant_map_ro:
-return flags | P2M_BASE_FLAGS | _PAGE_NX_BIT;
+flags |= P2M_BASE_FLAGS | _PAGE_NX_BIT;
+break;
case p2m_ioreq_server:
flags |= P2M_BASE_FLAGS | _PAGE_RW | _PAGE_NX_BIT;
if ( p2m->ioreq.flags & XEN_DMOP_IOREQ_MEM_ACCESS_WRITE )
-return flags & ~_PAGE_RW;
-return flags;
+flags &= ~_PAGE_RW;
+break;
case p2m_ram_ro:
case p2m_ram_logdirty:
case p2m_ram_shared:
-return flags | P2M_BASE_FLAGS;
+flags |= P2M_BASE_FLAGS;
+break;
case p2m_ram_rw:
-return flags | P2M_BASE_FLAGS | _PAGE_RW;
+flags |= P2M_BASE_FLAGS | _PAGE_RW;
+break;
case p2m_grant_map_rw:
case p2m_map_foreign:
-return flags | P2M_BASE_FLAGS | _PAGE_RW | _PAGE_NX_BIT;
+flags |= P2M_BASE_FLAGS | _PAGE_RW | _PAGE_NX_BIT;
+break;
case p2m_mmio_direct:
if (
[Xen-devel] [RFC PATCH 1/2] mem_access: Fix npfec.kind propagation
The name of the "with_gla" flag is confusing; it has nothing to do
with the existence or lack thereof of a faulting GLA, but rather where
the fault originated. The npfec.kind value is always valid, and
should thus be propagated, regardless of whether gla_valid is set or
not.
In particular, gla_valid will never be set on AMD systems; but
npfec.kind will still be valid and should still be propagated.
Signed-off-by: Alexandru Isaila
Signed-off-by: George Dunlap
---
CC: Andrew Cooper
CC: Jan Beulich
CC: Tim Deegan
CC: Tamas K Lengyel
CC: Razvan Cojocaru
---
xen/arch/x86/mm/mem_access.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c
index d9e64fcbb9..699a315076 100644
--- a/xen/arch/x86/mm/mem_access.c
+++ b/xen/arch/x86/mm/mem_access.c
@@ -232,12 +232,12 @@ bool p2m_mem_access_check(paddr_t gpa, unsigned long gla,
{
req->u.mem_access.flags |= MEM_ACCESS_GLA_VALID;
req->u.mem_access.gla = gla;
-
-if ( npfec.kind == npfec_kind_with_gla )
-req->u.mem_access.flags |= MEM_ACCESS_FAULT_WITH_GLA;
-else if ( npfec.kind == npfec_kind_in_gpt )
-req->u.mem_access.flags |= MEM_ACCESS_FAULT_IN_GPT;
}
+
+if ( npfec.kind == npfec_kind_with_gla )
+req->u.mem_access.flags |= MEM_ACCESS_FAULT_WITH_GLA;
+else if ( npfec.kind == npfec_kind_in_gpt )
+req->u.mem_access.flags |= MEM_ACCESS_FAULT_IN_GPT;
req->u.mem_access.flags |= npfec.read_access? MEM_ACCESS_R : 0;
req->u.mem_access.flags |= npfec.write_access ? MEM_ACCESS_W : 0;
req->u.mem_access.flags |= npfec.insn_fetch ? MEM_ACCESS_X : 0;
--
2.19.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [distros-debian-squeeze test] 75294: trouble: blocked/broken
flight 75294 distros-debian-squeeze real [real] http://osstest.xensource.com/osstest/logs/75294/ 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 Tests which did not succeed, but are not blocking: test-amd64-i386-i386-squeeze-netboot-pygrub 1 build-check(1) blocked n/a test-amd64-amd64-i386-squeeze-netboot-pygrub 1 build-check(1) blocked n/a test-amd64-i386-amd64-squeeze-netboot-pygrub 1 build-check(1) blocked n/a test-amd64-amd64-amd64-squeeze-netboot-pygrub 1 build-check(1)blocked n/a build-armhf 4 host-install(4) broken like 75245 build-armhf-pvops 4 host-install(4) broken like 75245 build-amd64-pvops 4 host-install(4) broken like 75245 build-amd64 4 host-install(4) broken like 75245 build-i386-pvops 4 host-install(4) broken like 75245 build-i3864 host-install(4) broken like 75245 baseline version: flight 75245 jobs: build-amd64 broken build-armhf broken build-i386 broken build-amd64-pvopsbroken build-armhf-pvopsbroken build-i386-pvops broken test-amd64-amd64-amd64-squeeze-netboot-pygrubblocked test-amd64-i386-amd64-squeeze-netboot-pygrub blocked test-amd64-amd64-i386-squeeze-netboot-pygrub blocked test-amd64-i386-i386-squeeze-netboot-pygrub blocked sg-report-flight on osstest.xs.citrite.net logs: /home/osstest/logs images: /home/osstest/images Logs, config files, etc. are available at http://osstest.xensource.com/osstest/logs Test harness code can be found at http://xenbits.xensource.com/gitweb?p=osstest.git;a=summary Push not applicable. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [OSSTEST PATCH v3] ts-xen-build-prep: install libgnutls28-dev for libvirt build
d54ecf31b2 placed the build dependency in a wrong file. This patch
adds the dependency to the right file. Add a runtime dependency in
libvirt.pm.
Signed-off-by: Wei Liu
---
Cc: Jim Fehlig
---
Osstest/Toolstack/libvirt.pm | 6 +-
ts-xen-build-prep| 3 ++-
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/Osstest/Toolstack/libvirt.pm b/Osstest/Toolstack/libvirt.pm
index d5cda77e..7ebeaf66 100644
--- a/Osstest/Toolstack/libvirt.pm
+++ b/Osstest/Toolstack/libvirt.pm
@@ -26,10 +26,14 @@ use XML::LibXML;
sub new {
my ($class, $ho, $methname,$asset) = @_;
-my @extra_packages = qw(libavahi-client3 libgnutls28-dev);
+my @extra_packages = qw(libavahi-client3);
my $nl_lib = "libnl-3-200";
+my $libgnutls = "libgnutls30";
+
$nl_lib = "libnl1" if ($ho->{Suite} =~ m/wheezy/);
+$libgnutls = "libgnutls-deb0-28" if ($ho->{Suite} =~ m/jessie/);
push(@extra_packages, $nl_lib);
+push(@extra_packages, $libgnutls);
return bless { Name => "libvirt",
Host => $ho,
NewDaemons => [qw(libvirtd)],
diff --git a/ts-xen-build-prep b/ts-xen-build-prep
index 77a2d284..23bbbeb9 100755
--- a/ts-xen-build-prep
+++ b/ts-xen-build-prep
@@ -208,7 +208,8 @@ sub prep () {
libxml2-utils libxml2-dev
libdevmapper-dev w3c-dtd-xhtml libxml-xpath-perl
libelf-dev
- ccache nasm checkpolicy ebtables);
+ ccache nasm checkpolicy ebtables
+ libgnutls28-dev);
if ($ho->{Suite} !~ m/squeeze|wheezy/) {
push(@packages, qw(ocaml-nox ocaml-findlib));
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4] x86/mm: Add mem access rights to NPT
[Resending]
On Wed, Sep 26, 2018 at 5:02 PM George Dunlap
wrote:
>
> On Mon, Jul 23, 2018 at 2:48 PM Alexandru Isaila
> wrote:
> >
> > From: Isaila Alexandru
> >
> > This patch adds access control for NPT mode.
> >
> > There aren’t enough extra bits to store the access rights in the NPT p2m
> > table, so we add a radix tree to store the rights. For efficiency,
> > remove entries which match the default permissions rather than
> > continuing to store them.
> >
> > Modify p2m-pt.c:p2m_type_to_flags() to mirror the ept version: taking an
> > access, and removing / adding RW or NX flags as appropriate.
> >
> > Note: It was tested with xen-access write
> >
> > Signed-off-by: Alexandru Isaila
> >
> > ---
> > Changes since V3:
> > - Add p2m_pt_check_access() to filter n, w, wx, n2rwx from
> > supported page rights
> > - Add rights check for the default_access change in the
> > IVALID_GFN case
> > - Add blank lines
> > - Remove cpu_has_svm if from p2m_mem_access_check()
> > - Add xfree(msr_bitmap) in case of error on
> > xalloc(raxid_tree_root).
> > ---
> > xen/arch/x86/mm/mem_access.c | 17 +++---
> > xen/arch/x86/mm/p2m-ept.c| 7 +++
> > xen/arch/x86/mm/p2m-pt.c | 124
> > ++-
> > xen/arch/x86/mm/p2m.c| 6 ++
> > xen/arch/x86/monitor.c | 15 +
> > xen/include/asm-x86/mem_access.h | 2 +-
> > xen/include/asm-x86/p2m.h| 7 +++
> > 7 files changed, 156 insertions(+), 22 deletions(-)
> >
> > diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c
> > index c0cd017..cab72bc 100644
> > --- a/xen/arch/x86/mm/mem_access.c
> > +++ b/xen/arch/x86/mm/mem_access.c
> > @@ -221,12 +221,12 @@ bool p2m_mem_access_check(paddr_t gpa, unsigned long
> > gla,
> > {
> > req->u.mem_access.flags |= MEM_ACCESS_GLA_VALID;
> > req->u.mem_access.gla = gla;
> > -
> > -if ( npfec.kind == npfec_kind_with_gla )
> > -req->u.mem_access.flags |= MEM_ACCESS_FAULT_WITH_GLA;
> > -else if ( npfec.kind == npfec_kind_in_gpt )
> > -req->u.mem_access.flags |= MEM_ACCESS_FAULT_IN_GPT;
> > }
> > +
> > +if ( npfec.kind == npfec_kind_with_gla )
> > +req->u.mem_access.flags |= MEM_ACCESS_FAULT_WITH_GLA;
> > +else if ( npfec.kind == npfec_kind_in_gpt )
> > +req->u.mem_access.flags |= MEM_ACCESS_FAULT_IN_GPT;
> > req->u.mem_access.flags |= npfec.read_access? MEM_ACCESS_R : 0;
> > req->u.mem_access.flags |= npfec.write_access ? MEM_ACCESS_W : 0;
> > req->u.mem_access.flags |= npfec.insn_fetch ? MEM_ACCESS_X : 0;
> > @@ -366,8 +366,11 @@ long p2m_set_mem_access(struct domain *d, gfn_t gfn,
> > uint32_t nr,
> > /* If request to set default access. */
> > if ( gfn_eq(gfn, INVALID_GFN) )
> > {
> > -p2m->default_access = a;
> > -return 0;
> > +if ( (rc = p2m->check_access(a)) == 0 )
> > +{
> > +p2m->default_access = a;
> > +return 0;
> > +}
> > }
>
> BTW this introduces a bug -- if the check fails, this will fall
> through into the gfn loop below, rather than returning the error.
>
> > @@ -87,23 +88,27 @@ static unsigned long p2m_type_to_flags(const struct
> > p2m_domain *p2m,
> > case p2m_ram_paged:
> > case p2m_ram_paging_in:
> > default:
> > -return flags | _PAGE_NX_BIT;
> > +flags |= P2M_BASE_FLAGS | _PAGE_NX_BIT;
> > +break;
>
> Why did you add in P2M_BASE_FLAGS here?
>
> > case p2m_grant_map_ro:
> > return flags | P2M_BASE_FLAGS | _PAGE_NX_BIT;
>
> And is this `return` left here on purpose, or was it missed?
>
> > /* Returns: 0 for success, -errno for failure */
> > static int
> > p2m_next_level(struct p2m_domain *p2m, void **table,
> > @@ -201,6 +268,7 @@ p2m_next_level(struct p2m_domain *p2m, void **table,
> > new_entry = l1e_from_mfn(mfn, P2M_BASE_FLAGS | _PAGE_RW);
> >
> > p2m_add_iommu_flags(_entry, level,
> > IOMMUF_readable|IOMMUF_writable);
> > +p2m_set_access(p2m, gfn, p2m->default_access);
>
> This is clearly wrong -- this isn't a leaf node, it's an intermediate
> p2m table; and p2m_next_level() is just acting "under the covers",
> filling in missing bits of the p2m table or breaking down superpages.
> Since the access information is in a completely separate structure, it
> shouldn't need to be modified here at all (indeed, it would be a bug
> to do so).
>
> But that does bring up an important issue -- it would appear that this
> code is incorrect when setting superpages -- if we set a 2MiB page but
> then read a non-2MiB-aligned entry within that page, we'll get the
> default access rather than the one we set; same thing with splintering
> a superpage into smaller pages.
>
> There's a draft patch addressing this on
Re: [Xen-devel] [PATCH v4] x86/mm: Add mem access rights to NPT
On Mon, Jul 23, 2018 at 2:48 PM Alexandru Isaila
wrote:
>
> From: Isaila Alexandru
>
> This patch adds access control for NPT mode.
>
> There aren’t enough extra bits to store the access rights in the NPT p2m
> table, so we add a radix tree to store the rights. For efficiency,
> remove entries which match the default permissions rather than
> continuing to store them.
>
> Modify p2m-pt.c:p2m_type_to_flags() to mirror the ept version: taking an
> access, and removing / adding RW or NX flags as appropriate.
>
> Note: It was tested with xen-access write
>
> Signed-off-by: Alexandru Isaila
>
> ---
> Changes since V3:
> - Add p2m_pt_check_access() to filter n, w, wx, n2rwx from
> supported page rights
> - Add rights check for the default_access change in the
> IVALID_GFN case
> - Add blank lines
> - Remove cpu_has_svm if from p2m_mem_access_check()
> - Add xfree(msr_bitmap) in case of error on
> xalloc(raxid_tree_root).
> ---
> xen/arch/x86/mm/mem_access.c | 17 +++---
> xen/arch/x86/mm/p2m-ept.c| 7 +++
> xen/arch/x86/mm/p2m-pt.c | 124
> ++-
> xen/arch/x86/mm/p2m.c| 6 ++
> xen/arch/x86/monitor.c | 15 +
> xen/include/asm-x86/mem_access.h | 2 +-
> xen/include/asm-x86/p2m.h| 7 +++
> 7 files changed, 156 insertions(+), 22 deletions(-)
>
> diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c
> index c0cd017..cab72bc 100644
> --- a/xen/arch/x86/mm/mem_access.c
> +++ b/xen/arch/x86/mm/mem_access.c
> @@ -221,12 +221,12 @@ bool p2m_mem_access_check(paddr_t gpa, unsigned long
> gla,
> {
> req->u.mem_access.flags |= MEM_ACCESS_GLA_VALID;
> req->u.mem_access.gla = gla;
> -
> -if ( npfec.kind == npfec_kind_with_gla )
> -req->u.mem_access.flags |= MEM_ACCESS_FAULT_WITH_GLA;
> -else if ( npfec.kind == npfec_kind_in_gpt )
> -req->u.mem_access.flags |= MEM_ACCESS_FAULT_IN_GPT;
> }
> +
> +if ( npfec.kind == npfec_kind_with_gla )
> +req->u.mem_access.flags |= MEM_ACCESS_FAULT_WITH_GLA;
> +else if ( npfec.kind == npfec_kind_in_gpt )
> +req->u.mem_access.flags |= MEM_ACCESS_FAULT_IN_GPT;
> req->u.mem_access.flags |= npfec.read_access? MEM_ACCESS_R : 0;
> req->u.mem_access.flags |= npfec.write_access ? MEM_ACCESS_W : 0;
> req->u.mem_access.flags |= npfec.insn_fetch ? MEM_ACCESS_X : 0;
> @@ -366,8 +366,11 @@ long p2m_set_mem_access(struct domain *d, gfn_t gfn,
> uint32_t nr,
> /* If request to set default access. */
> if ( gfn_eq(gfn, INVALID_GFN) )
> {
> -p2m->default_access = a;
> -return 0;
> +if ( (rc = p2m->check_access(a)) == 0 )
> +{
> +p2m->default_access = a;
> +return 0;
> +}
> }
BTW this introduces a bug -- if the check fails, this will fall
through into the gfn loop below, rather than returning the error.
> @@ -87,23 +88,27 @@ static unsigned long p2m_type_to_flags(const struct
> p2m_domain *p2m,
> case p2m_ram_paged:
> case p2m_ram_paging_in:
> default:
> -return flags | _PAGE_NX_BIT;
> +flags |= P2M_BASE_FLAGS | _PAGE_NX_BIT;
> +break;
Why did you add in P2M_BASE_FLAGS here?
> case p2m_grant_map_ro:
> return flags | P2M_BASE_FLAGS | _PAGE_NX_BIT;
And is this `return` left here on purpose, or was it missed?
> /* Returns: 0 for success, -errno for failure */
> static int
> p2m_next_level(struct p2m_domain *p2m, void **table,
> @@ -201,6 +268,7 @@ p2m_next_level(struct p2m_domain *p2m, void **table,
> new_entry = l1e_from_mfn(mfn, P2M_BASE_FLAGS | _PAGE_RW);
>
> p2m_add_iommu_flags(_entry, level,
> IOMMUF_readable|IOMMUF_writable);
> +p2m_set_access(p2m, gfn, p2m->default_access);
This is clearly wrong -- this isn't a leaf node, it's an intermediate
p2m table; and p2m_next_level() is just acting "under the covers",
filling in missing bits of the p2m table or breaking down superpages.
Since the access information is in a completely separate structure, it
shouldn't need to be modified here at all (indeed, it would be a bug
to do so).
But that does bring up an important issue -- it would appear that this
code is incorrect when setting superpages -- if we set a 2MiB page but
then read a non-2MiB-aligned entry within that page, we'll get the
default access rather than the one we set; same thing with splintering
a superpage into smaller pages.
There's a draft patch addressing this on the way.
-George
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 05/11] vpci/msi: add teardown cleanup
>>> On 17.07.18 at 11:48, wrote: > So that interrupts are properly freed. > > Signed-off-by: Roger Pau Monné Same remarks here as for patch 4. 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: use macros for filling parameter definition blocks
On Tue, 2018-09-18 at 08:02 +0200, Juergen Gross wrote: > Define macros for filling struct kernel_param when defining > parameters. > > Signed-off-by: Juergen Gross > Reviewed-by: Dario Faggioli Regards, Dario -- <> (Raistlin Majere) - Dario Faggioli, Ph.D, http://about.me/dario.faggioli Software Engineer @ SUSE https://www.suse.com/ signature.asc Description: This is a digitally signed message part ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 04/11] vpci/msix: add teardown cleanup
>>> On 17.07.18 at 11:48, wrote:
> --- a/xen/drivers/vpci/msix.c
> +++ b/xen/drivers/vpci/msix.c
> @@ -436,11 +436,6 @@ static int init_msix(struct pci_dev *pdev)
> vpci_msix_arch_init_entry(>vpci->msix->entries[i]);
> }
>
> -rc = vpci_add_register(pdev->vpci, control_read, control_write,
> - msix_control_reg(msix_offset), 2,
> pdev->vpci->msix);
> -if ( rc )
> -return rc;
> -
> write_lock(>arch.hvm_domain.msix_lock);
> if ( list_empty(>arch.hvm_domain.msix_tables) )
> register_mmio_handler(d, _msix_table_ops);
> @@ -448,9 +443,57 @@ static int init_msix(struct pci_dev *pdev)
> list_add(>vpci->msix->next, >arch.hvm_domain.msix_tables);
> write_unlock(>arch.hvm_domain.msix_lock);
>
> +rc = vpci_add_register(pdev->vpci, control_read, control_write,
> + msix_control_reg(msix_offset), 2,
> pdev->vpci->msix);
Without the description saying why, I can't figure or guess why
this wants/needs moving.
> +if ( rc )
> +/* The teardown function will free the msix struct. */
> +return rc;
> +
> return 0;
This can be a single return statement now, without even a need
for going through rc.
> +static void teardown_msix(struct pci_dev *pdev)
> +{
> +struct vpci_msix *msix = pdev->vpci->msix;
> +unsigned int i, pos;
> +uint16_t control;
> +
> +if ( !msix )
> +return;
> +
> +write_lock(>domain->arch.hvm_domain.msix_lock);
> +list_del(>vpci->msix->next);
> +write_unlock(>domain->arch.hvm_domain.msix_lock);
> +
> +if ( !msix->enabled )
> +goto out;
> +
> +/* Disable MSIX. */
> +pos = pci_find_cap_offset(pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
> + PCI_FUNC(pdev->devfn), PCI_CAP_ID_MSIX);
> +ASSERT(pos);
> +control = pci_conf_read16(pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
> + PCI_FUNC(pdev->devfn), msix_control_reg(pos));
> +pci_conf_write16(pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
> + PCI_FUNC(pdev->devfn), msix_control_reg(pos),
> + (control & ~PCI_MSIX_FLAGS_ENABLE));
To avoid subsequent surprises, perhaps better also clear
PCI_MSIX_FLAGS_MASKALL?
> +for ( i = 0; i < msix->max_entries; i++ )
> +{
> +int rc = vpci_msix_arch_disable_entry(>entries[i], pdev);
> +
> +if ( rc && rc != -ENOENT )
> +gprintk(XENLOG_WARNING,
> +"%04x:%02x:%02x.%u: unable to disable MSIX entry %u:
> %d\n",
> +pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
> +PCI_FUNC(pdev->devfn), i, rc);
> +}
> +
> +out:
Labels indented by at least one blank please.
> +xfree(msix);
> +pdev->vpci->msix = NULL;
Perhaps better to zap the field before freeing the structure?
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 02/12] xen: use a structure to define parsing parameters
On Tue, 2018-09-18 at 08:02 +0200, Juergen Gross wrote: > Instead of passing the start and end pointers of the parameter > definition array to the parsing function use a struct containing that > information. This will allow to add other parameters to control the > parsing later. > > Signed-off-by: Juergen Gross > Reviewed-by: Dario Faggioli Regards, Dario -- <> (Raistlin Majere) - Dario Faggioli, Ph.D, http://about.me/dario.faggioli Software Engineer @ SUSE https://www.suse.com/ signature.asc Description: This is a digitally signed message part ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 00/12] add per-domain and per-cpupool generic parameters
[Hey, is it me/my mailer, or threading is weird for this series? :-O] On Tue, 2018-09-18 at 14:57 +0100, George Dunlap wrote: > On 09/18/2018 02:36 PM, Juergen Gross wrote: > > > > The string variant is much more flexible. > > > > It is easy possible to e.g. add a per-domain trace parameter to > > specify > > rather complex trace instrumentations. Doing something like that > > via a > > struct based interface is in the best case complicated. > > ...or, for instance, specifying the runqueue layout of a cpupool (for > schedulers like credit2 which allow such things). Yes, that is true; > but probably a very niche case. > Exactly. As another example, I want to follow up on this: https://lists.xenproject.org/archives/html/xen-devel/2018-08/msg01644.html More precisely, I want to add a per-cpupool "smt=[on|off|force]" (or cpupool-smt, or something like that), with the following meaning: - smt=on: cpus that are hyperthread siblings can be added to the cpupool. Adding a cpu whose sibling is in another pool would fail; - smt=off: only one cpu per core can be added to the cpupool. Adding a cpu whose sibling is already in the pool would fail. Adding a cpu whose sibling is in another pool would also fail; - smt=force: (and I particularly dislike the name, but let's ignore it for now) any cpu can be added to any pool What I was putting together was something along the lines of: https://lists.xenproject.org/archives/html/xen-devel/2017-09/msg01552.html And then there will be the support for having a line like this, in a cpupool config file: smt = "off" With this approach, instead, there will have to be a line like this in there: parameters = "smt=off" Is that right? And when we will also have the support for, say, per-cpupool runqueue arrangement for Credit2, it will look like this: parameters = "credit2_runqueues=socket smt=off" Right again? If yes, I think I'm fine with this. The per-cpupool parameters case is, I think, probably less controversial than the per-domain case. In facte, the parsing of, e.g., credit2_runqueue=, happens in Xen already. And most (if not all) of the scheduling parameters that we allow as command line options, also make sense as per-cpupool parameters, so... :-) I'm not sure where to draw the line, assuming we even want to draw one. I.e., if we take this approach and these patches, _any_ new parameter will have to be handled like this? If no, how do we decide which ones better use the "hypervisor centric" string blobs, and which ones better use the current "toolstack centric" one? About this (and especially for per-domain params), I've much less clear ideas. But as far as per-cpupools parameters are concerned, I do like this. 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] [xen-4.10-testing test] 128055: regressions - FAIL
flight 128055 xen-4.10-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/128055/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-i386-libvirt6 libvirt-buildfail REGR. vs. 127761 test-amd64-amd64-rumprun-amd64 15 rumprun-demo-xenstorels/xenstorels fail REGR. vs. 127761 build-i386-pvops 6 kernel-build fail REGR. vs. 127761 Tests which did not succeed, but are not blocking: test-amd64-i386-qemuu-rhel6hvm-intel 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a test-amd64-i386-rumprun-i386 1 build-check(1) blocked n/a test-amd64-i386-xl1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-win7-amd64 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-amd64 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-win10-i386 1 build-check(1) blocked n/a test-amd64-i386-qemut-rhel6hvm-intel 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win10-i386 1 build-check(1) blocked n/a test-amd64-i386-qemut-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-i386-xl-xsm1 build-check(1) blocked n/a test-amd64-i386-xl-raw1 build-check(1) blocked n/a test-amd64-i386-pair 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ws16-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-debianhvm-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-i386 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked n/a test-amd64-i386-xl-shadow 1 build-check(1) blocked n/a test-amd64-i386-migrupgrade 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-ws16-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 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-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-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 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-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-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt
Re: [Xen-devel] clean up physical merging helpers
On 9/25/18 2:30 PM, Christoph Hellwig wrote: > Hi Jens, > > this series moves Xen special handling of block merges from arch hooks > into common code. A previous version has been reviewed by Boris. Applied, thanks. -- Jens Axboe ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH V5] x86/altp2m: Add a subop for obtaining the mem access of a page
On 9/26/18 4:39 PM, Jan Beulich wrote:
On 26.09.18 at 15:27, wrote:
>> On 9/26/18 4:20 PM, Jan Beulich wrote:
>> On 26.09.18 at 14:26, wrote:
To clarify the question, I'll of course do this:
diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c
index 67b4a1d..2b5a621 100644
--- a/xen/arch/x86/mm/mem_access.c
+++ b/xen/arch/x86/mm/mem_access.c
@@ -489,14 +489,13 @@ long p2m_set_mem_access_multi(struct domain *d,
int p2m_get_mem_access(struct domain *d, gfn_t gfn, xenmem_access_t
*access,
unsigned int altp2m_idx)
{
-struct p2m_domain *p2m;
+struct p2m_domain *p2m = p2m_get_hostp2m(d);
+#ifdef CONFIG_HVM
if ( !altp2m_active(d) )
{
if ( altp2m_idx )
return -EINVAL;
-
-p2m = p2m_get_hostp2m(d);
}
else
{
@@ -506,6 +505,9 @@ int p2m_get_mem_access(struct domain *d, gfn_t gfn,
xenmem_access_t *access,
p2m = d->arch.altp2m_p2m[altp2m_idx];
}
+#else
+ASSERT(!altp2m_idx);
+#endif
return _p2m_get_mem_access(p2m, gfn, access);
}
but is it OK that the hypervisor builds with a set of flags that
includes CONFIG_HVM and the firmware code with a set that doesn't?
>>>
>>> Is this perhaps simply (so far unnoticed) fallout from Wei's CONFIG_HVM-
>>> disabling work? Or insufficient re-basing of your change on top of his
>>> work? The shim now builds with HVM=n, while the hypervisor (unless
>>> you've overridden the default) uses HVM=y.
>>
>> I believe I'm up-to-date:
>>
>> $ git pull --rebase origin staging
>> From git://xenbits.xenproject.org/xen
>> * branchstaging-> FETCH_HEAD
>> Current branch altp2m-work is up to date.
>>
>> I've also ran "make clean", "make distclean", "configure" - again, and
>> "make dist" one more time, with the same results (mem_access.c won't
>> compile in the shim).
>
> I didn't imply you're on an outdated tree, but rather that you may not
> have done all changes necessary while re-basing your change over
> upstream commits.
Other than the above #ifdef-ery, I don't think I've missed anything
else, no. I've also now done an full introspection test with the patch
and everything seems to behave the way it's supposed to.
Thanks,
Razvan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3] x86: use VMLOAD for PV context switch
>>> On 26.09.18 at 14:47, wrote:
> On 26/09/18 07:33, Jan Beulich wrote:
> On 25.09.18 at 18:14, wrote:
>>> On 18/09/18 13:28, Jan Beulich wrote:
@@ -1281,11 +1282,35 @@ static void load_segments(struct vcpu *n
struct cpu_user_regs *uregs = >arch.user_regs;
int all_segs_okay = 1;
unsigned int dirty_segment_mask, cpu = smp_processor_id();
+bool fs_gs_done = false;
/* Load and clear the dirty segment mask. */
dirty_segment_mask = per_cpu(dirty_segment_mask, cpu);
per_cpu(dirty_segment_mask, cpu) = 0;
+#ifdef CONFIG_HVM
+if ( !is_pv_32bit_vcpu(n) && !cpu_has_fsgsbase && cpu_has_svm &&
+ !((uregs->fs | uregs->gs) & ~3) &&
+ /*
+ * The remaining part is just for optimization: If only shadow GS
+ * needs loading, there's nothing to be gained here.
>>> VMLOAD also loads LDT, and LLDT is fully serialising, so an even heavier
>>> perf hit than wrmsr.
>> I don't understand how your remark relates to the comment
>
> Because the comment is false in the case that the LDT also needs loading.
True (and the comment is a result of me having written it before paying
attention to the fact that the LDT can also be loaded this way); I'll OR
n->arch.pv.ldt_ents into that extra (optimization) condition, which
I think will then render the comment correct again.
+
+if ( !ldt_base )
+{
+/*
+ * The actual structure field used here was arbitrarily chosen.
+ * Empirically it doesn't seem to matter much which element is
used,
+ * and a clear explanation of the otherwise poor performance has
not
+ * been found/provided so far.
+ */
+asm volatile ( "prefetch %0" :: "m" (vmcb->ldtr) );
>>> prefetchw(), which already exists and is used.
>> I've specifically decided against using it, as we don't mean to write any
>> part of the VMCB.
>
> I think you need to double check your reasoning here. There is a reason
> why this function wont compile if you made vmcb a const pointer.
Oh, right you are. It's been way too long since I wrote this code,
and hence I should have looked back at it before replying rather
than just going from the function's name.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2] amd-iommu: use correct constants in amd_iommu_get_next_table_from_pte()...
...and change the name to amd_iommu_get_address_from_pte() since the
address read is not necessarily the address of a next level page table.
(If the 'next level' field is not 1 - 6 then the address is a page
address).
The constants in use prior to this patch relate to device table entries
rather than page table entries. Although they do have the same value, it
makes the code confusing to read.
This patch also changes the PDE/PTE pointer argument to void *, and
removes any u32/uint32_t casts in the call sites. Unnecessary casts
surrounding call sites are also removed.
No functional change.
NOTE: The patch also adds emacs boilerplate to iommu_map.c
Signed-off-by: Paul Durrant
---
Cc: Suravee Suthikulpanit
Cc: Brian Woods
---
xen/drivers/passthrough/amd/iommu_map.c | 40 ---
xen/drivers/passthrough/amd/pci_amd_iommu.c | 10 +++
xen/include/asm-x86/hvm/svm/amd-iommu-proto.h | 2 +-
3 files changed, 30 insertions(+), 22 deletions(-)
diff --git a/xen/drivers/passthrough/amd/iommu_map.c
b/xen/drivers/passthrough/amd/iommu_map.c
index 70b4345b37..9fa5cd3bd3 100644
--- a/xen/drivers/passthrough/amd/iommu_map.c
+++ b/xen/drivers/passthrough/amd/iommu_map.c
@@ -285,19 +285,18 @@ void iommu_dte_set_guest_cr3(u32 *dte, u16 dom_id, u64
gcr3,
dte[1] = entry;
}
-u64 amd_iommu_get_next_table_from_pte(u32 *entry)
+uint64_t amd_iommu_get_address_from_pte(void *pte)
{
-u64 addr_lo, addr_hi, ptr;
+uint32_t *entry = pte;
+uint64_t addr_lo, addr_hi, ptr;
-addr_lo = get_field_from_reg_u32(
-entry[0],
-IOMMU_DEV_TABLE_PAGE_TABLE_PTR_LOW_MASK,
-IOMMU_DEV_TABLE_PAGE_TABLE_PTR_LOW_SHIFT);
+addr_lo = get_field_from_reg_u32(entry[0],
+ IOMMU_PTE_ADDR_LOW_MASK,
+ IOMMU_PTE_ADDR_LOW_SHIFT);
-addr_hi = get_field_from_reg_u32(
-entry[1],
-IOMMU_DEV_TABLE_PAGE_TABLE_PTR_HIGH_MASK,
-IOMMU_DEV_TABLE_PAGE_TABLE_PTR_HIGH_SHIFT);
+addr_hi = get_field_from_reg_u32(entry[1],
+ IOMMU_PTE_ADDR_HIGH_MASK,
+ IOMMU_PTE_ADDR_HIGH_SHIFT);
ptr = (addr_hi << 32) | (addr_lo << PAGE_SHIFT);
return ptr;
@@ -350,11 +349,11 @@ static int iommu_update_pde_count(struct domain *d,
unsigned long pt_mfn,
pde = table + pfn_to_pde_idx(gfn, merge_level);
/* get page table of next level */
-ntable_maddr = amd_iommu_get_next_table_from_pte((u32*)pde);
+ntable_maddr = amd_iommu_get_address_from_pte(pde);
ntable = map_domain_page(_mfn(paddr_to_pfn(ntable_maddr)));
/* get the first mfn of next level */
-first_mfn = amd_iommu_get_next_table_from_pte((u32*)ntable) >> PAGE_SHIFT;
+first_mfn = amd_iommu_get_address_from_pte(ntable) >> PAGE_SHIFT;
if ( first_mfn == 0 )
goto out;
@@ -401,7 +400,7 @@ static int iommu_merge_pages(struct domain *d, unsigned
long pt_mfn,
pde = table + pfn_to_pde_idx(gfn, merge_level);
/* get first mfn */
-ntable_mfn = amd_iommu_get_next_table_from_pte((u32*)pde) >> PAGE_SHIFT;
+ntable_mfn = amd_iommu_get_address_from_pte(pde) >> PAGE_SHIFT;
if ( ntable_mfn == 0 )
{
@@ -410,7 +409,7 @@ static int iommu_merge_pages(struct domain *d, unsigned
long pt_mfn,
}
ntable = map_domain_page(_mfn(ntable_mfn));
-first_mfn = amd_iommu_get_next_table_from_pte((u32*)ntable) >> PAGE_SHIFT;
+first_mfn = amd_iommu_get_address_from_pte(ntable) >> PAGE_SHIFT;
if ( first_mfn == 0 )
{
@@ -468,8 +467,7 @@ static int iommu_pde_from_gfn(struct domain *d, unsigned
long pfn,
pde = next_table_vaddr + pfn_to_pde_idx(pfn, level);
/* Here might be a super page frame */
-next_table_mfn = amd_iommu_get_next_table_from_pte((uint32_t*)pde)
- >> PAGE_SHIFT;
+next_table_mfn = amd_iommu_get_address_from_pte(pde) >> PAGE_SHIFT;
/* Split super page frame into smaller pieces.*/
if ( iommu_is_pte_present((u32*)pde) &&
@@ -815,3 +813,13 @@ void amd_iommu_share_p2m(struct domain *d)
mfn_x(pgd_mfn));
}
}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c
b/xen/drivers/passthrough/amd/pci_amd_iommu.c
index 4a633ca940..80d9ae6561 100644
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -430,11 +430,11 @@ static void deallocate_page_table(struct page_info *pg)
for ( index = 0; index < PTE_PER_TABLE_SIZE; index++ )
{
pde = table_vaddr + (index * IOMMU_PAGE_TABLE_ENTRY_SIZE);
-next_table_maddr = amd_iommu_get_next_table_from_pte(pde);
-next_level = iommu_next_level((u32*)pde);
+next_table_maddr = amd_iommu_get_address_from_pte(pde);
+
[Xen-devel] [qemu-mainline baseline-only test] 75293: trouble: blocked/broken
This run is configured for baseline tests only. flight 75293 qemu-mainline real [real] http://osstest.xensource.com/osstest/logs/75293/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64 broken build-i386 broken build-armhf-pvopsbroken build-i386-xsm broken build-amd64-xsm broken build-amd64-pvopsbroken build-i386-pvops broken build-armhf broken Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-debianhvm-amd64 1 build-check(1)blocked n/a test-amd64-i386-freebsd10-i386 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-nested-intel 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-shadow1 build-check(1) blocked n/a test-armhf-armhf-xl-midway1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win10-i386 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-multivcpu 1 build-check(1) blocked n/a test-amd64-i386-xl-pvshim 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-amd64 1 build-check(1) blocked n/a test-amd64-amd64-pair 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit2 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-win10-i386 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-pygrub 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ws16-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qcow2 1 build-check(1) blocked n/a test-amd64-amd64-amd64-pvgrub 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-i386-xl1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-amd64-xl-credit2 1 build-check(1) blocked n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) blocked n/a test-amd64-i386-xl-xsm1 build-check(1) blocked n/a build-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvshim1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 1 build-check(1)blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-raw1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-amd64-i386-pvgrub 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 1 build-check(1) blocked n/a build-armhf-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ws16-amd64 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvhv2-intel 1 build-check(1) blocked n/a test-armhf-armhf-xl 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-shadow 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-intel 1 build-check(1) blocked n/a test-armhf-armhf-xl-vhd 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 1
Re: [Xen-devel] [PATCH V5] x86/altp2m: Add a subop for obtaining the mem access of a page
>>> On 26.09.18 at 15:27, wrote:
> On 9/26/18 4:20 PM, Jan Beulich wrote:
> On 26.09.18 at 14:26, wrote:
>>> To clarify the question, I'll of course do this:
>>>
>>> diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c
>>> index 67b4a1d..2b5a621 100644
>>> --- a/xen/arch/x86/mm/mem_access.c
>>> +++ b/xen/arch/x86/mm/mem_access.c
>>> @@ -489,14 +489,13 @@ long p2m_set_mem_access_multi(struct domain *d,
>>> int p2m_get_mem_access(struct domain *d, gfn_t gfn, xenmem_access_t
>>> *access,
>>> unsigned int altp2m_idx)
>>> {
>>> -struct p2m_domain *p2m;
>>> +struct p2m_domain *p2m = p2m_get_hostp2m(d);
>>>
>>> +#ifdef CONFIG_HVM
>>> if ( !altp2m_active(d) )
>>> {
>>> if ( altp2m_idx )
>>> return -EINVAL;
>>> -
>>> -p2m = p2m_get_hostp2m(d);
>>> }
>>> else
>>> {
>>> @@ -506,6 +505,9 @@ int p2m_get_mem_access(struct domain *d, gfn_t gfn,
>>> xenmem_access_t *access,
>>>
>>> p2m = d->arch.altp2m_p2m[altp2m_idx];
>>> }
>>> +#else
>>> +ASSERT(!altp2m_idx);
>>> +#endif
>>>
>>> return _p2m_get_mem_access(p2m, gfn, access);
>>> }
>>>
>>> but is it OK that the hypervisor builds with a set of flags that
>>> includes CONFIG_HVM and the firmware code with a set that doesn't?
>>
>> Is this perhaps simply (so far unnoticed) fallout from Wei's CONFIG_HVM-
>> disabling work? Or insufficient re-basing of your change on top of his
>> work? The shim now builds with HVM=n, while the hypervisor (unless
>> you've overridden the default) uses HVM=y.
>
> I believe I'm up-to-date:
>
> $ git pull --rebase origin staging
> From git://xenbits.xenproject.org/xen
> * branchstaging-> FETCH_HEAD
> Current branch altp2m-work is up to date.
>
> I've also ran "make clean", "make distclean", "configure" - again, and
> "make dist" one more time, with the same results (mem_access.c won't
> compile in the shim).
I didn't imply you're on an outdated tree, but rather that you may not
have done all changes necessary while re-basing your change over
upstream commits.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] amd-iommu: use correct constants in amd_iommu_get_next_table_from_pte()...
> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: 26 September 2018 14:32
> To: Paul Durrant
> Cc: Brian Woods ; Suravee Suthikulpanit
> ; xen-devel de...@lists.xenproject.org>
> Subject: Re: [Xen-devel] [PATCH] amd-iommu: use correct constants in
> amd_iommu_get_next_table_from_pte()...
>
> >>> On 26.09.18 at 14:56, wrote:
> > ...and change the name to amd_iommu_get_address_from_pte() since the
> > address read is not necessarily a 'next level' page table.
>
> There was no "level" in the original name. Which isn't meant to be an
> objection to the rename.
Ok. I'll re-word it a bit.
>
> > --- a/xen/drivers/passthrough/amd/iommu_map.c
> > +++ b/xen/drivers/passthrough/amd/iommu_map.c
> > @@ -285,19 +285,18 @@ void iommu_dte_set_guest_cr3(u32 *dte, u16 dom_id,
> u64 gcr3,
> > dte[1] = entry;
> > }
> >
> > -u64 amd_iommu_get_next_table_from_pte(u32 *entry)
> > +uint64_t amd_iommu_get_address_from_pte(void *pte)
> > {
> > -u64 addr_lo, addr_hi, ptr;
> > +uint32_t *entry = pte;
> > +uint64_t addr_lo, addr_hi, ptr;
> >
> > -addr_lo = get_field_from_reg_u32(
> > -entry[0],
> > -IOMMU_DEV_TABLE_PAGE_TABLE_PTR_LOW_MASK,
> > -IOMMU_DEV_TABLE_PAGE_TABLE_PTR_LOW_SHIFT);
> > +addr_lo = get_field_from_reg_u32(entry[0],
> > + IOMMU_PDE_ADDR_LOW_MASK,
> > + IOMMU_PDE_ADDR_LOW_SHIFT);
> >
> > -addr_hi = get_field_from_reg_u32(
> > -entry[1],
> > -IOMMU_DEV_TABLE_PAGE_TABLE_PTR_HIGH_MASK,
> > -IOMMU_DEV_TABLE_PAGE_TABLE_PTR_HIGH_SHIFT);
> > +addr_hi = get_field_from_reg_u32(entry[1],
> > + IOMMU_PDE_ADDR_HIGH_MASK,
> > + IOMMU_PDE_ADDR_HIGH_SHIFT);
>
> With the function parameter named pte, perhaps better to also use
> IOMMU_PTE_ADDR_* (which is also more generic imo, as it covers
> both leaf and non-leaf entries).
>
Yes, agreed.
Paul
> Jan
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] amd-iommu: use correct constants in amd_iommu_get_next_table_from_pte()...
>>> On 26.09.18 at 14:56, wrote:
> ...and change the name to amd_iommu_get_address_from_pte() since the
> address read is not necessarily a 'next level' page table.
There was no "level" in the original name. Which isn't meant to be an
objection to the rename.
> --- a/xen/drivers/passthrough/amd/iommu_map.c
> +++ b/xen/drivers/passthrough/amd/iommu_map.c
> @@ -285,19 +285,18 @@ void iommu_dte_set_guest_cr3(u32 *dte, u16 dom_id, u64
> gcr3,
> dte[1] = entry;
> }
>
> -u64 amd_iommu_get_next_table_from_pte(u32 *entry)
> +uint64_t amd_iommu_get_address_from_pte(void *pte)
> {
> -u64 addr_lo, addr_hi, ptr;
> +uint32_t *entry = pte;
> +uint64_t addr_lo, addr_hi, ptr;
>
> -addr_lo = get_field_from_reg_u32(
> -entry[0],
> -IOMMU_DEV_TABLE_PAGE_TABLE_PTR_LOW_MASK,
> -IOMMU_DEV_TABLE_PAGE_TABLE_PTR_LOW_SHIFT);
> +addr_lo = get_field_from_reg_u32(entry[0],
> + IOMMU_PDE_ADDR_LOW_MASK,
> + IOMMU_PDE_ADDR_LOW_SHIFT);
>
> -addr_hi = get_field_from_reg_u32(
> -entry[1],
> -IOMMU_DEV_TABLE_PAGE_TABLE_PTR_HIGH_MASK,
> -IOMMU_DEV_TABLE_PAGE_TABLE_PTR_HIGH_SHIFT);
> +addr_hi = get_field_from_reg_u32(entry[1],
> + IOMMU_PDE_ADDR_HIGH_MASK,
> + IOMMU_PDE_ADDR_HIGH_SHIFT);
With the function parameter named pte, perhaps better to also use
IOMMU_PTE_ADDR_* (which is also more generic imo, as it covers
both leaf and non-leaf entries).
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH V5] x86/altp2m: Add a subop for obtaining the mem access of a page
On 9/26/18 4:20 PM, Jan Beulich wrote:
On 26.09.18 at 14:26, wrote:
>> To clarify the question, I'll of course do this:
>>
>> diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c
>> index 67b4a1d..2b5a621 100644
>> --- a/xen/arch/x86/mm/mem_access.c
>> +++ b/xen/arch/x86/mm/mem_access.c
>> @@ -489,14 +489,13 @@ long p2m_set_mem_access_multi(struct domain *d,
>> int p2m_get_mem_access(struct domain *d, gfn_t gfn, xenmem_access_t
>> *access,
>> unsigned int altp2m_idx)
>> {
>> -struct p2m_domain *p2m;
>> +struct p2m_domain *p2m = p2m_get_hostp2m(d);
>>
>> +#ifdef CONFIG_HVM
>> if ( !altp2m_active(d) )
>> {
>> if ( altp2m_idx )
>> return -EINVAL;
>> -
>> -p2m = p2m_get_hostp2m(d);
>> }
>> else
>> {
>> @@ -506,6 +505,9 @@ int p2m_get_mem_access(struct domain *d, gfn_t gfn,
>> xenmem_access_t *access,
>>
>> p2m = d->arch.altp2m_p2m[altp2m_idx];
>> }
>> +#else
>> +ASSERT(!altp2m_idx);
>> +#endif
>>
>> return _p2m_get_mem_access(p2m, gfn, access);
>> }
>>
>> but is it OK that the hypervisor builds with a set of flags that
>> includes CONFIG_HVM and the firmware code with a set that doesn't?
>
> Is this perhaps simply (so far unnoticed) fallout from Wei's CONFIG_HVM-
> disabling work? Or insufficient re-basing of your change on top of his
> work? The shim now builds with HVM=n, while the hypervisor (unless
> you've overridden the default) uses HVM=y.
I believe I'm up-to-date:
$ git pull --rebase origin staging
From git://xenbits.xenproject.org/xen
* branchstaging-> FETCH_HEAD
Current branch altp2m-work is up to date.
I've also ran "make clean", "make distclean", "configure" - again, and
"make dist" one more time, with the same results (mem_access.c won't
compile in the shim).
In any case, I'll resend a corrected (as above) version tomorrow, to
hopefully get more comments on the current version in the meantime.
Thanks,
Razvan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH V5] x86/altp2m: Add a subop for obtaining the mem access of a page
>>> On 26.09.18 at 14:26, wrote:
> To clarify the question, I'll of course do this:
>
> diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c
> index 67b4a1d..2b5a621 100644
> --- a/xen/arch/x86/mm/mem_access.c
> +++ b/xen/arch/x86/mm/mem_access.c
> @@ -489,14 +489,13 @@ long p2m_set_mem_access_multi(struct domain *d,
> int p2m_get_mem_access(struct domain *d, gfn_t gfn, xenmem_access_t
> *access,
> unsigned int altp2m_idx)
> {
> -struct p2m_domain *p2m;
> +struct p2m_domain *p2m = p2m_get_hostp2m(d);
>
> +#ifdef CONFIG_HVM
> if ( !altp2m_active(d) )
> {
> if ( altp2m_idx )
> return -EINVAL;
> -
> -p2m = p2m_get_hostp2m(d);
> }
> else
> {
> @@ -506,6 +505,9 @@ int p2m_get_mem_access(struct domain *d, gfn_t gfn,
> xenmem_access_t *access,
>
> p2m = d->arch.altp2m_p2m[altp2m_idx];
> }
> +#else
> +ASSERT(!altp2m_idx);
> +#endif
>
> return _p2m_get_mem_access(p2m, gfn, access);
> }
>
> but is it OK that the hypervisor builds with a set of flags that
> includes CONFIG_HVM and the firmware code with a set that doesn't?
Is this perhaps simply (so far unnoticed) fallout from Wei's CONFIG_HVM-
disabling work? Or insufficient re-basing of your change on top of his
work? The shim now builds with HVM=n, while the hypervisor (unless
you've overridden the default) uses HVM=y.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4] x86/mm: Add mem access rights to NPT
On 09/26/2018 09:17 AM, Isaila Alexandru wrote:
> On Wed, 2018-07-25 at 04:37 -0600, Jan Beulich wrote:
> On 25.07.18 at 11:25, wrote:
>>>
>>> On 07/24/2018 01:02 PM, Jan Beulich wrote:
>>> On 24.07.18 at 13:26, wrote:
>
> On 07/24/2018 09:55 AM, Jan Beulich wrote:
> On 23.07.18 at 15:48, wrote:
>>>
>>> +{
>>> +xfree(d->arch.monitor.msr_bitmap);
>>> +return -ENOMEM;
>>> +}
>>> +radix_tree_init(p2m->mem_access_settings);
>>> +}
>>
>> What's the SVM connection here? Please don't forget that p2m-
>> pt.c
>> also serves the shadow case. Perhaps struct p2m_domain should
>> contain a boolean indicator whether this auxiliary data
>> structure is
>> needed?
>
> It's basically just "hap_enabled()" isn't it?
Only if we can't make it there when EPT is active.
>>>
>>> It can make it here when VMX is active and shadow is enabled, but
>>> it
>>> shouldn't be able to get here when EPT is active. We could add an
>>> ASSERT() to that effect; it should be safe in production, as the
>>> only
>>> side effect should be that we do a small pointless allocation.
>>
>> So I've looked a little more closely: This is being added to
>> arch_monitor_init_domain(), called from vm_event_domctl(). I can't
>> see why this wouldn't be reachable with EPT enabled.
>>
> Hi George,
>
> Any input on this?
Sorry, you're still waiting for me to weigh in on whether you can get to
arch_monitor_init_domain() with EPT enabled?
The obvious answer is 'yes'; I hope you don't need me to tell you that. :-)
Going back through this conversation, I'm not 100% clear what I meant
with my "hap_enabled()" comment -- my best guess is that I was
suggesting the check be `( cpu_has_svm || !hap_enabled() )`.
But in any case, on the whole patch, I think that:
1) The feature is not abstracted well enough. mem_access.c should not
have to know whether additional storage is required or not; that should
be all taken care of within p2m-pt.c (or p2m-ept.c, should that ever
become necessary).
2) We've been talking for a long time about having a place to store
additional per-pfn information; it would be good if this mechanism were
made general enough to be used for other types of storage.
Let me play around with what you have and see if I can get a mock-up of
something like what I'm looking for.
-George
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] amd-iommu: use correct constants in amd_iommu_get_next_table_from_pte()...
...and change the name to amd_iommu_get_address_from_pte() since the
address read is not necessarily a 'next level' page table.
The constants in use prior to this patch relate to device table entries
rather than page table entries. Although they do have the same value, it
makes the code confusing to read.
This patch also changes the PDE/PTE pointer argument to void *, and
removes any u32/uint32_t casts in the call sites. Unnecessary casts
surrounding call sites are also removed.
No functional change.
NOTE: The patch also adds emacs boilerplate to iommu_map.c
Signed-off-by: Paul Durrant
---
Cc: Suravee Suthikulpanit
Cc: Brian Woods
---
xen/drivers/passthrough/amd/iommu_map.c | 40 ---
xen/drivers/passthrough/amd/pci_amd_iommu.c | 10 +++
xen/include/asm-x86/hvm/svm/amd-iommu-proto.h | 2 +-
3 files changed, 30 insertions(+), 22 deletions(-)
diff --git a/xen/drivers/passthrough/amd/iommu_map.c
b/xen/drivers/passthrough/amd/iommu_map.c
index 70b4345b37..41247d8e9f 100644
--- a/xen/drivers/passthrough/amd/iommu_map.c
+++ b/xen/drivers/passthrough/amd/iommu_map.c
@@ -285,19 +285,18 @@ void iommu_dte_set_guest_cr3(u32 *dte, u16 dom_id, u64
gcr3,
dte[1] = entry;
}
-u64 amd_iommu_get_next_table_from_pte(u32 *entry)
+uint64_t amd_iommu_get_address_from_pte(void *pte)
{
-u64 addr_lo, addr_hi, ptr;
+uint32_t *entry = pte;
+uint64_t addr_lo, addr_hi, ptr;
-addr_lo = get_field_from_reg_u32(
-entry[0],
-IOMMU_DEV_TABLE_PAGE_TABLE_PTR_LOW_MASK,
-IOMMU_DEV_TABLE_PAGE_TABLE_PTR_LOW_SHIFT);
+addr_lo = get_field_from_reg_u32(entry[0],
+ IOMMU_PDE_ADDR_LOW_MASK,
+ IOMMU_PDE_ADDR_LOW_SHIFT);
-addr_hi = get_field_from_reg_u32(
-entry[1],
-IOMMU_DEV_TABLE_PAGE_TABLE_PTR_HIGH_MASK,
-IOMMU_DEV_TABLE_PAGE_TABLE_PTR_HIGH_SHIFT);
+addr_hi = get_field_from_reg_u32(entry[1],
+ IOMMU_PDE_ADDR_HIGH_MASK,
+ IOMMU_PDE_ADDR_HIGH_SHIFT);
ptr = (addr_hi << 32) | (addr_lo << PAGE_SHIFT);
return ptr;
@@ -350,11 +349,11 @@ static int iommu_update_pde_count(struct domain *d,
unsigned long pt_mfn,
pde = table + pfn_to_pde_idx(gfn, merge_level);
/* get page table of next level */
-ntable_maddr = amd_iommu_get_next_table_from_pte((u32*)pde);
+ntable_maddr = amd_iommu_get_address_from_pte(pde);
ntable = map_domain_page(_mfn(paddr_to_pfn(ntable_maddr)));
/* get the first mfn of next level */
-first_mfn = amd_iommu_get_next_table_from_pte((u32*)ntable) >> PAGE_SHIFT;
+first_mfn = amd_iommu_get_address_from_pte(ntable) >> PAGE_SHIFT;
if ( first_mfn == 0 )
goto out;
@@ -401,7 +400,7 @@ static int iommu_merge_pages(struct domain *d, unsigned
long pt_mfn,
pde = table + pfn_to_pde_idx(gfn, merge_level);
/* get first mfn */
-ntable_mfn = amd_iommu_get_next_table_from_pte((u32*)pde) >> PAGE_SHIFT;
+ntable_mfn = amd_iommu_get_address_from_pte(pde) >> PAGE_SHIFT;
if ( ntable_mfn == 0 )
{
@@ -410,7 +409,7 @@ static int iommu_merge_pages(struct domain *d, unsigned
long pt_mfn,
}
ntable = map_domain_page(_mfn(ntable_mfn));
-first_mfn = amd_iommu_get_next_table_from_pte((u32*)ntable) >> PAGE_SHIFT;
+first_mfn = amd_iommu_get_address_from_pte(ntable) >> PAGE_SHIFT;
if ( first_mfn == 0 )
{
@@ -468,8 +467,7 @@ static int iommu_pde_from_gfn(struct domain *d, unsigned
long pfn,
pde = next_table_vaddr + pfn_to_pde_idx(pfn, level);
/* Here might be a super page frame */
-next_table_mfn = amd_iommu_get_next_table_from_pte((uint32_t*)pde)
- >> PAGE_SHIFT;
+next_table_mfn = amd_iommu_get_address_from_pte(pde) >> PAGE_SHIFT;
/* Split super page frame into smaller pieces.*/
if ( iommu_is_pte_present((u32*)pde) &&
@@ -815,3 +813,13 @@ void amd_iommu_share_p2m(struct domain *d)
mfn_x(pgd_mfn));
}
}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c
b/xen/drivers/passthrough/amd/pci_amd_iommu.c
index 4a633ca940..80d9ae6561 100644
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -430,11 +430,11 @@ static void deallocate_page_table(struct page_info *pg)
for ( index = 0; index < PTE_PER_TABLE_SIZE; index++ )
{
pde = table_vaddr + (index * IOMMU_PAGE_TABLE_ENTRY_SIZE);
-next_table_maddr = amd_iommu_get_next_table_from_pte(pde);
-next_level = iommu_next_level((u32*)pde);
+next_table_maddr = amd_iommu_get_address_from_pte(pde);
+next_level = iommu_next_level(pde);
if ( (next_table_maddr != 0) &&
Re: [Xen-devel] [PATCH v3] x86: use VMLOAD for PV context switch
On 26/09/18 07:33, Jan Beulich wrote:
On 25.09.18 at 18:14, wrote:
>> On 18/09/18 13:28, Jan Beulich wrote:
>>> @@ -1281,11 +1282,35 @@ static void load_segments(struct vcpu *n
>>> struct cpu_user_regs *uregs = >arch.user_regs;
>>> int all_segs_okay = 1;
>>> unsigned int dirty_segment_mask, cpu = smp_processor_id();
>>> +bool fs_gs_done = false;
>>>
>>> /* Load and clear the dirty segment mask. */
>>> dirty_segment_mask = per_cpu(dirty_segment_mask, cpu);
>>> per_cpu(dirty_segment_mask, cpu) = 0;
>>>
>>> +#ifdef CONFIG_HVM
>>> +if ( !is_pv_32bit_vcpu(n) && !cpu_has_fsgsbase && cpu_has_svm &&
>>> + !((uregs->fs | uregs->gs) & ~3) &&
>>> + /*
>>> + * The remaining part is just for optimization: If only shadow GS
>>> + * needs loading, there's nothing to be gained here.
>> VMLOAD also loads LDT, and LLDT is fully serialising, so an even heavier
>> perf hit than wrmsr.
> I don't understand how your remark relates to the comment
Because the comment is false in the case that the LDT also needs loading.
> , or ...
>
>>> + */
>>> + (n->arch.pv.fs_base | n->arch.pv.gs_base_user) )
> ... the commented code. There's nothing LDT-ish here. Or are you
> meaning to suggest something LDT-ish should be added? I'd rather not,
> as the common case (afaict) is for there to be no LDT.
>
> I also don't understand the "even heavier" part - WRMSR (for the MSRs
> in question) is as serializing as is LLDT, isn't it?
No - I'd mixed up which MSRs weren't serialising.
>>> +
>>> +if ( !ldt_base )
>>> +{
>>> +/*
>>> + * The actual structure field used here was arbitrarily chosen.
>>> + * Empirically it doesn't seem to matter much which element is
>>> used,
>>> + * and a clear explanation of the otherwise poor performance has
>>> not
>>> + * been found/provided so far.
>>> + */
>>> +asm volatile ( "prefetch %0" :: "m" (vmcb->ldtr) );
>> prefetchw(), which already exists and is used.
> I've specifically decided against using it, as we don't mean to write any
> part of the VMCB.
I think you need to double check your reasoning here. There is a reason
why this function wont compile if you made vmcb a const pointer.
Irrespective of the writeable aspect, we also have prefetch() which
should be used in preference to inline assembly.
~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-4.9-testing test] 128054: regressions - FAIL
flight 128054 xen-4.9-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/128054/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-arm64-arm64-libvirt-xsm 7 xen-boot fail REGR. vs. 127753 build-i386-pvops 6 kernel-build fail REGR. vs. 127753 Regressions which are regarded as allowable (not blocking): test-amd64-amd64-rumprun-amd64 17 rumprun-demo-xenstorels/xenstorels.repeat fail REGR. vs. 127753 Tests which did not succeed, but are not blocking: test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-raw1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-i386-xl1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-pair 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-win7-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ws16-amd64 1 build-check(1) blocked n/a test-amd64-i386-qemut-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-ws16-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-xsm1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-debianhvm-amd64 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-livepatch 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-i386 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-shadow 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-win10-i386 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-intel 1 build-check(1) blocked n/a test-amd64-i386-qemut-rhel6hvm-intel 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win10-i386 1 build-check(1) blocked n/a test-amd64-i386-migrupgrade 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-rumprun-i386 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ws16-amd64 14 guest-localmigratefail like 127692 test-amd64-amd64-xl-qemut-ws16-amd64 16 guest-localmigrate/x10 fail like 127692 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-localmigrate/x10 fail like 127753 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail like 127753 test-amd64-amd64-xl-rtds 10 debian-install fail like 127753 test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-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-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 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-multivcpu 13 migrate-support-check
Re: [Xen-devel] [PATCH v4 7/8] libxc/pvh: set default MTRR type to write-back
On 08/06/18 10:59, Roger Pau Monne wrote:
> @@ -1014,6 +1034,30 @@ static int vcpu_hvm(struct xc_dom_image *dom)
> if ( dom->start_info_seg.pfn )
> bsp_ctx.cpu.rbx = dom->start_info_seg.pfn << PAGE_SHIFT;
>
> +/* Set the MTRR. */
> +bsp_ctx.mtrr_d.typecode = HVM_SAVE_CODE(MTRR);
> +bsp_ctx.mtrr_d.instance = 0;
> +bsp_ctx.mtrr_d.length = HVM_SAVE_LENGTH(MTRR);
> +
> +mtrr_record = hvm_get_save_record(full_ctx, HVM_SAVE_CODE(MTRR), 0);
> +if ( !mtrr_record )
> +{
> +xc_dom_panic(dom->xch, XC_INTERNAL_ERROR,
> + "%s: unable to get MTRR save record", __func__);
> +goto out;
> +}
> +
> +memcpy(_ctx.mtrr, mtrr_record, sizeof(bsp_ctx.mtrr));
> +
> +/* TODO: maybe this should be a firmware option instead? */
> +if ( !dom->device_model )
> +/*
> + * Enable MTRR, set default type to WB.
> + * TODO: add MMIO areas as UC when passthrough is supported.
> + */
> +bsp_ctx.mtrr.msr_mtrr_def_type = MTRR_TYPE_WRBACK |
> + MTRR_DEF_TYPE_ENABLE;
This is buggy. MTRRs are per-vcpu and expected to match. This only
works by chance in the HVM case because all settings are still 0 at this
point.
Currently, booting a multi-vcpu PVH guest (the shim, specifically) yields:
(d6) (XEN) mtrr: your CPUs had inconsistent MTRRdefType settings
(d6) (XEN) mtrr: probably your BIOS does not setup all CPUs.
(d6) (XEN) mtrr: corrected configuration.
(d6) (XEN) MTRR default type: write-back
(d6) (XEN) MTRR fixed ranges disabled:
(d6) (XEN) 0-f uncachable
(d6) (XEN) MTRR variable ranges enabled:
(d6) (XEN) 0 disabled
(d6) (XEN) 1 disabled
(d6) (XEN) 2 disabled
(d6) (XEN) 3 disabled
(d6) (XEN) 4 disabled
(d6) (XEN) 5 disabled
(d6) (XEN) 6 disabled
(d6) (XEN) 7 disabled
~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH V5] x86/altp2m: Add a subop for obtaining the mem access of a page
On 9/26/18 2:39 PM, Razvan Cojocaru wrote:
> On 9/26/18 10:34 AM, Razvan Cojocaru wrote:
>> Currently there is a subop for setting the memaccess of a page, but not
>> for consulting it. The new HVMOP_altp2m_get_mem_access adds this
>> functionality.
>>
>> Both altp2m get/set mem access functions use the struct
>> xen_hvm_altp2m_mem_access which has now dropped the `set' part and has
>> been renamed from xen_hvm_altp2m_set_mem_access.
>>
>> Signed-off-by: Adrian Pop
>> Signed-off-by: Razvan Cojocaru
>
> Sorry, I've just found that this change will break the build of
> tools/firmware/xen-dir/xen-root/xen/arch/x86/mm/mem_access.c, because -
> while it's building just fine when doing so for the hypervisor (i.e.
> "make" in xen/), building the tools somehow doesn't take into account
> CONFIG_HVM (output of "make dist"):
>
> gcc -m64 -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall
> -Wstrict-prototypes -Wdeclaration-after-statement
> -Wno-unused-but-set-variable -Wno-unused-local-typedefs -O2
> -fomit-frame-pointer -nostdinc -fno-builtin -fno-common -Werror
> -Wredundant-decls -Wno-pointer-arith -Wvla -pipe -D__XEN__ -include
> /home/red/work/pristine/xen.git/tools/firmware/xen-dir/xen-root/xen/include/xen/config.h
> '-D__OBJECT_FILE__="mem_access.o"' -Wa,--strip-local-absolute -g -MMD
> -MF ./.mem_access.o.d
> -I/home/red/work/pristine/xen.git/tools/firmware/xen-dir/xen-root/xen/include
> -I/home/red/work/pristine/xen.git/tools/firmware/xen-dir/xen-root/xen/include/asm-x86/mach-generic
> -I/home/red/work/pristine/xen.git/tools/firmware/xen-dir/xen-root/xen/include/asm-x86/mach-default
> -DXEN_IMG_OFFSET=0x20 '-D__OBJECT_LABEL__=arch$x86$mm$mem_access.o'
> -msoft-float -fno-stack-protector -fno-exceptions -Wnested-externs
> -DHAVE_AS_VMX -DHAVE_AS_SSE4_2 -DHAVE_AS_EPT -DHAVE_AS_RDRAND
> -DHAVE_AS_FSGSBASE -DHAVE_AS_RDSEED -U__OBJECT_LABEL__
> -DHAVE_AS_QUOTED_SYM '-D__OBJECT_LABEL__=arch/x86/mm/mem_access.o'
> -DHAVE_AS_INVPCID -DHAVE_AS_NEGATIVE_TRUE -mno-red-zone -fpic
> -fno-asynchronous-unwind-tables -mno-sse -mskip-rax-setup
> -DGCC_HAS_VISIBILITY_ATTRIBUTE -mindirect-branch=thunk-extern
> -mindirect-branch-register -DCONFIG_INDIRECT_THUNK
> -Wa,-I/home/red/work/pristine/xen.git/tools/firmware/xen-dir/xen-root/xen/include
> -c mem_access.c -o mem_access.o
> mem_access.c: In function ‘p2m_get_mem_access’:
> mem_access.c:504:21: error: ‘struct arch_domain’ has no member named
> ‘altp2m_eptp’
> d->arch.altp2m_eptp[altp2m_idx] == mfn_x(INVALID_MFN) )
> ^
> mem_access.c:507:22: error: ‘struct arch_domain’ has no member named
> ‘altp2m_p2m’
> p2m = d->arch.altp2m_p2m[altp2m_idx];
> ^
>
> Wei, thoughts on how this would be best approached?
To clarify the question, I'll of course do this:
diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c
index 67b4a1d..2b5a621 100644
--- a/xen/arch/x86/mm/mem_access.c
+++ b/xen/arch/x86/mm/mem_access.c
@@ -489,14 +489,13 @@ long p2m_set_mem_access_multi(struct domain *d,
int p2m_get_mem_access(struct domain *d, gfn_t gfn, xenmem_access_t
*access,
unsigned int altp2m_idx)
{
-struct p2m_domain *p2m;
+struct p2m_domain *p2m = p2m_get_hostp2m(d);
+#ifdef CONFIG_HVM
if ( !altp2m_active(d) )
{
if ( altp2m_idx )
return -EINVAL;
-
-p2m = p2m_get_hostp2m(d);
}
else
{
@@ -506,6 +505,9 @@ int p2m_get_mem_access(struct domain *d, gfn_t gfn,
xenmem_access_t *access,
p2m = d->arch.altp2m_p2m[altp2m_idx];
}
+#else
+ASSERT(!altp2m_idx);
+#endif
return _p2m_get_mem_access(p2m, gfn, access);
}
but is it OK that the hypervisor builds with a set of flags that
includes CONFIG_HVM and the firmware code with a set that doesn't?
Thanks,
Razvan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86: assert MBI is large enough in pvh-boot.c
>>> On 26.09.18 at 13:00, wrote: > --- a/xen/arch/x86/guest/pvh-boot.c > +++ b/xen/arch/x86/guest/pvh-boot.c > @@ -44,6 +44,13 @@ static void __init convert_pvh_info(void) > > ASSERT(pvh_info->magic == XEN_HVM_START_MAGIC_VALUE); > > +/* > + * Temporary MBI array needs to be at least one element bigger than > + * required. The extra element is used to aid relocation. See > + * arch/x86/setup.c:__start_xen(). > + */ > +ASSERT(ARRAY_SIZE(pvh_mbi_mods) > pvh_info->nr_modules); Are ASSERT()s (also the other one in context) actually the right thing here? I think we'd better panic(): That'll also cover release builds and is imo more appropriate for data coming from the outside. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86: silence false log messages for plain "xpti" / "pv-l1tf"
On 26/09/18 08:40, Jan Beulich wrote:
> While commit 2a3b34ec47 ("x86/spec-ctrl: Yet more fixes for xpti=
> parsing") claimed to have got rid of the 'parameter "xpti" has invalid
> value "", rc=-22!' log message for "xpti" alone on the command line,
> this wasn't the case (the option took effect nevertheless).
>
> Fix this there as well as for plain "pv-l1tf".
>
> Signed-off-by: Jan Beulich
Acked-by: Andrew Cooper
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] IOREQ server on Arm
> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: 26 September 2018 12:57
> To: Paul Durrant
> Cc: Julien Grall ; Andrew Cooper
> ; Roger Pau Monne ;
> Stefano Stabellini ; xen-devel de...@lists.xenproject.org>
> Subject: RE: IOREQ server on Arm
>
> >>> On 26.09.18 at 13:02, wrote:
> > --- a/xen/common/memory.c
> > +++ b/xen/common/memory.c
> > @@ -1105,8 +1105,11 @@ static int acquire_resource(
> >
> > for ( i = 0; !rc && i < xmar.nr_frames; i++ )
> > {
> > -rc = set_foreign_p2m_entry(currd, gfn_list[i],
> > - _mfn(mfn_list[i]));
> > +rc = (xmar.flags & XENMEM_rsrc_acq_caller_owned) ?
> > +guest_physmap_add_entry(currd, gfn_list[i],
> > +_mfn(mfn_list[i]), 0,
> p2m_ram_rw) :
> > +set_foreign_p2m_entry(currd, gfn_list[i],
> > + _mfn(mfn_list[i]));
> > /* rc should be -EIO for any iteration other than the first
> */
> > if ( rc && i )
> > rc = -EIO;
> >
> > But the guest_physmap_add_entry() is problematic as it will IOMMU map
> pages
> > as well, which is probably not wanted.
>
> Yeah, I'd prefer if we avoided establishing IOMMU mappings here.
> How about transforming set_foreign_p2m_entry() into
> set_special_p2m_entry(), with a type passed in?
>
That sounds like it might work.
Julien, do you want page types to distinguish caller-owned resources from
normal RAM are you ok with p2m_ram_rw even though it could be subject of
another domain's foreign map?
Paul
> Jan
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] IOREQ server on Arm
>>> On 26.09.18 at 13:02, wrote:
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -1105,8 +1105,11 @@ static int acquire_resource(
>
> for ( i = 0; !rc && i < xmar.nr_frames; i++ )
> {
> -rc = set_foreign_p2m_entry(currd, gfn_list[i],
> - _mfn(mfn_list[i]));
> +rc = (xmar.flags & XENMEM_rsrc_acq_caller_owned) ?
> +guest_physmap_add_entry(currd, gfn_list[i],
> +_mfn(mfn_list[i]), 0, p2m_ram_rw) :
> +set_foreign_p2m_entry(currd, gfn_list[i],
> + _mfn(mfn_list[i]));
> /* rc should be -EIO for any iteration other than the first */
> if ( rc && i )
> rc = -EIO;
>
> But the guest_physmap_add_entry() is problematic as it will IOMMU map pages
> as well, which is probably not wanted.
Yeah, I'd prefer if we avoided establishing IOMMU mappings here.
How about transforming set_foreign_p2m_entry() into
set_special_p2m_entry(), with a type passed in?
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH V5] x86/altp2m: Add a subop for obtaining the mem access of a page
On 9/26/18 10:34 AM, Razvan Cojocaru wrote: > Currently there is a subop for setting the memaccess of a page, but not > for consulting it. The new HVMOP_altp2m_get_mem_access adds this > functionality. > > Both altp2m get/set mem access functions use the struct > xen_hvm_altp2m_mem_access which has now dropped the `set' part and has > been renamed from xen_hvm_altp2m_set_mem_access. > > Signed-off-by: Adrian Pop > Signed-off-by: Razvan Cojocaru Sorry, I've just found that this change will break the build of tools/firmware/xen-dir/xen-root/xen/arch/x86/mm/mem_access.c, because - while it's building just fine when doing so for the hypervisor (i.e. "make" in xen/), building the tools somehow doesn't take into account CONFIG_HVM (output of "make dist"): gcc -m64 -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-but-set-variable -Wno-unused-local-typedefs -O2 -fomit-frame-pointer -nostdinc -fno-builtin -fno-common -Werror -Wredundant-decls -Wno-pointer-arith -Wvla -pipe -D__XEN__ -include /home/red/work/pristine/xen.git/tools/firmware/xen-dir/xen-root/xen/include/xen/config.h '-D__OBJECT_FILE__="mem_access.o"' -Wa,--strip-local-absolute -g -MMD -MF ./.mem_access.o.d -I/home/red/work/pristine/xen.git/tools/firmware/xen-dir/xen-root/xen/include -I/home/red/work/pristine/xen.git/tools/firmware/xen-dir/xen-root/xen/include/asm-x86/mach-generic -I/home/red/work/pristine/xen.git/tools/firmware/xen-dir/xen-root/xen/include/asm-x86/mach-default -DXEN_IMG_OFFSET=0x20 '-D__OBJECT_LABEL__=arch$x86$mm$mem_access.o' -msoft-float -fno-stack-protector -fno-exceptions -Wnested-externs -DHAVE_AS_VMX -DHAVE_AS_SSE4_2 -DHAVE_AS_EPT -DHAVE_AS_RDRAND -DHAVE_AS_FSGSBASE -DHAVE_AS_RDSEED -U__OBJECT_LABEL__ -DHAVE_AS_QUOTED_SYM '-D__OBJECT_LABEL__=arch/x86/mm/mem_access.o' -DHAVE_AS_INVPCID -DHAVE_AS_NEGATIVE_TRUE -mno-red-zone -fpic -fno-asynchronous-unwind-tables -mno-sse -mskip-rax-setup -DGCC_HAS_VISIBILITY_ATTRIBUTE -mindirect-branch=thunk-extern -mindirect-branch-register -DCONFIG_INDIRECT_THUNK -Wa,-I/home/red/work/pristine/xen.git/tools/firmware/xen-dir/xen-root/xen/include -c mem_access.c -o mem_access.o mem_access.c: In function ‘p2m_get_mem_access’: mem_access.c:504:21: error: ‘struct arch_domain’ has no member named ‘altp2m_eptp’ d->arch.altp2m_eptp[altp2m_idx] == mfn_x(INVALID_MFN) ) ^ mem_access.c:507:22: error: ‘struct arch_domain’ has no member named ‘altp2m_p2m’ p2m = d->arch.altp2m_p2m[altp2m_idx]; ^ Wei, thoughts on how this would be best approached? Thanks, Razvan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 5/6] xen/arm: smccc: Add wrapper to automatically select the calling convention
Hi Julien,
On 25.09.18 20:20, Julien Grall wrote:
Signed-off-by: Julien Grall
Reviewed-by: Volodymyr Babchuk
---
Changes in v2:
- Invert the condition
- Add missing includes
---
xen/arch/arm/psci.c | 4
xen/include/asm-arm/cpufeature.h | 3 ++-
xen/include/asm-arm/smccc.h | 11 +++
3 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
index 3cf5ecf0f3..941eec921b 100644
--- a/xen/arch/arm/psci.c
+++ b/xen/arch/arm/psci.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include
#include
@@ -118,6 +119,9 @@ static void __init psci_init_smccc(void)
smccc_ver = ret;
}
+if ( smccc_ver >= SMCCC_VERSION(1, 1) )
+cpus_set_cap(ARM_SMCCC_1_1);
+
printk(XENLOG_INFO "Using SMC Calling Convention v%u.%u\n",
SMCCC_VERSION_MAJOR(smccc_ver), SMCCC_VERSION_MINOR(smccc_ver));
}
diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
index c6cbc2ec84..2d82264427 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -44,8 +44,9 @@
#define SKIP_CTXT_SWITCH_SERROR_SYNC 6
#define ARM_HARDEN_BRANCH_PREDICTOR 7
#define ARM_SSBD 8
+#define ARM_SMCCC_1_1 9
-#define ARM_NCAPS 9
+#define ARM_NCAPS 10
#ifndef __ASSEMBLY__
diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
index 1ed6cbaa48..126399dd70 100644
--- a/xen/include/asm-arm/smccc.h
+++ b/xen/include/asm-arm/smccc.h
@@ -16,6 +16,9 @@
#ifndef __ASM_ARM_SMCCC_H__
#define __ASM_ARM_SMCCC_H__
+#include
+#include
+
#define SMCCC_VERSION_MAJOR_SHIFT16
#define SMCCC_VERSION_MINOR_MASK \
((1U << SMCCC_VERSION_MAJOR_SHIFT) - 1)
@@ -213,6 +216,7 @@ struct arm_smccc_res {
*/
#ifdef CONFIG_ARM_32
#define arm_smccc_1_0_smc(...) arm_smccc_1_1_smc(__VA_ARGS__)
+#define arm_smccc_smc(...) arm_smccc_1_1_smc(__VA_ARGS__)
#else
void __arm_smccc_1_0_smc(register_t a0, register_t a1, register_t a2,
@@ -254,6 +258,13 @@ void __arm_smccc_1_0_smc(register_t a0, register_t a1,
register_t a2,
#define arm_smccc_1_0_smc(...) \
__arm_smccc_1_0_smc_count(__count_args(__VA_ARGS__), __VA_ARGS__)
+#define arm_smccc_smc(...) \
+do {\
+if ( cpus_have_const_cap(ARM_SMCCC_1_1) ) \
+arm_smccc_1_1_smc(__VA_ARGS__); \
+else\
+arm_smccc_1_0_smc(__VA_ARGS__); \
+} while ( 0 )
#endif /* CONFIG_ARM_64 */
#endif /* __ASSEMBLY__ */
--
Volodymyr Babchuk
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 128100: tolerable all pass - PUSHED
flight 128100 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/128100/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass 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 c759fb5bc303411e70322948a6ced81b6219ad3a baseline version: xen 940185b2f6f343251c2b83bd96e599398cea51ec Last test of basis 127928 2018-09-22 10:00:53 Z4 days Failing since128013 2018-09-24 14:00:44 Z1 days 15 attempts Testing same since 128100 2018-09-26 09:00:41 Z0 days1 attempts People who touched revisions under test: Alexandru Isaila Amit Singh Tomar Andrew Cooper Christopher Clark Dario Faggioli Doug Goldstein George Dunlap Jan Beulich Julien Grall Razvan Cojocaru Roger Pau Monné Tamas K Lengyel Wei Liu jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass 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 Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git 940185b2f6..c759fb5bc3 c759fb5bc303411e70322948a6ced81b6219ad3a -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] IOREQ server on Arm
> -Original Message- > From: Julien Grall [mailto:julien.gr...@arm.com] > Sent: 26 September 2018 12:01 > To: Paul Durrant ; Jan Beulich > > Cc: Andrew Cooper ; Roger Pau Monne > ; Stefano Stabellini ; xen- > devel > Subject: Re: IOREQ server on Arm > > Hi Paul, > > On 09/26/2018 11:51 AM, Paul Durrant wrote: > >> -Original Message- > >> From: Julien Grall [mailto:julien.gr...@arm.com] > >> Sent: 26 September 2018 11:41 > >> To: Jan Beulich ; Paul Durrant > >> > >> Cc: Andrew Cooper ; Roger Pau Monne > >> ; Stefano Stabellini ; > xen- > >> devel > >> Subject: Re: IOREQ server on Arm > >> > >> Hi Jan, > >> > >> On 09/26/2018 09:08 AM, Jan Beulich wrote:_ > >> On 26.09.18 at 00:39, wrote: > Hi Paul, > > I am looking at porting the IOREQ server infrastructure on Arm. I > >> didn't > need much modification to make it run for Arm. Although, the > implementation could be simplified over the x86 implementation. > > I noticed some issue while trying to implement the hypercall > XENMEM_acquire_resource. Per my understanding, all the page mapped > via > that hypercall will use the type p2m_mapping_foreign. > > This will result to trigger the ASSERT(fdom != dom) in > >> get_page_from_gfn > (asm-arm/p2m.h) because the IOREQ page has been allocated to the > emulator domain and mapped to it. AFAICT x86 has the same assert in > p2m_get_page_from_gfn(...). > > IHMO, the ASSERT makes sense because you are only meant to map page > belonging to other domain with that type. > > So I am wondering whether IOREQ server running in PVH Dom0 has been > tested? What would be the best course of action to fix the issue? > >>> > >>> I think the p2m type needs to be chosen based on > >>> XENMEM_rsrc_acq_caller_owned. > >> > >> I am thinking to introduce p2m_mapping_owned. Or do we have a p2m_type > >> that we could re-use? > >> > > > > I think we should be able to just use p2m_ram_rw if it is caller owned. > > I thought about p2m_ram_rw but discarded because of the security > implications. At least on Arm, this type can be used for foreign > mapping, guest_copy helpers. This is not the case for p2m_mapping_foreign. > Not sure. The emulator has to have privilege over the target, so any domain having privilege over the emulator would have privilege over the target, wouldn't it? So I don't think there is any security issue. > Do we want to allow thoses resources to be used in hypercall buffer > and/or mapped by other guest via the foreign API? If not, then we want > to use a different type. > I can't think of a use-case where we would want that, but I'm not sure there is any particular problem allowing it. Same goes for the IOMMU mappings that I mentioned to Jan though... not really desirable but not necessarily a problem (for existing resource types). A new p2m type may well be a better option. Paul > Cheers, > > -- > Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 1/9] x86: infrastructure to allow converting certain indirect calls to direct ones
On Fri, Sep 21, 2018 at 09:26:27AM -0600, Jan Beulich wrote:
> >>> On 21.09.18 at 15:48, wrote:
> > On Fri, Sep 21, 2018 at 05:47:54AM -0600, Jan Beulich wrote:
> >> >>> On 21.09.18 at 12:49, wrote:
> >> > On Tue, Sep 11, 2018 at 07:32:04AM -0600, Jan Beulich wrote:
> >> >> @@ -218,6 +219,13 @@ void init_or_livepatch apply_alternative
> >> >
> >> > I think you need to fix the comment before this if statement. At the
> >> > very least you're now using two ->priv to make decision on patching.
> >>
> >> I've been considering this, but even a very close look didn't turn up
> >> anything I could do to this comment to improve it. Suggestions
> >> welcome.
> >
> > Just remove the sentence about using single ->priv field?
>
> That would go too far. But I'll make it "for some of our patching decisions".
Fair enough.
>
> >> >> @@ -236,20 +244,74 @@ void init_or_livepatch apply_alternative
> >> >> continue;
> >> >> }
> >> >>
> >> >> -base->priv = 1;
> >> >> -
> >> >> memcpy(buf, repl, a->repl_len);
> >> >>
> >> >> /* 0xe8/0xe9 are relative branches; fix the offset. */
> >> >> if ( a->repl_len >= 5 && (*buf & 0xfe) == 0xe8 )
> >> >> -*(int32_t *)(buf + 1) += repl - orig;
> >> >> +{
> >> >> +/*
> >> >> + * Detect the special case of indirect-to-direct branch
> >> >> patching:
> >> >> + * - replacement is a direct CALL/JMP (opcodes 0xE8/0xE9;
> >> >> already
> >> >> + * checked above),
> >> >> + * - replacement's displacement is -5 (pointing back at
> >> >> the very
> >> >> + * insn, which makes no sense in a real replacement
> >> >> insn),
> >> >> + * - original is an indirect CALL/JMP (opcodes 0xFF/2 or
> >> >> 0xFF/4)
> >> >> + * using RIP-relative addressing.
> >> >> + * Some function targets may not be available when we come
> >> >> here
> >> >> + * the first time. Defer patching of those until the
> >> >> post-presmp-
> >> >> + * initcalls re-invocation. If at that point the target
> >> >> pointer is
> >> >> + * still NULL, insert "UD2; UD0" (for ease of recognition)
> >> >> instead
> >> >> + * of CALL/JMP.
> >> >> + */
> >> >> +if ( a->cpuid == X86_FEATURE_ALWAYS &&
> >> >> + *(int32_t *)(buf + 1) == -5 &&
> >> >> + a->orig_len >= 6 &&
> >> >> + orig[0] == 0xff &&
> >> >> + orig[1] == (*buf & 1 ? 0x25 : 0x15) )
> >> >> +{
> >> >> +long disp = *(int32_t *)(orig + 2);
> >> >> +const uint8_t *dest = *(void **)(orig + 6 + disp);
> >> >> +
> >> >> +if ( dest )
> >> >> +{
> >> >> +disp = dest - (orig + 5);
> >> >> +ASSERT(disp == (int32_t)disp);
> >> >> +*(int32_t *)(buf + 1) = disp;
> >> >> +}
> >> >> +else if ( force )
> >> >> +{
> >> >> +buf[0] = 0x0f;
> >> >> +buf[1] = 0x0b;
> >> >> +buf[2] = 0x0f;
> >> >> +buf[3] = 0xff;
> >> >> +buf[4] = 0xff;
> >> >
> >> > I think these are opcodes for "UD2; UD0". Please add a comment for them.
> >> > Having to go through SDM to figure out what they are isn't nice.
> >>
> >> Well, I'm saying so in the relatively big comment ahead of this block of
> >> code. I don't want to say the same thing twice.
> >
> > It is all fine when one is rather familiar with the code and x86-ism,
> > but it is rather difficult for a casual reader when you refer to
> > "target" in comment but "dest" in code.
>
> Would "function pointers" / "branch destinations" (or both) in the
> comment be better?
I think "branch destination" is better because it matches "dest" in
code.
>
> > Lacking comment of what "force" means also doesn't help.
> >
> >>
> >> > At this point I also think the name "force" is not very good. What/who
> >> > is forced here? Why not use a more descriptive name like "post_init" or
> >> > "system_active"?
> >>
> >> _Patching_ is being forced here, i.e. even if we still can't find a
> >> non-NULL
> >> pointer, we still patch the site. I'm certainly open for suggestions, but
> >> I don't really like either of the two suggestions you make any better than
> >> the current "force". The next best option I had been thinking about back
> >> then was to pass in a number, to identify the stage / phase / pass we're
> >> in.
> >
> > I had to reverse-engineer when force is supposed to be true. It would
> > help a lot if you add a comment regarding "force" at the beginning of
> > the function.
>
> Will do.
Thanks, that would certainly help.
Wei.
>
> Jan
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
Re: [Xen-devel] [PATCH v3 3/4] x86/HVM: implement memory read caching
On Tue, Sep 25, 2018 at 08:25:50AM -0600, Jan Beulich wrote: > Emulation requiring device model assistance uses a form of instruction > re-execution, assuming that the second (and any further) pass takes > exactly the same path. This is a valid assumption as far use of CPU > registers goes (as those can't change without any other instruction > executing in between), but is wrong for memory accesses. In particular > it has been observed that Windows might page out buffers underneath an > instruction currently under emulation (hitting between two passes). If > the first pass translated a linear address successfully, any subsequent > pass needs to do so too, yielding the exact same translation. > > Introduce a cache (used by just guest page table accesses for now) to > make sure above described assumption holds. This is a very simplistic > implementation for now: Only exact matches are satisfied (no overlaps or > partial reads or anything). > > As to the actual data page in this scenario, there are a couple of > aspects to take into consideration: > - We must be talking about an insn accessing two locations (two memory > ones, one of which is MMIO, or a memory and an I/O one). > - If the non I/O / MMIO side is being read, the re-read (if it occurs at > all) is having its result discarded, by taking the shortcut through > the first switch()'s STATE_IORESP_READY case in hvmemul_do_io(). Note > how, among all the re-issue sanity checks there, we avoid comparing > the actual data. > - If the non I/O / MMIO side is being written, it is the OSes > responsibility to avoid actually moving page contents to disk while > there might still be a write access in flight - this is no different > in behavior from bare hardware. > - Read-modify-write accesses are, as always, complicated, and while we > deal with them better nowadays than we did in the past, we're still > not quite there to guarantee hardware like behavior in all cases > anyway. Nothing is getting worse by the changes made here, afaict. > > Signed-off-by: Jan Beulich > Acked-by: Tim Deegan > Reviewed-by: Paul Durrant FWIW: Reviewed-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86: assert MBI is large enough in pvh-boot.c
On 26/09/18 12:00, Wei Liu wrote: > The relocation code in __start_xen requires one extra element in the > MBI structure. By the looks of it the temporary MBI array is already > large enough. Add an assertion to catch any issue in the future. > > Signed-off-by: Wei Liu While this is all well and good, the ASSERT() never actually gets out onto the console. This is because, when a failure occurs, the console hasn't been configured yet. For development purposes I use: diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c index e48039d..64febfa 100644 --- a/xen/drivers/char/console.c +++ b/xen/drivers/char/console.c @@ -91,7 +91,7 @@ static uint32_t conringc, conringp; static int __read_mostly sercon_handle = -1; #ifdef CONFIG_X86 -static bool __read_mostly opt_console_xen; /* console=xen */ +static bool __read_mostly opt_console_xen = true; /* console=xen */ #endif static DEFINE_SPINLOCK(console_lock); which gets the details out into the L0 log. As an addendum, might it be worth having opt_console_xen be tristate, and enabled by the start of the PVH path, so log messages before the command line is parsed end up being emitted? ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] IOREQ server on Arm
> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: 26 September 2018 11:58
> To: Julien Grall ; Paul Durrant
>
> Cc: Andrew Cooper ; Roger Pau Monne
> ; Stefano Stabellini ; xen-
> devel
> Subject: RE: IOREQ server on Arm
>
> >>> On 26.09.18 at 12:51, wrote:
> >> -Original Message-
> >> From: Julien Grall [mailto:julien.gr...@arm.com]
> >> Sent: 26 September 2018 11:41
> >> To: Jan Beulich ; Paul Durrant
> >>
> >> Cc: Andrew Cooper ; Roger Pau Monne
> >> ; Stefano Stabellini ;
> xen-
> >> devel
> >> Subject: Re: IOREQ server on Arm
> >>
> >> Hi Jan,
> >>
> >> On 09/26/2018 09:08 AM, Jan Beulich wrote:
> >> On 26.09.18 at 00:39, wrote:
> >> >> Hi Paul,
> >> >>
> >> >> I am looking at porting the IOREQ server infrastructure on Arm. I
> >> didn't
> >> >> need much modification to make it run for Arm. Although, the
> >> >> implementation could be simplified over the x86 implementation.
> >> >>
> >> >> I noticed some issue while trying to implement the hypercall
> >> >> XENMEM_acquire_resource. Per my understanding, all the page mapped
> via
> >> >> that hypercall will use the type p2m_mapping_foreign.
> >> >>
> >> >> This will result to trigger the ASSERT(fdom != dom) in
> >> get_page_from_gfn
> >> >> (asm-arm/p2m.h) because the IOREQ page has been allocated to the
> >> >> emulator domain and mapped to it. AFAICT x86 has the same assert in
> >> >> p2m_get_page_from_gfn(...).
> >> >>
> >> >> IHMO, the ASSERT makes sense because you are only meant to map page
> >> >> belonging to other domain with that type.
> >> >>
> >> >> So I am wondering whether IOREQ server running in PVH Dom0 has been
> >> >> tested? What would be the best course of action to fix the issue?
> >> >
> >> > I think the p2m type needs to be chosen based on
> >> > XENMEM_rsrc_acq_caller_owned.
> >>
> >> I am thinking to introduce p2m_mapping_owned. Or do we have a p2m_type
> >> that we could re-use?
> >>
> >
> > I think we should be able to just use p2m_ram_rw if it is caller owned.
>
> Yes, that's what I too would have thought. If there ever was a resource
> which may only be mapped r/o, p2m_ram_ro should then be equally usable.
>
Yes, that's true. The only existent resources are read-write though so I guess
another flag passed back to the caller could be used to indicate a read-only
resource.
I was thinking along the lines of a patch like this:
diff --git a/xen/common/memory.c b/xen/common/memory.c
index 996f94b103..82c18fa9ad 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -1105,8 +1105,11 @@ static int acquire_resource(
for ( i = 0; !rc && i < xmar.nr_frames; i++ )
{
-rc = set_foreign_p2m_entry(currd, gfn_list[i],
- _mfn(mfn_list[i]));
+rc = (xmar.flags & XENMEM_rsrc_acq_caller_owned) ?
+guest_physmap_add_entry(currd, gfn_list[i],
+_mfn(mfn_list[i]), 0, p2m_ram_rw) :
+set_foreign_p2m_entry(currd, gfn_list[i],
+ _mfn(mfn_list[i]));
/* rc should be -EIO for any iteration other than the first */
if ( rc && i )
rc = -EIO;
But the guest_physmap_add_entry() is problematic as it will IOMMU map pages as
well, which is probably not wanted.
Paul
> Jan
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] IOREQ server on Arm
Hi Paul, On 09/26/2018 11:51 AM, Paul Durrant wrote: -Original Message- From: Julien Grall [mailto:julien.gr...@arm.com] Sent: 26 September 2018 11:41 To: Jan Beulich ; Paul Durrant Cc: Andrew Cooper ; Roger Pau Monne ; Stefano Stabellini ; xen- devel Subject: Re: IOREQ server on Arm Hi Jan, On 09/26/2018 09:08 AM, Jan Beulich wrote:_ On 26.09.18 at 00:39, wrote: Hi Paul, I am looking at porting the IOREQ server infrastructure on Arm. I didn't need much modification to make it run for Arm. Although, the implementation could be simplified over the x86 implementation. I noticed some issue while trying to implement the hypercall XENMEM_acquire_resource. Per my understanding, all the page mapped via that hypercall will use the type p2m_mapping_foreign. This will result to trigger the ASSERT(fdom != dom) in get_page_from_gfn (asm-arm/p2m.h) because the IOREQ page has been allocated to the emulator domain and mapped to it. AFAICT x86 has the same assert in p2m_get_page_from_gfn(...). IHMO, the ASSERT makes sense because you are only meant to map page belonging to other domain with that type. So I am wondering whether IOREQ server running in PVH Dom0 has been tested? What would be the best course of action to fix the issue? I think the p2m type needs to be chosen based on XENMEM_rsrc_acq_caller_owned. I am thinking to introduce p2m_mapping_owned. Or do we have a p2m_type that we could re-use? I think we should be able to just use p2m_ram_rw if it is caller owned. I thought about p2m_ram_rw but discarded because of the security implications. At least on Arm, this type can be used for foreign mapping, guest_copy helpers. This is not the case for p2m_mapping_foreign. Do we want to allow thoses resources to be used in hypercall buffer and/or mapped by other guest via the foreign API? If not, then we want to use a different type. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] x86: assert MBI is large enough in pvh-boot.c
The relocation code in __start_xen requires one extra element in the MBI structure. By the looks of it the temporary MBI array is already large enough. Add an assertion to catch any issue in the future. Signed-off-by: Wei Liu --- xen/arch/x86/guest/pvh-boot.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/xen/arch/x86/guest/pvh-boot.c b/xen/arch/x86/guest/pvh-boot.c index 0e9e5bfdf6..7e13e7604b 100644 --- a/xen/arch/x86/guest/pvh-boot.c +++ b/xen/arch/x86/guest/pvh-boot.c @@ -44,6 +44,13 @@ static void __init convert_pvh_info(void) ASSERT(pvh_info->magic == XEN_HVM_START_MAGIC_VALUE); +/* + * Temporary MBI array needs to be at least one element bigger than + * required. The extra element is used to aid relocation. See + * arch/x86/setup.c:__start_xen(). + */ +ASSERT(ARRAY_SIZE(pvh_mbi_mods) > pvh_info->nr_modules); + /* * Turn hvm_start_info into mbi. Luckily all modules are placed under 4GB * boundary on x86. -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 6/6] RFC: tools/dm_restrict: Enable QEMU sandboxing
George Dunlap writes ("Re: [PATCH v2 6/6] RFC: tools/dm_restrict: Enable QEMU
sandboxing"):
> On 09/25/2018 12:02 PM, Ian Jackson wrote:
> > If you don't say set -e then you need to wrap every everything in your
> > entire script with an error check.
(I have deleted the whole comparative programming languages
discussion.)
> > For example, you write
> >
> dmpid=$(xenstore-read /local/domain/$domid/image/device-model-pid
> 2>/dev/null)
> if [[ -z "$dmpid" ]] ; then
> echo "xenstore-read failed"
> exit 1
> fi
...
> > but with set -e you can write only
> >
> > dmpid=$(xenstore-read /local/domain/$domid/image/device-model-pid)
> >
> > and the subsequent if is not needed.
>
> I think you misunderstood what I meant. The original code looked
> something like this:
Yes. You agreed to the removal of the 2>/dev/null. My point here is
that you if you use set -e you can get rid of the entire error
clause.
> In the current code, printing the error message is obviously better than
> throwing it away. But I still feel better checking the result and
> giving a sensible follow-up error message than having a failure silently
> exit, because I prefer to explicitly think about what happens when
> things fail (and remind the people reading the code to do so as well).
This approach is not very usual in shell scripting (at least, in
circles whose behaviour should be emulated)[1], and it leads to bugs.
Your script as originally presented is riddled with unchecked calls to
subprograms.
If you absolutely want to handle a particular error case specifically
then IMO it is much better to make a specific exception to that
particular case, by using either a set +e / set -e pair, or a
construct like if or ||.
> > The expression
> > ${input#* }
...
> That's an idea. I'm not sure I like it much better than using a regexp
> (now that the runes have been written), but I'll think about it.
Fair enough.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] IOREQ server on Arm
>>> On 26.09.18 at 12:51, wrote: >> -Original Message- >> From: Julien Grall [mailto:julien.gr...@arm.com] >> Sent: 26 September 2018 11:41 >> To: Jan Beulich ; Paul Durrant >> >> Cc: Andrew Cooper ; Roger Pau Monne >> ; Stefano Stabellini ; xen- >> devel >> Subject: Re: IOREQ server on Arm >> >> Hi Jan, >> >> On 09/26/2018 09:08 AM, Jan Beulich wrote: >> On 26.09.18 at 00:39, wrote: >> >> Hi Paul, >> >> >> >> I am looking at porting the IOREQ server infrastructure on Arm. I >> didn't >> >> need much modification to make it run for Arm. Although, the >> >> implementation could be simplified over the x86 implementation. >> >> >> >> I noticed some issue while trying to implement the hypercall >> >> XENMEM_acquire_resource. Per my understanding, all the page mapped via >> >> that hypercall will use the type p2m_mapping_foreign. >> >> >> >> This will result to trigger the ASSERT(fdom != dom) in >> get_page_from_gfn >> >> (asm-arm/p2m.h) because the IOREQ page has been allocated to the >> >> emulator domain and mapped to it. AFAICT x86 has the same assert in >> >> p2m_get_page_from_gfn(...). >> >> >> >> IHMO, the ASSERT makes sense because you are only meant to map page >> >> belonging to other domain with that type. >> >> >> >> So I am wondering whether IOREQ server running in PVH Dom0 has been >> >> tested? What would be the best course of action to fix the issue? >> > >> > I think the p2m type needs to be chosen based on >> > XENMEM_rsrc_acq_caller_owned. >> >> I am thinking to introduce p2m_mapping_owned. Or do we have a p2m_type >> that we could re-use? >> > > I think we should be able to just use p2m_ram_rw if it is caller owned. Yes, that's what I too would have thought. If there ever was a resource which may only be mapped r/o, p2m_ram_ro should then be equally usable. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] memory_hotplug: Free pages as higher order
On 2018-09-25 23:48, Michal Hocko wrote: On Tue 25-09-18 11:59:09, Vlastimil Babka wrote: [...] This seems like almost complete copy of __free_pages_boot_core(), could you do some code reuse instead? I think Michal Hocko also suggested that. Yes, please try to reuse as much code as possible Sure, Will address in next spin. Regards, Arun ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] IOREQ server on Arm
> -Original Message- > From: Julien Grall [mailto:julien.gr...@arm.com] > Sent: 26 September 2018 11:41 > To: Jan Beulich ; Paul Durrant > > Cc: Andrew Cooper ; Roger Pau Monne > ; Stefano Stabellini ; xen- > devel > Subject: Re: IOREQ server on Arm > > Hi Jan, > > On 09/26/2018 09:08 AM, Jan Beulich wrote: > On 26.09.18 at 00:39, wrote: > >> Hi Paul, > >> > >> I am looking at porting the IOREQ server infrastructure on Arm. I > didn't > >> need much modification to make it run for Arm. Although, the > >> implementation could be simplified over the x86 implementation. > >> > >> I noticed some issue while trying to implement the hypercall > >> XENMEM_acquire_resource. Per my understanding, all the page mapped via > >> that hypercall will use the type p2m_mapping_foreign. > >> > >> This will result to trigger the ASSERT(fdom != dom) in > get_page_from_gfn > >> (asm-arm/p2m.h) because the IOREQ page has been allocated to the > >> emulator domain and mapped to it. AFAICT x86 has the same assert in > >> p2m_get_page_from_gfn(...). > >> > >> IHMO, the ASSERT makes sense because you are only meant to map page > >> belonging to other domain with that type. > >> > >> So I am wondering whether IOREQ server running in PVH Dom0 has been > >> tested? What would be the best course of action to fix the issue? > > > > I think the p2m type needs to be chosen based on > > XENMEM_rsrc_acq_caller_owned. > > I am thinking to introduce p2m_mapping_owned. Or do we have a p2m_type > that we could re-use? > I think we should be able to just use p2m_ram_rw if it is caller owned. Paul > 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 03/11] vpci: add tear down functions
>>> On 17.07.18 at 11:48, wrote:
> --- a/xen/drivers/vpci/vpci.c
> +++ b/xen/drivers/vpci/vpci.c
> @@ -31,14 +31,26 @@ struct vpci_register {
> };
>
> #ifdef __XEN__
> -extern vpci_register_init_t *const __start_vpci_array[];
> -extern vpci_register_init_t *const __end_vpci_array[];
> +extern const struct vpci_handler __start_vpci_array[];
> +extern const struct vpci_handler __end_vpci_array[];
> #define NUM_VPCI_INIT (__end_vpci_array - __start_vpci_array)
>
> static void vpci_remove_device_locked(struct pci_dev *pdev)
> {
> +unsigned int i;
> +
> ASSERT(spin_is_locked(>vpci_lock));
>
> +for ( i = 0; i < NUM_VPCI_INIT; i++ )
> +{
> +vpci_teardown_t *teardown =
> +__start_vpci_array[NUM_VPCI_INIT - i - 1].teardown;
Perhaps slightly easier to read if you made the loop count downwards?
> #define VPCI_PRIORITY_HIGH "1"
> #define VPCI_PRIORITY_MIDDLE"5"
> #define VPCI_PRIORITY_LOW "9"
>
> -#define REGISTER_VPCI_INIT(x, p)\
> - static vpci_register_init_t *const x##_entry \
> - __used_section(".data.vpci." p) = x
> +#define REGISTER_VPCI_INIT(i, t, p) \
> + const static struct vpci_handler i ## t ## _entry \
Please flip static and const. Iirc we had a case in the past where some
tool (perhaps not a compiler but an analysis tool) choked about this
uncommon ordering.
With these taken care of
Acked-by: Jan Beulich
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Out of bounds access in early boot code related to GRUB
On Wed, Sep 26, 2018 at 12:18:43PM +0200, Daniel Kiper wrote: > On Fri, Sep 21, 2018 at 08:56:45PM +0200, Daniel Kiper wrote: > > On Wed, Sep 19, 2018 at 10:34:47AM +0100, Wei Liu wrote: > > > Hi Daniel, > > > > > > I discovered an out of bounds access issue related to GRUB relocation > > > code path when inspecting early boot code. > > > > > > 9589927e5b changed an EFI only path to work with GRUB. Yet the following > > > two lines within an if condition remained untouched. > > > > > > mod[mbi->mods_count].mod_start = virt_to_mfn(_stext); > > > mod[mbi->mods_count].mod_end = __2M_rwdata_end - _stext; > > > > > > Before your change they were fine because the mod array was created one > > > element larger in Xen (see e22e1c47958a). I don't think GRUB does the > > > same. So this is an out of bounds access for GRUB case. > > > > You are right! I will post a fix next week. > > I think that the issue can be quickly fixed by changing line 180 > in xen/arch/x86/boot/reloc.c with: > > mbi_out->mods_addr = alloc_mem((mbi_out->mods_count + 1) * > sizeof(*mbi_out_mods)); > > This way we will get extra space for Xen hypervisor if it is needed. > > If you are OK with that fix I will post a patch. Sure. That looks fine to me. But you will need Jan or Andrew's ack. :) Now I realise I'd better at least add an assert to the PVH boot path. Wei. > > Daniel ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] IOREQ server on Arm
> -Original Message- > From: Julien Grall [mailto:julien.gr...@arm.com] > Sent: 26 September 2018 11:33 > To: Paul Durrant ; 'Jan Beulich' > > Cc: Andrew Cooper ; Roger Pau Monne > ; Stefano Stabellini ; xen- > devel > Subject: Re: IOREQ server on Arm > > Hi, > > On 09/26/2018 10:14 AM, Paul Durrant wrote: > >> -Original Message- > >> From: Jan Beulich [mailto:jbeul...@suse.com] > >> Sent: 26 September 2018 09:09 > >> To: Julien Grall ; Paul Durrant > >> > >> Cc: Andrew Cooper ; Roger Pau Monne > >> ; Stefano Stabellini ; > xen- > >> devel > >> Subject: Re: IOREQ server on Arm > >> > > On 26.09.18 at 00:39, wrote: > >>> Hi Paul, > >>> > >>> I am looking at porting the IOREQ server infrastructure on Arm. I > didn't > >>> need much modification to make it run for Arm. Although, the > >>> implementation could be simplified over the x86 implementation. > >>> > >>> I noticed some issue while trying to implement the hypercall > >>> XENMEM_acquire_resource. Per my understanding, all the page mapped via > >>> that hypercall will use the type p2m_mapping_foreign. > >>> > >>> This will result to trigger the ASSERT(fdom != dom) in > get_page_from_gfn > >>> (asm-arm/p2m.h) because the IOREQ page has been allocated to the > >>> emulator domain and mapped to it. AFAICT x86 has the same assert in > >>> p2m_get_page_from_gfn(...). > >>> > >>> IHMO, the ASSERT makes sense because you are only meant to map page > >>> belonging to other domain with that type. > >>> > >>> So I am wondering whether IOREQ server running in PVH Dom0 has been > >>> tested? What would be the best course of action to fix the issue? > >> > >> I think the p2m type needs to be chosen based on > >> XENMEM_rsrc_acq_caller_owned. > >> > > > > Yes, that's correct. There is a FIXME clause in acquire_resource so that > that only caller owned resources can be mapped by HVM/PVH domains. Thus > the new call can be used for IOREQ server pages, but not grant frames. > > I don't understand your last sentence. IOREQ is caller owned and > therefore should work. Yes, I was just saying that it is currently the only resource that should work. > > As I said, I don't have any problem with mapping the resource. The > problem is when unmapping it because the region is set with > p2m_mapping_foreign. You will reach the ASSERT(fdom != p2m->domain) in > p2m_get_page_from_gfn. > > Regardless the reference problem (we support it on Arm). Can you explain > how this is working on PVH Dom0 today? > I never tested with a PVH dom0, but I did run tests with an HVM domU (and a Xen with the privilege checks hacked out). I guess I didn't re-test with HVM after the switch to caller-owned (which came quite late in review) and so missed this problem. Sorry about that. Paul > 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 02/11] vpci/msix: add lock to protect the list of MSIX regions
>>> On 17.07.18 at 11:48, wrote:
> --- a/xen/drivers/vpci/msix.c
> +++ b/xen/drivers/vpci/msix.c
> @@ -148,10 +148,11 @@ static void control_write(const struct pci_dev *pdev,
> unsigned int reg,
> pci_conf_write16(pdev->seg, pdev->bus, slot, func, reg, val);
> }
>
> -static struct vpci_msix *msix_find(const struct domain *d, unsigned long
> addr)
> +static struct vpci_msix *msix_find(struct domain *d, unsigned long addr)
> {
> struct vpci_msix *msix;
>
> +read_lock(>arch.hvm_domain.msix_lock);
> list_for_each_entry ( msix, >arch.hvm_domain.msix_tables, next )
> {
> const struct vpci_bar *bars = msix->pdev->vpci->header.bars;
> @@ -160,8 +161,12 @@ static struct vpci_msix *msix_find(const struct domain
> *d, unsigned long addr)
> for ( i = 0; i < ARRAY_SIZE(msix->tables); i++ )
> if ( bars[msix->tables[i] & PCI_MSIX_BIRMASK].enabled &&
> VMSIX_ADDR_IN_RANGE(addr, msix->pdev->vpci, i) )
> +{
> +read_unlock(>arch.hvm_domain.msix_lock);
> return msix;
> +}
> }
> +read_unlock(>arch.hvm_domain.msix_lock);
>
> return NULL;
> }
Don't you rather need the caller to acquire the lock, so that the return
value is guaranteed non-stale by the time the caller looks at it?
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] IOREQ server on Arm
Hi Jan, On 09/26/2018 09:08 AM, Jan Beulich wrote: On 26.09.18 at 00:39, wrote: Hi Paul, I am looking at porting the IOREQ server infrastructure on Arm. I didn't need much modification to make it run for Arm. Although, the implementation could be simplified over the x86 implementation. I noticed some issue while trying to implement the hypercall XENMEM_acquire_resource. Per my understanding, all the page mapped via that hypercall will use the type p2m_mapping_foreign. This will result to trigger the ASSERT(fdom != dom) in get_page_from_gfn (asm-arm/p2m.h) because the IOREQ page has been allocated to the emulator domain and mapped to it. AFAICT x86 has the same assert in p2m_get_page_from_gfn(...). IHMO, the ASSERT makes sense because you are only meant to map page belonging to other domain with that type. So I am wondering whether IOREQ server running in PVH Dom0 has been tested? What would be the best course of action to fix the issue? I think the p2m type needs to be chosen based on XENMEM_rsrc_acq_caller_owned. I am thinking to introduce p2m_mapping_owned. Or do we have a p2m_type that we could re-use? Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] IOREQ server on Arm
On 26/09/18 11:32, Julien Grall wrote: > Hi, > > On 09/26/2018 10:14 AM, Paul Durrant wrote: >>> -Original Message- >>> From: Jan Beulich [mailto:jbeul...@suse.com] >>> Sent: 26 September 2018 09:09 >>> To: Julien Grall ; Paul Durrant >>> >>> Cc: Andrew Cooper ; Roger Pau Monne >>> ; Stefano Stabellini ; >>> xen- >>> devel >>> Subject: Re: IOREQ server on Arm >>> >> On 26.09.18 at 00:39, wrote: Hi Paul, I am looking at porting the IOREQ server infrastructure on Arm. I didn't need much modification to make it run for Arm. Although, the implementation could be simplified over the x86 implementation. I noticed some issue while trying to implement the hypercall XENMEM_acquire_resource. Per my understanding, all the page mapped via that hypercall will use the type p2m_mapping_foreign. This will result to trigger the ASSERT(fdom != dom) in get_page_from_gfn (asm-arm/p2m.h) because the IOREQ page has been allocated to the emulator domain and mapped to it. AFAICT x86 has the same assert in p2m_get_page_from_gfn(...). IHMO, the ASSERT makes sense because you are only meant to map page belonging to other domain with that type. So I am wondering whether IOREQ server running in PVH Dom0 has been tested? What would be the best course of action to fix the issue? >>> >>> I think the p2m type needs to be chosen based on >>> XENMEM_rsrc_acq_caller_owned. >>> >> >> Yes, that's correct. There is a FIXME clause in acquire_resource so >> that that only caller owned resources can be mapped by HVM/PVH >> domains. Thus the new call can be used for IOREQ server pages, but >> not grant frames. > > I don't understand your last sentence. IOREQ is caller owned and > therefore should work. > > As I said, I don't have any problem with mapping the resource. The > problem is when unmapping it because the region is set with > p2m_mapping_foreign. You will reach the ASSERT(fdom != p2m->domain) in > p2m_get_page_from_gfn. > > Regardless the reference problem (we support it on Arm). Can you > explain how this is working on PVH Dom0 today? I doubt anyone has tried using it with a PVH Dom0. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 01/11] vpci: move lock
>>> On 17.07.18 at 11:48, wrote:
> @@ -62,12 +70,15 @@ int __hwdom_init vpci_add_handlers(struct pci_dev *pdev)
> if ( !has_vpci(pdev->domain) )
> return 0;
>
> +spin_lock(>vpci_lock);
> pdev->vpci = xzalloc(struct vpci);
Patterns like this should be avoided where possible: I'd prefer if you
called xzalloc() before acquiring the lock, storing the result in a local
variable.
Also the locked region becomes pretty big this way.
> @@ -148,8 +160,6 @@ int vpci_add_register(struct vpci *vpci, vpci_read_t
> *read_handler,
> r->offset = offset;
> r->private = data;
>
> -spin_lock(>lock);
> -
> /* The list of handlers must be kept sorted at all times. */
> list_for_each ( prev, >handlers )
> {
> @@ -161,14 +171,12 @@ int vpci_add_register(struct vpci *vpci, vpci_read_t
> *read_handler,
> break;
> if ( cmp == 0 )
> {
> -spin_unlock(>lock);
> xfree(r);
> return -EEXIST;
> }
> }
>
> list_add_tail(>node, prev);
> -spin_unlock(>lock);
>
> return 0;
> }
With the above in mind, this isn't very nice, because it puts an xmalloc()
invocation inside a locked region.
Can't you solve the race you mention in the description by simply latching
pdev->vpci into a local variable in vpci_remove_device(), and storing
NULL into the field with the lock still held? That way all the xfree()s
there could apparently move out of the locked region (if need be by
deferring all of this to an RCU callback).
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] IOREQ server on Arm
Hi, On 09/26/2018 10:14 AM, Paul Durrant wrote: -Original Message- From: Jan Beulich [mailto:jbeul...@suse.com] Sent: 26 September 2018 09:09 To: Julien Grall ; Paul Durrant Cc: Andrew Cooper ; Roger Pau Monne ; Stefano Stabellini ; xen- devel Subject: Re: IOREQ server on Arm On 26.09.18 at 00:39, wrote: Hi Paul, I am looking at porting the IOREQ server infrastructure on Arm. I didn't need much modification to make it run for Arm. Although, the implementation could be simplified over the x86 implementation. I noticed some issue while trying to implement the hypercall XENMEM_acquire_resource. Per my understanding, all the page mapped via that hypercall will use the type p2m_mapping_foreign. This will result to trigger the ASSERT(fdom != dom) in get_page_from_gfn (asm-arm/p2m.h) because the IOREQ page has been allocated to the emulator domain and mapped to it. AFAICT x86 has the same assert in p2m_get_page_from_gfn(...). IHMO, the ASSERT makes sense because you are only meant to map page belonging to other domain with that type. So I am wondering whether IOREQ server running in PVH Dom0 has been tested? What would be the best course of action to fix the issue? I think the p2m type needs to be chosen based on XENMEM_rsrc_acq_caller_owned. Yes, that's correct. There is a FIXME clause in acquire_resource so that that only caller owned resources can be mapped by HVM/PVH domains. Thus the new call can be used for IOREQ server pages, but not grant frames. I don't understand your last sentence. IOREQ is caller owned and therefore should work. As I said, I don't have any problem with mapping the resource. The problem is when unmapping it because the region is set with p2m_mapping_foreign. You will reach the ASSERT(fdom != p2m->domain) in p2m_get_page_from_gfn. Regardless the reference problem (we support it on Arm). Can you explain how this is working on PVH Dom0 today? Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] libxl: keep assigned pci devices across domain reboots
Hi,
On Thu, Sep 20, 2018 at 12:40:25PM +0200, Roger Pau Monne wrote:
> Fill the from_xenstore libxl_device_type hook for PCI devices so that
> libxl_retrieve_domain_configuration can properly retrieve PCI devices
> from xenstore.
>
> This fixes disappearing pci devices across domain reboots.
>
This patch seems to be committed now. Please backport this to Xen 4.10 stable
branch, for upcoming 4.10.3, because original bugreport was about Xen 4.10.
Thanks,
-- Pasi
> Reported-by: Andreas Kinzler
> Signed-off-by: Roger Pau Monné
> ---
> Cc: Ian Jackson
> Cc: Wei Liu
> Cc: Andreas Kinzler
> ---
> tools/libxl/libxl_pci.c | 9 +
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
> index 4755a0c93c..87afa03d9e 100644
> --- a/tools/libxl/libxl_pci.c
> +++ b/tools/libxl/libxl_pci.c
> @@ -1549,8 +1549,7 @@ int libxl_device_pci_destroy(libxl_ctx *ctx, uint32_t
> domid,
>
> static void libxl__device_pci_from_xs_be(libxl__gc *gc,
> const char *be_path,
> - libxl_device_pci *pci,
> - int nr)
> + int nr, libxl_device_pci *pci)
> {
> char *s;
> unsigned int domain = 0, bus = 0, dev = 0, func = 0, vdevfn = 0;
> @@ -1604,7 +1603,7 @@ libxl_device_pci *libxl_device_pci_list(libxl_ctx *ctx,
> uint32_t domid, int *num
> pcidevs = calloc(n, sizeof(libxl_device_pci));
>
> for (i = 0; i < n; i++)
> -libxl__device_pci_from_xs_be(gc, be_path, pcidevs + i, i);
> +libxl__device_pci_from_xs_be(gc, be_path, i, pcidevs + i);
>
> *num = n;
> out:
> @@ -1688,7 +1687,9 @@ static int libxl_device_pci_compare(libxl_device_pci
> *d1,
>
> #define libxl__device_pci_update_devid NULL
>
> -DEFINE_DEVICE_TYPE_STRUCT_X(pcidev, pci, PCI);
> +DEFINE_DEVICE_TYPE_STRUCT_X(pcidev, pci, PCI,
> +.from_xenstore = (device_from_xenstore_fn_t)libxl__device_pci_from_xs_be,
> +);
>
> /*
> * Local variables:
> --
> 2.19.0
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Out of bounds access in early boot code related to GRUB
On Fri, Sep 21, 2018 at 08:56:45PM +0200, Daniel Kiper wrote: > On Wed, Sep 19, 2018 at 10:34:47AM +0100, Wei Liu wrote: > > Hi Daniel, > > > > I discovered an out of bounds access issue related to GRUB relocation > > code path when inspecting early boot code. > > > > 9589927e5b changed an EFI only path to work with GRUB. Yet the following > > two lines within an if condition remained untouched. > > > > mod[mbi->mods_count].mod_start = virt_to_mfn(_stext); > > mod[mbi->mods_count].mod_end = __2M_rwdata_end - _stext; > > > > Before your change they were fine because the mod array was created one > > element larger in Xen (see e22e1c47958a). I don't think GRUB does the > > same. So this is an out of bounds access for GRUB case. > > You are right! I will post a fix next week. I think that the issue can be quickly fixed by changing line 180 in xen/arch/x86/boot/reloc.c with: mbi_out->mods_addr = alloc_mem((mbi_out->mods_count + 1) * sizeof(*mbi_out_mods)); This way we will get extra space for Xen hypervisor if it is needed. If you are OK with that fix I will post a patch. Daniel ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 5/6] powerpc/powernv: hold device_hotplug_lock when calling memtrace_offline_pages()
On 25/09/2018 14:15, Balbir Singh wrote:
> On Tue, Sep 25, 2018 at 11:14:56AM +0200, David Hildenbrand wrote:
>> Let's perform all checking + offlining + removing under
>> device_hotplug_lock, so nobody can mess with these devices via
>> sysfs concurrently.
>>
>> Cc: Benjamin Herrenschmidt
>> Cc: Paul Mackerras
>> Cc: Michael Ellerman
>> Cc: Rashmica Gupta
>> Cc: Balbir Singh
>> Cc: Michael Neuling
>> Reviewed-by: Pavel Tatashin
>> Reviewed-by: Rashmica Gupta
>> Signed-off-by: David Hildenbrand
>> ---
>> arch/powerpc/platforms/powernv/memtrace.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/platforms/powernv/memtrace.c
>> b/arch/powerpc/platforms/powernv/memtrace.c
>> index fdd48f1a39f7..d84d09c56af9 100644
>> --- a/arch/powerpc/platforms/powernv/memtrace.c
>> +++ b/arch/powerpc/platforms/powernv/memtrace.c
>> @@ -70,6 +70,7 @@ static int change_memblock_state(struct memory_block *mem,
>> void *arg)
>> return 0;
>> }
>>
>> +/* called with device_hotplug_lock held */
>> static bool memtrace_offline_pages(u32 nid, u64 start_pfn, u64 nr_pages)
>> {
>> u64 end_pfn = start_pfn + nr_pages - 1;
>> @@ -111,6 +112,7 @@ static u64 memtrace_alloc_node(u32 nid, u64 size)
>> end_pfn = round_down(end_pfn - nr_pages, nr_pages);
>>
>> for (base_pfn = end_pfn; base_pfn > start_pfn; base_pfn -= nr_pages) {
>> +lock_device_hotplug();
>
> Why not grab the lock before the for loop? That way we can avoid bad cases
> like a
> large node being scanned for a small number of pages (nr_pages). Ideally we
> need
> a cond_resched() in the loop, but I guess offline_pages() has one.
Yes, it does.
I can move it out of the loop, thanks!
>
> Acked-by: Balbir Singh
>
--
Thanks,
David / dhildenb
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] x86/HVM: correct hvmemul_map_linear_addr() for multi-page case
>>> On 25.09.18 at 17:30, wrote: > On 25/09/18 13:41, Jan Beulich wrote: > On 20.09.18 at 14:41, wrote: >>> On 13/09/18 11:12, Jan Beulich wrote: The function does two translations in one go for a single guest access. Any failure of the first translation step (guest linear -> guest physical), resulting in #PF, ought to take precedence over any failure of the second step (guest physical -> host physical). >>> Why? What is the basis of this presumption? >>> >>> As far as what real hardware does... >>> >>> This test sets up a ballooned page and a read-only page. I.e. a second >>> stage fault on the first part of a misaligned access, and a first stage >>> fault on the second part of the access. >>> >>> (d1) --- Xen Test Framework --- >>> (d1) Environment: HVM 64bit (Long mode 4 levels) >>> (d1) Test splitfault >>> (d1) About to read >>> (XEN) *** EPT qual 0181, gpa 0011cffc >>> (d1) Reading PTR: got >>> (d1) About to write >>> (XEN) *** EPT qual 0182, gpa 0011cffc >>> (d1) ** >>> (d1) PANIC: Unhandled exception at 0008:001047e0 >>> (d1) Vec 14 #PF[-d-sWP] %cr2 0011d000 >>> (d1) ** >>> >>> The second stage fault is recognised first, which is contrary to your >>> presumption, i.e. the code in its current form appears to be correct. >> Coming back to this example of yours: As a first step, are we in >> agreement that with the exception of very complex instructions >> (FSAVE, FXSAVE, XSAVE etc) instructions are supposed to work in an >> all-or-nothing manner when it comes to updating of architectural >> state (be it registers or memory)? > > No. Read Chapter Intel Vol3 8.1 and 8.2, which makes it quite clear > that misaligned accesses may be split access, and observably so to other > processors in the system. > > I've even found a new bit in it which says that >quadword SSE accesses > may even result in a partial write being completed before #PF is raised. But note that this is indeed limited to x87 / SSE insns. And there's nothing said that this behavior is mandatory. Hence if we emulated things such that (a) we meet the requirements for MOV and ALU insns and (b) we make x87 / SSE ones match (a), all would still be within spec. Furthermore both section individually state that LOCKed insns perform their accesses atomically, regardless of alignment. To me this implies no #PF when part of the write has already happened (presumably achieved by the walks needed for the reads already done as write-access walks). I think hvmemul_rmw() matches this already, yet even there a possible #PF on the second part of a split access could be detected and reported without doing two walks, by way of the change proposed here. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH net-next] net: xen-netback: fix return type of ndo_start_xmit function
The method ndo_start_xmit() is defined as returning an 'netdev_tx_t',
which is a typedef for an enum type, so make sure the implementation in
this driver has returns 'netdev_tx_t' value, and change the function
return type to netdev_tx_t.
Found by coccinelle.
Signed-off-by: YueHaibing
Acked-by: Wei Liu
---
drivers/net/xen-netback/interface.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/xen-netback/interface.c
b/drivers/net/xen-netback/interface.c
index 92274c2..7e3ea39 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -165,7 +165,8 @@ static u16 xenvif_select_queue(struct net_device *dev,
struct sk_buff *skb,
return vif->hash.mapping[skb_get_hash_raw(skb) % size];
}
-static int xenvif_start_xmit(struct sk_buff *skb, struct net_device *dev)
+static netdev_tx_t
+xenvif_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct xenvif *vif = netdev_priv(dev);
struct xenvif_queue *queue = NULL;
--
1.8.3.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] IOREQ server on Arm
> -Original Message- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: 26 September 2018 09:09 > To: Julien Grall ; Paul Durrant > > Cc: Andrew Cooper ; Roger Pau Monne > ; Stefano Stabellini ; xen- > devel > Subject: Re: IOREQ server on Arm > > >>> On 26.09.18 at 00:39, wrote: > > Hi Paul, > > > > I am looking at porting the IOREQ server infrastructure on Arm. I didn't > > need much modification to make it run for Arm. Although, the > > implementation could be simplified over the x86 implementation. > > > > I noticed some issue while trying to implement the hypercall > > XENMEM_acquire_resource. Per my understanding, all the page mapped via > > that hypercall will use the type p2m_mapping_foreign. > > > > This will result to trigger the ASSERT(fdom != dom) in get_page_from_gfn > > (asm-arm/p2m.h) because the IOREQ page has been allocated to the > > emulator domain and mapped to it. AFAICT x86 has the same assert in > > p2m_get_page_from_gfn(...). > > > > IHMO, the ASSERT makes sense because you are only meant to map page > > belonging to other domain with that type. > > > > So I am wondering whether IOREQ server running in PVH Dom0 has been > > tested? What would be the best course of action to fix the issue? > > I think the p2m type needs to be chosen based on > XENMEM_rsrc_acq_caller_owned. > Yes, that's correct. There is a FIXME clause in acquire_resource so that that only caller owned resources can be mapped by HVM/PVH domains. Thus the new call can be used for IOREQ server pages, but not grant frames. Paul > Jan > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 2/5] x86: use PDEP/PEXT for maddr/direct-map-offset conversion when available
>>> On 25.09.18 at 19:15, wrote:
> On 10/09/18 11:00, Jan Beulich wrote:
>> Well, I continue to not really agree. First and foremost, as said before,
>> the common (exclusive?) case is going to be that with "x86: use MOV
>> for PFN/PDX conversion when possible" no calls will exist at runtime at
>> all.
>
> Taking this one step further, why don't we drop PDX entirely?
>
> I seem to recall you saying that the one system it was introduced for
> never shipped, at which point, why bother keeping the code around?
For one I don't know if they or anyone else have plans to ship something
like this in the future. And then ARM uses PDX as well.
> A separate point which has only just occurred to me is the humongous
> pipeline stall which occurs when mixing legacy and VEX SSE instructions
> on SandyBridge and later hardware. I severely doubt that a single
> transformation from ALU operations to PDEP/PEXT is going to make up for
> the pipeline stall if the guest is using legacy SSE, although given how
> common the PDX conversions are, I could easily believe that the net is
> in the same ballpark.
I think you're mixing up SIMD and VEX-encoded GPR insns. I'm not
aware of the latter (which PDEP and PEXT belong to) falling into the
category where that SIMD register related stall would occur. That's
only related to non-VEX-encoded SIMD insns not updating the full
YMM / ZMM registers afaik (and iirc has been largely taken care of
in newer hardware).
>> At that point all function instances could collectively be purged just
>> like .init.text, if we cared enough. And then, for this particular case,
>> leaving the compiler the widest possible choice of register allocation
>> still seems pretty desirable to me. I'd agree with your "register
>> renames at compile time are free" only if there weren't special uses of
>> quite a few of the registers.
>>
>> As perhaps a prime example, consider the case where the
>> transformation here gets done in the course of setting up another
>> function's arguments. The compiler would have to avoid certain
>> registers (or generate extra MOVs) if it had to first pass the input
>> in a fixed register to the helper here (which then additionally also
>> needs to be assumed to clobber several other ones).
>
> Once again, I think you are focusing on the wrong aspect, and ending up
> with something which is worse overall.
>
> First, is there a single example here where the compiler sets up
> registers before a function call? Given the sequence point, it is
> distinctly non-trivial to optimise around.
First and foremost
#define __map_domain_page(pg)map_domain_page(page_to_mfn(pg))
static inline void *__map_domain_page_global(const struct page_info *pg)
{
return map_domain_page_global(page_to_mfn(pg));
}
Plus efi_rs_enter() has
switch_cr3_cr4(virt_to_maddr(efi_l4_pgtable), read_cr4());
and gnttab_unpopulate_status_frames() has
: guest_physmap_remove_page(d, gfn,
page_to_mfn(pg), 0);
just to give a few examples, and while looking for some I've already
skipped printk() invocations, init-only code and alike.
> Irrespective, a couple of extra movs (which are handled during register
> renaming) is far less overhead than hitting a cold icache line, and
> having 256 variations of this stub function is a very good way to hit a
> lot of cold icache lines.
Why do you continue to think that the called functions would be any
more cold than the call sites? I'm not going to claim I perfectly know
all details of when and how prefetching works on the various CPU
models, but I think direct calls should be pretty easy to handle in
hardware.
> Ultimately, real performance numbers are the only way to say for sure,
> but I expect you'll be surprised by the results you'd see.
I don't think I would, because for there to be surprises I'd have to
have hardware where I actually end up using the stub functions (or
revive/reconstruct the patch I used to have to fake memory holes).
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86emul: fix test harness build after e8dfbc2962
On Wed, Sep 26, 2018 at 02:03:36AM -0600, Jan Beulich wrote: > There was another stdio.h inclusion left in place. Re-order #include-s > altogether in test_x86_emulator.c. > > Signed-off-by: Jan Beulich Reviewed-by: Wei Liu Tested-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 128093: regressions - FAIL
flight 128093 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/128093/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64 6 xen-buildfail REGR. vs. 127928 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-i386 1 build-check(1) blocked n/a test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 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 version targeted for testing: xen 6cf4d8d3aa2699ff1ffa9e56240a6d188f91938c baseline version: xen 940185b2f6f343251c2b83bd96e599398cea51ec Last test of basis 127928 2018-09-22 10:00:53 Z3 days Failing since128013 2018-09-24 14:00:44 Z1 days 14 attempts Testing same since 128062 2018-09-25 18:00:52 Z0 days5 attempts People who touched revisions under test: Alexandru Isaila Amit Singh Tomar Andrew Cooper Christopher Clark Dario Faggioli George Dunlap Jan Beulich Julien Grall Razvan Cojocaru Roger Pau Monné Tamas K Lengyel Wei Liu jobs: build-arm64-xsm pass build-amd64 fail build-armhf pass build-amd64-libvirt blocked test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-i386 blocked test-amd64-amd64-libvirt blocked sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Not pushing. (No revision log; it would be 384 lines long.) ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] fuzz, test x86_emulator: disable sse before including always_inline fns
On Wed, Sep 26, 2018 at 09:36:39AM +0100, Wei Liu wrote: > This patch is causing build failure. > > See: > > http://logs.test-lab.xenproject.org/osstest/logs/128087/build-amd64/6.ts-xen-build.log Oh, it appears that Jan already posted a fix. Wei. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] fuzz, test x86_emulator: disable sse before including always_inline fns
This patch is causing build failure. See: http://logs.test-lab.xenproject.org/osstest/logs/128087/build-amd64/6.ts-xen-build.log Wei. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4] x86/mm: Add mem access rights to NPT
On Wed, 2018-07-25 at 04:37 -0600, Jan Beulich wrote:
> > > > On 25.07.18 at 11:25, wrote:
> >
> > On 07/24/2018 01:02 PM, Jan Beulich wrote:
> > > > > > On 24.07.18 at 13:26, wrote:
> > > >
> > > > On 07/24/2018 09:55 AM, Jan Beulich wrote:
> > > > > > > > On 23.07.18 at 15:48, wrote:
> > > > > >
> > > > > > +{
> > > > > > +xfree(d->arch.monitor.msr_bitmap);
> > > > > > +return -ENOMEM;
> > > > > > +}
> > > > > > +radix_tree_init(p2m->mem_access_settings);
> > > > > > +}
> > > > >
> > > > > What's the SVM connection here? Please don't forget that p2m-
> > > > > pt.c
> > > > > also serves the shadow case. Perhaps struct p2m_domain should
> > > > > contain a boolean indicator whether this auxiliary data
> > > > > structure is
> > > > > needed?
> > > >
> > > > It's basically just "hap_enabled()" isn't it?
> > >
> > > Only if we can't make it there when EPT is active.
> >
> > It can make it here when VMX is active and shadow is enabled, but
> > it
> > shouldn't be able to get here when EPT is active. We could add an
> > ASSERT() to that effect; it should be safe in production, as the
> > only
> > side effect should be that we do a small pointless allocation.
>
> So I've looked a little more closely: This is being added to
> arch_monitor_init_domain(), called from vm_event_domctl(). I can't
> see why this wouldn't be reachable with EPT enabled.
>
Hi George,
Any input on this?
Regards,
Alex
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] IOREQ server on Arm
>>> On 26.09.18 at 00:39, wrote: > Hi Paul, > > I am looking at porting the IOREQ server infrastructure on Arm. I didn't > need much modification to make it run for Arm. Although, the > implementation could be simplified over the x86 implementation. > > I noticed some issue while trying to implement the hypercall > XENMEM_acquire_resource. Per my understanding, all the page mapped via > that hypercall will use the type p2m_mapping_foreign. > > This will result to trigger the ASSERT(fdom != dom) in get_page_from_gfn > (asm-arm/p2m.h) because the IOREQ page has been allocated to the > emulator domain and mapped to it. AFAICT x86 has the same assert in > p2m_get_page_from_gfn(...). > > IHMO, the ASSERT makes sense because you are only meant to map page > belonging to other domain with that type. > > So I am wondering whether IOREQ server running in PVH Dom0 has been > tested? What would be the best course of action to fix the issue? I think the p2m type needs to be chosen based on XENMEM_rsrc_acq_caller_owned. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] x86emul: fix test harness build after e8dfbc2962
There was another stdio.h inclusion left in place. Re-order #include-s altogether in test_x86_emulator.c. Signed-off-by: Jan Beulich --- a/tools/tests/x86_emulator/test_x86_emulator.c +++ b/tools/tests/x86_emulator/test_x86_emulator.c @@ -1,10 +1,10 @@ -#include -#include -#include -#include - #include "x86-emulate.h" +#include +#include +#include +#include + asm ( ".pushsection .test, \"ax\", @progbits; .popsection" ); #include "blowfish.h" ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v4] x86: use VMLOAD for PV context switch
Having noticed that VMLOAD alone is about as fast as a single of the
involved WRMSRs, I thought it might be a reasonable idea to also use it
for PV. Measurements, however, have shown that an actual improvement can
be achieved only with an early prefetch of the VMCB (thanks to Andrew
for suggesting to try this), which I have to admit I can't really
explain. This way on my Fam15 box context switch takes over 100 clocks
less on average (the measured values are heavily varying in all cases,
though).
This is intentionally not using a new hvm_funcs hook: For one, this is
all about PV, and something similar can hardly be done for VMX.
Furthermore the indirect to direct call patching that is meant to be
applied to most hvm_funcs hooks would be ugly to make work with
functions having more than 6 parameters.
Signed-off-by: Jan Beulich
Acked-by: Brian Woods
Reviewed-by: Boris Ostrovsky
---
v4: Cosmetics as per Andrew (none of which I've considered invalidating
the already present tags).
v3: Add/extend comments.
v2: Re-base.
---
Besides the mentioned oddity with measured performance, I've also
noticed a significant difference (of at least 150 clocks) between
measuring immediately around the calls to svm_load_segs() and measuring
immediately inside the function.
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -52,6 +52,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -1281,11 +1282,34 @@ static void load_segments(struct vcpu *n
struct cpu_user_regs *uregs = >arch.user_regs;
int all_segs_okay = 1;
unsigned int dirty_segment_mask, cpu = smp_processor_id();
+bool fs_gs_done = false;
/* Load and clear the dirty segment mask. */
dirty_segment_mask = per_cpu(dirty_segment_mask, cpu);
per_cpu(dirty_segment_mask, cpu) = 0;
+#ifdef CONFIG_HVM
+if ( !is_pv_32bit_vcpu(n) && !cpu_has_fsgsbase && cpu_has_svm &&
+ !((uregs->fs | uregs->gs) & ~3) &&
+ /*
+ * The remaining part is just for optimization: If only shadow GS
+ * needs loading, there's nothing to be gained here.
+ */
+ (n->arch.pv.fs_base | n->arch.pv.gs_base_user) )
+{
+unsigned long gsb = n->arch.flags & TF_kernel_mode
+? n->arch.pv.gs_base_kernel : n->arch.pv.gs_base_user;
+unsigned long gss = n->arch.flags & TF_kernel_mode
+? n->arch.pv.gs_base_user : n->arch.pv.gs_base_kernel;
+
+fs_gs_done = svm_load_segs(n->arch.pv.ldt_ents, LDT_VIRT_START(n),
+ uregs->fs, n->arch.pv.fs_base,
+ uregs->gs, gsb, gss);
+}
+#endif
+if ( !fs_gs_done )
+load_LDT(n);
+
/* Either selector != 0 ==> reload. */
if ( unlikely((dirty_segment_mask & DIRTY_DS) | uregs->ds) )
{
@@ -1301,7 +1325,7 @@ static void load_segments(struct vcpu *n
}
/* Either selector != 0 ==> reload. */
-if ( unlikely((dirty_segment_mask & DIRTY_FS) | uregs->fs) )
+if ( unlikely((dirty_segment_mask & DIRTY_FS) | uregs->fs) && !fs_gs_done )
{
all_segs_okay &= loadsegment(fs, uregs->fs);
/* non-nul selector updates fs_base */
@@ -1310,7 +1334,7 @@ static void load_segments(struct vcpu *n
}
/* Either selector != 0 ==> reload. */
-if ( unlikely((dirty_segment_mask & DIRTY_GS) | uregs->gs) )
+if ( unlikely((dirty_segment_mask & DIRTY_GS) | uregs->gs) && !fs_gs_done )
{
all_segs_okay &= loadsegment(gs, uregs->gs);
/* non-nul selector updates gs_base_user */
@@ -1318,7 +1342,7 @@ static void load_segments(struct vcpu *n
dirty_segment_mask &= ~DIRTY_GS_BASE;
}
-if ( !is_pv_32bit_vcpu(n) )
+if ( !fs_gs_done && !is_pv_32bit_vcpu(n) )
{
/* This can only be non-zero if selector is NULL. */
if ( n->arch.pv.fs_base | (dirty_segment_mask & DIRTY_FS_BASE) )
@@ -1653,6 +1677,13 @@ static void __context_switch(void)
write_ptbase(n);
+#if defined(CONFIG_PV) && defined(CONFIG_HVM)
+/* Prefetch the VMCB if we expect to use it later in the context switch */
+if ( is_pv_domain(nd) && !is_pv_32bit_domain(nd) && !is_idle_domain(nd) &&
+ !cpu_has_fsgsbase && cpu_has_svm )
+svm_load_segs(0, 0, 0, 0, 0, 0, 0);
+#endif
+
if ( need_full_gdt(nd) &&
((p->vcpu_id != n->vcpu_id) || !need_full_gdt(pd)) )
{
@@ -1714,10 +1745,7 @@ void context_switch(struct vcpu *prev, s
local_irq_enable();
if ( is_pv_domain(nextd) )
-{
-load_LDT(next);
load_segments(next);
-}
ctxt_switch_levelling(next);
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -78,6 +78,9 @@ static struct hvm_function_table svm_fun
*/
static DEFINE_PER_CPU_READ_MOSTLY(paddr_t, hsa);
static DEFINE_PER_CPU_READ_MOSTLY(paddr_t, host_vmcb);
+#ifdef CONFIG_PV
+static DEFINE_PER_CPU(struct vmcb_struct *, host_vmcb_va);
[Xen-devel] [PATCH] x86: silence false log messages for plain "xpti" / "pv-l1tf"
While commit 2a3b34ec47 ("x86/spec-ctrl: Yet more fixes for xpti=
parsing") claimed to have got rid of the 'parameter "xpti" has invalid
value "", rc=-22!' log message for "xpti" alone on the command line,
this wasn't the case (the option took effect nevertheless).
Fix this there as well as for plain "pv-l1tf".
Signed-off-by: Jan Beulich
---
Split off from "x86: fix "xpti=" and "pv-l1tf=" yet again".
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -257,7 +257,7 @@ static __init int parse_pv_l1tf(const ch
else if ( (val = parse_boolean("domu", s, ss)) >= 0 )
opt_pv_l1tf = ((opt_pv_l1tf & ~OPT_PV_L1TF_DOMU) |
(val ? OPT_PV_L1TF_DOMU : 0));
-else
+else if ( *s )
rc = -EINVAL;
break;
}
@@ -715,7 +715,7 @@ static __init int parse_xpti(const char
else if ( (val = parse_boolean("domu", s, ss)) >= 0 )
opt_xpti = (opt_xpti & ~OPT_XPTI_DOMU) |
(val ? OPT_XPTI_DOMU : 0);
-else
+else if ( *s )
rc = -EINVAL;
break;
}
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH V5] x86/altp2m: Add a subop for obtaining the mem access of a page
Currently there is a subop for setting the memaccess of a page, but not
for consulting it. The new HVMOP_altp2m_get_mem_access adds this
functionality.
Both altp2m get/set mem access functions use the struct
xen_hvm_altp2m_mem_access which has now dropped the `set' part and has
been renamed from xen_hvm_altp2m_set_mem_access.
Signed-off-by: Adrian Pop
Signed-off-by: Razvan Cojocaru
---
Changes since V4:
- Replaced the "if ( altp2m_idx ) return -EINVAL;" test in
p2m_get_mem_access() with "ASSERT(altp2m_idx == 0);" for ARM.
- Removed XEN_INTERFACE_VERSION #ifdeffery from the toolstack and
hypervisor (only kept them in hvm_op.h).
- Renamed hvmmem_access to simply access (on Tamas' request) in
xen_hvm_altp2m_set_mem_access and xen_hvm_altp2m_mem_access.
---
tools/libxc/include/xenctrl.h | 3 +++
tools/libxc/xc_altp2m.c | 33 ++---
xen/arch/arm/mem_access.c | 7 +--
xen/arch/x86/hvm/hvm.c | 30 --
xen/arch/x86/mm/mem_access.c| 21 +++--
xen/common/mem_access.c | 2 +-
xen/include/public/hvm/hvm_op.h | 21 -
xen/include/public/xen-compat.h | 2 +-
xen/include/xen/mem_access.h| 3 ++-
9 files changed, 105 insertions(+), 17 deletions(-)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index dad96a9..618f3cb 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1949,6 +1949,9 @@ int xc_altp2m_set_mem_access(xc_interface *handle,
uint32_t domid,
int xc_altp2m_set_mem_access_multi(xc_interface *handle, uint32_t domid,
uint16_t view_id, uint8_t *access,
uint64_t *gfns, uint32_t nr);
+int xc_altp2m_get_mem_access(xc_interface *handle, uint32_t domid,
+ uint16_t view_id, xen_pfn_t gfn,
+ xenmem_access_t *access);
int xc_altp2m_change_gfn(xc_interface *handle, uint32_t domid,
uint16_t view_id, xen_pfn_t old_gfn,
xen_pfn_t new_gfn);
diff --git a/tools/libxc/xc_altp2m.c b/tools/libxc/xc_altp2m.c
index be5bfd2..844b9f1 100644
--- a/tools/libxc/xc_altp2m.c
+++ b/tools/libxc/xc_altp2m.c
@@ -226,9 +226,9 @@ int xc_altp2m_set_mem_access(xc_interface *handle, uint32_t
domid,
arg->version = HVMOP_ALTP2M_INTERFACE_VERSION;
arg->cmd = HVMOP_altp2m_set_mem_access;
arg->domain = domid;
-arg->u.set_mem_access.view = view_id;
-arg->u.set_mem_access.hvmmem_access = access;
-arg->u.set_mem_access.gfn = gfn;
+arg->u.mem_access.view = view_id;
+arg->u.mem_access.access = access;
+arg->u.mem_access.gfn = gfn;
rc = xencall2(handle->xcall, __HYPERVISOR_hvm_op, HVMOP_altp2m,
HYPERCALL_BUFFER_AS_ARG(arg));
@@ -303,3 +303,30 @@ int xc_altp2m_set_mem_access_multi(xc_interface *xch,
uint32_t domid,
return rc;
}
+
+int xc_altp2m_get_mem_access(xc_interface *handle, uint32_t domid,
+ uint16_t view_id, xen_pfn_t gfn,
+ xenmem_access_t *access)
+{
+int rc;
+DECLARE_HYPERCALL_BUFFER(xen_hvm_altp2m_op_t, arg);
+
+arg = xc_hypercall_buffer_alloc(handle, arg, sizeof(*arg));
+if ( arg == NULL )
+return -1;
+
+arg->version = HVMOP_ALTP2M_INTERFACE_VERSION;
+arg->cmd = HVMOP_altp2m_get_mem_access;
+arg->domain = domid;
+arg->u.mem_access.view = view_id;
+arg->u.mem_access.gfn = gfn;
+
+rc = xencall2(handle->xcall, __HYPERVISOR_hvm_op, HVMOP_altp2m,
+ HYPERCALL_BUFFER_AS_ARG(arg));
+
+if ( !rc )
+*access = arg->u.mem_access.access;
+
+xc_hypercall_buffer_free(handle, arg);
+return rc;
+}
diff --git a/xen/arch/arm/mem_access.c b/xen/arch/arm/mem_access.c
index ba4ec78..653d960 100644
--- a/xen/arch/arm/mem_access.c
+++ b/xen/arch/arm/mem_access.c
@@ -236,7 +236,7 @@ bool p2m_mem_access_check(paddr_t gpa, vaddr_t gla, const
struct npfec npfec)
if ( !p2m->mem_access_enabled )
return true;
-rc = p2m_get_mem_access(v->domain, gaddr_to_gfn(gpa), );
+rc = p2m_get_mem_access(v->domain, gaddr_to_gfn(gpa), , 0);
if ( rc )
return true;
@@ -441,11 +441,14 @@ long p2m_set_mem_access_multi(struct domain *d,
}
int p2m_get_mem_access(struct domain *d, gfn_t gfn,
- xenmem_access_t *access)
+ xenmem_access_t *access, unsigned int altp2m_idx)
{
int ret;
struct p2m_domain *p2m = p2m_get_hostp2m(d);
+/* altp2m is not yet implemented on Arm. The altp2m_idx should be 0. */
+ASSERT(altp2m_idx == 0);
+
p2m_read_lock(p2m);
ret = __p2m_get_mem_access(d, gfn, access);
p2m_read_unlock(p2m);
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 51fc3ec..8a9abf3 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4473,6 +4473,7 @@
