Re: [Xen-devel] [PATCH v6 5/5] x86/vm_event: Add HVM debug exception vm_events

[Jan Beulich]

>>> On 23.06.16 at 19:07,  wrote:
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -3373,10 +3373,39 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
>  HVMTRACE_1D(TRAP_DEBUG, exit_qualification);
>  write_debugreg(6, exit_qualification | DR_STATUS_RESERVED_ONE);
>  if ( !v->domain->debugger_attached )
> -vmx_propagate_intr(intr_info);
> +{
> +unsigned long insn_len = 0;
> +int rc;
> +unsigned long trap_type = MASK_EXTR(intr_info,
> +
> INTR_INFO_INTR_TYPE_MASK);
> +
> +if ( trap_type >= X86_EVENTTYPE_SW_INTERRUPT )
> +__vmread(VM_EXIT_INSTRUCTION_LEN, &insn_len);
> +
> +rc = hvm_monitor_debug(regs->eip,
> +   HVM_MONITOR_DEBUG_EXCEPTION,
> +   trap_type, insn_len);
> +
> +/*
> + * !rc  continue normally
> + * rc > paused waiting for response, work here is done
> + * rc < error, fall-through to exit_and_crash
> + */
> +if ( !rc )
> +{
> +vmx_propagate_intr(intr_info);
> +break;
> +}
> +if ( rc > 0 )
> +break;
> +}
>  else
> +{
>  domain_pause_for_debugger();
> -break;
> +break;
> +}
> +
> +goto exit_and_crash;

I continue to think this is sub-optimal structuring (at once needlessly
making the patch larger), but it'll be the VMX maintainers to judge.

For the few pieces it is relevant for:
Acked-by: Jan Beulich 

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 0/3] VMX: Properly handle pi descriptor and per-cpu blocking list

[Dario Faggioli]

On Fri, 2016-06-24 at 06:11 +, Wu, Feng wrote:
> > -Original Message-
> > From: Dario Faggioli [mailto:dario.faggi...@citrix.com]
> > No, because we call cpu_disable_scheduler() from __cpu_disable(),
> > only
> > when system state is SYS_STATE_suspend already, and hence we take
> > the
> > then branch of the 'if', which does never return an error.
> Thanks for the elaboration. I find __cpu_disable() can be called with
> system state not being SYS_STATE_suspend. Here is my experiment:
> 
> 1. Launch a guest and pin vCPU 3 to pCPU 3 (which makes the
> experiment simpler)
> 2. offline pCPU 3 via "xen-hptool cpu-offline 3"
> 
Ah, yes, of course. I should have included cpu-hot(un)plug in my
analysis in the first place, sorry for not doing so.

> The call path of the above steps is:
> arch_do_sysctl()
>   -> cpu_down_helper()
> -> cpu_down()
>   -> take_cpu_down()
> -> __cpu_disable()
>   -> cpu_disable_scheduler() (enter the 'else'  part)
> 
Right, and the important part is this one:

> int stop_machine_run(int (*fn)(void *), void *data, unsigned int cpu)
> {
> 
> ..
> 
> point 1
> 
> for_each_cpu ( i, &allbutself )
> tasklet_schedule_on_cpu(&per_cpu(stopmachine_tasklet, i), i);
> 
> point 2
> ..
> 
> }
> 
> at point 1 above, 
> vCPU3->runstate.state: 0, vCPU3->is_running: 1
> while at point 2 above:
> vCPU3->runstate.state: 1, vCPU3->is_running: 0
> 
This is exactly as you describe. cpu hotplug is done in stop machine
context. Check the comment close to the definition of stop_machine_run:

/**
 * stop_machine_run: freeze the machine on all CPUs and run this function
 * @fn: the function to run
 * @data: the data ptr for the @fn()
 * @cpu: the cpu to run @fn() on (or all, if @cpu == NR_CPUS).
 *
 * Description: This causes every other cpu to enter a safe point, with
 * each of which disables interrupts, and finally interrupts are disabled
 * on the current CPU.  The result is that none is holding a spinlock
 * or inside any other preempt-disabled region when @fn() runs.
 *
 * This can be thought of as a very heavy write lock, equivalent to
 * grabbing every spinlock in the kernel. */

As you discovered yourself, this is achieved by forcing the execution
of a tasklet on all the pcpus, which include pCPU 3 of your example.

So, vCPU 3 was running, but then some called stop_machine_run(), which
causes the descheduling of vCPU 3, and the execution of the stopmachine
tasklet.

Thet's why you find is_running to be 0, and that's why  we never return
EAGAIN.

> I tested it for many times and got the same result. I am not sure the
> vcpu
> state transition just happens to occurs here or not? If the
> transition doesn't
> happen and is_running is still 1 when we get vcpu_migrate() in
> cpu_disable_scheduler() in the above case, should it be a problem?
>
I'm not sure what you mean here (in particular with "just happen to
occur"). If you're wondering whether the fact that vCPU 3 gets
descheduled happens by chance or by design, it's indeed the latter, and
so, no, we don't have a problem with this code path.

Regards,
Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)



signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [xen-unstable test] 96178: tolerable FAIL - PUSHED

[osstest service owner]

flight 96178 xen-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/96178/

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-xl-credit2   3 host-install(3)  broken in 96151 pass in 96178
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 3 host-install(3) broken 
in 96151 pass in 96178
 test-armhf-armhf-xl  18 leak-check/check   fail in 96151 pass in 96178
 test-armhf-armhf-libvirt-raw  9 debian-di-install   fail pass in 96151

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds 15 guest-start/debian.repeat fail in 96151 like 96125
 build-amd64-rumpuserxen   6 xen-buildfail   like 96125
 build-i386-rumpuserxen6 xen-buildfail   like 96125
 test-amd64-amd64-xl-rtds  9 debian-install   fail   like 96125
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail like 96125
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 96125
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 96125
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 96125

Tests which did not succeed, but are not blocking:
 test-amd64-i386-rumpuserxen-i386  1 build-check(1)   blocked  n/a
 test-amd64-amd64-rumpuserxen-amd64  1 build-check(1)   blocked n/a
 test-armhf-armhf-libvirt-raw 13 guest-saverestore fail in 96151 never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-check fail in 96151 never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass

version targeted for testing:
 xen  91b26a35914176db4d19dc145bc6e2db62ee7a2c
baseline version:
 xen  c6f7d21747805b50123fc1b8d73518fea2aa9096

Last test of basis96125  2016-06-22 12:58:07 Z1 days
Testing same since96151  2016-06-23 00:44:34 Z1 days2 attempts


People who touched revisions under test:
  Jan Beulich 
  Julien Grall 
  Kevin Tian 
  Quan Xu 

jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-armhf  

Re: [Xen-devel] [PATCH v5 05/14] libxl: Load guest BIOS from file

[Jan Beulich]

>>> On 22.06.16 at 19:15,  wrote:
> --- a/tools/libxl/libxl_paths.c
> +++ b/tools/libxl/libxl_paths.c
> @@ -35,6 +35,16 @@ const char *libxl__run_dir_path(void)
>  return XEN_RUN_DIR;
>  }
>  
> +const char *libxl__seabios_path(void)
> +{
> +return SEABIOS_PATH;
> +}
> +
> +const char *libxl__ovmf_path(void)
> +{
> +return OVMF_PATH;
> +}

With an earlier version of this series pulled into one of our branches,
I've run into a problem with this: The paths you return here are the
configured paths, and that's intended. Yet it breaks running the
tools out of the build area (i.e. without any "make install"), which so
far has been working fine (as apparently in all other relevant cases
where paths are needed, relative ones are being used), and which
I much prefer over the hassle of scattering around half a dozen of
different Xen tools versions in custom directories under, say,
/usr/local. I guess if I'm the only one using this, I'll have to find my
own local solution for this, but of course I'd prefer for this currently
working case not to get broken.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 3/3] x86/ioreq server: Add HVMOP to map guest ram with p2m_ioreq_server to an ioreq server.

[Yu Zhang]

On 6/24/2016 2:12 PM, Jan Beulich wrote:

On 24.06.16 at 06:16,  wrote:

I'm now willing to take your suggestions:
a> still need the p2m resetting when ioreq server is unbounded;
b> disable log dirty feature if one ioreq server is bounded.

Does anyone else has different opinions? Thanks!

Hmm, in particular for a) I don't think I read that out of George's
descriptions. But of course much depends on what you really
mean by that: Do you want to say we need to guarantee all
entries get reverted back, or do you instead mean to just kick
off the conversion (via the misconfig mechanism)?


Thanks for your reply, Jan. I mean the misconfig mechanism.



In any event, I think log-dirty shouldn't be disabled when an
ioreq server binds the type, but as long as there are outstanding
entries of that type. That way, the "cannot be migrated" state
of a VM has a chance to clear.


Do you mean to disable log dirty by checking if there's outstanding 
p2m_ioreq_server
entries instead of the p2m->ioreq.server? How about we check both? 
Because only
checking the count of outstanding p2m_ioreq_server may prevent the live 
migration

when an ioreq server is unbound, but with p2m type not entirely synced.


And then, thinking about it again especially in the context of
the hvmctl series - the unbinding of the type is happening in a
hypercall with built-in preemption capability. Hence there's not
really an issue with how long that conversion may take, as
long as there's no need to pause the guest for that time period.
Which means you could first initiate conversion via the
misconfig mechanism, but then immediately go ahead and walk
the entire guest address space (or the relevant part of it, if
the bounds got tracked) with continuations used as necessary.


Sorry, what's the misconfig mechanism good for if I still need to sweep 
the entire

p2m table immediately?

Thanks
Yu


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [qemu-upstream-4.3-testing test] 96179: regressions - FAIL

[osstest service owner]

flight 96179 qemu-upstream-4.3-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/96179/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-libvirt   5 libvirt-build fail REGR. vs. 80927
 build-i386-libvirt5 libvirt-build fail REGR. vs. 80927

Regressions which are regarded as allowable (not blocking):
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 80927
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 80927

Tests which did not succeed, but are not blocking:
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-vhd  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  9 debian-hvm-install fail never pass
 test-amd64-i386-xl-qemuu-ovmf-amd64  9 debian-hvm-install  fail never pass

version targeted for testing:
 qemuu12e8fccf5b5460be7aecddc71d27eceaba6e1f15
baseline version:
 qemuu10c1b763c26feb645627a1639e722515f3e1e876

Last test of basis80927  2016-02-06 13:30:02 Z  138 days
Failing since 93977  2016-05-10 11:09:16 Z   44 days  143 attempts
Testing same since95534  2016-06-11 00:59:46 Z   13 days   23 attempts


People who touched revisions under test:
  Anthony PERARD 
  Gerd Hoffmann 
  Ian Jackson 
  Stefano Stabellini 
  Wei Liu 

jobs:
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  fail
 build-i386-libvirt   fail
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl  pass
 test-amd64-i386-xl   pass
 test-amd64-i386-qemuu-rhel6hvm-amd   pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-i386-xl-qemuu-debianhvm-amd64 pass
 test-amd64-i386-freebsd10-amd64  pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 fail
 test-amd64-i386-xl-qemuu-ovmf-amd64  fail
 test-amd64-amd64-xl-qemuu-win7-amd64 fail
 test-amd64-i386-xl-qemuu-win7-amd64  fail
 test-amd64-amd64-xl-credit2  pass
 test-amd64-i386-freebsd10-i386   pass
 test-amd64-i386-qemuu-rhel6hvm-intel pass
 test-amd64-amd64-libvirt blocked 
 test-amd64-i386-libvirt  blocked 
 test-amd64-amd64-xl-multivcpupass
 test-amd64-amd64-pairpass
 test-amd64-i386-pair pass
 test-amd64-amd64-pv  pass
 test-amd64-i386-pv   pass
 test-amd64-amd64-amd64-pvgrubpass
 test-amd64-amd64-i386-pvgrub pass
 test-amd64-amd64-pygrub  pass
 test-amd64-amd64-xl-qcow2pass
 test-amd64-i386-xl-raw   pass
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 pass
 test-amd64-amd64-libvirt-vhd blocked 
 test-amd64-amd64-xl-qemuu-winxpsp3   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


Not pushing.


commit 12e8fccf5b5460be7aecddc71d27eceaba6e1f15
Author: Ian Jackson 
Date:   Thu May 26 16:21:56 2016 +0100

main loop: Big hammer to fix logfile disk DoS in Xen setups

Each time round the main loop, we now fstat stderr.  If it is too big,
we dup2 /dev/null onto it.  This is not a very pretty patch but it is

Re: [Xen-devel] [PATCH v5 08/14] hvmloader: Locate the BIOS blob

[Jan Beulich]

>>> On 22.06.16 at 19:15,  wrote:
> --- a/tools/firmware/hvmloader/hvmloader.c
> +++ b/tools/firmware/hvmloader/hvmloader.c
> @@ -253,10 +253,51 @@ static void acpi_enable_sci(void)
>  BUG_ON(!(pm1a_cnt_val & ACPI_PM1C_SCI_EN));
>  }
>  
> +const struct hvm_modlist_entry *get_module_entry(
> +const struct hvm_start_info *info,
> +const char *name)
> +{
> +const struct hvm_modlist_entry *modlist =
> +(struct hvm_modlist_entry *)(uint32_t)info->modlist_paddr;
> +unsigned int i;
> +
> +if ( !modlist || info->modlist_paddr > UINT_MAX)
> +return NULL;

How about info->modlist_paddr + info->nr_modules * sizeof()?
You check for overflow below, but not here. I think you should
either consistently rely on there being something right below 4Gb
which makes this impossible (and then say so in a comment), or
do full checks everywhere.

> +for ( i = 0; i < info->nr_modules; i++ )
> +{
> +uint32_t module_name = modlist[i].cmdline_paddr;
> +
> +/* Skip if the module or its cmdline is missing. */
> +if ( !module_name || !modlist[i].paddr )
> +continue;
> +
> +/* Skip if the cmdline can not be read. */
> +if ( modlist[i].cmdline_paddr > UINT_MAX )
> +continue;

Similarly here.

> +if ( !strcmp(name, (char*)module_name) )

Stray cast.

> +{
> +if ( modlist[i].paddr > UINT_MAX || modlist[i].size > UINT_MAX ||
> + (modlist[i].paddr + modlist[i].size) > UINT_MAX )

I think the last one could be >=.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 09/14] hvmloader: Check modules whereabouts in perform_tests

[Jan Beulich]

>>> On 22.06.16 at 19:15,  wrote:
> --- a/tools/firmware/hvmloader/tests.c
> +++ b/tools/firmware/hvmloader/tests.c
> @@ -20,6 +20,7 @@
>   */
>  
>  #include "util.h"
> +#include "config.h"
>  
>  #define TEST_FAIL 0
>  #define TEST_PASS 1
> @@ -189,6 +190,15 @@ static int shadow_gs_test(void)
>  return (ebx == 2) ? TEST_PASS : TEST_FAIL;
>  }
>  
> +static bool check_test_overlap(uint64_t start, uint64_t size)
> +{
> +if (start)

Missing blanks.

> +return check_overlap(start, size,
> + 4ul << 20,
> + (PT_START + 4 * PAGE_SIZE) - (4ul << 20));

Can the 4ul << 20 and the upper bound be made into descriptive
#define-s please, also to be used wherever (if anywhere) those
boundary are currently of relevance (but at least side by side with
the respective comment at the top of the file)?

> @@ -210,6 +220,49 @@ void perform_tests(void)
>  return;
>  }
>  
> +/* Check that tests does not use memory where modules are stored */
> +if ( check_test_overlap((uint32_t)hvm_start_info, 
> sizeof(hvm_start_info)) )

sizeof(*hvm_start_info) perhaps?

> +{
> +printf("Skipping tests due to memory used by hvm_start_info\n");
> +return;
> +}
> +if ( check_test_overlap(hvm_start_info->modlist_paddr,
> +hvm_start_info->nr_modules *
> +  sizeof(struct hvm_modlist_entry)) )
> +{
> +printf("Skipping tests due to memory used by"
> +   " hvm_start_info->modlist\n");
> +return;
> +}
> +for ( i = 0; i < hvm_start_info->nr_modules; i++ )
> +{
> +const struct hvm_modlist_entry *modlist =
> +(struct hvm_modlist_entry 
> *)(uint32_t)hvm_start_info->modlist_paddr;

To cut done on the length of such lines, perhaps better to use void *
in casts like this?

> +uint64_t cmdline_paddr = modlist[i].cmdline_paddr;
> +
> +if ( check_test_overlap(modlist[i].paddr, modlist[i].size) )
> +{
> +printf("Skipping tests due to memory used by module[%d]\n", i);
> +return;
> +}
> +if ( cmdline_paddr && cmdline_paddr < UINT_MAX &&
> + check_test_overlap(cmdline_paddr,
> +strlen((char*)(uint32_t)cmdline_paddr)) )

As said on the previous patch - cmdline_paddr being below 4Gb
doesn't necessarily mean the string not crossing that boundary.
Depending on the resolution for that other patch this may need
adjustment.

Also, thinking about it again, the use of UINT_MAX for bounding
pointers is unfortunate: I think this would better be UINTPTR_MAX.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 1/2] xen: sched: rtds refactor code

[Dario Faggioli]

On Thu, 2016-06-23 at 11:42 +0100, George Dunlap wrote:
> On 22/06/16 17:16, Meng Xu wrote:
> > 
> > I think he is trying to align those comments to make them start
> > from
> > the same column. I was confused at the reason at the very
> > beginning.
> > Then I pulled his repo and checked this change.
> Right -- well neither you as a reviewer nor anyone in the future
> looking
> back at this changeset should have to try to guess what the purpose
> was;
> if he did want to align them, that's perfectly fine, it just needs a
> brief mention in the changelog. :-)
> 
Indeed. BTW, I don't recall if we discussed this alignment
previously,neither, in case we did, what my position was back then :-P

In any case, I (now) think that having these comments aligned on a
per-struct base is just fine, and there really is no need to have _all_
of them aligned, across all structs.

I don't have a super strong opinion on this, and I'd be fine with it,
if Meng is. I just think it's not worth the effort (of patching,
reviewing, checking in, etc.)

Regards,
Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)



signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 10/14] hvmloader: Load SeaBIOS from hvm_start_info modules

[Jan Beulich]

>>> On 22.06.16 at 19:15,  wrote:
> ... and do not include the SeaBIOS ROM into hvmloader anymore.
> 
> This also fix the dependency on roms.inc, hvmloader.o does not include it.
> 
> Signed-off-by: Anthony PERARD 

Acked-by: Jan Beulich 


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v6 5/5] x86/vm_event: Add HVM debug exception vm_events

[Razvan Cojocaru]

On 06/23/2016 08:07 PM, Tamas K Lengyel wrote:
> Since in-guest debug exceptions are now unconditionally trapped to Xen, adding
> a hook for vm_event subscribers to tap into this new always-on guest event. We
> rename along the way hvm_event_breakpoint_type to hvm_event_type to better
> match the various events that can be passed with it. We also introduce the
> necessary monitor_op domctl's to enable subscribing to the events.
> 
> This patch also provides monitor subscribers to int3 events proper access
> to the instruction length necessary for accurate event-reinjection. Without
> this subscribers manually have to evaluate if the int3 instruction has any
> prefix attached which would change the instruction length.
> 
> Signed-off-by: Tamas K Lengyel 
> ---
> Cc: Ian Jackson 
> Cc: Wei Liu 
> Cc: Razvan Cojocaru 
> Cc: Jan Beulich 
> Cc: Andrew Cooper 
> Cc: Jun Nakajima 
> Cc: Kevin Tian 
> 
> v6: Add comment describing rc condition values for the monitor call
> v5: Transmit the proper insn_length for int3 events as well to fix
>  the current bug with prefixed int3 instructions.
> ---
>  tools/libxc/include/xenctrl.h   |  3 +-
>  tools/libxc/xc_monitor.c| 25 +++
>  tools/tests/xen-access/xen-access.c | 63 
> -
>  xen/arch/x86/hvm/monitor.c  | 21 +++--
>  xen/arch/x86/hvm/vmx/vmx.c  | 55 +---
>  xen/arch/x86/monitor.c  | 16 ++
>  xen/include/asm-x86/domain.h|  2 ++
>  xen/include/asm-x86/hvm/monitor.h   |  7 +++--
>  xen/include/asm-x86/monitor.h   |  3 +-
>  xen/include/public/domctl.h |  6 
>  xen/include/public/vm_event.h   | 14 +++--
>  11 files changed, 186 insertions(+), 29 deletions(-)
> 
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index 4f5d954..4a85b4a 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -2165,7 +2165,8 @@ int xc_monitor_software_breakpoint(xc_interface *xch, 
> domid_t domain_id,
> bool enable);
>  int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id,
>   bool enable, bool sync);
> -
> +int xc_monitor_debug_exceptions(xc_interface *xch, domid_t domain_id,
> +bool enable, bool sync);
>  /**
>   * This function enables / disables emulation for each REP for a
>   * REP-compatible instruction.
> diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c
> index 78131b2..264992c 100644
> --- a/tools/libxc/xc_monitor.c
> +++ b/tools/libxc/xc_monitor.c
> @@ -156,3 +156,28 @@ int xc_monitor_emulate_each_rep(xc_interface *xch, 
> domid_t domain_id,
>  
>  return do_domctl(xch, &domctl);
>  }
> +
> +int xc_monitor_debug_exceptions(xc_interface *xch, domid_t domain_id,
> +bool enable, bool sync)
> +{
> +DECLARE_DOMCTL;
> +
> +domctl.cmd = XEN_DOMCTL_monitor_op;
> +domctl.domain = domain_id;
> +domctl.u.monitor_op.op = enable ? XEN_DOMCTL_MONITOR_OP_ENABLE
> +: XEN_DOMCTL_MONITOR_OP_DISABLE;
> +domctl.u.monitor_op.event = XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION;
> +domctl.u.monitor_op.u.debug_exception.sync = sync;
> +
> +return do_domctl(xch, &domctl);
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/tools/tests/xen-access/xen-access.c 
> b/tools/tests/xen-access/xen-access.c
> index f26e723..e615476 100644
> --- a/tools/tests/xen-access/xen-access.c
> +++ b/tools/tests/xen-access/xen-access.c
> @@ -53,6 +53,10 @@
>  #define ERROR(a, b...) fprintf(stderr, a "\n", ## b)
>  #define PERROR(a, b...) fprintf(stderr, a ": %s\n", ## b, strerror(errno))
>  
> +/* From xen/include/asm-x86/processor.h */
> +#define X86_TRAP_DEBUG  1
> +#define X86_TRAP_INT3   3
> +
>  typedef struct vm_event {
>  domid_t domain_id;
>  xenevtchn_handle *xce_handle;
> @@ -333,7 +337,7 @@ void usage(char* progname)
>  {
>  fprintf(stderr, "Usage: %s [-m]  write|exec", progname);
>  #if defined(__i386__) || defined(__x86_64__)
> -fprintf(stderr, "|breakpoint|altp2m_write|altp2m_exec");
> +fprintf(stderr, "|breakpoint|altp2m_write|altp2m_exec|debug");
>  #endif
>  fprintf(stderr,
>  "\n"
> @@ -354,10 +358,12 @@ int main(int argc, char *argv[])
>  xc_interface *xch;
>  xenmem_access_t default_access = XENMEM_access_rwx;
>  xenmem_access_t after_first_access = XENMEM_access_rwx;
> +int memaccess = 0;
>  int required = 0;
>  int breakpoint = 0;
>  int shutting_down = 0;
>  int altp2m = 0;
> +int debug = 0;
>  uint16_t altp2m_view_id = 0;
>  
>  char* progname = argv[0];
> @@ -391,11 +397,13 @@ int main(int argc, char *argv[])
>  {
> 

Re: [Xen-devel] [PATCH 0/3] VMX: Properly handle pi descriptor and per-cpu blocking list

[Wu, Feng]

> -Original Message-
> From: Dario Faggioli [mailto:dario.faggi...@citrix.com]
> Sent: Friday, June 24, 2016 3:23 PM
> To: Wu, Feng ; Jan Beulich 
> Cc: Tian, Kevin ; k...@xen.org;
> george.dun...@eu.citrix.com; andrew.coop...@citrix.com; xen-
> de...@lists.xen.org
> Subject: Re: [Xen-devel] [PATCH 0/3] VMX: Properly handle pi descriptor and
> per-cpu blocking list
> 
> On Fri, 2016-06-24 at 06:11 +, Wu, Feng wrote:
> > > -Original Message-
> > > From: Dario Faggioli [mailto:dario.faggi...@citrix.com]
> > > No, because we call cpu_disable_scheduler() from __cpu_disable(),
> > > only
> > > when system state is SYS_STATE_suspend already, and hence we take
> > > the
> > > then branch of the 'if', which does never return an error.
> > Thanks for the elaboration. I find __cpu_disable() can be called with
> > system state not being SYS_STATE_suspend. Here is my experiment:
> >
> > 1. Launch a guest and pin vCPU 3 to pCPU 3 (which makes the
> > experiment simpler)
> > 2. offline pCPU 3 via "xen-hptool cpu-offline 3"
> >
> Ah, yes, of course. I should have included cpu-hot(un)plug in my
> analysis in the first place, sorry for not doing so.
> 
> > The call path of the above steps is:
> > arch_do_sysctl()
> >   -> cpu_down_helper()
> > -> cpu_down()
> >   -> take_cpu_down()
> > -> __cpu_disable()
> >   -> cpu_disable_scheduler() (enter the 'else'  part)
> >
> Right, and the important part is this one:
> 
> > int stop_machine_run(int (*fn)(void *), void *data, unsigned int cpu)
> > {
> >
> > ..
> >
> > point 1
> >
> > for_each_cpu ( i, &allbutself )
> > tasklet_schedule_on_cpu(&per_cpu(stopmachine_tasklet, i), i);
> >
> > point 2
> > ..
> >
> > }
> >
> > at point 1 above,
> > vCPU3->runstate.state: 0, vCPU3->is_running: 1
> > while at point 2 above:
> > vCPU3->runstate.state: 1, vCPU3->is_running: 0
> >
> This is exactly as you describe. cpu hotplug is done in stop machine
> context. Check the comment close to the definition of stop_machine_run:
> 
> /**
>  * stop_machine_run: freeze the machine on all CPUs and run this function
>  * @fn: the function to run
>  * @data: the data ptr for the @fn()
>  * @cpu: the cpu to run @fn() on (or all, if @cpu == NR_CPUS).
>  *
>  * Description: This causes every other cpu to enter a safe point, with
>  * each of which disables interrupts, and finally interrupts are disabled
>  * on the current CPU.  The result is that none is holding a spinlock
>  * or inside any other preempt-disabled region when @fn() runs.
>  *
>  * This can be thought of as a very heavy write lock, equivalent to
>  * grabbing every spinlock in the kernel. */
> 
> As you discovered yourself, this is achieved by forcing the execution
> of a tasklet on all the pcpus, which include pCPU 3 of your example.
> 
> So, vCPU 3 was running, but then some called stop_machine_run(), which
> causes the descheduling of vCPU 3, and the execution of the stopmachine
> tasklet.

Thanks for your replay. Yes, I think this is point. Here descheduling of vCPU3
happens, and the reason we will choose the tasklet as the next running
unit for sure (not choosing another vCPU or vCPU3 itself as the next
running unit) is because tasklet will overrides all other choose as
stated in csched_schedule() as below, right?

static struct task_slice
csched_schedule(
const struct scheduler *ops, s_time_t now, bool_t tasklet_work_scheduled) 
{
..

snext = __runq_elem(runq->next);
ret.migrated = 0;

/* Tasklet work (which runs in idle VCPU context) overrides all else. */
if ( tasklet_work_scheduled )
{
TRACE_0D(TRC_CSCHED_SCHED_TASKLET);
snext = CSCHED_VCPU(idle_vcpu[cpu]);
snext->pri = CSCHED_PRI_TS_BOOST;
}


..
}

Thanks,
Feng
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 3/3] x86/ioreq server: Add HVMOP to map guest ram with p2m_ioreq_server to an ioreq server.

[Jan Beulich]

>>> On 24.06.16 at 09:12,  wrote:
> On 6/24/2016 2:12 PM, Jan Beulich wrote:
>> In any event, I think log-dirty shouldn't be disabled when an
>> ioreq server binds the type, but as long as there are outstanding
>> entries of that type. That way, the "cannot be migrated" state
>> of a VM has a chance to clear.
> 
> Do you mean to disable log dirty by checking if there's outstanding 
> p2m_ioreq_server
> entries instead of the p2m->ioreq.server? How about we check both? 
> Because only
> checking the count of outstanding p2m_ioreq_server may prevent the live 
> migration
> when an ioreq server is unbound, but with p2m type not entirely synced.

Checking both would limit things further, which seems to
contradict your intention of relaxing things. What may be a
reasonable combination is to check for a registered ioreq server
when enabling log-dirty, and for outstanding pages when
registering a new ioreq server. Yet then again it feels like we're
moving in circles: Didn't we mean de-registration to fail when
there are outstanding pages? In which case checking for a
registered server would be slightly more tight than checking for
outstanding pages: The server could have removed all pages,
but not de-registered, which would - afaict - still allow log-dirty
to function (of course new registration of pages would need to
fail until log-dirty got disabled again).

>> And then, thinking about it again especially in the context of
>> the hvmctl series - the unbinding of the type is happening in a
>> hypercall with built-in preemption capability. Hence there's not
>> really an issue with how long that conversion may take, as
>> long as there's no need to pause the guest for that time period.
>> Which means you could first initiate conversion via the
>> misconfig mechanism, but then immediately go ahead and walk
>> the entire guest address space (or the relevant part of it, if
>> the bounds got tracked) with continuations used as necessary.
> 
> Sorry, what's the misconfig mechanism good for if I still need to sweep 
> the entire
> p2m table immediately?

To avoid the need to pause the guest for an extended time: At
least between scheduling a continuation and executing it, the
guest could happily run (and perhaps cause some of the
otherwise synchronous - to the hypercall - work to get carried
out already, with the involved execution time accounted to the
guest instead of the host).

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 3/4] VMX: Assign the right value to 'NDST' field in a concern case

[Wu, Feng]

> -Original Message-
> From: dunl...@gmail.com [mailto:dunl...@gmail.com] On Behalf Of George
> Dunlap
> Sent: Thursday, June 23, 2016 2:01 AM
> To: Wu, Feng 
> Cc: xen-devel@lists.xen.org; Tian, Kevin ; Keir Fraser
> ; Andrew Cooper ; Dario Faggioli
> ; Jan Beulich 
> Subject: Re: [Xen-devel] [PATCH v2 3/4] VMX: Assign the right value to 'NDST'
> field in a concern case
> 
> On Thu, May 26, 2016 at 2:39 PM, Feng Wu  wrote:
> > Normally, in vmx_cpu_block() 'NDST' filed should have the same
> > value with 'dest' or 'MASK_INSR(dest, PI_xAPIC_NDST_MASK)' depending
> > on whether x2apic is enabled. However, in the following scenario,
> > 'NDST' has different value:
> >
> > 'vcpu_block' hook gets assigned in vmx_pi_hooks_assign(), but all
> > other three PI hooks have not been assigned or not been excuted yet.
> > And during this interval, we are running in vmx_vcpu_block(), then
> > 'NDST' may have different value.
> >
> > This patch fix this concern case.
> >
> > Signed-off-by: Feng Wu 
> 
> I agree with Jan that a cleaner solution here would be making sure
> that all the appropriate state is actually set up for all vcpus before
> leaving vmx_pi_hooks_assign().  With the patch you propose, the
> following sequence of events is possible:
> 
> * vcpu 0 starts running on a pcpu
> * a device is assigned, causing the hooks to be set
> * an interrupt from the device is routed to vcpu 0, but it is not
> actually delivered properly, since ndst is not pointing to the right
> processor.
> 
> One option would be to pause all vcpus before setting the hooks and
> then un-pause them; this would force all the vcpus to go through
> vmx_pi_switch_to() before vmx_vcpu_block().  Another would be to grab
> the scheduler lock for each pcpu and write the vcpu's ndst with the
> appropriate value before setting the hooks.

That sounds a great idea. Besides that, maybe we can also pause/unpause
the domain before/after unsetting the hooks, then we don't need to
care about the race condition when vmx_pi_hooks_deassign() and
vmx_vcpu_block() get called at the same time. After unpause the domain,
we can safely remove the vCPUs from the per-cpu blocking list if needed.

Thanks,
Feng

> 
>  -George
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v3 1/2] xen-pciback: return proper values during BAR sizing

[Jan Beulich]

Reads following writes with all address bits set to 1 should return all
changeable address bits as one, not the BAR size (nor, as was the case
for the upper half of 64-bit BARs, the high half of the region's end
address). Presumably this didn't cause any problems so far because
consumers use the value to calculate the size (usually via val & -val),
and do nothing else with it.

But also consider the exception here: Unimplemented BARs should always
return all zeroes.

And finally, the check for whether to return the sizing address on read
for the ROM BAR should ignore all non-address bits, not just the ROM
Enable one.

Signed-off-by: Jan Beulich 
Reviewed-by: Boris Ostrovsky 
---
v3: Use ~0U in rom_write(), to account for PCI_ROM_ADDRESS_MASK being
of unsigned long type (relevant on 64-bit). (Note: Patch 2 is
unchanged, and hence not being re-sent. I hope that, despite this
being a bug fix from v2, retaining the R-b is okay.)
---
 drivers/xen/xen-pciback/conf_space_header.c |   18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

--- 4.7-rc4-xen-pciback-BAR.orig/drivers/xen/xen-pciback/conf_space_header.c
+++ 4.7-rc4-xen-pciback-BAR/drivers/xen/xen-pciback/conf_space_header.c
@@ -145,7 +145,7 @@ static int rom_write(struct pci_dev *dev
/* A write to obtain the length must happen as a 32-bit write.
 * This does not (yet) support writing individual bytes
 */
-   if (value == ~PCI_ROM_ADDRESS_ENABLE)
+   if ((value | ~PCI_ROM_ADDRESS_MASK) == ~0U)
bar->which = 1;
else {
u32 tmpval;
@@ -225,38 +225,42 @@ static inline void read_dev_bar(struct p
   (PCI_BASE_ADDRESS_SPACE_MEMORY |
PCI_BASE_ADDRESS_MEM_TYPE_64))) {
bar_info->val = res[pos - 1].start >> 32;
-   bar_info->len_val = res[pos - 1].end >> 32;
+   bar_info->len_val = -resource_size(&res[pos - 1]) >> 32;
return;
}
}
 
+   if (!res[pos].flags ||
+   (res[pos].flags & (IORESOURCE_DISABLED | IORESOURCE_UNSET |
+  IORESOURCE_BUSY)))
+   return;
+
bar_info->val = res[pos].start |
(res[pos].flags & PCI_REGION_FLAG_MASK);
-   bar_info->len_val = resource_size(&res[pos]);
+   bar_info->len_val = -resource_size(&res[pos]) |
+   (res[pos].flags & PCI_REGION_FLAG_MASK);
 }
 
 static void *bar_init(struct pci_dev *dev, int offset)
 {
-   struct pci_bar_info *bar = kmalloc(sizeof(*bar), GFP_KERNEL);
+   struct pci_bar_info *bar = kzalloc(sizeof(*bar), GFP_KERNEL);
 
if (!bar)
return ERR_PTR(-ENOMEM);
 
read_dev_bar(dev, bar, offset, ~0);
-   bar->which = 0;
 
return bar;
 }
 
 static void *rom_init(struct pci_dev *dev, int offset)
 {
-   struct pci_bar_info *bar = kmalloc(sizeof(*bar), GFP_KERNEL);
+   struct pci_bar_info *bar = kzalloc(sizeof(*bar), GFP_KERNEL);
 
if (!bar)
return ERR_PTR(-ENOMEM);
 
read_dev_bar(dev, bar, offset, ~PCI_ROM_ADDRESS_ENABLE);
-   bar->which = 0;
 
return bar;
 }




___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH] init: fix build with older gcc

[Jan Beulich]

__setup_str is used on arrays of char, so there aren't any relocatable
items. Hence __initconst has to be used here, not __initconstrel.

Whatever led to the revert of commit 59b151d2c0 (the original version
of a6066af5b1 "xen/init: Annotate all command line parameter
infrastructure as const" must have got addressed meanwhile - with the
patch here I can't see that old gcc (4.3ish) report a section type
conflict anymore.

Signed-off-by: Jan Beulich 

--- a/xen/include/xen/init.h
+++ b/xen/include/xen/init.h
@@ -88,7 +88,7 @@ struct kernel_param {
 
 extern const struct kernel_param __setup_start[], __setup_end[];
 
-#define __setup_str static const  __initconstrel \
+#define __setup_str static const __initconst \
 __attribute__((__aligned__(1))) char
 #define __kparam static const __initsetup \
 __attribute__((__aligned__(sizeof(void * struct kernel_param



init: fix build with older gcc

__setup_str is used on arrays of char, so there aren't any relocatable
items. Hence __initconst has to be used here, not __initconstrel.

Whatever led to the revert of commit 59b151d2c0 (the original version
of a6066af5b1 "xen/init: Annotate all command line parameter
infrastructure as const" must have got addressed meanwhile - with the
patch here I can't see that old gcc (4.3ish) report a section type
conflict anymore.

Signed-off-by: Jan Beulich 

--- a/xen/include/xen/init.h
+++ b/xen/include/xen/init.h
@@ -88,7 +88,7 @@ struct kernel_param {
 
 extern const struct kernel_param __setup_start[], __setup_end[];
 
-#define __setup_str static const  __initconstrel \
+#define __setup_str static const __initconst \
 __attribute__((__aligned__(1))) char
 #define __kparam static const __initsetup \
 __attribute__((__aligned__(sizeof(void * struct kernel_param
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 7/7] vm-event/arm: implement support for control-register write vm-events

[Corneliu ZUZU]

On 6/23/2016 2:11 PM, Julien Grall wrote:

Hello,

On 23/06/16 06:49, Corneliu ZUZU wrote:

On 6/23/2016 8:31 AM, Corneliu ZUZU wrote:

On 6/22/2016 10:41 PM, Julien Grall wrote:



On 22/06/2016 20:37, Corneliu ZUZU wrote:

I've also realized that it's a bit complicated to avoid writing HCR
from
2 places.
That's because:
- p2m_restore_state is part of the process of switching to another 
vCPU

and the HCR write _must be committed_ here because other components
depend on that, like address-translation functions
- I want vm_event_vcpu_enter to be called _after_ the switch to the
vCPU
is completed
- I want HCR_TVM to be set in vm_event_vcpu_enter because setting
necessary traps _for cr vm-events_ to work should be done there
(setting
HCR_TVM bit makes sense to be there and the purpose is to centralize
operations such as this for code comprehensibility; also, on the X86
counterpart a similar operation is done for trapping CR3, so it
would be
nice to keep the symmetry)

Would it be such a stretch to have HCR written in 2 places? (the 
second

time happens rarely anyway: it's unlikely(..) to have to do the
write in
vm_event_vcpu_enter)


Not really. It was mostly to avoid setting/clearing HCR bits in
different place in the code. It makes more difficult to know what is
the final result of the register.

Anyway, let's skip it for now, if it is too difficult.

Regards,



Then perhaps something like the following would be suitable:

1. store hcr in arch_domain (register_t hcr)

2. add a function in asm-arm/processor.h (or where else?) which only
does:
static inline void update_hcr(struct domain *d)
{
WRITE_SYSREG(d->arch.hcr, HCR_EL2);
isb();
}

3.  modify p2m_restore_state to do:
n->domain->arch.hcr &= ~HCR_VM;
update_hcr(n->domain);
p2m_load_VTTBR(n->domain);

n->domain->arch.hcr |= HCR_VM;

if ( is_32bit_domain(n->domain) )
n->domain->arch.hcr &= ~HCR_RW;
else
n->domain->arch.hcr |= HCR_RW;


This is not safe at all, p2m_restore_state is vCPU specific at you 
modify domain information.


However, if we store the hcr per domain, overriding every context 
switch is pointless as the domain will always be 32-bit/64-bit.


Oh right, the RW bit needs not be set/unset anymore with this change.





update_hcr(n->domain);

WRITE_SYSREG(n->arch.sctlr, SCTLR_EL1);
isb();

4. and vcpu_enter_adjust_traps to

if ( unlikely(0 != v->domain->arch.monitor.write_ctrlreg_enabled) )
{
 if ( likely(v->domain->arch.hcr & HCR_TVM) )
 return;
 v->domain->arch.hcr |= HCR_TVM;
}
else
{
 if ( likely(!(v->domain->arch.hcr & HCR_TVM)) )
 return;
 v->domain->arch.hcr &= ~HCR_TVM;
}


This does not need to be done in vcpu_enter_adjust_traps everytime. 
You can set the bit in arch.hcr in DOMCTL_MONITOR_EVENT_WRITE_CTRLREG.


I wanted to keep X86-ARM symmetry and it seemed more intuitive to have 
these kind of adjustments with the vcpu_enter code motion. But now that 
I think about it, given the fact that we have the guarantee that after 
monitor_domctl and before reentering the vCPU p2m_restore_state gets 
called (due to domain_pause/domain_unpause) - thus actually committing 
the hcr update at the proper time - technically monitor_domctl _is_ the 
optimal place to set arch.hcr in. In conclusion, I'm thinking of 
discarding the entire idea of introducing vm_event_vcpu_enter, it seems 
to me now that this would also render a simpler code.






update_hcr(v->domain);

That way at least it's easier to follow where update_hcr is called.


I don't see much reason to store the value in the domain and have 
multiple update_hcr. If we store the value, then we should only call 
update_hcr once when returning to the guest.


Yep, that will happen with the above-mentioned discarding of 
vm_event_vcpu_enter idea.





And also set the initial value of HCR at the moment of creation, i.e. in
arch_domain_create as

d->arch.hcr = READ_SYSREG(HCR_EL2)


We control the value of HCR_EL2, so it would be better to assign the 
list of flags here.


Right, that will happen too.



Regards,



Thanks for the useful insights,
Corneliu.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] init: fix build with older gcc

[Andrew Cooper]

On 24/06/16 10:29, Jan Beulich wrote:
> __setup_str is used on arrays of char, so there aren't any relocatable
> items. Hence __initconst has to be used here, not __initconstrel.
>
> Whatever led to the revert of commit 59b151d2c0 (the original version
> of a6066af5b1 "xen/init: Annotate all command line parameter
> infrastructure as const" must have got addressed meanwhile - with the
> patch here I can't see that old gcc (4.3ish) report a section type
> conflict anymore.
>
> Signed-off-by: Jan Beulich 

Acked-by: Andrew Cooper 

>
> --- a/xen/include/xen/init.h
> +++ b/xen/include/xen/init.h
> @@ -88,7 +88,7 @@ struct kernel_param {
>  
>  extern const struct kernel_param __setup_start[], __setup_end[];
>  
> -#define __setup_str static const  __initconstrel \
> +#define __setup_str static const __initconst \
>  __attribute__((__aligned__(1))) char
>  #define __kparam static const __initsetup \
>  __attribute__((__aligned__(sizeof(void * struct kernel_param
>
>
>


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [linux-4.1 test] 96183: regressions - FAIL

[osstest service owner]

flight 96183 linux-4.1 real [real]
http://logs.test-lab.xenproject.org/osstest/logs/96183/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-xl  11 guest-start   fail REGR. vs. 95848

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-libvirt 11 guest-start fail pass in 96160
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail 
pass in 96160

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds 11 guest-start   fail REGR. vs. 95848
 test-armhf-armhf-xl-credit2  11 guest-start   fail in 96160 like 95591
 test-armhf-armhf-xl-multivcpu 15 guest-start/debian.repeat fail in 96160 like 
95848
 test-armhf-armhf-xl-xsm  15 guest-start/debian.repeatfail   like 95763
 test-armhf-armhf-xl-credit2  15 guest-start/debian.repeatfail   like 95818
 build-amd64-rumpuserxen   6 xen-buildfail   like 95848
 build-i386-rumpuserxen6 xen-buildfail   like 95848
 test-armhf-armhf-xl-cubietruck 15 guest-start/debian.repeatfail like 95848
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 95848
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 95848
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail like 95848
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 95848
 test-armhf-armhf-xl-vhd   9 debian-di-installfail   like 95848

Tests which did not succeed, but are not blocking:
 test-amd64-i386-rumpuserxen-i386  1 build-check(1)   blocked  n/a
 test-amd64-amd64-rumpuserxen-amd64  1 build-check(1)   blocked n/a
 test-armhf-armhf-libvirt 14 guest-saverestore fail in 96160 never pass
 test-armhf-armhf-libvirt 12 migrate-support-check fail in 96160 never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail in 96160 never pass
 test-amd64-amd64-xl-pvh-intel 14 guest-saverestorefail  never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw  9 debian-di-installfail   never pass

version targeted for testing:
 linux95123c0b81d9478b8155fe15093b88f57ef7d0bd
baseline version:
 linux888172862fa78505c4e4674c205a06586443d83f

Last test of basis95848  2016-06-17 05:58:53 Z7 days
Testing same since96160  2016-06-23 05:07:18 Z1 days2 attempts


People who touched revisions under test:
  "Eric W. Biederman" 
  AceLan Kao 
  Al Viro 
  Andy Lutomirski 
  Arnd Bergmann 
  Ben Dooks 
  Ben Skeggs 
  Bob Copeland 
  Borislav Petkov 
  Chris Wilson 
  Dibyajyoti Ghosh 
  Eduardo Valentin 
  Eric W. Biederman 
  Ewan D. Milne 
  Fred Veldini 
  Gavin Shan 
  H. Peter Anvin 
  Helge Deller 
  Herbert Xu 
  Hongkun Cao 
  hongkun.cao 
  Ilia Mirkin 
  Ingo Molnar 
  Jack Wang 
  James Bottomley 
  James E.J. Bottomley 
  Jan Stancek 
  Jann Horn 
  Javi Merino 
  Johannes Berg 
  Kailang Yang 
  Linus Torvalds 
  Linus Walleij 
  Lukasz Luba 
  Marc 

Re: [Xen-devel] [PATCH v4 3/3] x86/ioreq server: Add HVMOP to map guest ram with p2m_ioreq_server to an ioreq server.

[Yu Zhang]

On 6/24/2016 4:01 PM, Jan Beulich wrote:

On 24.06.16 at 09:12,  wrote:

On 6/24/2016 2:12 PM, Jan Beulich wrote:

In any event, I think log-dirty shouldn't be disabled when an
ioreq server binds the type, but as long as there are outstanding
entries of that type. That way, the "cannot be migrated" state
of a VM has a chance to clear.

Do you mean to disable log dirty by checking if there's outstanding
p2m_ioreq_server
entries instead of the p2m->ioreq.server? How about we check both?
Because only
checking the count of outstanding p2m_ioreq_server may prevent the live
migration
when an ioreq server is unbound, but with p2m type not entirely synced.

Checking both would limit things further, which seems to
contradict your intention of relaxing things. What may be a
reasonable combination is to check for a registered ioreq server
when enabling log-dirty, and for outstanding pages when
registering a new ioreq server. Yet then again it feels like we're
moving in circles: Didn't we mean de-registration to fail when
there are outstanding pages? In which case checking for a
registered server would be slightly more tight than checking for
outstanding pages: The server could have removed all pages,
but not de-registered, which would - afaict - still allow log-dirty
to function (of course new registration of pages would need to
fail until log-dirty got disabled again).


OK. To disable logdirty, the judgement could be based on the existence of
outstanding p2m_ioreq_sever entries.


And then, thinking about it again especially in the context of
the hvmctl series - the unbinding of the type is happening in a
hypercall with built-in preemption capability. Hence there's not
really an issue with how long that conversion may take, as
long as there's no need to pause the guest for that time period.
Which means you could first initiate conversion via the
misconfig mechanism, but then immediately go ahead and walk
the entire guest address space (or the relevant part of it, if
the bounds got tracked) with continuations used as necessary.

Sorry, what's the misconfig mechanism good for if I still need to sweep
the entire
p2m table immediately?

To avoid the need to pause the guest for an extended time: At
least between scheduling a continuation and executing it, the
guest could happily run (and perhaps cause some of the
otherwise synchronous - to the hypercall - work to get carried
out already, with the involved execution time accounted to the
guest instead of the host).



By "scheduling a continuation and executing it", do you mean code like
hypercall_create_continuation? I'll need to study on this part. But one
difference I can imagine is that the hypercall_create_continuation is of
uncompleted hypercalls I guess, yet the p2m table sweeping may only
be a side effect of the unbound hypercall here.
I may have some misunderstandings here, so please correct me if so,
and I'll have try some investigation at the same time. Thanks!

Yu


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] Xen on QEMU

[George Dunlap]

On Mon, Jun 20, 2016 at 9:00 AM, Fadwa Messaoudi
 wrote:
> Hi,
> is it possible to install Xen on QEMU , and if so could you send me a
> details tutorial on how to do it.

Did you try it?

Also, you might want to read this:

http://wiki.xenproject.org/wiki/Asking_Developer_Questions

Good luck,
 -George

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2 00/11] hvmctl hypercall

[Jan Beulich]

A long while back separating out all control kind operations (intended
for use by only the control domain or device model) from the currect
hvmop hypercall has been discussed. This series aims at finally making
this reality (at once allowing to streamline the associated XSM checking).

01: public / x86: introduce hvmctl hypercall
02: convert HVMOP_set_pci_intx_level
03: convert HVMOP_set_isa_irq_level
04: convert HVMOP_set_pci_link_route
05: convert HVMOP_track_dirty_vram
06: convert HVMOP_modified_memory
07: convert HVMOP_set_mem_type
08: convert HVMOP_inject_trap
09: convert HVMOP_inject_msi
10: convert HVMOP_*ioreq_server*
11: x86/HVM: serialize trap injecting producer and consumer

Signed-off-by: Jan Beulich 
---
v2: Largely just addressing review comments for patch 1 (see there),
but those required mechanical adjustments to most other patches.


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v1 Altp2m cleanup 2/3] Move altp2m specific functions to altp2m files.

[George Dunlap]

On Tue, Jun 21, 2016 at 5:04 PM, Paul Lai  wrote:
> Move altp2m specific functions to altp2m files.  This makes the code
> a little easier to read.
>
> Signed-off-by: Paul Lai 
> ---
>  xen/arch/x86/mm/altp2m.c  | 43 
> +++
>  xen/arch/x86/mm/hap/hap.c | 35 +--
>  xen/arch/x86/mm/p2m-ept.c | 38 ++
>  xen/arch/x86/mm/p2m.c | 41 +
>  xen/include/asm-x86/altp2m.h  |  3 ++-
>  xen/include/asm-x86/hvm/vmx/vmx.h |  3 +++
>  xen/include/asm-x86/p2m.h |  9 +++-

You forgot to CC' the MM maintainer (me).  :-)

Please see MAINTAINERS file, and/or use get-maintainers.pl in the
future, or your patches risk dropping through the cracks.

(I've put this on my list to review now.)

 -George

>  7 files changed, 95 insertions(+), 77 deletions(-)
>
> diff --git a/xen/arch/x86/mm/altp2m.c b/xen/arch/x86/mm/altp2m.c
> index 10605c8..1caf6b4 100644
> --- a/xen/arch/x86/mm/altp2m.c
> +++ b/xen/arch/x86/mm/altp2m.c
> @@ -17,6 +17,7 @@
>
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>
> @@ -65,6 +66,48 @@ altp2m_vcpu_destroy(struct vcpu *v)
>  vcpu_unpause(v);
>  }
>
> +int
> +hvm_altp2m_init( struct domain *d) {
> +int rv = 0;
> +unsigned int i = 0;
> +
> +/* Init alternate p2m data */
> +if ( (d->arch.altp2m_eptp = alloc_xenheap_page()) == NULL )
> +{
> +rv = -ENOMEM;
> +goto out;
> +}
> +
> +for ( i = 0; i < MAX_EPTP; i++ )
> +d->arch.altp2m_eptp[i] = INVALID_MFN;
> +
> +for ( i = 0; i < MAX_ALTP2M; i++ )
> +{
> +rv = p2m_alloc_table(d->arch.altp2m_p2m[i]);
> +if ( rv != 0 )
> +   goto out;
> +}
> +
> +d->arch.altp2m_active = 0;
> + out:
> +return rv;
> +}
> +
> +void
> +hvm_altp2m_teardown( struct domain *d) {
> +unsigned int i = 0;
> +d->arch.altp2m_active = 0;
> +
> +if ( d->arch.altp2m_eptp )
> +{
> +free_xenheap_page(d->arch.altp2m_eptp);
> +d->arch.altp2m_eptp = NULL;
> +}
> +
> +for ( i = 0; i < MAX_ALTP2M; i++ )
> +p2m_teardown(d->arch.altp2m_p2m[i]);
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
> index 9c2cd49..07833b7 100644
> --- a/xen/arch/x86/mm/hap/hap.c
> +++ b/xen/arch/x86/mm/hap/hap.c
> @@ -37,6 +37,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -501,24 +502,9 @@ int hap_enable(struct domain *d, u32 mode)
>
>  if ( hvm_altp2m_supported() )
>  {
> -/* Init alternate p2m data */
> -if ( (d->arch.altp2m_eptp = alloc_xenheap_page()) == NULL )
> -{
> -rv = -ENOMEM;
> -goto out;
> -}
> -
> -for ( i = 0; i < MAX_EPTP; i++ )
> -d->arch.altp2m_eptp[i] = INVALID_MFN;
> -
> -for ( i = 0; i < MAX_ALTP2M; i++ )
> -{
> -rv = p2m_alloc_table(d->arch.altp2m_p2m[i]);
> -if ( rv != 0 )
> -   goto out;
> -}
> -
> -d->arch.altp2m_active = 0;
> +rv = hvm_altp2m_init(d);
> +if ( rv != 0 )
> +   goto out;
>  }
>
>  /* Now let other users see the new mode */
> @@ -534,18 +520,7 @@ void hap_final_teardown(struct domain *d)
>  unsigned int i;
>
>  if ( hvm_altp2m_supported() )
> -{
> -d->arch.altp2m_active = 0;
> -
> -if ( d->arch.altp2m_eptp )
> -{
> -free_xenheap_page(d->arch.altp2m_eptp);
> -d->arch.altp2m_eptp = NULL;
> -}
> -
> -for ( i = 0; i < MAX_ALTP2M; i++ )
> -p2m_teardown(d->arch.altp2m_p2m[i]);
> -}
> +hvm_altp2m_teardown(d);
>
>  /* Destroy nestedp2m's first */
>  for (i = 0; i < MAX_NESTEDP2M; i++) {
> diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
> index 7166c71..dff34b1 100644
> --- a/xen/arch/x86/mm/p2m-ept.c
> +++ b/xen/arch/x86/mm/p2m-ept.c
> @@ -1329,6 +1329,44 @@ void setup_ept_dump(void)
>  register_keyhandler('D', ept_dump_p2m_table, "dump VT-x EPT tables", 0);
>  }
>
> +void p2m_init_altp2m_helper( struct domain *d, unsigned int i) {
> +struct p2m_domain *p2m = d->arch.altp2m_p2m[i];
> +struct ept_data *ept;
> +
> +p2m->min_remapped_gfn = INVALID_GFN;
> +p2m->max_remapped_gfn = 0;
> +ept = &p2m->ept;
> +ept->asr = pagetable_get_pfn(p2m_get_pagetable(p2m));
> +d->arch.altp2m_eptp[i] = ept_get_eptp(ept);
> +}
> +
> +unsigned int p2m_find_altp2m_by_eptp(struct domain *d, uint64_t eptp)
> +{
> +struct p2m_domain *p2m;
> +struct ept_data *ept;
> +unsigned int i;
> +
> +altp2m_list_lock(d);
> +
> +for ( i = 0; i < MAX_ALTP2M; i++ )
> +{
> +if ( d->arch.altp2m_eptp[i] == INVALID_MFN )
> +continue;
> +
> +p2m = d->arch.altp2m_p2m[i];
> +ept = &p2

Re: [Xen-devel] [PATCH v4 3/3] x86/ioreq server: Add HVMOP to map guest ram with p2m_ioreq_server to an ioreq server.

[Jan Beulich]

>>> On 24.06.16 at 11:57,  wrote:
> On 6/24/2016 4:01 PM, Jan Beulich wrote:
> On 24.06.16 at 09:12,  wrote:
>>> On 6/24/2016 2:12 PM, Jan Beulich wrote:
 And then, thinking about it again especially in the context of
 the hvmctl series - the unbinding of the type is happening in a
 hypercall with built-in preemption capability. Hence there's not
 really an issue with how long that conversion may take, as
 long as there's no need to pause the guest for that time period.
 Which means you could first initiate conversion via the
 misconfig mechanism, but then immediately go ahead and walk
 the entire guest address space (or the relevant part of it, if
 the bounds got tracked) with continuations used as necessary.
>>> Sorry, what's the misconfig mechanism good for if I still need to sweep
>>> the entire
>>> p2m table immediately?
>> To avoid the need to pause the guest for an extended time: At
>> least between scheduling a continuation and executing it, the
>> guest could happily run (and perhaps cause some of the
>> otherwise synchronous - to the hypercall - work to get carried
>> out already, with the involved execution time accounted to the
>> guest instead of the host).
> 
> By "scheduling a continuation and executing it", do you mean code like
> hypercall_create_continuation? I'll need to study on this part. But one
> difference I can imagine is that the hypercall_create_continuation is of
> uncompleted hypercalls I guess, yet the p2m table sweeping may only
> be a side effect of the unbound hypercall here.

Side effect or not, it'll take long and hence you can't do it in one go.
Hence the need for a continuation. We have at least one other such
example - see paging_domctl(). I don't think you'll need an auxiliary
new hypercall here, though.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 0/3] VMX: Properly handle pi descriptor and per-cpu blocking list

[Dario Faggioli]

On Fri, 2016-06-24 at 07:59 +, Wu, Feng wrote:
> > -Original Message-
> > From: Dario Faggioli [mailto:dario.faggi...@citrix.com]
> > So, vCPU 3 was running, but then some called stop_machine_run(),
> > which
> > causes the descheduling of vCPU 3, and the execution of the
> > stopmachine
> > tasklet.
> Thanks for your replay. Yes, I think this is point. Here descheduling
> of vCPU3
> happens, and the reason we will choose the tasklet as the next
> running
> unit for sure (not choosing another vCPU or vCPU3 itself as the next
> running unit) is because tasklet will overrides all other choose as
> stated in csched_schedule() as below, right?
> 
Exactly, tasklets preempt the running vcpu and take precedence of
runnable vcpus.

Then, in this case, the reason why we are sure that all the pcpus are
executing the body of the tasklet, is indeed the structure of
stop_machine_run() and stopmachine_action() themselves, which are built
to make sure of that, much rather than just the fact that tasklet are
higher priority.

But yes, this is what happens.

Regards,
Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)



signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2 01/11] public / x86: introduce hvmctl hypercall

[Jan Beulich]

... as a means to replace all HVMOP_* which a domain can't issue on
itself (i.e. intended for use by only the control domain or device
model).

Signed-off-by: Jan Beulich 
Reviewed-by: Wei Liu 
---
v2: Widen cmd field to 32 bits and opaque one to 64. Drop
HVMCTL_iter_*.

--- a/xen/arch/x86/hvm/Makefile
+++ b/xen/arch/x86/hvm/Makefile
@@ -2,6 +2,7 @@ subdir-y += svm
 subdir-y += vmx
 
 obj-y += asid.o
+obj-y += control.o
 obj-y += emulate.o
 obj-y += event.o
 obj-y += hpet.o
--- /dev/null
+++ b/xen/arch/x86/hvm/control.c
@@ -0,0 +1,82 @@
+/*
+ * control.c: Hardware virtual machine control operations.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; If not, see .
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+long do_hvmctl(XEN_GUEST_HANDLE_PARAM(xen_hvmctl_t) u_hvmctl)
+{
+xen_hvmctl_t op;
+struct domain *d;
+int rc;
+
+BUILD_BUG_ON(sizeof(op.u) > sizeof(op.u.pad));
+
+if ( copy_from_guest(&op, u_hvmctl, 1) )
+return -EFAULT;
+
+if ( op.interface_version != XEN_HVMCTL_INTERFACE_VERSION )
+return -EACCES;
+
+rc = rcu_lock_remote_domain_by_id(op.domain, &d);
+if ( rc )
+return rc;
+
+if ( !has_hvm_container_domain(d) )
+{
+rcu_unlock_domain(d);
+return -EINVAL;
+}
+
+rc = xsm_hvm_control(XSM_DM_PRIV, d, op.cmd);
+if ( rc )
+{
+rcu_unlock_domain(d);
+return rc;
+}
+
+switch ( op.cmd )
+{
+default:
+rc = -EOPNOTSUPP;
+break;
+}
+
+rcu_unlock_domain(d);
+
+if ( rc == -ERESTART )
+{
+if ( unlikely(copy_field_to_guest(u_hvmctl, &op, opaque)) )
+rc = -EFAULT;
+else
+rc = hypercall_create_continuation(__HYPERVISOR_hvmctl, "h",
+   u_hvmctl);
+}
+
+return rc;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4111,6 +4111,7 @@ static const struct {
 COMPAT_CALL(platform_op),
 COMPAT_CALL(mmuext_op),
 HYPERCALL(xenpmu_op),
+HYPERCALL(hvmctl),
 HYPERCALL(arch_1)
 };
 
--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -469,6 +469,7 @@ ENTRY(compat_hypercall_table)
 .quad do_tmem_op
 .quad do_ni_hypercall   /* reserved for XenClient */
 .quad do_xenpmu_op  /* 40 */
+.quad do_hvmctl
 .rept __HYPERVISOR_arch_0-((.-compat_hypercall_table)/8)
 .quad compat_ni_hypercall
 .endr
@@ -520,6 +521,7 @@ ENTRY(compat_hypercall_args_table)
 .byte 1 /* do_tmem_op   */
 .byte 0 /* reserved for XenClient   */
 .byte 2 /* do_xenpmu_op */  /* 40 */
+.byte 1 /* do_hvmctl*/
 .rept __HYPERVISOR_arch_0-(.-compat_hypercall_args_table)
 .byte 0 /* compat_ni_hypercall  */
 .endr
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -792,6 +792,7 @@ ENTRY(hypercall_table)
 .quad do_tmem_op
 .quad do_ni_hypercall   /* reserved for XenClient */
 .quad do_xenpmu_op  /* 40 */
+.quad do_hvmctl
 .rept __HYPERVISOR_arch_0-((.-hypercall_table)/8)
 .quad do_ni_hypercall
 .endr
@@ -843,6 +844,7 @@ ENTRY(hypercall_args_table)
 .byte 1 /* do_tmem_op   */
 .byte 0 /* reserved for XenClient */
 .byte 2 /* do_xenpmu_op */  /* 40 */
+.byte 1 /* do_hvmctl*/
 .rept __HYPERVISOR_arch_0-(.-hypercall_args_table)
 .byte 0 /* do_ni_hypercall  */
 .endr
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -93,7 +93,7 @@ all: headers.chk headers++.chk
 
 PUBLIC_HEADERS := $(filter-out public/arch-% public/dom0_ops.h, $(wildcard 
public/*.h public/*/*.h) $(public-y))
 
-PUBLIC_ANSI_HEADERS := $(filter-out public/%ctl.h public/xsm/% 
public/%hvm/save.h, $(PUBLIC_HEADERS))
+PUBLIC_ANSI_HEADERS := $(filter-out public/%ctl.h public/hvm/control.h 
public/xsm/% public/%hvm/save.h,$(PUBLIC_HEADERS))
 
 headers.chk: $(PUBLIC_ANSI_HEADERS) Makefile
for i in $(filter %.h,$^); do \
--- /dev/null
+++ b/xen/include/public/hvm/control.h
@@ -0,0 +1,54 @@
+/*
+ * Permission is hereby granted, free of charge, to any person obtaining a co

[Xen-devel] [PATCH v2 02/11] hvmctl: convert HVMOP_set_pci_intx_level

[Jan Beulich]

Note that this adds validation of the "domain" interface structure
field, which previously got ignored.

Note further that this retains the hvmop interface definitions as those
had (wrongly) been exposed to non-tool stack consumers (albeit the
operation wouldn't have succeeded when requested by a domain for
itself).

Signed-off-by: Jan Beulich 
Acked-by: Daniel De Graaf 
Reviewed-by: Wei Liu 
Reviewed-by: Andrew Cooper 
---
v2: Drop bogus comment from xen/xsm/flask/policy/access_vectors.

--- a/tools/libxc/xc_misc.c
+++ b/tools/libxc/xc_misc.c
@@ -473,30 +473,14 @@ int xc_hvm_set_pci_intx_level(
 uint8_t domain, uint8_t bus, uint8_t device, uint8_t intx,
 unsigned int level)
 {
-DECLARE_HYPERCALL_BUFFER(struct xen_hvm_set_pci_intx_level, arg);
-int rc;
+DECLARE_HVMCTL(set_pci_intx_level, dom,
+   .domain = domain,
+   .bus= bus,
+   .device = device,
+   .intx   = intx,
+   .level  = level);
 
-arg = xc_hypercall_buffer_alloc(xch, arg, sizeof(*arg));
-if ( arg == NULL )
-{
-PERROR("Could not allocate memory for xc_hvm_set_pci_intx_level 
hypercall");
-return -1;
-}
-
-arg->domid  = dom;
-arg->domain = domain;
-arg->bus= bus;
-arg->device = device;
-arg->intx   = intx;
-arg->level  = level;
-
-rc = xencall2(xch->xcall, __HYPERVISOR_hvm_op,
-  HVMOP_set_pci_intx_level,
-  HYPERCALL_BUFFER_AS_ARG(arg));
-
-xc_hypercall_buffer_free(xch, arg);
-
-return rc;
+return do_hvmctl(xch, &hvmctl);
 }
 
 int xc_hvm_set_isa_irq_level(
--- a/tools/libxc/xc_private.h
+++ b/tools/libxc/xc_private.h
@@ -34,6 +34,8 @@
 #define XC_INTERNAL_COMPAT_MAP_FOREIGN_API
 #include "xenctrl.h"
 
+#include 
+
 #include 
 #include 
 
@@ -61,6 +63,13 @@ struct iovec {
 
 #define DECLARE_DOMCTL struct xen_domctl domctl
 #define DECLARE_SYSCTL struct xen_sysctl sysctl
+#define DECLARE_HVMCTL(op, dom, init...) \
+struct xen_hvmctl hvmctl = { \
+.interface_version = XEN_HVMCTL_INTERFACE_VERSION, \
+.domain = (dom), \
+.cmd = XEN_HVMCTL_##op, \
+.u.op = { init } \
+}
 #define DECLARE_PHYSDEV_OP struct physdev_op physdev_op
 #define DECLARE_FLASK_OP struct xen_flask_op op
 #define DECLARE_PLATFORM_OP struct xen_platform_op platform_op
@@ -311,6 +320,31 @@ static inline int do_sysctl(xc_interface
 return ret;
 }
 
+static inline int do_hvmctl(xc_interface *xch, struct xen_hvmctl *hvmctl)
+{
+int ret = -1;
+DECLARE_HYPERCALL_BOUNCE(hvmctl, sizeof(*hvmctl), 
XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
+
+if ( xc_hypercall_bounce_pre(xch, hvmctl) )
+{
+PERROR("Could not bounce buffer for hvmctl hypercall");
+return -1;
+}
+
+ret = xencall1(xch->xcall, __HYPERVISOR_hvmctl,
+   HYPERCALL_BUFFER_AS_ARG(hvmctl));
+if ( ret < 0 )
+{
+if ( errno == EACCES )
+DPRINTF("hvmctl operation failed -- need to"
+" rebuild the user-space tool set?\n");
+}
+
+xc_hypercall_bounce_post(xch, hvmctl);
+
+return ret;
+}
+
 static inline int do_platform_op(xc_interface *xch,
  struct xen_platform_op *platform_op)
 {
--- a/xen/arch/x86/hvm/control.c
+++ b/xen/arch/x86/hvm/control.c
@@ -19,6 +19,30 @@
 #include 
 #include 
 
+static int set_pci_intx_level(struct domain *d,
+  const struct xen_hvm_set_pci_intx_level *op)
+{
+if ( op->domain || op->bus || (op->device > 31) || (op->intx > 3) )
+return -EINVAL;
+
+if ( !is_hvm_domain(d) )
+return -EINVAL;
+
+switch ( op->level )
+{
+case 0:
+hvm_pci_intx_deassert(d, op->device, op->intx);
+break;
+case 1:
+hvm_pci_intx_assert(d, op->device, op->intx);
+break;
+default:
+return -EINVAL;
+}
+
+return 0;
+}
+
 long do_hvmctl(XEN_GUEST_HANDLE_PARAM(xen_hvmctl_t) u_hvmctl)
 {
 xen_hvmctl_t op;
@@ -52,6 +76,10 @@ long do_hvmctl(XEN_GUEST_HANDLE_PARAM(xe
 
 switch ( op.cmd )
 {
+case XEN_HVMCTL_set_pci_intx_level:
+rc = set_pci_intx_level(d, &op.u.set_pci_intx_level);
+break;
+
 default:
 rc = -EOPNOTSUPP;
 break;
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4303,50 +4303,6 @@ void hvm_hypercall_page_initialise(struc
 hvm_funcs.init_hypercall_page(d, hypercall_page);
 }
 
-static int hvmop_set_pci_intx_level(
-XEN_GUEST_HANDLE_PARAM(xen_hvm_set_pci_intx_level_t) uop)
-{
-struct xen_hvm_set_pci_intx_level op;
-struct domain *d;
-int rc;
-
-if ( copy_from_guest(&op, uop, 1) )
-return -EFAULT;
-
-if ( (op.domain > 0) || (op.bus > 0) || (op.device > 31) || (op.intx > 3) )
-return -EINVAL;
-
-rc = rcu_lock_remote_domain_by_id(op.domid, &d);
-if ( rc != 0 )
-return rc;
-
-rc = -EINVAL;
-if ( !is_hvm_do

Re: [Xen-devel] [PATCH v2 00/11] hvmctl hypercall

[David Vrabel]

On 24/06/16 11:21, Jan Beulich wrote:
> A long while back separating out all control kind operations (intended
> for use by only the control domain or device model) from the currect
> hvmop hypercall has been discussed. This series aims at finally making
> this reality (at once allowing to streamline the associated XSM checking).
> 
> 01: public / x86: introduce hvmctl hypercall
> 02: convert HVMOP_set_pci_intx_level
> 03: convert HVMOP_set_isa_irq_level
> 04: convert HVMOP_set_pci_link_route
> 05: convert HVMOP_track_dirty_vram
> 06: convert HVMOP_modified_memory
> 07: convert HVMOP_set_mem_type
> 08: convert HVMOP_inject_trap
> 09: convert HVMOP_inject_msi
> 10: convert HVMOP_*ioreq_server*
> 11: x86/HVM: serialize trap injecting producer and consumer

Is hvmctl going to have a stable ABI?

David

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] xen/arm: map_dev_mmio_region: The iomem permission check should be done on MFN

[Julien Grall]

Hi,

Ping?

Cheers,

On 14/06/16 12:50, Julien Grall wrote:

The helper iomem_access_permitted expects MFNs in parameters and not
GNFs. Thankfully only the hardware domain can call this function and
it will always be with GFNS == MFNs for now.

Also, fix the printf to use the MFN range and not the GFN one.

Signed-off-by: Julien Grall 
Cc: Shannon Zhao 

---
 This patch is a good candidate to backport to Xen 4.7. Without
 it, the hardware domain can map any MMIO because the permission
 check is done on the GPFNs and not the MNFs.
---
  xen/arch/arm/p2m.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 6a19c57..4c6547d 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1275,14 +1275,14 @@ int map_dev_mmio_region(struct domain *d,
  {
  int res;

-if ( !(nr && iomem_access_permitted(d, start_gfn, start_gfn + nr - 1)) )
+if ( !(nr && iomem_access_permitted(d, mfn, mfn + nr - 1)) )
  return 0;

  res = map_mmio_regions(d, start_gfn, nr, mfn);
  if ( res < 0 )
  {
  printk(XENLOG_G_ERR "Unable to map [%#lx - %#lx] in Dom%d\n",
-   start_gfn, start_gfn + nr - 1, d->domain_id);
+   mfn, mfn + nr - 1, d->domain_id);
  return res;
  }




--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2 03/11] hvmctl: convert HVMOP_set_isa_irq_level

[Jan Beulich]

Note that this retains the hvmop interface definitions as those had
(wrongly) been exposed to non-tool stack consumers (albeit the
operation wouldn't have succeeded when requested by a domain for
itself).

Signed-off-by: Jan Beulich 
Reviewed-by: Wei Liu 
Reviewed-by: Andrew Cooper 

--- a/tools/libxc/xc_misc.c
+++ b/tools/libxc/xc_misc.c
@@ -488,27 +488,11 @@ int xc_hvm_set_isa_irq_level(
 uint8_t isa_irq,
 unsigned int level)
 {
-DECLARE_HYPERCALL_BUFFER(struct xen_hvm_set_isa_irq_level, arg);
-int rc;
+DECLARE_HVMCTL(set_isa_irq_level, dom,
+   .isa_irq = isa_irq,
+   .level   = level);
 
-arg = xc_hypercall_buffer_alloc(xch, arg, sizeof(*arg));
-if ( arg == NULL )
-{
-PERROR("Could not allocate memory for xc_hvm_set_isa_irq_level 
hypercall");
-return -1;
-}
-
-arg->domid   = dom;
-arg->isa_irq = isa_irq;
-arg->level   = level;
-
-rc = xencall2(xch->xcall, __HYPERVISOR_hvm_op,
-  HVMOP_set_isa_irq_level,
-  HYPERCALL_BUFFER_AS_ARG(arg));
-
-xc_hypercall_buffer_free(xch, arg);
-
-return rc;
+return do_hvmctl(xch, &hvmctl);
 }
 
 int xc_hvm_set_pci_link_route(
--- a/xen/arch/x86/hvm/control.c
+++ b/xen/arch/x86/hvm/control.c
@@ -43,6 +43,30 @@ static int set_pci_intx_level(struct dom
 return 0;
 }
 
+static int set_isa_irq_level(struct domain *d,
+ const struct xen_hvm_set_isa_irq_level *op)
+{
+if ( op->isa_irq > 15 )
+return -EINVAL;
+
+if ( !is_hvm_domain(d) )
+return -EINVAL;
+
+switch ( op->level )
+{
+case 0:
+hvm_isa_irq_deassert(d, op->isa_irq);
+break;
+case 1:
+hvm_isa_irq_assert(d, op->isa_irq);
+break;
+default:
+return -EINVAL;
+}
+
+return 0;
+}
+
 long do_hvmctl(XEN_GUEST_HANDLE_PARAM(xen_hvmctl_t) u_hvmctl)
 {
 xen_hvmctl_t op;
@@ -80,6 +104,10 @@ long do_hvmctl(XEN_GUEST_HANDLE_PARAM(xe
 rc = set_pci_intx_level(d, &op.u.set_pci_intx_level);
 break;
 
+case XEN_HVMCTL_set_isa_irq_level:
+rc = set_isa_irq_level(d, &op.u.set_isa_irq_level);
+break;
+
 default:
 rc = -EOPNOTSUPP;
 break;
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4446,50 +4446,6 @@ static void hvm_s3_resume(struct domain
 }
 }
 
-static int hvmop_set_isa_irq_level(
-XEN_GUEST_HANDLE_PARAM(xen_hvm_set_isa_irq_level_t) uop)
-{
-struct xen_hvm_set_isa_irq_level op;
-struct domain *d;
-int rc;
-
-if ( copy_from_guest(&op, uop, 1) )
-return -EFAULT;
-
-if ( op.isa_irq > 15 )
-return -EINVAL;
-
-rc = rcu_lock_remote_domain_by_id(op.domid, &d);
-if ( rc != 0 )
-return rc;
-
-rc = -EINVAL;
-if ( !is_hvm_domain(d) )
-goto out;
-
-rc = xsm_hvm_set_isa_irq_level(XSM_DM_PRIV, d);
-if ( rc )
-goto out;
-
-rc = 0;
-switch ( op.level )
-{
-case 0:
-hvm_isa_irq_deassert(d, op.isa_irq);
-break;
-case 1:
-hvm_isa_irq_assert(d, op.isa_irq);
-break;
-default:
-rc = -EINVAL;
-break;
-}
-
- out:
-rcu_unlock_domain(d);
-return rc;
-}
-
 static int hvmop_set_pci_link_route(
 XEN_GUEST_HANDLE_PARAM(xen_hvm_set_pci_link_route_t) uop)
 {
@@ -5364,11 +5320,6 @@ long do_hvm_op(unsigned long op, XEN_GUE
 guest_handle_cast(arg, xen_hvm_param_t));
 break;
 
-case HVMOP_set_isa_irq_level:
-rc = hvmop_set_isa_irq_level(
-guest_handle_cast(arg, xen_hvm_set_isa_irq_level_t));
-break;
-
 case HVMOP_inject_msi:
 rc = hvmop_inject_msi(
 guest_handle_cast(arg, xen_hvm_inject_msi_t));
--- a/xen/include/public/hvm/control.h
+++ b/xen/include/public/hvm/control.h
@@ -38,14 +38,25 @@ struct xen_hvm_set_pci_intx_level {
 uint8_t level;
 };
 
+/* XEN_HVMCTL_set_isa_irq_level */
+/* Set the logical level of one of a domain's ISA IRQ wires. */
+struct xen_hvm_set_isa_irq_level {
+/* ISA device identification, by ISA IRQ (0-15). */
+uint8_t  isa_irq;
+/* Assertion level (0 = unasserted, 1 = asserted). */
+uint8_t  level;
+};
+
 struct xen_hvmctl {
 uint16_t interface_version;/* XEN_HVMCTL_INTERFACE_VERSION */
 domid_t domain;
 uint32_t cmd;
 #define XEN_HVMCTL_set_pci_intx_level1
+#define XEN_HVMCTL_set_isa_irq_level 2
 uint64_t opaque;   /* Must be zero on initial invocation. */
 union {
 struct xen_hvm_set_pci_intx_level set_pci_intx_level;
+struct xen_hvm_set_isa_irq_level set_isa_irq_level;
 uint8_t pad[120];
 } u;
 };
--- a/xen/include/public/hvm/hvm_op.h
+++ b/xen/include/public/hvm/hvm_op.h
@@ -53,8 +53,6 @@ struct xen_hvm_set_pci_intx_level {
 typedef struct xen_hvm_set_pci_intx_level xen_hvm_set_pci_intx_level_t;
 DEFINE_XEN_GUEST_HANDLE(xen_hvm_set_pci_intx_level_t)

Re: [Xen-devel] [PATCH v2 0/4] VMX: Properly handle pi descriptor and per-cpu blocking list

[Dario Faggioli]

On Fri, 2016-06-24 at 06:33 +, Wu, Feng wrote:
> > -Original Message-
> > From: Dario Faggioli [mailto:dario.faggi...@citrix.com]
> > In your case, AFAICUI, it's:
> >  - the vCPU should block, waiting for an event
> >  - the event is _not_ arrived, so we indeed should block
> >  - we do *not* block, due to some other reason
> > 
> > That does not look right to me... what about the fact that we
> > wanted to
> > actually wait for the event? :-O
> I understand your point. In my understanding, currently, vcpu_block()
> is
> for guest's HLT operation, which means, guest has nothing to do. In
> this case, even we return (not blocking), seems the function is not
> broken.
>
So, basically, you're saying that the vcpu has nothing to do, and in
fact it executed an HLT instruction, and you want to let it continue to
run? :-O

Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)



signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2 04/11] hvmctl: convert HVMOP_set_pci_link_route

[Jan Beulich]

Note that this retains the hvmop interface definitions as those had
(wrongly) been exposed to non-tool stack consumers (albeit the
operation wouldn't have succeeded when requested by a domain for
itself).

Signed-off-by: Jan Beulich 
Reviewed-by: Wei Liu 
Reviewed-by: Andrew Cooper 

--- a/tools/libxc/xc_misc.c
+++ b/tools/libxc/xc_misc.c
@@ -498,27 +498,11 @@ int xc_hvm_set_isa_irq_level(
 int xc_hvm_set_pci_link_route(
 xc_interface *xch, domid_t dom, uint8_t link, uint8_t isa_irq)
 {
-DECLARE_HYPERCALL_BUFFER(struct xen_hvm_set_pci_link_route, arg);
-int rc;
+DECLARE_HVMCTL(set_pci_link_route, dom,
+   .link= link,
+   .isa_irq = isa_irq);
 
-arg = xc_hypercall_buffer_alloc(xch, arg, sizeof(*arg));
-if ( arg == NULL )
-{
-PERROR("Could not allocate memory for xc_hvm_set_pci_link_route 
hypercall");
-return -1;
-}
-
-arg->domid   = dom;
-arg->link= link;
-arg->isa_irq = isa_irq;
-
-rc = xencall2(xch->xcall, __HYPERVISOR_hvm_op,
-  HVMOP_set_pci_link_route,
-  HYPERCALL_BUFFER_AS_ARG(arg));
-
-xc_hypercall_buffer_free(xch, arg);
-
-return rc;
+return do_hvmctl(xch, &hvmctl);
 }
 
 int xc_hvm_inject_msi(
--- a/xen/arch/x86/hvm/control.c
+++ b/xen/arch/x86/hvm/control.c
@@ -108,6 +108,11 @@ long do_hvmctl(XEN_GUEST_HANDLE_PARAM(xe
 rc = set_isa_irq_level(d, &op.u.set_isa_irq_level);
 break;
 
+case XEN_HVMCTL_set_pci_link_route:
+rc = hvm_set_pci_link_route(d, op.u.set_pci_link_route.link,
+op.u.set_pci_link_route.isa_irq);
+break;
+
 default:
 rc = -EOPNOTSUPP;
 break;
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4446,39 +4446,6 @@ static void hvm_s3_resume(struct domain
 }
 }
 
-static int hvmop_set_pci_link_route(
-XEN_GUEST_HANDLE_PARAM(xen_hvm_set_pci_link_route_t) uop)
-{
-struct xen_hvm_set_pci_link_route op;
-struct domain *d;
-int rc;
-
-if ( copy_from_guest(&op, uop, 1) )
-return -EFAULT;
-
-if ( (op.link > 3) || (op.isa_irq > 15) )
-return -EINVAL;
-
-rc = rcu_lock_remote_domain_by_id(op.domid, &d);
-if ( rc != 0 )
-return rc;
-
-rc = -EINVAL;
-if ( !is_hvm_domain(d) )
-goto out;
-
-rc = xsm_hvm_set_pci_link_route(XSM_DM_PRIV, d);
-if ( rc )
-goto out;
-
-rc = 0;
-hvm_set_pci_link_route(d, op.link, op.isa_irq);
-
- out:
-rcu_unlock_domain(d);
-return rc;
-}
-
 static int hvmop_inject_msi(
 XEN_GUEST_HANDLE_PARAM(xen_hvm_inject_msi_t) uop)
 {
@@ -5325,11 +5292,6 @@ long do_hvm_op(unsigned long op, XEN_GUE
 guest_handle_cast(arg, xen_hvm_inject_msi_t));
 break;
 
-case HVMOP_set_pci_link_route:
-rc = hvmop_set_pci_link_route(
-guest_handle_cast(arg, xen_hvm_set_pci_link_route_t));
-break;
-
 case HVMOP_flush_tlbs:
 rc = guest_handle_is_null(arg) ? hvmop_flush_tlb_all() : -ENOSYS;
 break;
--- a/xen/arch/x86/hvm/irq.c
+++ b/xen/arch/x86/hvm/irq.c
@@ -229,13 +229,17 @@ void hvm_assert_evtchn_irq(struct vcpu *
 hvm_set_callback_irq_level(v);
 }
 
-void hvm_set_pci_link_route(struct domain *d, u8 link, u8 isa_irq)
+int hvm_set_pci_link_route(struct domain *d, u8 link, u8 isa_irq)
 {
 struct hvm_irq *hvm_irq = &d->arch.hvm_domain.irq;
 u8 old_isa_irq;
 int i;
 
-ASSERT((link <= 3) && (isa_irq <= 15));
+if ( link > 3 || isa_irq > 15 )
+return -EINVAL;
+
+if ( !is_hvm_domain(d) )
+return -EINVAL;
 
 spin_lock(&d->arch.hvm_domain.irq_lock);
 
@@ -273,6 +277,8 @@ void hvm_set_pci_link_route(struct domai
 
 dprintk(XENLOG_G_INFO, "Dom%u PCI link %u changed %u -> %u\n",
 d->domain_id, link, old_isa_irq, isa_irq);
+
+return 0;
 }
 
 int hvm_inject_msi(struct domain *d, uint64_t addr, uint32_t data)
--- a/xen/include/public/hvm/control.h
+++ b/xen/include/public/hvm/control.h
@@ -47,16 +47,26 @@ struct xen_hvm_set_isa_irq_level {
 uint8_t  level;
 };
 
+/* XEN_HVMCTL_set_pci_link_route */
+struct xen_hvm_set_pci_link_route {
+/* PCI link identifier (0-3). */
+uint8_t  link;
+/* ISA IRQ (1-15), or 0 (disable link). */
+uint8_t  isa_irq;
+};
+
 struct xen_hvmctl {
 uint16_t interface_version;/* XEN_HVMCTL_INTERFACE_VERSION */
 domid_t domain;
 uint32_t cmd;
 #define XEN_HVMCTL_set_pci_intx_level1
 #define XEN_HVMCTL_set_isa_irq_level 2
+#define XEN_HVMCTL_set_pci_link_route3
 uint64_t opaque;   /* Must be zero on initial invocation. */
 union {
 struct xen_hvm_set_pci_intx_level set_pci_intx_level;
 struct xen_hvm_set_isa_irq_level set_isa_irq_level;
+struct xen_hvm_set_pci_link_route set_pci_link_route;
 uint8_t pad[120];
 } u;
 };
--- a/xen/include/public/hvm/hvm_op.h
+++ b/xen/inclu

[Xen-devel] [PATCH v2 06/11] hvmctl: convert HVMOP_modified_memory

[Jan Beulich]

Also limiting "nr" at the libxc level to 32 bits (the high 32 bits of
the previous 64-bit parameter got ignore so far).

Signed-off-by: Jan Beulich 
Reviewed-by: Wei Liu 
Reviewed-by: Andrew Cooper 

--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1620,7 +1620,7 @@ int xc_hvm_track_dirty_vram(
  * Notify that some pages got modified by the Device Model
  */
 int xc_hvm_modified_memory(
-xc_interface *xch, domid_t dom, uint64_t first_pfn, uint64_t nr);
+xc_interface *xch, domid_t dom, uint64_t first_gfn, uint32_t nr);
 
 /*
  * Set a range of memory to a specific type.
--- a/tools/libxc/xc_misc.c
+++ b/tools/libxc/xc_misc.c
@@ -558,29 +558,13 @@ int xc_hvm_track_dirty_vram(
 }
 
 int xc_hvm_modified_memory(
-xc_interface *xch, domid_t dom, uint64_t first_pfn, uint64_t nr)
+xc_interface *xch, domid_t dom, uint64_t first_gfn, uint32_t nr)
 {
-DECLARE_HYPERCALL_BUFFER(struct xen_hvm_modified_memory, arg);
-int rc;
+DECLARE_HVMCTL(modified_memory, dom,
+   .first_gfn = first_gfn,
+   .nr= nr);
 
-arg = xc_hypercall_buffer_alloc(xch, arg, sizeof(*arg));
-if ( arg == NULL )
-{
-PERROR("Could not allocate memory for xc_hvm_modified_memory 
hypercall");
-return -1;
-}
-
-arg->domid = dom;
-arg->first_pfn = first_pfn;
-arg->nr= nr;
-
-rc = xencall2(xch->xcall, __HYPERVISOR_hvm_op,
-  HVMOP_modified_memory,
-  HYPERCALL_BUFFER_AS_ARG(arg));
-
-xc_hypercall_buffer_free(xch, arg);
-
-return rc;
+return do_hvmctl(xch, &hvmctl);
 }
 
 int xc_hvm_set_mem_type(
--- a/xen/arch/x86/hvm/control.c
+++ b/xen/arch/x86/hvm/control.c
@@ -14,6 +14,7 @@
  * this program; If not, see .
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -90,6 +91,51 @@ static int track_dirty_vram(struct domai
: hap_track_dirty_vram(d, op->first_gfn, op->nr, op->dirty_bitmap);
 }
 
+static int modified_memory(struct domain *d,
+   const struct xen_hvm_modified_memory *op,
+   uint64_t *iter)
+{
+if ( !is_hvm_domain(d) )
+return -EINVAL;
+
+if ( op->rsvd || op->nr < *iter ||
+ ((op->first_gfn + op->nr - 1) < op->first_gfn) ||
+ ((op->first_gfn + op->nr - 1) > domain_get_maximum_gpfn(d)) )
+return -EINVAL;
+
+if ( !paging_mode_log_dirty(d) )
+return 0;
+
+while ( op->nr > *iter )
+{
+unsigned long gfn = op->first_gfn + *iter;
+struct page_info *page = get_page_from_gfn(d, gfn, NULL, P2M_UNSHARE);
+
+if ( page )
+{
+unsigned long mfn = page_to_mfn(page);
+
+paging_mark_dirty(d, mfn);
+/*
+ * These are most probably not page tables any more:
+ * Don't take a long time, and don't die either.
+ */
+sh_remove_shadows(d, _mfn(mfn), 1, 0);
+put_page(page);
+}
+
+/*
+ * Check for continuation every once in a while, and if it's not the
+ * last interation.
+ */
+if ( op->nr > ++*iter && !(*iter & 0xff) &&
+ hypercall_preempt_check() )
+return -ERESTART;
+}
+
+return 0;
+}
+
 long do_hvmctl(XEN_GUEST_HANDLE_PARAM(xen_hvmctl_t) u_hvmctl)
 {
 xen_hvmctl_t op;
@@ -140,6 +186,10 @@ long do_hvmctl(XEN_GUEST_HANDLE_PARAM(xe
 rc = track_dirty_vram(d, &op.u.track_dirty_vram);
 break;
 
+case XEN_HVMCTL_modified_memory:
+rc = modified_memory(d, &op.u.modified_memory, &op.opaque);
+break;
+
 default:
 rc = -EOPNOTSUPP;
 break;
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -5233,7 +5233,6 @@ long do_hvm_op(unsigned long op, XEN_GUE
 default:
 mask = ~0UL;
 break;
-case HVMOP_modified_memory:
 case HVMOP_set_mem_type:
 mask = HVMOP_op_mask;
 break;
@@ -5296,65 +5295,6 @@ long do_hvm_op(unsigned long op, XEN_GUE
 rc = guest_handle_is_null(arg) ? hvmop_flush_tlb_all() : -ENOSYS;
 break;
 
-case HVMOP_modified_memory:
-{
-struct xen_hvm_modified_memory a;
-struct domain *d;
-
-if ( copy_from_guest(&a, arg, 1) )
-return -EFAULT;
-
-rc = rcu_lock_remote_domain_by_id(a.domid, &d);
-if ( rc != 0 )
-return rc;
-
-rc = -EINVAL;
-if ( !is_hvm_domain(d) )
-goto modmem_fail;
-
-rc = xsm_hvm_control(XSM_DM_PRIV, d, op);
-if ( rc )
-goto modmem_fail;
-
-rc = -EINVAL;
-if ( a.nr < start_iter ||
- ((a.first_pfn + a.nr - 1) < a.first_pfn) ||
- ((a.first_pfn + a.nr - 1) > domain_get_maximum_gpfn(d)) )
-goto modmem_fail;
-
-rc = 0;
-if ( !paging_mode_log_dirty(d) )
-goto modmem_fail;
-
-while ( a.nr 

[Xen-devel] [PATCH v2 05/11] hvmctl: convert HVMOP_track_dirty_vram

[Jan Beulich]

Also limiting "nr" at the libxc level to 32 bits (the high 32 bits of
the previous 64-bit parameter got ignore so far).

Signed-off-by: Jan Beulich 
Reviewed-by: Wei Liu 
Reviewed-by: Andrew Cooper 

--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1613,7 +1613,7 @@ int xc_hvm_inject_msi(
  */
 int xc_hvm_track_dirty_vram(
 xc_interface *xch, domid_t dom,
-uint64_t first_pfn, uint64_t nr,
+uint64_t first_gfn, uint32_t nr,
 unsigned long *bitmap);
 
 /*
--- a/tools/libxc/xc_misc.c
+++ b/tools/libxc/xc_misc.c
@@ -533,33 +533,27 @@ int xc_hvm_inject_msi(
 
 int xc_hvm_track_dirty_vram(
 xc_interface *xch, domid_t dom,
-uint64_t first_pfn, uint64_t nr,
+uint64_t first_gfn, uint32_t nr,
 unsigned long *dirty_bitmap)
 {
+DECLARE_HVMCTL(track_dirty_vram, dom,
+   .first_gfn = first_gfn,
+   .nr= nr);
 DECLARE_HYPERCALL_BOUNCE(dirty_bitmap, (nr+7) / 8, 
XC_HYPERCALL_BUFFER_BOUNCE_OUT);
-DECLARE_HYPERCALL_BUFFER(struct xen_hvm_track_dirty_vram, arg);
 int rc;
 
-arg = xc_hypercall_buffer_alloc(xch, arg, sizeof(*arg));
-if ( arg == NULL || xc_hypercall_bounce_pre(xch, dirty_bitmap) )
+if ( xc_hypercall_bounce_pre(xch, dirty_bitmap) )
 {
 PERROR("Could not bounce memory for xc_hvm_track_dirty_vram 
hypercall");
-rc = -1;
-goto out;
+return -1;
 }
 
-arg->domid = dom;
-arg->first_pfn = first_pfn;
-arg->nr= nr;
-set_xen_guest_handle(arg->dirty_bitmap, dirty_bitmap);
-
-rc = xencall2(xch->xcall, __HYPERVISOR_hvm_op,
-  HVMOP_track_dirty_vram,
-  HYPERCALL_BUFFER_AS_ARG(arg));
+set_xen_guest_handle(hvmctl.u.track_dirty_vram.dirty_bitmap, dirty_bitmap);
+
+rc = do_hvmctl(xch, &hvmctl);
 
-out:
-xc_hypercall_buffer_free(xch, arg);
 xc_hypercall_bounce_post(xch, dirty_bitmap);
+
 return rc;
 }
 
--- a/xen/arch/x86/hvm/control.c
+++ b/xen/arch/x86/hvm/control.c
@@ -17,6 +17,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 
 static int set_pci_intx_level(struct domain *d,
@@ -67,6 +69,27 @@ static int set_isa_irq_level(struct doma
 return 0;
 }
 
+static int track_dirty_vram(struct domain *d,
+const struct xen_hvm_track_dirty_vram *op)
+{
+if ( !is_hvm_domain(d) )
+return -EINVAL;
+
+if ( op->rsvd || op->nr > (GB(1) >> PAGE_SHIFT) )
+return -EINVAL;
+
+if ( d->is_dying )
+return -ESRCH;
+
+if ( !d->max_vcpus || !d->vcpu[0] )
+return -EINVAL;
+
+return shadow_mode_enabled(d)
+   ? shadow_track_dirty_vram(d, op->first_gfn, op->nr,
+ op->dirty_bitmap)
+   : hap_track_dirty_vram(d, op->first_gfn, op->nr, op->dirty_bitmap);
+}
+
 long do_hvmctl(XEN_GUEST_HANDLE_PARAM(xen_hvmctl_t) u_hvmctl)
 {
 xen_hvmctl_t op;
@@ -113,6 +136,10 @@ long do_hvmctl(XEN_GUEST_HANDLE_PARAM(xe
 op.u.set_pci_link_route.isa_irq);
 break;
 
+case XEN_HVMCTL_track_dirty_vram:
+rc = track_dirty_vram(d, &op.u.track_dirty_vram);
+break;
+
 default:
 rc = -EOPNOTSUPP;
 break;
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -5296,47 +5296,6 @@ long do_hvm_op(unsigned long op, XEN_GUE
 rc = guest_handle_is_null(arg) ? hvmop_flush_tlb_all() : -ENOSYS;
 break;
 
-case HVMOP_track_dirty_vram:
-{
-struct xen_hvm_track_dirty_vram a;
-struct domain *d;
-
-if ( copy_from_guest(&a, arg, 1) )
-return -EFAULT;
-
-rc = rcu_lock_remote_domain_by_id(a.domid, &d);
-if ( rc != 0 )
-return rc;
-
-rc = -EINVAL;
-if ( !is_hvm_domain(d) )
-goto tdv_fail;
-
-if ( a.nr > GB(1) >> PAGE_SHIFT )
-goto tdv_fail;
-
-rc = xsm_hvm_control(XSM_DM_PRIV, d, op);
-if ( rc )
-goto tdv_fail;
-
-rc = -ESRCH;
-if ( d->is_dying )
-goto tdv_fail;
-
-rc = -EINVAL;
-if ( d->vcpu == NULL || d->vcpu[0] == NULL )
-goto tdv_fail;
-
-if ( shadow_mode_enabled(d) )
-rc = shadow_track_dirty_vram(d, a.first_pfn, a.nr, a.dirty_bitmap);
-else
-rc = hap_track_dirty_vram(d, a.first_pfn, a.nr, a.dirty_bitmap);
-
-tdv_fail:
-rcu_unlock_domain(d);
-break;
-}
-
 case HVMOP_modified_memory:
 {
 struct xen_hvm_modified_memory a;
--- a/xen/include/public/hvm/control.h
+++ b/xen/include/public/hvm/control.h
@@ -55,6 +55,18 @@ struct xen_hvm_set_pci_link_route {
 uint8_t  isa_irq;
 };
 
+/* XEN_HVMCTL_track_dirty_vram */
+struct xen_hvm_track_dirty_vram {
+/* Number of pages to track. */
+uint32_t nr;
+uint32_t rsvd;
+/* First GFN to track. */
+uint64_aligned_t first_gfn;
+/* OUT variable. */
+/*

[Xen-devel] [PATCH v2 07/11] hvmctl: convert HVMOP_set_mem_type

[Jan Beulich]

This allows elimination of the (ab)use of the high operation number
bits for encoding continuations.

Also limiting "nr" at the libxc level to 32 bits (the high 32 bits of
the previous 64-bit parameter got ignore so far).

Signed-off-by: Jan Beulich 
Reviewed-by: Wei Liu 
Reviewed-by: Andrew Cooper 

--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1627,7 +1627,8 @@ int xc_hvm_modified_memory(
  * Allowed types are HVMMEM_ram_rw, HVMMEM_ram_ro, HVMMEM_mmio_dm
  */
 int xc_hvm_set_mem_type(
-xc_interface *xch, domid_t dom, hvmmem_type_t memtype, uint64_t first_pfn, 
uint64_t nr);
+xc_interface *xch, domid_t dom, hvmmem_type_t memtype,
+uint64_t first_gfn, uint32_t nr);
 
 /*
  * Injects a hardware/software CPU trap, to take effect the next time the HVM 
--- a/tools/libxc/xc_misc.c
+++ b/tools/libxc/xc_misc.c
@@ -568,30 +568,15 @@ int xc_hvm_modified_memory(
 }
 
 int xc_hvm_set_mem_type(
-xc_interface *xch, domid_t dom, hvmmem_type_t mem_type, uint64_t 
first_pfn, uint64_t nr)
+xc_interface *xch, domid_t dom, hvmmem_type_t mem_type,
+uint64_t first_gfn, uint32_t nr)
 {
-DECLARE_HYPERCALL_BUFFER(struct xen_hvm_set_mem_type, arg);
-int rc;
+DECLARE_HVMCTL(set_mem_type, dom,
+   .hvmmem_type = mem_type,
+   .first_gfn   = first_gfn,
+   .nr  = nr);
 
-arg = xc_hypercall_buffer_alloc(xch, arg, sizeof(*arg));
-if ( arg == NULL )
-{
-PERROR("Could not allocate memory for xc_hvm_set_mem_type hypercall");
-return -1;
-}
-
-arg->domid= dom;
-arg->hvmmem_type  = mem_type;
-arg->first_pfn= first_pfn;
-arg->nr   = nr;
-
-rc = xencall2(xch->xcall, __HYPERVISOR_hvm_op,
-  HVMOP_set_mem_type,
-  HYPERCALL_BUFFER_AS_ARG(arg));
-
-xc_hypercall_buffer_free(xch, arg);
-
-return rc;
+return do_hvmctl(xch, &hvmctl);
 }
 
 int xc_hvm_inject_trap(
--- a/xen/arch/x86/hvm/control.c
+++ b/xen/arch/x86/hvm/control.c
@@ -136,6 +136,70 @@ static int modified_memory(struct domain
 return 0;
 }
 
+static int set_mem_type(struct domain *d,
+const struct xen_hvm_set_mem_type *op, uint64_t *iter)
+{
+/* Interface types to internal p2m types. */
+static const p2m_type_t memtype[] = {
+[HVMMEM_ram_rw]  = p2m_ram_rw,
+[HVMMEM_ram_ro]  = p2m_ram_ro,
+[HVMMEM_mmio_dm] = p2m_mmio_dm,
+[HVMMEM_unused]  = p2m_invalid
+};
+
+if ( !is_hvm_domain(d) )
+return -EINVAL;
+
+if ( op->rsvd || op->nr < *iter ||
+ ((op->first_gfn + op->nr - 1) < op->first_gfn) ||
+ ((op->first_gfn + op->nr - 1) > domain_get_maximum_gpfn(d)) )
+return -EINVAL;
+
+if ( op->hvmmem_type >= ARRAY_SIZE(memtype) ||
+ unlikely(op->hvmmem_type == HVMMEM_unused) )
+return -EINVAL;
+
+while ( op->nr > *iter )
+{
+unsigned long gfn = op->first_gfn + *iter;
+p2m_type_t t;
+int rc;
+
+get_gfn_unshare(d, gfn, &t);
+
+if ( p2m_is_paging(t) )
+{
+put_gfn(d, gfn);
+p2m_mem_paging_populate(d, gfn);
+return -EAGAIN;
+}
+
+if ( p2m_is_shared(t) )
+rc = -EAGAIN;
+else if ( !p2m_is_ram(t) &&
+  (!p2m_is_hole(t) || op->hvmmem_type != HVMMEM_mmio_dm) &&
+  (t != p2m_mmio_write_dm || op->hvmmem_type != HVMMEM_ram_rw) 
)
+rc = -EINVAL;
+else
+rc = p2m_change_type_one(d, gfn, t, memtype[op->hvmmem_type]);
+
+put_gfn(d, gfn);
+
+if ( rc )
+return rc;
+
+/*
+ * Check for continuation every once in a while, and if it's not the
+ * last interation.
+ */
+if ( op->nr > ++*iter && !(*iter & 0xff) &&
+ hypercall_preempt_check() )
+return -ERESTART;
+}
+
+return 0;
+}
+
 long do_hvmctl(XEN_GUEST_HANDLE_PARAM(xen_hvmctl_t) u_hvmctl)
 {
 xen_hvmctl_t op;
@@ -190,6 +254,10 @@ long do_hvmctl(XEN_GUEST_HANDLE_PARAM(xe
 rc = modified_memory(d, &op.u.modified_memory, &op.opaque);
 break;
 
+case XEN_HVMCTL_set_mem_type:
+rc = set_mem_type(d, &op.u.set_mem_type, &op.opaque);
+break;
+
 default:
 rc = -EOPNOTSUPP;
 break;
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -5215,31 +5215,11 @@ static int do_altp2m_op(
 return rc;
 }
 
-/*
- * Note that this value is effectively part of the ABI, even if we don't need
- * to make it a formal part of it: A guest suspended for migration in the
- * middle of a continuation would fail to work if resumed on a hypervisor
- * using a different value.
- */
-#define HVMOP_op_mask 0xff
-
 long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
-unsigned long start_iter, mask;
 long rc = 0;
 
-switch ( op & HVMOP_op_mask )
-{
-def

[Xen-devel] [PATCH v2 08/11] hvmctl: convert HVMOP_inject_trap

[Jan Beulich]

Signed-off-by: Jan Beulich 
Reviewed-by: Wei Liu 
Reviewed-by: Andrew Cooper 

--- a/tools/libxc/xc_misc.c
+++ b/tools/libxc/xc_misc.c
@@ -584,31 +584,15 @@ int xc_hvm_inject_trap(
 uint32_t type, uint32_t error_code, uint32_t insn_len,
 uint64_t cr2)
 {
-DECLARE_HYPERCALL_BUFFER(struct xen_hvm_inject_trap, arg);
-int rc;
+DECLARE_HVMCTL(inject_trap, dom,
+   .vcpuid = vcpu,
+   .type   = type,
+   .vector = vector,
+   .insn_len   = insn_len,
+   .error_code = error_code,
+   .cr2= cr2);
 
-arg = xc_hypercall_buffer_alloc(xch, arg, sizeof(*arg));
-if ( arg == NULL )
-{
-PERROR("Could not allocate memory for xc_hvm_inject_trap hypercall");
-return -1;
-}
-
-arg->domid   = dom;
-arg->vcpuid  = vcpu;
-arg->vector  = vector;
-arg->type= type;
-arg->error_code  = error_code;
-arg->insn_len= insn_len;
-arg->cr2 = cr2;
-
-rc = xencall2(xch->xcall, __HYPERVISOR_hvm_op,
-  HVMOP_inject_trap,
-  HYPERCALL_BUFFER_AS_ARG(arg));
-
-xc_hypercall_buffer_free(xch, arg);
-
-return rc;
+return do_hvmctl(xch, &hvmctl);
 }
 
 int xc_livepatch_upload(xc_interface *xch,
--- a/tools/tests/xen-access/xen-access.c
+++ b/tools/tests/xen-access/xen-access.c
@@ -41,6 +41,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #if defined(__arm__) || defined(__aarch64__)
 #include 
@@ -643,7 +644,7 @@ int main(int argc, char *argv[])
 /* Reinject */
 rc = xc_hvm_inject_trap(
 xch, domain_id, req.vcpu_id, 3,
-HVMOP_TRAP_sw_exc, -1, 0, 0);
+XEN_HVMCTL_TRAP_sw_exc, -1, 0, 0);
 if (rc < 0)
 {
 ERROR("Error %d injecting breakpoint\n", rc);
--- a/xen/arch/x86/hvm/control.c
+++ b/xen/arch/x86/hvm/control.c
@@ -200,6 +200,32 @@ static int set_mem_type(struct domain *d
 return 0;
 }
 
+static int inject_trap(struct domain *d,
+   const struct xen_hvm_inject_trap *op)
+{
+struct vcpu *v;
+
+if ( !is_hvm_domain(d) )
+return -EINVAL;
+
+if ( op->rsvd8 || op->rsvd32 )
+return -EINVAL;
+
+if ( op->vcpuid >= d->max_vcpus || (v = d->vcpu[op->vcpuid]) == NULL )
+return -ENOENT;
+
+if ( v->arch.hvm_vcpu.inject_trap.vector != -1 )
+return -EBUSY;
+
+v->arch.hvm_vcpu.inject_trap.vector = op->vector;
+v->arch.hvm_vcpu.inject_trap.type   = op->type;
+v->arch.hvm_vcpu.inject_trap.error_code = op->error_code;
+v->arch.hvm_vcpu.inject_trap.insn_len   = op->insn_len;
+v->arch.hvm_vcpu.inject_trap.cr2= op->cr2;
+
+return 0;
+}
+
 long do_hvmctl(XEN_GUEST_HANDLE_PARAM(xen_hvmctl_t) u_hvmctl)
 {
 xen_hvmctl_t op;
@@ -258,6 +284,10 @@ long do_hvmctl(XEN_GUEST_HANDLE_PARAM(xe
 rc = set_mem_type(d, &op.u.set_mem_type, &op.opaque);
 break;
 
+case XEN_HVMCTL_inject_trap:
+rc = inject_trap(d, &op.u.inject_trap);
+break;
+
 default:
 rc = -EOPNOTSUPP;
 break;
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -5366,48 +5366,6 @@ long do_hvm_op(unsigned long op, XEN_GUE
 break;
 }
 
-case HVMOP_inject_trap: 
-{
-xen_hvm_inject_trap_t tr;
-struct domain *d;
-struct vcpu *v;
-
-if ( copy_from_guest(&tr, arg, 1 ) )
-return -EFAULT;
-
-rc = rcu_lock_remote_domain_by_id(tr.domid, &d);
-if ( rc != 0 )
-return rc;
-
-rc = -EINVAL;
-if ( !is_hvm_domain(d) )
-goto injtrap_fail;
-
-rc = xsm_hvm_control(XSM_DM_PRIV, d, op);
-if ( rc )
-goto injtrap_fail;
-
-rc = -ENOENT;
-if ( tr.vcpuid >= d->max_vcpus || (v = d->vcpu[tr.vcpuid]) == NULL )
-goto injtrap_fail;
-
-if ( v->arch.hvm_vcpu.inject_trap.vector != -1 )
-rc = -EBUSY;
-else 
-{
-v->arch.hvm_vcpu.inject_trap.vector = tr.vector;
-v->arch.hvm_vcpu.inject_trap.type = tr.type;
-v->arch.hvm_vcpu.inject_trap.error_code = tr.error_code;
-v->arch.hvm_vcpu.inject_trap.insn_len = tr.insn_len;
-v->arch.hvm_vcpu.inject_trap.cr2 = tr.cr2;
-rc = 0;
-}
-
-injtrap_fail:
-rcu_unlock_domain(d);
-break;
-}
-
 case HVMOP_guest_request_vm_event:
 if ( guest_handle_is_null(arg) )
 vm_event_monitor_guest_request();
--- a/xen/include/public/hvm/control.h
+++ b/xen/include/public/hvm/control.h
@@ -89,6 +89,37 @@ struct xen_hvm_set_mem_type {
 uint64_aligned_t first_gfn;
 };
 
+/* XEN_HVMCTL_inject_trap */
+/*
+ * Inject a trap into a VCPU, which will get taken up on the next
+ * scheduling of it. Note that the cal

[Xen-devel] [PATCH v2 09/11] hvmctl: convert HVMOP_inject_msi

[Jan Beulich]

Signed-off-by: Jan Beulich 
Reviewed-by: Wei Liu 
Reviewed-by: Andrew Cooper 

--- a/tools/libxc/xc_misc.c
+++ b/tools/libxc/xc_misc.c
@@ -508,27 +508,11 @@ int xc_hvm_set_pci_link_route(
 int xc_hvm_inject_msi(
 xc_interface *xch, domid_t dom, uint64_t addr, uint32_t data)
 {
-DECLARE_HYPERCALL_BUFFER(struct xen_hvm_inject_msi, arg);
-int rc;
+DECLARE_HVMCTL(inject_msi, dom,
+   .data  = data,
+   .addr  = addr);
 
-arg = xc_hypercall_buffer_alloc(xch, arg, sizeof(*arg));
-if ( arg == NULL )
-{
-PERROR("Could not allocate memory for xc_hvm_inject_msi hypercall");
-return -1;
-}
-
-arg->domid = dom;
-arg->addr  = addr;
-arg->data  = data;
-
-rc = xencall2(xch->xcall, __HYPERVISOR_hvm_op,
-  HVMOP_inject_msi,
-  HYPERCALL_BUFFER_AS_ARG(arg));
-
-xc_hypercall_buffer_free(xch, arg);
-
-return rc;
+return do_hvmctl(xch, &hvmctl);
 }
 
 int xc_hvm_track_dirty_vram(
--- a/xen/arch/x86/hvm/control.c
+++ b/xen/arch/x86/hvm/control.c
@@ -288,6 +288,10 @@ long do_hvmctl(XEN_GUEST_HANDLE_PARAM(xe
 rc = inject_trap(d, &op.u.inject_trap);
 break;
 
+case XEN_HVMCTL_inject_msi:
+rc = hvm_inject_msi(d, op.u.inject_msi.addr, op.u.inject_msi.data);
+break;
+
 default:
 rc = -EOPNOTSUPP;
 break;
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4446,35 +4446,6 @@ static void hvm_s3_resume(struct domain
 }
 }
 
-static int hvmop_inject_msi(
-XEN_GUEST_HANDLE_PARAM(xen_hvm_inject_msi_t) uop)
-{
-struct xen_hvm_inject_msi op;
-struct domain *d;
-int rc;
-
-if ( copy_from_guest(&op, uop, 1) )
-return -EFAULT;
-
-rc = rcu_lock_remote_domain_by_id(op.domid, &d);
-if ( rc != 0 )
-return rc;
-
-rc = -EINVAL;
-if ( !is_hvm_domain(d) )
-goto out;
-
-rc = xsm_hvm_inject_msi(XSM_DM_PRIV, d);
-if ( rc )
-goto out;
-
-rc = hvm_inject_msi(d, op.addr, op.data);
-
- out:
-rcu_unlock_domain(d);
-return rc;
-}
-
 static int hvmop_flush_tlb_all(void)
 {
 struct domain *d = current->domain;
@@ -5266,11 +5237,6 @@ long do_hvm_op(unsigned long op, XEN_GUE
 guest_handle_cast(arg, xen_hvm_param_t));
 break;
 
-case HVMOP_inject_msi:
-rc = hvmop_inject_msi(
-guest_handle_cast(arg, xen_hvm_inject_msi_t));
-break;
-
 case HVMOP_flush_tlbs:
 rc = guest_handle_is_null(arg) ? hvmop_flush_tlb_all() : -ENOSYS;
 break;
--- a/xen/arch/x86/hvm/irq.c
+++ b/xen/arch/x86/hvm/irq.c
@@ -292,6 +292,9 @@ int hvm_inject_msi(struct domain *d, uin
 >> MSI_DATA_TRIGGER_SHIFT;
 uint8_t vector = data & MSI_DATA_VECTOR_MASK;
 
+if ( !is_hvm_domain(d) )
+return -EINVAL;
+
 if ( !vector )
 {
 int pirq = ((addr >> 32) & 0xff00) | dest;
--- a/xen/include/public/hvm/control.h
+++ b/xen/include/public/hvm/control.h
@@ -120,6 +120,16 @@ struct xen_hvm_inject_trap {
 uint64_aligned_t cr2;
 };
 
+/* XEN_HVMCTL_inject_msi */
+/* MSI injection for emulated devices. */
+struct xen_hvm_inject_msi {
+/* Message data. */
+uint32_t  data;
+uint32_t  rsvd;
+/* Message address (x86: 0xFEEx). */
+uint64_t  addr;
+};
+
 struct xen_hvmctl {
 uint16_t interface_version;/* XEN_HVMCTL_INTERFACE_VERSION */
 domid_t domain;
@@ -131,6 +141,7 @@ struct xen_hvmctl {
 #define XEN_HVMCTL_modified_memory   5
 #define XEN_HVMCTL_set_mem_type  6
 #define XEN_HVMCTL_inject_trap   7
+#define XEN_HVMCTL_inject_msi8
 uint64_t opaque;   /* Must be zero on initial invocation. */
 union {
 struct xen_hvm_set_pci_intx_level set_pci_intx_level;
@@ -140,6 +151,7 @@ struct xen_hvmctl {
 struct xen_hvm_modified_memory modified_memory;
 struct xen_hvm_set_mem_type set_mem_type;
 struct xen_hvm_inject_trap inject_trap;
+struct xen_hvm_inject_msi inject_msi;
 uint8_t pad[120];
 } u;
 };
--- a/xen/include/public/hvm/hvm_op.h
+++ b/xen/include/public/hvm/hvm_op.h
@@ -140,19 +140,6 @@ DEFINE_XEN_GUEST_HANDLE(xen_hvm_get_mem_
 /* Following tools-only interfaces may change in future. */
 #if defined(__XEN__) || defined(__XEN_TOOLS__)
 
-/* MSI injection for emulated devices */
-#define HVMOP_inject_msi 16
-struct xen_hvm_inject_msi {
-/* Domain to be injected */
-domid_t   domid;
-/* Data -- lower 32 bits */
-uint32_t  data;
-/* Address (0xfeex) */
-uint64_t  addr;
-};
-typedef struct xen_hvm_inject_msi xen_hvm_inject_msi_t;
-DEFINE_XEN_GUEST_HANDLE(xen_hvm_inject_msi_t);
-
 /*
  * IOREQ Servers
  *
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -609,12 +609,6 @@ static XSM_INLINE int xsm_shadow_control
 return xsm_default_action(action, current->domain, d);
 }
 
-static XSM_INLINE int 

[Xen-devel] [PATCH v2 10/11] hvmctl: convert HVMOP_*ioreq_server*

[Jan Beulich]

Note that we can't adjust HVM_IOREQSRV_BUFIOREQ_* to properly obey
name space rules, as these constants as in use by callers of the libxc
interface.

Signed-off-by: Jan Beulich 
Reviewed-by: Wei Liu 
Reviewed-by: Paul Durrant 
Reviewed-by: Andrew Cooper 

--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -41,6 +41,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -1416,23 +1416,14 @@ int xc_hvm_create_ioreq_server(xc_interf
int handle_bufioreq,
ioservid_t *id)
 {
-DECLARE_HYPERCALL_BUFFER(xen_hvm_create_ioreq_server_t, arg);
+DECLARE_HVMCTL(create_ioreq_server, domid,
+   .handle_bufioreq = handle_bufioreq);
 int rc;
 
-arg = xc_hypercall_buffer_alloc(xch, arg, sizeof(*arg));
-if ( arg == NULL )
-return -1;
-
-arg->domid = domid;
-arg->handle_bufioreq = handle_bufioreq;
-
-rc = xencall2(xch->xcall, __HYPERVISOR_hvm_op,
-  HVMOP_create_ioreq_server,
-  HYPERCALL_BUFFER_AS_ARG(arg));
+rc = do_hvmctl(xch, &hvmctl);
 
-*id = arg->id;
+*id = hvmctl.u.create_ioreq_server.id;
 
-xc_hypercall_buffer_free(xch, arg);
 return rc;
 }
 
@@ -1443,84 +1434,52 @@ int xc_hvm_get_ioreq_server_info(xc_inte
  xen_pfn_t *bufioreq_pfn,
  evtchn_port_t *bufioreq_port)
 {
-DECLARE_HYPERCALL_BUFFER(xen_hvm_get_ioreq_server_info_t, arg);
+DECLARE_HVMCTL(get_ioreq_server_info, domid,
+   .id = id);
 int rc;
 
-arg = xc_hypercall_buffer_alloc(xch, arg, sizeof(*arg));
-if ( arg == NULL )
-return -1;
-
-arg->domid = domid;
-arg->id = id;
-
-rc = xencall2(xch->xcall, __HYPERVISOR_hvm_op,
-  HVMOP_get_ioreq_server_info,
-  HYPERCALL_BUFFER_AS_ARG(arg));
+rc = do_hvmctl(xch, &hvmctl);
 if ( rc != 0 )
-goto done;
+return rc;
 
 if ( ioreq_pfn )
-*ioreq_pfn = arg->ioreq_pfn;
+*ioreq_pfn = hvmctl.u.get_ioreq_server_info.ioreq_pfn;
 
 if ( bufioreq_pfn )
-*bufioreq_pfn = arg->bufioreq_pfn;
+*bufioreq_pfn = hvmctl.u.get_ioreq_server_info.bufioreq_pfn;
 
 if ( bufioreq_port )
-*bufioreq_port = arg->bufioreq_port;
+*bufioreq_port = hvmctl.u.get_ioreq_server_info.bufioreq_port;
 
-done:
-xc_hypercall_buffer_free(xch, arg);
-return rc;
+return 0;
 }
 
 int xc_hvm_map_io_range_to_ioreq_server(xc_interface *xch, domid_t domid,
 ioservid_t id, int is_mmio,
 uint64_t start, uint64_t end)
 {
-DECLARE_HYPERCALL_BUFFER(xen_hvm_io_range_t, arg);
-int rc;
-
-arg = xc_hypercall_buffer_alloc(xch, arg, sizeof(*arg));
-if ( arg == NULL )
-return -1;
-
-arg->domid = domid;
-arg->id = id;
-arg->type = is_mmio ? HVMOP_IO_RANGE_MEMORY : HVMOP_IO_RANGE_PORT;
-arg->start = start;
-arg->end = end;
-
-rc = xencall2(xch->xcall, __HYPERVISOR_hvm_op,
-  HVMOP_map_io_range_to_ioreq_server,
-  HYPERCALL_BUFFER_AS_ARG(arg));
+DECLARE_HVMCTL(map_io_range_to_ioreq_server, domid,
+   .id = id,
+   .type = is_mmio ? XEN_HVMCTL_IO_RANGE_MEMORY
+   : XEN_HVMCTL_IO_RANGE_PORT,
+   .start = start,
+   .end = end);
 
-xc_hypercall_buffer_free(xch, arg);
-return rc;
+return do_hvmctl(xch, &hvmctl);
 }
 
 int xc_hvm_unmap_io_range_from_ioreq_server(xc_interface *xch, domid_t domid,
 ioservid_t id, int is_mmio,
 uint64_t start, uint64_t end)
 {
-DECLARE_HYPERCALL_BUFFER(xen_hvm_io_range_t, arg);
-int rc;
-
-arg = xc_hypercall_buffer_alloc(xch, arg, sizeof(*arg));
-if ( arg == NULL )
-return -1;
+DECLARE_HVMCTL(unmap_io_range_from_ioreq_server, domid,
+   .id = id,
+   .type = is_mmio ? XEN_HVMCTL_IO_RANGE_MEMORY
+   : XEN_HVMCTL_IO_RANGE_PORT,
+   .start = start,
+   .end = end);
 
-arg->domid = domid;
-arg->id = id;
-arg->type = is_mmio ? HVMOP_IO_RANGE_MEMORY : HVMOP_IO_RANGE_PORT;
-arg->start = start;
-arg->end = end;
-
-rc = xencall2(xch->xcall, __HYPERVISOR_hvm_op,
-  HVMOP_unmap_io_range_from_ioreq_server,
-  HYPERCALL_BUFFER_AS_ARG(arg));
-
-xc_hypercall_buffer_free(xch, arg);
-return rc;
+return do_hvmctl(xch, &hvmctl);
 }
 
 int xc_hvm_map_pcidev_to_ioreq_server(xc_interface *xch, domid_t domid,
@@ -1528,37 +1487,23 @@ int xc_hvm_map_pcidev_to_ioreq_server(xc
   uint8_t 

[Xen-devel] [PATCH v2 11/11] x86/HVM: serialize trap injecting producer and consumer

[Jan Beulich]

Since injection works on a remote vCPU, and since there's no
enforcement of the subject vCPU being paused, there's a potential race
between the producing and consuming sides. Fix this by leveraging the
vector field as synchronization variable.

Signed-off-by: Jan Beulich 
Reviewed-by: Andrew Cooper 

--- a/xen/arch/x86/hvm/control.c
+++ b/xen/arch/x86/hvm/control.c
@@ -215,14 +215,16 @@ static int inject_trap(struct domain *d,
 if ( op->vcpuid >= d->max_vcpus || (v = d->vcpu[op->vcpuid]) == NULL )
 return -ENOENT;
 
-if ( v->arch.hvm_vcpu.inject_trap.vector != -1 )
+if ( cmpxchg(&v->arch.hvm_vcpu.inject_trap.vector, HVM_TRAP_VECTOR_UNSET,
+ HVM_TRAP_VECTOR_UPDATING) != HVM_TRAP_VECTOR_UNSET )
 return -EBUSY;
 
-v->arch.hvm_vcpu.inject_trap.vector = op->vector;
 v->arch.hvm_vcpu.inject_trap.type   = op->type;
 v->arch.hvm_vcpu.inject_trap.error_code = op->error_code;
 v->arch.hvm_vcpu.inject_trap.insn_len   = op->insn_len;
 v->arch.hvm_vcpu.inject_trap.cr2= op->cr2;
+smp_wmb();
+v->arch.hvm_vcpu.inject_trap.vector = op->vector;
 
 return 0;
 }
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -510,10 +510,11 @@ void hvm_do_resume(struct vcpu *v)
 }
 
 /* Inject pending hw/sw trap */
-if ( v->arch.hvm_vcpu.inject_trap.vector != -1 )
+if ( v->arch.hvm_vcpu.inject_trap.vector >= 0 )
 {
+smp_rmb();
 hvm_inject_trap(&v->arch.hvm_vcpu.inject_trap);
-v->arch.hvm_vcpu.inject_trap.vector = -1;
+v->arch.hvm_vcpu.inject_trap.vector = HVM_TRAP_VECTOR_UNSET;
 }
 }
 
@@ -1508,7 +1509,7 @@ int hvm_vcpu_initialise(struct vcpu *v)
 (void(*)(unsigned long))hvm_assert_evtchn_irq,
 (unsigned long)v);
 
-v->arch.hvm_vcpu.inject_trap.vector = -1;
+v->arch.hvm_vcpu.inject_trap.vector = HVM_TRAP_VECTOR_UNSET;
 
 if ( is_pvh_domain(d) )
 {
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -78,6 +78,8 @@ enum hvm_intblk {
 #define HVM_HAP_SUPERPAGE_1GB   0x0002
 
 struct hvm_trap {
+#define HVM_TRAP_VECTOR_UNSET(-1)
+#define HVM_TRAP_VECTOR_UPDATING (-2)
 int16_t   vector;
 uint8_t   type; /* X86_EVENTTYPE_* */
 uint8_t   insn_len; /* Instruction length */



x86/HVM: serialize trap injecting producer and consumer

Since injection works on a remote vCPU, and since there's no
enforcement of the subject vCPU being paused, there's a potential race
between the producing and consuming sides. Fix this by leveraging the
vector field as synchronization variable.

Signed-off-by: Jan Beulich 
Reviewed-by: Andrew Cooper 

--- a/xen/arch/x86/hvm/control.c
+++ b/xen/arch/x86/hvm/control.c
@@ -215,14 +215,16 @@ static int inject_trap(struct domain *d,
 if ( op->vcpuid >= d->max_vcpus || (v = d->vcpu[op->vcpuid]) == NULL )
 return -ENOENT;
 
-if ( v->arch.hvm_vcpu.inject_trap.vector != -1 )
+if ( cmpxchg(&v->arch.hvm_vcpu.inject_trap.vector, HVM_TRAP_VECTOR_UNSET,
+ HVM_TRAP_VECTOR_UPDATING) != HVM_TRAP_VECTOR_UNSET )
 return -EBUSY;
 
-v->arch.hvm_vcpu.inject_trap.vector = op->vector;
 v->arch.hvm_vcpu.inject_trap.type   = op->type;
 v->arch.hvm_vcpu.inject_trap.error_code = op->error_code;
 v->arch.hvm_vcpu.inject_trap.insn_len   = op->insn_len;
 v->arch.hvm_vcpu.inject_trap.cr2= op->cr2;
+smp_wmb();
+v->arch.hvm_vcpu.inject_trap.vector = op->vector;
 
 return 0;
 }
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -510,10 +510,11 @@ void hvm_do_resume(struct vcpu *v)
 }
 
 /* Inject pending hw/sw trap */
-if ( v->arch.hvm_vcpu.inject_trap.vector != -1 )
+if ( v->arch.hvm_vcpu.inject_trap.vector >= 0 )
 {
+smp_rmb();
 hvm_inject_trap(&v->arch.hvm_vcpu.inject_trap);
-v->arch.hvm_vcpu.inject_trap.vector = -1;
+v->arch.hvm_vcpu.inject_trap.vector = HVM_TRAP_VECTOR_UNSET;
 }
 }
 
@@ -1508,7 +1509,7 @@ int hvm_vcpu_initialise(struct vcpu *v)
 (void(*)(unsigned long))hvm_assert_evtchn_irq,
 (unsigned long)v);
 
-v->arch.hvm_vcpu.inject_trap.vector = -1;
+v->arch.hvm_vcpu.inject_trap.vector = HVM_TRAP_VECTOR_UNSET;
 
 if ( is_pvh_domain(d) )
 {
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -78,6 +78,8 @@ enum hvm_intblk {
 #define HVM_HAP_SUPERPAGE_1GB   0x0002
 
 struct hvm_trap {
+#define HVM_TRAP_VECTOR_UNSET(-1)
+#define HVM_TRAP_VECTOR_UPDATING (-2)
 int16_t   vector;
 uint8_t   type; /* X86_EVENTTYPE_* */
 uint8_t   insn_len; /* Instruction length */
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 00/11] hvmctl hypercall

[Jan Beulich]

>>> On 24.06.16 at 12:29,  wrote:
> On 24/06/16 11:21, Jan Beulich wrote:
>> A long while back separating out all control kind operations (intended
>> for use by only the control domain or device model) from the currect
>> hvmop hypercall has been discussed. This series aims at finally making
>> this reality (at once allowing to streamline the associated XSM checking).
>> 
>> 01: public / x86: introduce hvmctl hypercall
>> 02: convert HVMOP_set_pci_intx_level
>> 03: convert HVMOP_set_isa_irq_level
>> 04: convert HVMOP_set_pci_link_route
>> 05: convert HVMOP_track_dirty_vram
>> 06: convert HVMOP_modified_memory
>> 07: convert HVMOP_set_mem_type
>> 08: convert HVMOP_inject_trap
>> 09: convert HVMOP_inject_msi
>> 10: convert HVMOP_*ioreq_server*
>> 11: x86/HVM: serialize trap injecting producer and consumer
> 
> Is hvmctl going to have a stable ABI?

No, that's why it is being versioned - just like domctl and sysctl.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] xen: x86: remove duplicated MSR_IA32_FEATURE_CONTROL definition

[Huang, Kai]

On 6/22/2016 11:44 PM, Jan Beulich wrote:

On 22.06.16 at 12:17,  wrote:

@@ -288,7 +289,6 @@
 #define MSR_IA32_PLATFORM_ID   0x0017
 #define MSR_IA32_EBL_CR_POWERON0x002a
 #define MSR_IA32_EBC_FREQUENCY_ID  0x002c
-#define MSR_IA32_FEATURE_CONTROL   0x003a
 #define MSR_IA32_TSC_ADJUST0x003b


The latest when removing the definition here you should have noticed
that this variant follows the conventional naming, so if you want to
consolidate things, it should be the other way around imo. While not
the main reason, it'll also make sure mwait-idle.c won't needlessly
diverge from its Linux original.


You are right. I'll sent out another patch removing the old one.

Thanks,
-Kai


Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel



___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [xen-unstable-smoke test] 96210: regressions - FAIL

[osstest service owner]

flight 96210 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/96210/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-xl   6 xen-boot  fail REGR. vs. 96189

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass

version targeted for testing:
 xen  8ab94536d592393e053dc4a7acf84163054c2965
baseline version:
 xen  a6288d5bb8b970646368afe167a24eeba95e9e03

Last test of basis96189  2016-06-23 18:02:47 Z0 days
Testing same since96210  2016-06-24 09:01:07 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Dirk Behme 
  Jan Beulich 
  Julien Grall 
  Kevin Tian 
  Razvan Cojocaru 
  Tamas K Lengyel 

jobs:
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  fail
 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


Not pushing.


commit 8ab94536d592393e053dc4a7acf84163054c2965
Author: Dirk Behme 
Date:   Fri Jun 24 10:34:16 2016 +0200

MAINTAINERS: add ARM scif serial driver

The scif-uart.c is an ARM specific UART driver for the Renesas RCar
SoC family.

Signed-off-by: Dirk Behme 

commit ca63ceeb98dbbaddcdbadd5ef033ddd2469a9621
Author: Tamas K Lengyel 
Date:   Fri Jun 24 10:31:45 2016 +0200

monitor: Rename hvm/event to hvm/monitor

Mechanical renaming to better describe that the code in hvm/event is part of
the monitor subsystem.

Signed-off-by: Tamas K Lengyel 
Acked-by: Kevin Tian 
Acked-by: Razvan Cojocaru 
Acked-by: Jan Beulich 

commit 9648d483c03a56407a4340fde52f3a3780990485
Author: Tamas K Lengyel 
Date:   Fri Jun 24 10:31:00 2016 +0200

monitor: rename vm_event_monitor_guest_request

Mechanical renaming and relocation to the monitor subsystem.

Signed-off-by: Tamas K Lengyel 
Acked-by: Razvan Cojocaru 
Acked-by: Jan Beulich 
Acked-by: Julien Grall 

commit c4317ff6120bfa1cd6e7cbd4e453f7a3021c3a64
Author: Tamas K Lengyel 
Date:   Fri Jun 24 10:26:32 2016 +0200

monitor: rename vm_event_monitor_get_capabilities

The monitor_get_capabilities check actually belongs to the monitor 
subsystem so
relocating and renaming it to sanitize the code's name and location. 
Mechanical
patch, no code changes introduced.

Signed-off-by: Tamas K Lengyel 
Acked-by: Andrew Cooper 
Acked-by: Razvan Cojocaru 
Acked-by: Julien Grall 
(qemu changes not included)

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH] xen: x86: remove duplicated IA32_FEATURE_CONTROL MSR macro

[kaih . linux]

From: Kai Huang 

Below commit introduced a new macro MSR_IA32_FEATURE_CONTROL for
IA32_FEATURE_CONTROL MSR but it didn't remove old IA32_FEATURE_CONTROL_MSR
macro. The new one has better naming convention, so remove the old as
duplication and replace the relevant code with new one.

mwait-idle: prevent SKL-H boot failure when C8+C9+C10 enabled

Some SKL-H configurations require "max_cstate=7" to boot.
While that is an effective workaround, it disables C10.

..

Above commit also used SGX_ENABLE (bit 18) in IA32_FEATURE_CONTROL MSR without a
macro for it. A new macro IA32_FEATURE_CONTROL_MSR_SGX_ENABLE is also added for
better code and future use.

Signed-off-by: Kai Huang 
---
 xen/arch/x86/cpu/mwait-idle.c   | 2 +-
 xen/arch/x86/hvm/vmx/vmcs.c | 4 ++--
 xen/arch/x86/hvm/vmx/vmx.c  | 4 ++--
 xen/arch/x86/hvm/vmx/vvmx.c | 2 +-
 xen/include/asm-x86/msr-index.h | 4 ++--
 5 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/cpu/mwait-idle.c b/xen/arch/x86/cpu/mwait-idle.c
index e062e21..d6488f7 100644
--- a/xen/arch/x86/cpu/mwait-idle.c
+++ b/xen/arch/x86/cpu/mwait-idle.c
@@ -1006,7 +1006,7 @@ static void __init sklh_idle_state_table_update(void)
rdmsrl(MSR_IA32_FEATURE_CONTROL, msr);
 
/* if SGX is enabled */
-   if (msr & (1 << 18))
+   if (msr & IA32_FEATURE_CONTROL_MSR_SGX_ENABLE)
return;
}
 
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 848ac33..33d83fc 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -604,7 +604,7 @@ int vmx_cpu_up(void)
 return -EINVAL;
 }
 
-rdmsr(IA32_FEATURE_CONTROL_MSR, eax, edx);
+rdmsr(MSR_IA32_FEATURE_CONTROL, eax, edx);
 
 bios_locked = !!(eax & IA32_FEATURE_CONTROL_MSR_LOCK);
 if ( bios_locked )
@@ -623,7 +623,7 @@ int vmx_cpu_up(void)
 eax |= IA32_FEATURE_CONTROL_MSR_ENABLE_VMXON_OUTSIDE_SMX;
 if ( test_bit(X86_FEATURE_SMX, &boot_cpu_data.x86_capability) )
 eax |= IA32_FEATURE_CONTROL_MSR_ENABLE_VMXON_INSIDE_SMX;
-wrmsr(IA32_FEATURE_CONTROL_MSR, eax, 0);
+wrmsr(MSR_IA32_FEATURE_CONTROL, eax, 0);
 }
 
 if ( (rc = vmx_init_vmcs_config()) != 0 )
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 54cdb86..c23b1e9 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2622,7 +2622,7 @@ static int vmx_msr_read_intercept(unsigned int msr, 
uint64_t *msr_content)
 case MSR_IA32_DEBUGCTLMSR:
 __vmread(GUEST_IA32_DEBUGCTL, msr_content);
 break;
-case IA32_FEATURE_CONTROL_MSR:
+case MSR_IA32_FEATURE_CONTROL:
 case MSR_IA32_VMX_BASIC...MSR_IA32_VMX_VMFUNC:
 if ( !nvmx_msr_read_intercept(msr, msr_content) )
 goto gp_fault;
@@ -2848,7 +2848,7 @@ static int vmx_msr_write_intercept(unsigned int msr, 
uint64_t msr_content)
 
 break;
 }
-case IA32_FEATURE_CONTROL_MSR:
+case MSR_IA32_FEATURE_CONTROL:
 case MSR_IA32_VMX_BASIC...MSR_IA32_VMX_TRUE_ENTRY_CTLS:
 if ( !nvmx_msr_write_intercept(msr, msr_content) )
 goto gp_fault;
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index c6a39e9..4b9ec6a 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1941,7 +1941,7 @@ int nvmx_msr_read_intercept(unsigned int msr, u64 
*msr_content)
 data = gen_vmx_msr(data, VMX_ENTRY_CTLS_DEFAULT1, host_data);
 break;
 
-case IA32_FEATURE_CONTROL_MSR:
+case MSR_IA32_FEATURE_CONTROL:
 data = IA32_FEATURE_CONTROL_MSR_LOCK | 
IA32_FEATURE_CONTROL_MSR_ENABLE_VMXON_OUTSIDE_SMX;
 break;
diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
index e0f7f8d..5962cbf 100644
--- a/xen/include/asm-x86/msr-index.h
+++ b/xen/include/asm-x86/msr-index.h
@@ -133,12 +133,13 @@
 #define MSR_IA32_VMX_TRUE_EXIT_CTLS 0x48f
 #define MSR_IA32_VMX_TRUE_ENTRY_CTLS0x490
 #define MSR_IA32_VMX_VMFUNC 0x491
-#define IA32_FEATURE_CONTROL_MSR0x3a
+#define MSR_IA32_FEATURE_CONTROL0x3a
 #define IA32_FEATURE_CONTROL_MSR_LOCK 0x0001
 #define IA32_FEATURE_CONTROL_MSR_ENABLE_VMXON_INSIDE_SMX  0x0002
 #define IA32_FEATURE_CONTROL_MSR_ENABLE_VMXON_OUTSIDE_SMX 0x0004
 #define IA32_FEATURE_CONTROL_MSR_SENTER_PARAM_CTL 0x7f00
 #define IA32_FEATURE_CONTROL_MSR_ENABLE_SENTER0x8000
+#define IA32_FEATURE_CONTROL_MSR_SGX_ENABLE   0x4
 
 /* K7/K8 MSRs. Not complete. See the architecture manual for a more
complete list. */
@@ -288,7 +289,6 @@
 #define MSR_IA32_PLATFORM_ID   0x0017
 #define MSR_IA32_EBL_CR_POWERON0x002a
 #define MSR_IA32_EBC_FREQUENCY_ID  0x002c
-#define MSR_IA32_FEATURE_CONTROL   0x003a
 #define MSR_IA32_TSC_ADJUST  

Re: [Xen-devel] [PATCH] xen: x86: remove duplicated IA32_FEATURE_CONTROL MSR macro

[Tian, Kevin]

> From: kaih.li...@gmail.com [mailto:kaih.li...@gmail.com]
> Sent: Friday, June 24, 2016 6:45 PM
> 
> From: Kai Huang 
> 
> Below commit introduced a new macro MSR_IA32_FEATURE_CONTROL for
> IA32_FEATURE_CONTROL MSR but it didn't remove old IA32_FEATURE_CONTROL_MSR
> macro. The new one has better naming convention, so remove the old as
> duplication and replace the relevant code with new one.
> 
> mwait-idle: prevent SKL-H boot failure when C8+C9+C10 enabled
> 
> Some SKL-H configurations require "max_cstate=7" to boot.
> While that is an effective workaround, it disables C10.
> 
> ..
> 
> Above commit also used SGX_ENABLE (bit 18) in IA32_FEATURE_CONTROL MSR 
> without a
> macro for it. A new macro IA32_FEATURE_CONTROL_MSR_SGX_ENABLE is also added 
> for
> better code and future use.
> 
> Signed-off-by: Kai Huang 
> ---
>  xen/arch/x86/cpu/mwait-idle.c   | 2 +-
>  xen/arch/x86/hvm/vmx/vmcs.c | 4 ++--
>  xen/arch/x86/hvm/vmx/vmx.c  | 4 ++--
>  xen/arch/x86/hvm/vmx/vvmx.c | 2 +-
>  xen/include/asm-x86/msr-index.h | 4 ++--
>  5 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/arch/x86/cpu/mwait-idle.c b/xen/arch/x86/cpu/mwait-idle.c
> index e062e21..d6488f7 100644
> --- a/xen/arch/x86/cpu/mwait-idle.c
> +++ b/xen/arch/x86/cpu/mwait-idle.c
> @@ -1006,7 +1006,7 @@ static void __init sklh_idle_state_table_update(void)
>   rdmsrl(MSR_IA32_FEATURE_CONTROL, msr);
> 
>   /* if SGX is enabled */
> - if (msr & (1 << 18))
> + if (msr & IA32_FEATURE_CONTROL_MSR_SGX_ENABLE)
>   return;
>   }
> 
> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
> index 848ac33..33d83fc 100644
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -604,7 +604,7 @@ int vmx_cpu_up(void)
>  return -EINVAL;
>  }
> 
> -rdmsr(IA32_FEATURE_CONTROL_MSR, eax, edx);
> +rdmsr(MSR_IA32_FEATURE_CONTROL, eax, edx);
> 
>  bios_locked = !!(eax & IA32_FEATURE_CONTROL_MSR_LOCK);
>  if ( bios_locked )
> @@ -623,7 +623,7 @@ int vmx_cpu_up(void)
>  eax |= IA32_FEATURE_CONTROL_MSR_ENABLE_VMXON_OUTSIDE_SMX;
>  if ( test_bit(X86_FEATURE_SMX, &boot_cpu_data.x86_capability) )
>  eax |= IA32_FEATURE_CONTROL_MSR_ENABLE_VMXON_INSIDE_SMX;
> -wrmsr(IA32_FEATURE_CONTROL_MSR, eax, 0);
> +wrmsr(MSR_IA32_FEATURE_CONTROL, eax, 0);
>  }
> 
>  if ( (rc = vmx_init_vmcs_config()) != 0 )
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index 54cdb86..c23b1e9 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2622,7 +2622,7 @@ static int vmx_msr_read_intercept(unsigned int msr, 
> uint64_t
> *msr_content)
>  case MSR_IA32_DEBUGCTLMSR:
>  __vmread(GUEST_IA32_DEBUGCTL, msr_content);
>  break;
> -case IA32_FEATURE_CONTROL_MSR:
> +case MSR_IA32_FEATURE_CONTROL:
>  case MSR_IA32_VMX_BASIC...MSR_IA32_VMX_VMFUNC:
>  if ( !nvmx_msr_read_intercept(msr, msr_content) )
>  goto gp_fault;
> @@ -2848,7 +2848,7 @@ static int vmx_msr_write_intercept(unsigned int msr, 
> uint64_t
> msr_content)
> 
>  break;
>  }
> -case IA32_FEATURE_CONTROL_MSR:
> +case MSR_IA32_FEATURE_CONTROL:
>  case MSR_IA32_VMX_BASIC...MSR_IA32_VMX_TRUE_ENTRY_CTLS:
>  if ( !nvmx_msr_write_intercept(msr, msr_content) )
>  goto gp_fault;
> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
> index c6a39e9..4b9ec6a 100644
> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> @@ -1941,7 +1941,7 @@ int nvmx_msr_read_intercept(unsigned int msr, u64
> *msr_content)
>  data = gen_vmx_msr(data, VMX_ENTRY_CTLS_DEFAULT1, host_data);
>  break;
> 
> -case IA32_FEATURE_CONTROL_MSR:
> +case MSR_IA32_FEATURE_CONTROL:
>  data = IA32_FEATURE_CONTROL_MSR_LOCK |
> IA32_FEATURE_CONTROL_MSR_ENABLE_VMXON_OUTSIDE_SMX;
>  break;
> diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
> index e0f7f8d..5962cbf 100644
> --- a/xen/include/asm-x86/msr-index.h
> +++ b/xen/include/asm-x86/msr-index.h
> @@ -133,12 +133,13 @@
>  #define MSR_IA32_VMX_TRUE_EXIT_CTLS 0x48f
>  #define MSR_IA32_VMX_TRUE_ENTRY_CTLS0x490
>  #define MSR_IA32_VMX_VMFUNC 0x491
> -#define IA32_FEATURE_CONTROL_MSR0x3a
> +#define MSR_IA32_FEATURE_CONTROL0x3a

Instead of moving MSR definition up here, better move all related lines
down since original place is more sorted regarding to 0x3a.

>  #define IA32_FEATURE_CONTROL_MSR_LOCK 0x0001
>  #define IA32_FEATURE_CONTROL_MSR_ENABLE_VMXON_INSIDE_SMX  0x0002
>  #define IA32_FEATURE_CONTROL_MSR_ENABLE_VMXON_OUTSIDE_SMX 0x0004
>  #define IA32_FEATURE_CONTROL_MSR_SENTER_PARAM_CTL 0x7f00
>  #define IA32_FEATURE_CONTROL_MSR_ENABLE_SE

[Xen-devel] [linux-3.18 test] 96188: tolerable FAIL - PUSHED

[osstest service owner]

flight 96188 linux-3.18 real [real]
http://logs.test-lab.xenproject.org/osstest/logs/96188/

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail in 
96161 pass in 96188
 test-amd64-amd64-qemuu-nested-intel 16 debian-hvm-install/l1/l2 fail in 96161 
pass in 96188
 test-armhf-armhf-libvirt-raw  9 debian-di-install  fail in 96161 pass in 96188
 test-armhf-armhf-xl-arndale  15 guest-start/debian.repeat   fail pass in 96161

Regressions which are regarded as allowable (not blocking):
 build-i386-rumpuserxen6 xen-buildfail   like 95844
 build-amd64-rumpuserxen   6 xen-buildfail   like 95844
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 95844
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail like 95844
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 95844
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 95844

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-rumpuserxen-amd64  1 build-check(1)   blocked n/a
 test-amd64-i386-rumpuserxen-i386  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-pvh-intel 14 guest-saverestorefail  never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-armhf-armhf-libvirt-raw 13 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass

version targeted for testing:
 linuxd420f00c7bfb405884dd71fb7f87974f0d1be455
baseline version:
 linuxb5076139991c6b12c62346d9880eec1d4227d99f

Last test of basis95844  2016-06-17 01:16:12 Z7 days
Testing same since96161  2016-06-23 05:18:02 Z1 days2 attempts


People who touched revisions under test:
  "Eric W. Biederman" 
  AceLan Kao 
  Al Viro 
  Andre Przywara 
  Arnd Bergmann 
  Ben Dooks 
  Ben Skeggs 
  Bob Copeland 
  Chris Wilson 
  Christoffer Dall 
  Daniel Vetter 
  Dibyajyoti Ghosh 
  Eric W. Biederman 
  Ewan D. Milne 
  Fred Veldini 
  Gavin Shan 
  H. Peter Anvin 
  Helge Deller 
  Herbert Xu 
  Ingo Molnar 
  Jack Wang 
  James Bottomley 
  James E.J. Bottomley 
  Jan Stancek 
  Jann Horn 
  Johannes Berg 
  Linus Torvalds 
  Linus Walleij 
  Marc Zyngier 
  Martin K. Petersen 
  Martin Willi 
  Mathieu Malaterre 
  Micha

Re: [Xen-devel] [PATCH v5 1/9] vm_event: clear up return value of vm_event_monitor_traps

[Tian, Kevin]

> From: Tamas K Lengyel [mailto:ta...@tklengyel.com]
> Sent: Saturday, June 18, 2016 3:09 AM
> 
> On Thu, Jun 2, 2016 at 4:52 PM, Tamas K Lengyel  wrote:
> > The return value has not been clearly defined, with the function
> > never returning 0 which seemingly indicated a condition where the
> > guest should crash. In this patch we define -rc as error condition
> > where the notification was not sent, 0 where the notification was sent
> > and the vCPU is not paused and 1 that the notification was sent and
> > that the vCPU is paused.
> >
> > Signed-off-by: Tamas K Lengyel 
> > ---
> > Cc: Jun Nakajima 
> > Cc: Kevin Tian 
> > Cc: Andrew Cooper 
> 
> Pinging the rest of the maintainers to get an update on this patch.
> Current status is:
> 
> Acked-by: Razvan Cojocaru 
> Reviewed-by: Jan Beulich 

Acked-by: Kevin Tian 
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v6 4/5] vm_event: clear up return value of vm_event_monitor_traps

[Tian, Kevin]

> From: Tamas K Lengyel [mailto:ta...@tklengyel.com]
> Sent: Friday, June 24, 2016 1:07 AM
> 
> The return value has not been clearly defined, with the function
> never returning 0 which seemingly indicated a condition where the
> guest should crash.
> 
> In this patch we define -rc as error condition where a subscriber is
> present but an error prevented the notification from being sent;
> 0 where there is no subscriber or the notification was sent and the vCPU
> is not paused (i.e. safe to continue execution as normal); and 1 where the
> notification was sent with the vCPU paused and we are waiting for a
> response.
> 
> Signed-off-by: Tamas K Lengyel 
> Reviewed-by: Jan Beulich 
> Acked-by: Razvan Cojocaru 

Sorry I should ack this new version:

Acked-by: Kevin Tian 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] monitor access to pages with a specific p2m_type_t

[George Dunlap]

On Wed, Jun 22, 2016 at 12:38 PM, sepanta s  wrote:
> Hi,
> Is it possible to monitor the access on the pages withp2m_type_t
> p2m_ram_shared?

cc'ing Tamas and Razvan

 -George

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] get_maintainer.pl: Teach brace expansion

[George Dunlap]

On Wed, Jun 22, 2016 at 6:28 PM, Anthony PERARD
 wrote:
> This only implement a simpler non-nested brace expansion.
>
> This will convert brace expansion style use in MAINTAINER into a regex
> that get_maintainer.pl can use to match a path again a maintainer
> section.
>
> It is done by using two different regex, the first one will take care of
> converting ',' inside '{}' to a '|', one by one, as long as there is at
> least two commas. The second regex will do the final convertion of '{,}'
> to '(|)'.

Can you give some examples of the sorts of MAINTAINER entries this
would allow (and maybe one that you intend to implement once this is
accepted)?

Thanks,
 -George

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] Change of max-ram-below-4g initial value breaks Xen

[Anthony PERARD]

On Fri, Jun 24, 2016 at 07:46:23AM +0200, Gerd Hoffmann wrote:
> On Do, 2016-06-23 at 17:18 +0100, Anthony PERARD wrote:
> > On Thu, Jun 23, 2016 at 04:57:54PM +0200, Gerd Hoffmann wrote:
> > >   Hi,
> > > 
> > > > How could xen_ram_init() find out if the value of max-ram-below-4g is
> > > > the default or if a user have set it? Is there another way we could fix
> > > > this?
> > > 
> > > Attached patch should fix it.  Patch survived a quick smoke test on kvm
> > > so far, need to do some more testing tomorrow.  Can you give it a spin
> > > on xen?
> > 
> > Thanks. Unfortunately, it does not work :(.
> > 
> > In this patch, max_ram_below_4g is set before the call to xen_ram_init()
> > and xen_ram_init read it back (via object_property_get_int()).  So, in
> > xen_ram_init, user_lowmem is not 0.
> 
> Ah, I see.  We do the split calculation twice on xen.  That is pretty
> pointless.  New patch attached.

I've tested on Xen, it works fine. Thanks. Also, the patch look good.

Cheers,

-- 
Anthony PERARD

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] monitor access to pages with a specific p2m_type_t

[Razvan Cojocaru]

On 06/24/2016 02:05 PM, George Dunlap wrote:
> On Wed, Jun 22, 2016 at 12:38 PM, sepanta s  wrote:
>> Hi,
>> Is it possible to monitor the access on the pages withp2m_type_t
>> p2m_ram_shared?
> 
> cc'ing Tamas and Razvan

Thanks for the CC. Judging by the "if ( npfec.write_access && (p2mt ==
p2m_ram_shared) )" line in hvm_hap_nested_page_fault() (from
xen/arch/x86/hvm/hvm.c), I'd say it certainly looks possible. But I
don't know what the context of the question is.


Thanks,
Razvan

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v6 5/5] x86/vm_event: Add HVM debug exception vm_events

[Tian, Kevin]

> From: Tamas K Lengyel [mailto:ta...@tklengyel.com]
> Sent: Friday, June 24, 2016 1:07 AM
> 
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index 03fcba7..4b69ca6 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -3373,10 +3373,39 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
>  HVMTRACE_1D(TRAP_DEBUG, exit_qualification);
>  write_debugreg(6, exit_qualification | DR_STATUS_RESERVED_ONE);
>  if ( !v->domain->debugger_attached )
> -vmx_propagate_intr(intr_info);
> +{
> +unsigned long insn_len = 0;
> +int rc;
> +unsigned long trap_type = MASK_EXTR(intr_info,
> +
> INTR_INFO_INTR_TYPE_MASK);
> +
> +if ( trap_type >= X86_EVENTTYPE_SW_INTERRUPT )
> +__vmread(VM_EXIT_INSTRUCTION_LEN, &insn_len);
> +
> +rc = hvm_monitor_debug(regs->eip,
> +   HVM_MONITOR_DEBUG_EXCEPTION,
> +   trap_type, insn_len);
> +
> +/*
> + * !rc  continue normally
> + * rc > paused waiting for response, work here is done

Did you mean "rc > 0, vcpu is paused and waiting for response..."? Please
make the comment clear to understand.

> + * rc < error, fall-through to exit_and_crash
> + */
> +if ( !rc )
> +{
> +vmx_propagate_intr(intr_info);
> +break;
> +}
> +if ( rc > 0 )
> +break;
> +}
>  else
> +{
>  domain_pause_for_debugger();
> -break;
> +break;
> +}
> +
> +goto exit_and_crash;

Putting goto as the last line within a 'case' looks a bit strange. What
about putting goto directly under a "if ( rc < 0 )" check earlier?

if ( !rc )
...
if ( rc < 0 )
goto exit_and_crash;
}
else
domain_pause_for_debugger();
break;

Thanks
Kevin

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v6 5/5] x86/vm_event: Add HVM debug exception vm_events

[Jan Beulich]

>>> On 24.06.16 at 13:20,  wrote:
>>  From: Tamas K Lengyel [mailto:ta...@tklengyel.com]
>> Sent: Friday, June 24, 2016 1:07 AM
>> 
>> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
>> index 03fcba7..4b69ca6 100644
>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>> @@ -3373,10 +3373,39 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
>>  HVMTRACE_1D(TRAP_DEBUG, exit_qualification);
>>  write_debugreg(6, exit_qualification | DR_STATUS_RESERVED_ONE);
>>  if ( !v->domain->debugger_attached )
>> -vmx_propagate_intr(intr_info);
>> +{
>> +unsigned long insn_len = 0;
>> +int rc;
>> +unsigned long trap_type = MASK_EXTR(intr_info,
>> +
>> INTR_INFO_INTR_TYPE_MASK);
>> +
>> +if ( trap_type >= X86_EVENTTYPE_SW_INTERRUPT )
>> +__vmread(VM_EXIT_INSTRUCTION_LEN, &insn_len);
>> +
>> +rc = hvm_monitor_debug(regs->eip,
>> +   HVM_MONITOR_DEBUG_EXCEPTION,
>> +   trap_type, insn_len);
>> +
>> +/*
>> + * !rc  continue normally
>> + * rc > paused waiting for response, work here is done
> 
> Did you mean "rc > 0, vcpu is paused and waiting for response..."? Please
> make the comment clear to understand.
> 
>> + * rc < error, fall-through to exit_and_crash
>> + */
>> +if ( !rc )
>> +{
>> +vmx_propagate_intr(intr_info);
>> +break;
>> +}
>> +if ( rc > 0 )
>> +break;
>> +}
>>  else
>> +{
>>  domain_pause_for_debugger();
>> -break;
>> +break;
>> +}
>> +
>> +goto exit_and_crash;
> 
> Putting goto as the last line within a 'case' looks a bit strange. What
> about putting goto directly under a "if ( rc < 0 )" check earlier?
> 
>   if ( !rc )
>   ...
>   if ( rc < 0 )
>   goto exit_and_crash;
>   }
>   else
>   domain_pause_for_debugger();
>   break;

Thanks, Kevin - indeed that's exactly what I had asked for already
on the previous iteration.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v12 1/6] IOMMU: add a timeout parameter for device IOTLB invalidation

[Tian, Kevin]

> From: Xu, Quan
> Sent: Friday, June 24, 2016 1:52 PM
> 
> From: Quan Xu 
> 
> The parameter 'iommu_dev_iotlb_timeout' specifies the timeout
> of device IOTLB invalidation in milliseconds. By default, the
> timeout is 1000 milliseconds, which can be boot-time changed.
> 
> We also confirmed with VT-d hardware team that 1 milliseconds
> is large enough for VT-d IOMMU internal invalidation.
> 
> the existing panic() is eliminated and we bubble up the timeout
> of device IOTLB invalidation for further processing, as the
> PCI-e Address Translation Services (ATS) mandates a timeout of
> 60 seconds for device IOTLB invalidation. Obviously we can't
> spin for 60 seconds or otherwise Xen hypervisor hangs.
> 
> Add a __must_check annotation. The followup patch titled
> 'VT-d IOTLB/Context/IEC flush issue' addresses the __mustcheck.
> That is the other callers of this routine (two or three levels up)
> ignore the return code. This patch does not address this but the
> other does.
> 
> Signed-off-by: Quan Xu 
> 

Acked-by: Kevin Tian 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] xen: x86: remove duplicated IA32_FEATURE_CONTROL MSR macro

[Jan Beulich]

>>> On 24.06.16 at 12:56,  wrote:
>>  From: kaih.li...@gmail.com [mailto:kaih.li...@gmail.com]
>> Sent: Friday, June 24, 2016 6:45 PM
>> --- a/xen/include/asm-x86/msr-index.h
>> +++ b/xen/include/asm-x86/msr-index.h
>> @@ -133,12 +133,13 @@
>>  #define MSR_IA32_VMX_TRUE_EXIT_CTLS 0x48f
>>  #define MSR_IA32_VMX_TRUE_ENTRY_CTLS0x490
>>  #define MSR_IA32_VMX_VMFUNC 0x491
>> -#define IA32_FEATURE_CONTROL_MSR0x3a
>> +#define MSR_IA32_FEATURE_CONTROL0x3a
> 
> Instead of moving MSR definition up here, better move all related lines
> down since original place is more sorted regarding to 0x3a.

I agree.

>>  #define IA32_FEATURE_CONTROL_MSR_LOCK 0x0001
>>  #define IA32_FEATURE_CONTROL_MSR_ENABLE_VMXON_INSIDE_SMX  0x0002
>>  #define IA32_FEATURE_CONTROL_MSR_ENABLE_VMXON_OUTSIDE_SMX 0x0004
>>  #define IA32_FEATURE_CONTROL_MSR_SENTER_PARAM_CTL 0x7f00
>>  #define IA32_FEATURE_CONTROL_MSR_ENABLE_SENTER0x8000
>> +#define IA32_FEATURE_CONTROL_MSR_SGX_ENABLE   0x4
> 
> suppose above macros better be changed in same style? Or is it
> really meaningful to keep whole MSR name in every bit definition?
> Is it clearly enough to just keep strings after _MSR_?

I partly agree. The _MSR_ infix is clearly pointless. I wouldn't,
however, like to see the IA32_FEATURE_CONTROL_ prefix
dropped, as it helps associating the bits with their MSR.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v12 2/6] vt-d: synchronize for Device-TLB flush one by one

[Tian, Kevin]

> From: Xu, Quan
> Sent: Friday, June 24, 2016 1:52 PM
> 
> From: Quan Xu 
> 
> Today we do Device-TLB flush synchronization after issuing flush
> requests for all ATS devices belonging to a VM. Doing so however
> imposes a limitation, i.e. that we can not figure out which flush
> request is blocked in the flush queue list, based on VT-d spec.
> 
> To prepare correct Device-TLB flush timeout handling in next patch,
> we change the behavior to synchronize for every Device-TLB flush
> request. So the Device-TLB flush interface is changed a little bit,
> by checking timeout within the function instead of outside of function.
> 
> Accordingly we also do a similar change for flush interfaces of
> IOTLB/IEC/Context, i.e. moving synchronization into the function.
> Since there is no user of a non-synced interface, we just rename
> existing ones with _sync suffix.
> 
> Signed-off-by: Quan Xu 
> Reviewed-by: Jan Beulich 
> 

Acked-by: Kevin Tian 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] get_maintainer.pl: Teach brace expansion

[Anthony PERARD]

On Fri, Jun 24, 2016 at 12:09:53PM +0100, George Dunlap wrote:
> On Wed, Jun 22, 2016 at 6:28 PM, Anthony PERARD
>  wrote:
> > This only implement a simpler non-nested brace expansion.
> >
> > This will convert brace expansion style use in MAINTAINER into a regex
> > that get_maintainer.pl can use to match a path again a maintainer
> > section.
> >
> > It is done by using two different regex, the first one will take care of
> > converting ',' inside '{}' to a '|', one by one, as long as there is at
> > least two commas. The second regex will do the final convertion of '{,}'
> > to '(|)'.
> 
> Can you give some examples of the sorts of MAINTAINER entries this
> would allow (and maybe one that you intend to implement once this is
> accepted)?

With this patch, every entry of the current MAINTAINER file would work!
The one below for example would return the right sections/maintainers.
$ ./scripts/get_maintainer.pl --sections -f xen/common/kexec.c

Also, for every invocation of the script, I've got this message (x3)
(without the patch):
Unescaped left brace in regex is deprecated, passed through in regex; marked by 
<-- HERE in m/^xen/common/{ <-- HERE kexec,kimage}\.c/ at 
./scripts/get_maintainer.pl line 731.

One example I've tried the patch with, by adding a bogus entry:
  xen/common/{kexec,kimage,extra,patterns}.{c,h,S}

By the way, --sections does not print the original pattern, but it print
the pattern converted by to globing from the regex.

-- 
Anthony PERARD

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v12 3/6] vt-d: convert conditionals of qi_ctrl->qinval_maddr into ASSERT()s

[Tian, Kevin]

> From: Xu, Quan
> Sent: Friday, June 24, 2016 1:52 PM
> 
> From: Quan Xu 
> 
> QI ought to have got disabled if any of the IOMMU table setup
> failed. A QI function (other than enable_qinval) is unreachable
> when qi_ctrl->qinval_maddr is zero.
> 
> Signed-off-by: Quan Xu 

Acked-by: Kevin Tian 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH] xen: fix ram init regression

[Gerd Hoffmann]

Commit "8156d48 pc: allow raising low memory via max-ram-below-4g
option" causes a regression on xen, because it uses a different
memory split.

This patch initializes max-ram-below-4g to zero and leaves the
initialization to the memory initialization functions.  That way
they can pick different default values (max-ram-below-4g is zero
still) or use the user supplied value (max-ram-below-4g is non-zero).

Also skip the whole ram split calculation on Xen.  xen_ram_init()
does its own split calculation anyway so it is superfluous, also
this way xen_ram_init can actually see whenever max-ram-below-4g
is zero or not.

Reported-by: Anthony PERARD 
Tested-by: Anthony PERARD 
Signed-off-by: Gerd Hoffmann 
---
 hw/i386/pc.c  |  2 +-
 hw/i386/pc_piix.c | 52 +---
 hw/i386/pc_q35.c  |  3 +++
 xen-hvm.c |  3 +++
 4 files changed, 36 insertions(+), 24 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 7198ed5..66e1dae 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1886,7 +1886,7 @@ static void pc_machine_initfn(Object *obj)
 pc_machine_get_hotplug_memory_region_size,
 NULL, NULL, NULL, &error_abort);
 
-pcms->max_ram_below_4g = 0xe000; /* 3.5G */
+pcms->max_ram_below_4g = 0; /* use default */
 object_property_add(obj, PC_MACHINE_MAX_RAM_BELOW_4G, "size",
 pc_machine_get_max_ram_below_4g,
 pc_machine_set_max_ram_below_4g,
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 53bc968..f51fa77 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -108,37 +108,43 @@ static void pc_init1(MachineState *machine,
  *so legacy non-PAE guests can get as much memory as possible in
  *the 32bit address space below 4G.
  *
+ *  - Note that Xen has its own ram setp code in xen_ram_init(),
+ *called via xen_hvm_init().
+ *
  * Examples:
  *qemu -M pc-1.7 -m 4G(old default)-> 3584M low,  512M high
  *qemu -M pc -m 4G(new default)-> 3072M low, 1024M high
  *qemu -M pc,max-ram-below-4g=2G -m 4G -> 2048M low, 2048M high
  *qemu -M pc,max-ram-below-4g=4G -m 3968M  -> 3968M low (=4G-128M)
  */
-lowmem = pcms->max_ram_below_4g;
-if (machine->ram_size >= pcms->max_ram_below_4g) {
-if (pcmc->gigabyte_align) {
-if (lowmem > 0xc000) {
-lowmem = 0xc000;
-}
-if (lowmem & ((1ULL << 30) - 1)) {
-error_report("Warning: Large machine and max_ram_below_4g "
- "(%" PRIu64 ") not a multiple of 1G; "
- "possible bad performance.",
- pcms->max_ram_below_4g);
-}
-}
-}
-
-if (machine->ram_size >= lowmem) {
-pcms->above_4g_mem_size = machine->ram_size - lowmem;
-pcms->below_4g_mem_size = lowmem;
-} else {
-pcms->above_4g_mem_size = 0;
-pcms->below_4g_mem_size = machine->ram_size;
-}
-
 if (xen_enabled()) {
 xen_hvm_init(pcms, &ram_memory);
+} else {
+if (!pcms->max_ram_below_4g) {
+pcms->max_ram_below_4g = 0xe000; /* default: 3.5G */
+}
+lowmem = pcms->max_ram_below_4g;
+if (machine->ram_size >= pcms->max_ram_below_4g) {
+if (pcmc->gigabyte_align) {
+if (lowmem > 0xc000) {
+lowmem = 0xc000;
+}
+if (lowmem & ((1ULL << 30) - 1)) {
+error_report("Warning: Large machine and max_ram_below_4g "
+ "(%" PRIu64 ") not a multiple of 1G; "
+ "possible bad performance.",
+ pcms->max_ram_below_4g);
+}
+}
+}
+
+if (machine->ram_size >= lowmem) {
+pcms->above_4g_mem_size = machine->ram_size - lowmem;
+pcms->below_4g_mem_size = lowmem;
+} else {
+pcms->above_4g_mem_size = 0;
+pcms->below_4g_mem_size = machine->ram_size;
+}
 }
 
 pc_cpus_init(pcms);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index e4b541f..1b653e2 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -93,6 +93,9 @@ static void pc_q35_init(MachineState *machine)
 /* Handle the machine opt max-ram-below-4g.  It is basically doing
  * min(qemu limit, user limit).
  */
+if (!pcms->max_ram_below_4g) {
+pcms->max_ram_below_4g = 1ULL << 32; /* default: 4G */;
+}
 if (lowmem > pcms->max_ram_below_4g) {
 lowmem = pcms->max_ram_below_4g;
 if (machine->ram_size - lowmem > lowmem &&
diff --git a/xen-hvm.c b/xen-hvm.c
index 98ea44f..eb57792 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -190,6 +190,9 @@ static void xen_ram_init(PCMachineState *pcms,
 /* Handle the m

Re: [Xen-devel] [PATCH 1/2] xen: sched: rtds refactor code

[Meng Xu]

On Fri, Jun 24, 2016 at 3:45 AM, Dario Faggioli
 wrote:
> On Thu, 2016-06-23 at 11:42 +0100, George Dunlap wrote:
>> On 22/06/16 17:16, Meng Xu wrote:
>> >
>> > I think he is trying to align those comments to make them start
>> > from
>> > the same column. I was confused at the reason at the very
>> > beginning.
>> > Then I pulled his repo and checked this change.
>> Right -- well neither you as a reviewer nor anyone in the future
>> looking
>> back at this changeset should have to try to guess what the purpose
>> was;
>> if he did want to align them, that's perfectly fine, it just needs a
>> brief mention in the changelog. :-)
>>
> Indeed. BTW, I don't recall if we discussed this alignment
> previously,neither, in case we did, what my position was back then :-P
>
> In any case, I (now) think that having these comments aligned on a
> per-struct base is just fine, and there really is no need to have _all_
> of them aligned, across all structs.

Agree..

>
> I don't have a super strong opinion on this, and I'd be fine with it,
> if Meng is. I just think it's not worth the effort (of patching,
> reviewing, checking in, etc.)

Hmm, I don't have a strong opinion on this either.
I agree with George's comments on the commit message. I think it
should make this code change easier to understand in the future.
(Well, the code change is not hard to understand. So the improvement
of the commit message is not that critical.)

Now it's a matter of perfection or not.
If we want a "better" commit, I don't mind reviewing a new version. I
don't think this patch has been committed yet. So the extra work is in
our side, if we want to resend the patch. The work is as follows:
1) Replace "refactor" with "clean-up" for the patch title.
2) Go through and enumerate the different clean-ups this patch does.
Explain the reason for each type of the clean-ups. For instance, why
remove the __ at the head of functions (as described in the cover
letter)

OK. We need an action.
How about: Tianyang send another version for the work listed above. I
will review it again?

Thanks,

Meng

---
Meng Xu
PhD Student in Computer and Information Science
University of Pennsylvania
http://www.cis.upenn.edu/~mengxu/

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] pcie error containment: kill domain and dm without xend

[George Dunlap]

On Wed, Jun 22, 2016 at 9:16 PM, Elena Ufimtseva  wrote:
> Hello
>
> I am working on PCIe errors containment and XSA-124 relevant problem.
> This is only small part of the problem and I can provide more details later
> if that is of someone's interest.
> As the temporary solution, guest domain with passthrough device
> without SRIOV  gets killed when certain AER errors are triggered by
> dom0 AER code.
> In versions of xen with xend present, xenwatch can be used and pciback can
> write some fields to xenstore (as "aerfail" which is already present)
> and destroy device model and then domain itself.
> What would be the best way to initiate similar behaviour when xend is
> not used? Or maybe what is the best way to initiate device model
> destruction and domain itself without xend?

xl forks a background process per VM to monitor VMs and destroy device
models at the appropriate times -- see
tools/xl_cmdimpl.c:create_domain() (and in particular search for
"need_daemon").  This is the place to implement VM-watching features
such as xend had.

 -George

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v12 4/6] IOMMU/x86: using a struct pci_dev* instead of SBDF

[Tian, Kevin]

> From: Xu, Quan
> Sent: Friday, June 24, 2016 1:52 PM
> 
> From: Quan Xu 
> 
> a struct pci_dev* instead of SBDF is stored inside struct
> pci_ats_dev and parameter to enable_ats_device().
> 
> Signed-off-by: Quan Xu 

Can we unify the naming convention throughout the patch, e.g. 
always using ats_pdev for "struct pci_ats_dev" variable, while
pdev for "struct pci_dev". It's quite confusing when reading the
patch which has both named as pdev in various places I know 
the confusion is also in original code, but please take this chance 
to clean them up. :-)

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v12 5/6] IOMMU: move the domain crash logic up to the generic IOMMU layer

[Tian, Kevin]

> From: Xu, Quan
> Sent: Friday, June 24, 2016 1:52 PM
> 
> From: Quan Xu 
> 
> Signed-off-by: Quan Xu 
> 
> CC: Julien Grall 
> CC: Kevin Tian 
> CC: Feng Wu 
> CC: Jan Beulich 
> CC: Suravee Suthikulpanit 
> ---
>  xen/drivers/passthrough/iommu.c | 30
> --
>  xen/drivers/passthrough/vtd/iommu.c | 11 +++
>  2 files changed, 39 insertions(+), 2 deletions(-)

when you say "moving the logic up", I don't see any lines being
deleted. Looks you are just "adding the domain crash logic"?

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v12 6/6] vt-d: fix vt-d Device-TLB flush timeout issue

[Tian, Kevin]

> From: Xu, Quan
> Sent: Friday, June 24, 2016 1:52 PM
> 
> From: Quan Xu 
> 
> If Device-TLB flush timed out, we hide the target ATS device
> immediately and crash the domain owning this ATS device. If
> impacted domain is hardware domain, just throw out a warning.
> 
> By hiding the device, we make sure it can't be assigned to any
> domain any longer (see device_assigned).
> 
> Signed-off-by: Quan Xu 
> 
> CC: Jan Beulich 
> CC: Kevin Tian 
> CC: Feng Wu 
> 
> ---
> v12:
>1. a forward declaration struct pci_ats_dev*, instead of
>   including ats.h.
>2. eliminate the loop.
>3. use the same logic for logging and crashing as I did in
>   other series (despite I have moved the domain crash logic
>   up to the generic IOMMU layer, I think I am better still
>   leave it as is).
>4. enhance dev_invalidate_sync() with ASSERT().
> ---
>  xen/drivers/passthrough/pci.c |  6 +--
>  xen/drivers/passthrough/vtd/extern.h  |  5 ++-
>  xen/drivers/passthrough/vtd/qinval.c  | 75
> +--
>  xen/drivers/passthrough/vtd/x86/ats.c |  9 +
>  xen/include/xen/pci.h |  1 +
>  5 files changed, 71 insertions(+), 25 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
> index 98936f55c..843dc88 100644
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -419,7 +419,7 @@ static void free_pdev(struct pci_seg *pseg, struct 
> pci_dev *pdev)
>  xfree(pdev);
>  }
> 
> -static void _pci_hide_device(struct pci_dev *pdev)
> +void pci_hide_existing_device(struct pci_dev *pdev)

Don't understand what is the meaning of "hiding existing device".
Is there case where you may want to hide a non-existing device?

>  {
>  if ( pdev->domain )
>  return;
> @@ -436,7 +436,7 @@ int __init pci_hide_device(int bus, int devfn)
>  pdev = alloc_pdev(get_pseg(0), bus, devfn);
>  if ( pdev )
>  {
> -_pci_hide_device(pdev);
> +pci_hide_existing_device(pdev);
>  rc = 0;
>  }
>  pcidevs_unlock();
> @@ -466,7 +466,7 @@ int __init pci_ro_device(int seg, int bus, int devfn)
>  }
> 
>  __set_bit(PCI_BDF2(bus, devfn), pseg->ro_map);
> -_pci_hide_device(pdev);
> +pci_hide_existing_device(pdev);
> 
>  return 0;
>  }
> diff --git a/xen/drivers/passthrough/vtd/extern.h
> b/xen/drivers/passthrough/vtd/extern.h
> index 45357f2..efaff28 100644
> --- a/xen/drivers/passthrough/vtd/extern.h
> +++ b/xen/drivers/passthrough/vtd/extern.h
> @@ -25,6 +25,7 @@
> 
>  #define VTDPREFIX "[VT-D]"
> 
> +struct pci_ats_dev;
>  extern bool_t rwbf_quirk;
> 
>  void print_iommu_regs(struct acpi_drhd_unit *drhd);
> @@ -60,8 +61,8 @@ int dev_invalidate_iotlb(struct iommu *iommu, u16 did,
>   u64 addr, unsigned int size_order, u64 type);
> 
>  int __must_check qinval_device_iotlb_sync(struct iommu *iommu,
> -  u32 max_invs_pend,
> -  u16 sid, u16 size, u64 addr);
> +  struct pci_ats_dev *ats_dev,
> +  u16 did, u16 size, u64 addr);
> 
>  unsigned int get_cache_line_size(void);
>  void cacheline_flush(char *);
> diff --git a/xen/drivers/passthrough/vtd/qinval.c 
> b/xen/drivers/passthrough/vtd/qinval.c
> index 4492b29..e4e2771 100644
> --- a/xen/drivers/passthrough/vtd/qinval.c
> +++ b/xen/drivers/passthrough/vtd/qinval.c
> @@ -27,11 +27,11 @@
>  #include "dmar.h"
>  #include "vtd.h"
>  #include "extern.h"
> +#include "../ats.h"

Earlier you said:
>1. a forward declaration struct pci_ats_dev*, instead of
>   including ats.h.

But above you still have ats.h included.

> 
>  #define VTD_QI_TIMEOUT   1
> 
> -static int __must_check invalidate_sync(struct iommu *iommu,
> -bool_t flush_dev_iotlb);
> +static int __must_check invalidate_sync(struct iommu *iommu);

I don't understand the rationale behind. In earlier patch you introduce
a new parameter which is however just removed later here

Thanks
Kevin


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v12 6/6] vt-d: fix vt-d Device-TLB flush timeout issue

[Jan Beulich]

>>> On 24.06.16 at 13:55,  wrote:
>> --- a/xen/drivers/passthrough/pci.c
>> +++ b/xen/drivers/passthrough/pci.c
>> @@ -419,7 +419,7 @@ static void free_pdev(struct pci_seg *pseg, struct 
>> pci_dev *pdev)
>>  xfree(pdev);
>>  }
>> 
>> -static void _pci_hide_device(struct pci_dev *pdev)
>> +void pci_hide_existing_device(struct pci_dev *pdev)
> 
> Don't understand what is the meaning of "hiding existing device".
> Is there case where you may want to hide a non-existing device?

The distinction is whether alloc_pdev() needs calling, as can be seen ...

>> @@ -436,7 +436,7 @@ int __init pci_hide_device(int bus, int devfn)
>>  pdev = alloc_pdev(get_pseg(0), bus, devfn);
>>  if ( pdev )
>>  {
>> -_pci_hide_device(pdev);
>> +pci_hide_existing_device(pdev);
>>  rc = 0;
>>  }

... as the beginning context of this hunk.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 0/3] VMX: Properly handle pi descriptor and per-cpu blocking list

[Wu, Feng]

> > Thanks for your replay. Yes, I think this is point. Here descheduling
> > of vCPU3
> > happens, and the reason we will choose the tasklet as the next
> > running
> > unit for sure (not choosing another vCPU or vCPU3 itself as the next
> > running unit) is because tasklet will overrides all other choose as
> > stated in csched_schedule() as below, right?
> >
> Exactly, tasklets preempt the running vcpu and take precedence of
> runnable vcpus.
> 
> Then, in this case, the reason why we are sure that all the pcpus are
> executing the body of the tasklet, is indeed the structure of
> stop_machine_run() and stopmachine_action() themselves, which are built
> to make sure of that,

Thanks for the reply, I am sorry I don't quite understand the above
comment. In my understanding, the tasklet has higher priority, so
stopmachine_action() as the body of the tasklet preempts vCPU3.
Is this the case?

Thanks,
Feng

> much rather than just the fact that tasklet are
> higher priority.
> 
> But yes, this is what happens.
> 
Thanks for the clarification!

Thanks,
Feng

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 00/11] hvmctl hypercall

[David Vrabel]

On 24/06/16 11:35, Jan Beulich wrote:
 On 24.06.16 at 12:29,  wrote:
>> On 24/06/16 11:21, Jan Beulich wrote:
>>> A long while back separating out all control kind operations (intended
>>> for use by only the control domain or device model) from the currect
>>> hvmop hypercall has been discussed. This series aims at finally making
>>> this reality (at once allowing to streamline the associated XSM checking).
>>>
>>> 01: public / x86: introduce hvmctl hypercall
>>> 02: convert HVMOP_set_pci_intx_level
>>> 03: convert HVMOP_set_isa_irq_level
>>> 04: convert HVMOP_set_pci_link_route
>>> 05: convert HVMOP_track_dirty_vram
>>> 06: convert HVMOP_modified_memory
>>> 07: convert HVMOP_set_mem_type
>>> 08: convert HVMOP_inject_trap
>>> 09: convert HVMOP_inject_msi
>>> 10: convert HVMOP_*ioreq_server*
>>> 11: x86/HVM: serialize trap injecting producer and consumer
>>
>> Is hvmctl going to have a stable ABI?
> 
> No, that's why it is being versioned - just like domctl and sysctl.

Isn't this a backward step?  Don't we want to be able to (for example)
produce qemu stubdom images that aren't tied to specific Xen versions?

David

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 00/11] hvmctl hypercall

[Jan Beulich]

>>> On 24.06.16 at 15:27,  wrote:
> On 24/06/16 11:35, Jan Beulich wrote:
> On 24.06.16 at 12:29,  wrote:
>>> On 24/06/16 11:21, Jan Beulich wrote:
 A long while back separating out all control kind operations (intended
 for use by only the control domain or device model) from the currect
 hvmop hypercall has been discussed. This series aims at finally making
 this reality (at once allowing to streamline the associated XSM checking).

 01: public / x86: introduce hvmctl hypercall
 02: convert HVMOP_set_pci_intx_level
 03: convert HVMOP_set_isa_irq_level
 04: convert HVMOP_set_pci_link_route
 05: convert HVMOP_track_dirty_vram
 06: convert HVMOP_modified_memory
 07: convert HVMOP_set_mem_type
 08: convert HVMOP_inject_trap
 09: convert HVMOP_inject_msi
 10: convert HVMOP_*ioreq_server*
 11: x86/HVM: serialize trap injecting producer and consumer
>>>
>>> Is hvmctl going to have a stable ABI?
>> 
>> No, that's why it is being versioned - just like domctl and sysctl.
> 
> Isn't this a backward step?

No. This series merely splits off the unstable portion of HVMOP to a
separate hypercall.

>  Don't we want to be able to (for example)
> produce qemu stubdom images that aren't tied to specific Xen versions?

Yes. With libxc sitting in between this is no problem, at least if
carefully used (see patches 2, 3, and 4  for examples where full
conversion could not be done because of parts of the unstable
interface having leaked beyond libxc).

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 0/4] VMX: Properly handle pi descriptor and per-cpu blocking list

[Wu, Feng]

> -Original Message-
> From: Dario Faggioli [mailto:dario.faggi...@citrix.com]
> Sent: Friday, June 24, 2016 6:29 PM
> To: Wu, Feng ; xen-devel@lists.xen.org
> Cc: k...@xen.org; Tian, Kevin ; jbeul...@suse.com;
> andrew.coop...@citrix.com; george.dun...@eu.citrix.com;
> konrad.w...@oracle.com
> Subject: Re: [PATCH v2 0/4] VMX: Properly handle pi descriptor and per-cpu
> blocking list
> 
> On Fri, 2016-06-24 at 06:33 +, Wu, Feng wrote:
> > > -Original Message-
> > > From: Dario Faggioli [mailto:dario.faggi...@citrix.com]
> > > In your case, AFAICUI, it's:
> > >  - the vCPU should block, waiting for an event
> > >  - the event is _not_ arrived, so we indeed should block
> > >  - we do *not* block, due to some other reason
> > >
> > > That does not look right to me... what about the fact that we
> > > wanted to
> > > actually wait for the event? :-O
> > I understand your point. In my understanding, currently, vcpu_block()
> > is
> > for guest's HLT operation, which means, guest has nothing to do. In
> > this case, even we return (not blocking), seems the function is not
> > broken.
> >
> So, basically, you're saying that the vcpu has nothing to do, and in
> fact it executed an HLT instruction, and you want to let it continue to
> run? :-O

Here is my understanding, if the guest has nothing to do, it will
call HLT, and Xen hypervisor will call vcpu_block(), if we don't
block the vCPU and return to guest, guest will continue to run
HLT if it still has nothing to do. If this is the case, I feel there aren’t
problems with it. Anyway, I have replied to George's mail and
based on his suggestion, I proposed a new method to handle
the case that the last assigned devices is detached. Could you
please have a look at it when you have time. If that works, I think
we don't need to the above changes anymore. Thanks in advance!

Thanks,
Feng
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [xen-unstable-smoke test] 96214: tolerable all pass - PUSHED

[osstest service owner]

flight 96214 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/96214/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  8384dc2d95538c5910d98db3df3ff5448bf0af48
baseline version:
 xen  a6288d5bb8b970646368afe167a24eeba95e9e03

Last test of basis96189  2016-06-23 18:02:47 Z0 days
Failing since 96210  2016-06-24 09:01:07 Z0 days2 attempts
Testing same since96214  2016-06-24 11:02:52 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Dirk Behme 
  Jan Beulich 
  Julien Grall 
  Kevin Tian 
  Razvan Cojocaru 
  Tamas K Lengyel 

jobs:
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  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 :

+ branch=xen-unstable-smoke
+ revision=8384dc2d95538c5910d98db3df3ff5448bf0af48
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x '!=' x/home/osstest/repos/lock ']'
++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock
++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push xen-unstable-smoke 
8384dc2d95538c5910d98db3df3ff5448bf0af48
+ branch=xen-unstable-smoke
+ revision=8384dc2d95538c5910d98db3df3ff5448bf0af48
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']'
+ . ./cri-common
++ . ./cri-getconfig
++ umask 002
+ select_xenbranch
+ case "$branch" in
+ tree=xen
+ xenbranch=xen-unstable-smoke
+ qemuubranch=qemu-upstream-unstable
+ '[' xxen = xlinux ']'
+ linuxbranch=
+ '[' xqemu-upstream-unstable = x ']'
+ select_prevxenbranch
++ ./cri-getprevxenbranch xen-unstable-smoke
+ prevxenbranch=xen-4.7-testing
+ '[' x8384dc2d95538c5910d98db3df3ff5448bf0af48 = x ']'
+ : tested/2.6.39.x
+ . ./ap-common
++ : osst...@xenbits.xen.org
+++ getconfig OsstestUpstream
+++ perl -e '
use Osstest;
readglobalconfig();
print $c{"OsstestUpstream"} or die $!;
'
++ :
++ : git://xenbits.xen.org/xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/xen.git
++ : git://xenbits.xen.org/qemu-xen-traditional.git
++ : git://git.kernel.org
++ : git://git.kernel.org/pub/scm/linux/kernel/git
++ : git
++ : git://xenbits.xen.org/libvirt.git
++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git
++ : git://xenbits.xen.org/libvirt.git
++ : git://xenbits.xen.org/rumpuser-xen.git
++ : git
++ : git://xenbits.xen.org/rumpuser-xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/rumpuser-xen.git
+++ besteffort_repo https://github.com/rumpkernel/rumpkernel-netbsd-src
+++ local repo=https://github.com/rumpkernel/rumpkernel-netbsd-src
+++ cached_repo https://github.com/rumpkernel/rumpkernel-netbsd-src 
'[fetch=try]'
+++ local repo=https://github.com/rumpkernel/rumpkernel-netbsd-src
+++ local 'options=[fetch=try]'
 getconfig GitCacheProxy
 perl -e '
use Osstest;
readglobalconfig();

Re: [Xen-devel] [PATCH v2 0/4] VMX: Properly handle pi descriptor and per-cpu blocking list

[George Dunlap]

On 24/06/16 14:42, Wu, Feng wrote:
> 
> 
>> -Original Message-
>> From: Dario Faggioli [mailto:dario.faggi...@citrix.com]
>> Sent: Friday, June 24, 2016 6:29 PM
>> To: Wu, Feng ; xen-devel@lists.xen.org
>> Cc: k...@xen.org; Tian, Kevin ; jbeul...@suse.com;
>> andrew.coop...@citrix.com; george.dun...@eu.citrix.com;
>> konrad.w...@oracle.com
>> Subject: Re: [PATCH v2 0/4] VMX: Properly handle pi descriptor and per-cpu
>> blocking list
>>
>> On Fri, 2016-06-24 at 06:33 +, Wu, Feng wrote:
 -Original Message-
 From: Dario Faggioli [mailto:dario.faggi...@citrix.com]
 In your case, AFAICUI, it's:
  - the vCPU should block, waiting for an event
  - the event is _not_ arrived, so we indeed should block
  - we do *not* block, due to some other reason

 That does not look right to me... what about the fact that we
 wanted to
 actually wait for the event? :-O
>>> I understand your point. In my understanding, currently, vcpu_block()
>>> is
>>> for guest's HLT operation, which means, guest has nothing to do. In
>>> this case, even we return (not blocking), seems the function is not
>>> broken.
>>>
>> So, basically, you're saying that the vcpu has nothing to do, and in
>> fact it executed an HLT instruction, and you want to let it continue to
>> run? :-O
> 
> Here is my understanding, if the guest has nothing to do, it will
> call HLT, and Xen hypervisor will call vcpu_block(), if we don't
> block the vCPU and return to guest, guest will continue to run
> HLT if it still has nothing to do. 

As it happens, most operating systems will handle this situation just
fine; but I'm not sure this is something we want to rely on.

 -George

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 00/11] hvmctl hypercall

[David Vrabel]

On 24/06/16 14:37, Jan Beulich wrote:
 On 24.06.16 at 15:27,  wrote:
>> On 24/06/16 11:35, Jan Beulich wrote:
>> On 24.06.16 at 12:29,  wrote:
 On 24/06/16 11:21, Jan Beulich wrote:
> A long while back separating out all control kind operations (intended
> for use by only the control domain or device model) from the currect
> hvmop hypercall has been discussed. This series aims at finally making
> this reality (at once allowing to streamline the associated XSM checking).
>
> 01: public / x86: introduce hvmctl hypercall
> 02: convert HVMOP_set_pci_intx_level
> 03: convert HVMOP_set_isa_irq_level
> 04: convert HVMOP_set_pci_link_route
> 05: convert HVMOP_track_dirty_vram
> 06: convert HVMOP_modified_memory
> 07: convert HVMOP_set_mem_type
> 08: convert HVMOP_inject_trap
> 09: convert HVMOP_inject_msi
> 10: convert HVMOP_*ioreq_server*
> 11: x86/HVM: serialize trap injecting producer and consumer

 Is hvmctl going to have a stable ABI?
>>>
>>> No, that's why it is being versioned - just like domctl and sysctl.
>>
>> Isn't this a backward step?
> 
> No. This series merely splits off the unstable portion of HVMOP to a
> separate hypercall.

It's not a forward step either.

>>  Don't we want to be able to (for example)
>> produce qemu stubdom images that aren't tied to specific Xen versions?
> 
> Yes. With libxc sitting in between this is no problem, at least if
> carefully used (see patches 2, 3, and 4  for examples where full
> conversion could not be done because of parts of the unstable
> interface having leaked beyond libxc).

There has been discussion in the past about creating a stable hypervisor
ABI for use by device models (and thus a userspace library with a stable
ABI and API).

Why is this conversion not working towards this?

David

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3 7/8] xen/arm: Rework the interface of p2m_lookup and use typesafe gfn and mfn

[Julien Grall]

Hi Stefano,

On 23/06/16 15:14, Stefano Stabellini wrote:

On Tue, 21 Jun 2016, Julien Grall wrote:

The prototype and the declaration of p2m_lookup disagree on how the
function should be used. One expect a frame number whilst the other
an address.

Thankfully, everyone is using with an address today. However, most of
the callers have to convert a guest frame to an address. Modify
the interface to take a guest physical frame in parameter and return
a machine frame.

Whilst modifying the interface, use typesafe gfn and mfn for clarity
and catching possible misusage.

Signed-off-by: Julien Grall 
---
  xen/arch/arm/p2m.c| 37 -
  xen/arch/arm/traps.c  | 21 +++--
  xen/include/asm-arm/p2m.h |  7 +++
  3 files changed, 34 insertions(+), 31 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 47cb383..f3330dd 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -140,14 +140,15 @@ void flush_tlb_domain(struct domain *d)
  }

  /*
- * Lookup the MFN corresponding to a domain's PFN.
+ * Lookup the MFN corresponding to a domain's GFN.
   *
   * There are no processor functions to do a stage 2 only lookup therefore we
   * do a a software walk.
   */
-static paddr_t __p2m_lookup(struct domain *d, paddr_t paddr, p2m_type_t *t)
+static mfn_t __p2m_lookup(struct domain *d, gfn_t gfn, p2m_type_t *t)
  {
  struct p2m_domain *p2m = &d->arch.p2m;
+const paddr_t paddr = pfn_to_paddr(gfn_x(gfn));
  const unsigned int offsets[4] = {
  zeroeth_table_offset(paddr),
  first_table_offset(paddr),
@@ -158,7 +159,7 @@ static paddr_t __p2m_lookup(struct domain *d, paddr_t 
paddr, p2m_type_t *t)
  ZEROETH_MASK, FIRST_MASK, SECOND_MASK, THIRD_MASK
  };
  lpae_t pte, *map;
-paddr_t maddr = INVALID_PADDR;
+mfn_t mfn = _mfn(INVALID_MFN);


It might be worth defining INVALID_MFN_T and just assign that to mfn.


Good idea. It will be useful in other places too.




  paddr_t mask = 0;
  p2m_type_t _t;
  unsigned int level, root_table;



[...]


@@ -1561,11 +1565,10 @@ p2m_mem_access_check_and_get_page(vaddr_t gva, unsigned 
long flag)
   * We had a mem_access permission limiting the access, but the page type
   * could also be limiting, so we need to check that as well.
   */
-maddr = __p2m_lookup(current->domain, ipa, &t);
-if ( maddr == INVALID_PADDR )
+mfn = mfn_x(__p2m_lookup(current->domain, gfn, &t));
+if ( mfn == INVALID_MFN )


The conversion would go away if we had an INVALID_MFN which is mfn_t


Will do.

Regards,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [GIT PULL] xen: bug fixes for 4.7-rc4

[David Vrabel]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

Linus,

Please git pull the following tag:

 git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git 
for-linus-4.7b-rc4-tag

xen: bug fixes for 4.7-rc4

- - Fix x86 PV dom0 crash during early boot on some hardware.
- - Fix two pciback bugs affects certain devices.
- - Fix potential overflow when clearing page tables in x86 PV.

Thanks.

David

 arch/x86/xen/mmu.c  | 74 +
 drivers/xen/balloon.c   | 28 +--
 drivers/xen/xen-pciback/conf_space.c|  6 +--
 drivers/xen/xen-pciback/conf_space_header.c | 18 ---
 4 files changed, 58 insertions(+), 68 deletions(-)

Andrey Grodzovsky (1):
  xen/pciback: Fix conf_space read/write overlap check.

David Vrabel (1):
  x86/xen: avoid m2p lookup when setting early page table entries

Jan Beulich (1):
  xen-pciback: return proper values during BAR sizing

Juergen Gross (1):
  x86/xen: fix upper bound of pmd loop in xen_cleanhighmap()

Ross Lagerwall (1):
  xen/balloon: Fix declared-but-not-defined warning
-BEGIN PGP SIGNATURE-
Version: GnuPG v2

iQEcBAEBCAAGBQJXbTzSAAoJEFxbo/MsZsTRW8UIAIPh6MsgfLOtobIp69PiSUXz
+xE/q+2TXJTv81jOm95lFmxM8WPwusM7G9M8WpT7AdiXOTmnYGYAuG8qkwEhytGc
17cV6tOjwDAKIVTZmCwfkrwQy2FsrQ4zsX59AgqqjB16s5J5HqPaazM/NAXCqMej
PFt9edlCLOw1FbdYCZGRsxa02tEnuh1qldGVwNIs5TQWrB8+hXp7KQmAETcujZJe
UDCMkBQ5BYMiI2E3e54wi06nQlTG8JQFuCqFhe3BGOycvD6t/DhikbkGwavS/4nN
78xpr+cUtE7NpDZqrqQCdk8t0VtmACSL9vTOFqPdgRctmLPt71VX3n4ebjjXpN8=
=fM1R
-END PGP SIGNATURE-

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3 7/8] xen/arm: Rework the interface of p2m_lookup and use typesafe gfn and mfn

[Andrew Cooper]

On 24/06/16 14:58, Julien Grall wrote:
> Hi Stefano,
>
> On 23/06/16 15:14, Stefano Stabellini wrote:
>> On Tue, 21 Jun 2016, Julien Grall wrote:
>>> The prototype and the declaration of p2m_lookup disagree on how the
>>> function should be used. One expect a frame number whilst the other
>>> an address.
>>>
>>> Thankfully, everyone is using with an address today. However, most of
>>> the callers have to convert a guest frame to an address. Modify
>>> the interface to take a guest physical frame in parameter and return
>>> a machine frame.
>>>
>>> Whilst modifying the interface, use typesafe gfn and mfn for clarity
>>> and catching possible misusage.
>>>
>>> Signed-off-by: Julien Grall 
>>> ---
>>>   xen/arch/arm/p2m.c| 37 -
>>>   xen/arch/arm/traps.c  | 21 +++--
>>>   xen/include/asm-arm/p2m.h |  7 +++
>>>   3 files changed, 34 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
>>> index 47cb383..f3330dd 100644
>>> --- a/xen/arch/arm/p2m.c
>>> +++ b/xen/arch/arm/p2m.c
>>> @@ -140,14 +140,15 @@ void flush_tlb_domain(struct domain *d)
>>>   }
>>>
>>>   /*
>>> - * Lookup the MFN corresponding to a domain's PFN.
>>> + * Lookup the MFN corresponding to a domain's GFN.
>>>*
>>>* There are no processor functions to do a stage 2 only lookup
>>> therefore we
>>>* do a a software walk.
>>>*/
>>> -static paddr_t __p2m_lookup(struct domain *d, paddr_t paddr,
>>> p2m_type_t *t)
>>> +static mfn_t __p2m_lookup(struct domain *d, gfn_t gfn, p2m_type_t *t)
>>>   {
>>>   struct p2m_domain *p2m = &d->arch.p2m;
>>> +const paddr_t paddr = pfn_to_paddr(gfn_x(gfn));
>>>   const unsigned int offsets[4] = {
>>>   zeroeth_table_offset(paddr),
>>>   first_table_offset(paddr),
>>> @@ -158,7 +159,7 @@ static paddr_t __p2m_lookup(struct domain *d,
>>> paddr_t paddr, p2m_type_t *t)
>>>   ZEROETH_MASK, FIRST_MASK, SECOND_MASK, THIRD_MASK
>>>   };
>>>   lpae_t pte, *map;
>>> -paddr_t maddr = INVALID_PADDR;
>>> +mfn_t mfn = _mfn(INVALID_MFN);
>>
>> It might be worth defining INVALID_MFN_T and just assign that to mfn.
>
> Good idea. It will be useful in other places too.
>
>>
>>>   paddr_t mask = 0;
>>>   p2m_type_t _t;
>>>   unsigned int level, root_table;
>
>
> [...]
>
>>> @@ -1561,11 +1565,10 @@ p2m_mem_access_check_and_get_page(vaddr_t
>>> gva, unsigned long flag)
>>>* We had a mem_access permission limiting the access, but the
>>> page type
>>>* could also be limiting, so we need to check that as well.
>>>*/
>>> -maddr = __p2m_lookup(current->domain, ipa, &t);
>>> -if ( maddr == INVALID_PADDR )
>>> +mfn = mfn_x(__p2m_lookup(current->domain, gfn, &t));
>>> +if ( mfn == INVALID_MFN )
>>
>> The conversion would go away if we had an INVALID_MFN which is mfn_t

Be careful.

INVALID_MFN_T is fine for assignment, but you can't do plain equality
tests of opaque structures.

mfn_t m = _mfn(0);
mfn_t inval = INVALID_MFN_T; // Ok
mfn_equal(m, INVALID_MFN_T); // Ok
mfn_equal(m, inval); // Ok
m == inval; // Compilation error

This was the reason that I didn't previously introduce INVALID_MFN_T,
although with mfn_equal(), perhaps the time has come.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3 1/2] xen-pciback: return proper values during BAR sizing

[David Vrabel]

On 24/06/16 10:13, Jan Beulich wrote:
> Reads following writes with all address bits set to 1 should return all
> changeable address bits as one, not the BAR size (nor, as was the case
> for the upper half of 64-bit BARs, the high half of the region's end
> address). Presumably this didn't cause any problems so far because
> consumers use the value to calculate the size (usually via val & -val),
> and do nothing else with it.
> 
> But also consider the exception here: Unimplemented BARs should always
> return all zeroes.
> 
> And finally, the check for whether to return the sizing address on read
> for the ROM BAR should ignore all non-address bits, not just the ROM
> Enable one.

Applied to for-linus-4.7b, thanks.

David

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [ovmf test] 96196: regressions - FAIL

[osstest service owner]

flight 96196 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/96196/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemuu-ovmf-amd64 17 guest-start/debianhvm.repeat fail REGR. 
vs. 94748
 test-amd64-amd64-xl-qemuu-ovmf-amd64 17 guest-start/debianhvm.repeat fail 
REGR. vs. 94748

version targeted for testing:
 ovmf c7fefb690661f2e38afcb8200bd318ecf38ab961
baseline version:
 ovmf dc99315b8732b6e3032d01319d3f534d440b43d0

Last test of basis94748  2016-05-24 22:43:25 Z   30 days
Failing since 94750  2016-05-25 03:43:08 Z   30 days   54 attempts
Testing same since96196  2016-06-23 22:45:51 Z0 days1 attempts


People who touched revisions under test:
  Ard Biesheuvel 
  Chao Zhang 
  Cinnamon Shia 
  Cohen, Eugene 
  Dandan Bi 
  Darbin Reyes 
  Eric Dong 
  Eugene Cohen 
  Evan Lloyd 
  Fu Siyuan 
  Fu, Siyuan 
  Gary Li 
  Gary Lin 
  Giri P Mudusuru 
  Hao Wu 
  Hegde Nagaraj P 
  hegdenag 
  Heyi Guo 
  Jan D?bro? 
  Jan Dabros 
  Jeff Fan 
  Jiaxin Wu 
  Jiewen Yao 
  Katie Dellaquila 
  Laszlo Ersek 
  Liming Gao 
  Lu, ShifeiX A 
  lushifex 
  Marcin Wojtas 
  Marvin H?user 
  Marvin Haeuser 
  Maurice Ma 
  Michael Zimmermann 
  Qiu Shumin 
  Ruiyu Ni 
  Ryan Harkin 
  Sami Mujawar 
  Satya Yarlagadda 
  Sriram Subramanian 
  Star Zeng 
  Tapan Shah 
  Thomas Palmer 
  Yonghong Zhu 
  Zhang Lubo 
  Zhang, Chao B 

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 fail
 test-amd64-i386-xl-qemuu-ovmf-amd64  fail



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Not pushing.

(No revision log; it would be 2934 lines long.)

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 05/14] libxl: Load guest BIOS from file

[Anthony PERARD]

On Fri, Jun 24, 2016 at 01:23:19AM -0600, Jan Beulich wrote:
> >>> On 22.06.16 at 19:15,  wrote:
> > --- a/tools/libxl/libxl_paths.c
> > +++ b/tools/libxl/libxl_paths.c
> > @@ -35,6 +35,16 @@ const char *libxl__run_dir_path(void)
> >  return XEN_RUN_DIR;
> >  }
> >  
> > +const char *libxl__seabios_path(void)
> > +{
> > +return SEABIOS_PATH;
> > +}
> > +
> > +const char *libxl__ovmf_path(void)
> > +{
> > +return OVMF_PATH;
> > +}
> 
> With an earlier version of this series pulled into one of our branches,
> I've run into a problem with this: The paths you return here are the
> configured paths, and that's intended. Yet it breaks running the
> tools out of the build area (i.e. without any "make install"), which so
> far has been working fine (as apparently in all other relevant cases
> where paths are needed, relative ones are being used), and which

I'm not sure that true about the relative paths, I think most, if not
all are full path and happen to match what is already install on the
system. I've tried to run xl from the build dir (and also dist dir) and
with a different --prefix, xl can not find qemu and if I run xl, this
time configure with the same --prefix and I remove hvmloader from my
system, xl can not find hvmloader.

You could copy both firmware manually to be in the same directory as
hvmloader. A better solution would be to use all the _override xl
config, I've added `bios_path_override' so one can supply a different
bios/firmware to libxl.

To be honest, I don't know which relative path to use since it depend on
where is xl executed from. And I don't think the path to the firmwares
(like hvmloader) can be change at execution time unless change for each
of them.

Is that answer your question?

> I much prefer over the hassle of scattering around half a dozen of
> different Xen tools versions in custom directories under, say,
> /usr/local. I guess if I'm the only one using this, I'll have to find my
> own local solution for this, but of course I'd prefer for this currently
> working case not to get broken.
> 
> Jan
> 

-- 
Anthony PERARD

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] Binary compatibility report for Xen base libraries

[Ponomarenko Andrey]

Hello,

I maintain a new project for backward compatibility analysis of the Linux ABIs. 
The report for Xen base libraries has been recently added to the project: 
http://abi-laboratory.pro/tracker/timeline/xen/

The report is generated daily with the help of the abi-compliance-checker, 
abi-dumper and abi-tracker tools: https://github.com/lvc/abi-tracker

Hope this will help users, maintainers and developers to maintain backward 
compatibility.

Thank you.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 00/11] hvmctl hypercall

[Jan Beulich]

>>> On 24.06.16 at 15:51,  wrote:
> On 24/06/16 14:37, Jan Beulich wrote:
> On 24.06.16 at 15:27,  wrote:
>>> On 24/06/16 11:35, Jan Beulich wrote:
>>> On 24.06.16 at 12:29,  wrote:
> On 24/06/16 11:21, Jan Beulich wrote:
>> A long while back separating out all control kind operations (intended
>> for use by only the control domain or device model) from the currect
>> hvmop hypercall has been discussed. This series aims at finally making
>> this reality (at once allowing to streamline the associated XSM 
>> checking).
>>
>> 01: public / x86: introduce hvmctl hypercall
>> 02: convert HVMOP_set_pci_intx_level
>> 03: convert HVMOP_set_isa_irq_level
>> 04: convert HVMOP_set_pci_link_route
>> 05: convert HVMOP_track_dirty_vram
>> 06: convert HVMOP_modified_memory
>> 07: convert HVMOP_set_mem_type
>> 08: convert HVMOP_inject_trap
>> 09: convert HVMOP_inject_msi
>> 10: convert HVMOP_*ioreq_server*
>> 11: x86/HVM: serialize trap injecting producer and consumer
>
> Is hvmctl going to have a stable ABI?

 No, that's why it is being versioned - just like domctl and sysctl.
>>>
>>> Isn't this a backward step?
>> 
>> No. This series merely splits off the unstable portion of HVMOP to a
>> separate hypercall.
> 
> It's not a forward step either.

Well - depends on what is relevant to you. It's not a step forward
for the one aspect you mention. The consolidation XSM-wise, otoh,
is a step forward imo.

>>>  Don't we want to be able to (for example)
>>> produce qemu stubdom images that aren't tied to specific Xen versions?
>> 
>> Yes. With libxc sitting in between this is no problem, at least if
>> carefully used (see patches 2, 3, and 4  for examples where full
>> conversion could not be done because of parts of the unstable
>> interface having leaked beyond libxc).
> 
> There has been discussion in the past about creating a stable hypervisor
> ABI for use by device models (and thus a userspace library with a stable
> ABI and API).

The two are really mostly independent: A properly designed user
mode library interface can shield against any changes in the
underlying hypervisor ABI. I don't see what this has to do with this
series - that's entirely a tool stack thing. (After all the instability
doesn't have to go as far as subops disappearing all of the sudden;
it may well just mean tweaks to the existing interface.)

> Why is this conversion not working towards this?

Because that wasn't the intention? I have to admit I don't
understand your questions: As said elsewhere in the discussion
of this series, this is not a result of IanC's outlining of a stable
ABI for qemu to use; instead the work item this removed from
my todo list was a much older one (which, as it happens, also
resulted from a discussion with IanC).

And then - what's your expectation? Any parts of this new
interface can subsequently be marked stable if we so wish. I
don't see why this needs to happen right away.

As to secure usability with a deprivileged qemu (stubdom or
otherwise), without any investigation as to whether the
_implementation_ of those operations actually permits them
being marked so I don't think this can reasonable be stated. And
the auditing required to be certain here is a whole big separate
work item.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 00/11] hvmctl hypercall

[George Dunlap]

On 24/06/16 15:25, Jan Beulich wrote:
 On 24.06.16 at 15:51,  wrote:
>> On 24/06/16 14:37, Jan Beulich wrote:
>> On 24.06.16 at 15:27,  wrote:
 On 24/06/16 11:35, Jan Beulich wrote:
 On 24.06.16 at 12:29,  wrote:
>> On 24/06/16 11:21, Jan Beulich wrote:
>>> A long while back separating out all control kind operations (intended
>>> for use by only the control domain or device model) from the currect
>>> hvmop hypercall has been discussed. This series aims at finally making
>>> this reality (at once allowing to streamline the associated XSM 
>>> checking).
>>>
>>> 01: public / x86: introduce hvmctl hypercall
>>> 02: convert HVMOP_set_pci_intx_level
>>> 03: convert HVMOP_set_isa_irq_level
>>> 04: convert HVMOP_set_pci_link_route
>>> 05: convert HVMOP_track_dirty_vram
>>> 06: convert HVMOP_modified_memory
>>> 07: convert HVMOP_set_mem_type
>>> 08: convert HVMOP_inject_trap
>>> 09: convert HVMOP_inject_msi
>>> 10: convert HVMOP_*ioreq_server*
>>> 11: x86/HVM: serialize trap injecting producer and consumer
>>
>> Is hvmctl going to have a stable ABI?
>
> No, that's why it is being versioned - just like domctl and sysctl.

 Isn't this a backward step?
>>>
>>> No. This series merely splits off the unstable portion of HVMOP to a
>>> separate hypercall.
>>
>> It's not a forward step either.
> 
> Well - depends on what is relevant to you. It's not a step forward
> for the one aspect you mention. The consolidation XSM-wise, otoh,
> is a step forward imo.
> 
  Don't we want to be able to (for example)
 produce qemu stubdom images that aren't tied to specific Xen versions?
>>>
>>> Yes. With libxc sitting in between this is no problem, at least if
>>> carefully used (see patches 2, 3, and 4  for examples where full
>>> conversion could not be done because of parts of the unstable
>>> interface having leaked beyond libxc).
>>
>> There has been discussion in the past about creating a stable hypervisor
>> ABI for use by device models (and thus a userspace library with a stable
>> ABI and API).
> 
> The two are really mostly independent: A properly designed user
> mode library interface can shield against any changes in the
> underlying hypervisor ABI. I don't see what this has to do with this
> series - that's entirely a tool stack thing. (After all the instability
> doesn't have to go as far as subops disappearing all of the sudden;
> it may well just mean tweaks to the existing interface.)
> 
>> Why is this conversion not working towards this?
> 
> Because that wasn't the intention? I have to admit I don't
> understand your questions: As said elsewhere in the discussion
> of this series, this is not a result of IanC's outlining of a stable
> ABI for qemu to use; instead the work item this removed from
> my todo list was a much older one (which, as it happens, also
> resulted from a discussion with IanC).

Yes -- I looked back over the discussion we had last year internally
about the deprivileged qemu, and a lot of this work was spoken of at
that time as a clean-up which had already been desired in its own right.

 -George


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 0/4] VMX: Properly handle pi descriptor and per-cpu blocking list

[Dario Faggioli]

On Fri, 2016-06-24 at 14:49 +0100, George Dunlap wrote:
> On 24/06/16 14:42, Wu, Feng wrote:
> > Here is my understanding, if the guest has nothing to do, it will
> > call HLT, and Xen hypervisor will call vcpu_block(), if we don't
> > block the vCPU and return to guest, guest will continue to run
> > HLT if it still has nothing to do. 
> As it happens, most operating systems will handle this situation just
> fine; but I'm not sure this is something we want to rely on.
> 
Exactly! I indeed don't think it would be wise to rely on that. But
more than that, I don't think we want to waste pCPU cycles considering
runnable, and hence potentially scheduling, vCPUs that have HTL-ed!

That may be acceptable in emulators like Bochs or "plain" Qemu... Xen
is an hypervisor, it does virtualization, and this is one of the points
of virtualization, IMO: when a virtual CPU has nothing to do, use the
physical CPU to execute _another_ virtual CPU.

And even if that would be just for the sake of handling a corner case,
I still think that:
 - we really want to try as hard as possible to avoid it,
 - if we really can't, we want a big comment in the code for:
  * explaining what actually happens,
  * proving that it will be something limited in time (and preferably
    rather short),
  * providing an explanations of why we could not do better.

Regards,
Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)



signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] Binary compatibility report for Xen base libraries

[Doug Goldstein]

On 6/24/16 9:21 AM, Ponomarenko Andrey wrote:
> Hello,
> 
> I maintain a new project for backward compatibility analysis of the Linux 
> ABIs. The report for Xen base libraries has been recently added to the 
> project: http://abi-laboratory.pro/tracker/timeline/xen/
> 
> The report is generated daily with the help of the abi-compliance-checker, 
> abi-dumper and abi-tracker tools: https://github.com/lvc/abi-tracker
> 
> Hope this will help users, maintainers and developers to maintain backward 
> compatibility.
> 
> Thank you.
> 

Outstanding! I had mentioned your tools in the #xen-devel channel. And I
had talked about utilizing them as part of the Travis build process with
the goal of making this data available. Thank you for doing this.

Do you have any advice for us if it would be positive to do some sort of
check on a per-commit basis or not?

-- 
Doug Goldstein



signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 13/17 v3] xen: move FLASK entry under XSM in Kconfig

[Doug Goldstein]

On 6/21/16 12:09 PM, Daniel De Graaf wrote:
> Since enabling XSM is required to enable FLASK, place the option for
> FLASK below the one for XSM.  In addition, since it does not make sense
> to enable XSM without any XSM providers, and FLASK is the only XSM
> provider, hide the option to disable FLASK under EXPERT.
> 
> Signed-off-by: Daniel De Graaf 

Thanks for the update Daniel.

Reviewed-by: Doug Goldstein 

-- 
Doug Goldstein



signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 2/2] xen-pciback: clean up {bar, rom}_init()

[David Vrabel]

On 07/06/16 07:31, Jan Beulich wrote:
> - drop unused function parameter of read_dev_bar()
> - drop rom_init() (now identical to bar_init())
> - fold read_dev_bar() into its now single caller
> - simplify determination of 64-bit memory resource
> - use const and unsigned

Please split this in 5 separate patches for easier review.

Especially as often anyone writing "simplify" means "accidentally break".

David

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 00/11] hvmctl hypercall

[David Vrabel]

On 24/06/16 15:25, Jan Beulich wrote:
> 
> And then - what's your expectation? Any parts of this new
> interface can subsequently be marked stable if we so wish. I
> don't see why this needs to happen right away.

Ok.

David

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] pcie error containment: kill domain and dm without xend

[Elena Ufimtseva]

Thanks George!


On Fri, Jun 24, 2016 at 4:37 AM, George Dunlap  wrote:
> On Wed, Jun 22, 2016 at 9:16 PM, Elena Ufimtseva  wrote:
>> Hello
>>
>> I am working on PCIe errors containment and XSA-124 relevant problem.
>> This is only small part of the problem and I can provide more details later
>> if that is of someone's interest.
>> As the temporary solution, guest domain with passthrough device
>> without SRIOV  gets killed when certain AER errors are triggered by
>> dom0 AER code.
>> In versions of xen with xend present, xenwatch can be used and pciback can
>> write some fields to xenstore (as "aerfail" which is already present)
>> and destroy device model and then domain itself.
>> What would be the best way to initiate similar behaviour when xend is
>> not used? Or maybe what is the best way to initiate device model
>> destruction and domain itself without xend?
>
> xl forks a background process per VM to monitor VMs and destroy device
> models at the appropriate times -- see
> tools/xl_cmdimpl.c:create_domain() (and in particular search for
> "need_daemon").  This is the place to implement VM-watching features
> such as xend had.
>
>  -George



-- 
Elena

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v1 Altp2m cleanup 1/3] altp2m cleanup work

[Lai, Paul C]

[Paul] inlined

-Original Message-
From: Jan Beulich [mailto:jbeul...@suse.com] 
Sent: Thursday, June 23, 2016 11:19 PM
To: Lai, Paul C 
Cc: Sahita, Ravi ; xen-devel 

Subject: RE: [PATCH v1 Altp2m cleanup 1/3] altp2m cleanup work

>>> On 23.06.16 at 20:23,  wrote:

First of all - please don't top post.
[Paul] Understood

> I'm opposed to moving HVMOP_cmd_min and HVMOP_cmd_max somewhere else.  
> That would make reading/understanding of the macros more difficult.  
> This practice is common.  Also, If min & max are defined elsewhere, it 
> will be more likely to lead to mistakes/bugs.

I understand that, but I'm going to nak any patch that introduces clutter 
(which then will need to be retained forever) to the public interface. A 
possible compromise might be to frame these in __XEN__ conditionals, but this 
would need to be outweighed by the resulting benefits, which I'm not convinced 
of.

[Paul] Still waiting for a better idea.

> The use of "_min" and "_max" should be quite clear and is common use 
> in linux code; Yes, I know this is xen code and I see it here too.  If 
> there's a better way, please propose the better.  Maybe you're 
> suggesting the macro names should be all caps:
>   HVMOP_CMD_MIN, HVMOP_CMD_MAX
> ?  I was following the coding style within the file itself.

The problem isn't the use of min and max, but the selected names suggesting 
these are the minimum/maximum HVMOPs, whereas
- afaict - you really mean to frame the altp2m subset. Which btw, together with 
the above, points at another issue here: What if later another altp2m subop 
gets added, and especially when its new value ends up not being adjacent to the 
existing range?

[Paul] Again, waiting for a better idea.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] monitor access to pages with a specific p2m_type_t

[Tamas K Lengyel]

On Jun 24, 2016 05:19, "Razvan Cojocaru"  wrote:
>
> On 06/24/2016 02:05 PM, George Dunlap wrote:
> > On Wed, Jun 22, 2016 at 12:38 PM, sepanta s 
wrote:
> >> Hi,
> >> Is it possible to monitor the access on the pages withp2m_type_t
> >> p2m_ram_shared?
> >
> > cc'ing Tamas and Razvan
>
> Thanks for the CC. Judging by the "if ( npfec.write_access && (p2mt ==
> p2m_ram_shared) )" line in hvm_hap_nested_page_fault() (from
> xen/arch/x86/hvm/hvm.c), I'd say it certainly looks possible. But I
> don't know what the context of the question is.
>
>
> Thanks,
> Razvan

Yes, p2m_ram_shared type pages can be monitored with mem_access just as
normal pages. The only part that may be tricky is if you map the page into
your monitoring application while the page is shared. Your handle will
continue to be valid even if the page is unshared but it will continue to
point to the shared page. However, even if you catch write access events to
the shared page that will lead to unsharing, the mem_access notification is
sent before unsharing. I just usually do unsharing myself in the mem_access
callback manually for monitored pages for this reason. I might change the
flow in 4.8 to send the notification after the unsharing happened to
simplify this.

Tamas
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [libvirt test] 96204: regressions - FAIL

[osstest service owner]

flight 96204 libvirt real [real]
http://logs.test-lab.xenproject.org/osstest/logs/96204/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail 
REGR. vs. 96157

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass

version targeted for testing:
 libvirt  541bd183f7caaa16cf325c9227e996458a2b8c4a
baseline version:
 libvirt  03ce1328086d6937d2647d616efff29941a3e80a

Last test of basis96157  2016-06-23 04:24:23 Z1 days
Testing same since96204  2016-06-24 04:26:09 Z0 days1 attempts


People who touched revisions under test:
  Cole Robinson 
  John Ferlan 
  Ján Tomko 
  Nikolay Shirokovskiy 
  Peter Krempa 

jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmfail
 test-amd64-amd64-libvirt-xsm pass
 test-armhf-armhf-libvirt-xsm fail
 test-amd64-i386-libvirt-xsm  pass
 test-amd64-amd64-libvirt pass
 test-armhf-armhf-libvirt fail
 test-amd64-i386-libvirt  pass
 test-amd64-amd64-libvirt-pairpass
 test-amd64-i386-libvirt-pair pass
 test-armhf-armhf-libvirt-qcow2   fail
 test-armhf-armhf-libvirt-raw fail
 test-amd64-amd64-libvirt-vhd pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.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 344 lines long.)

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 17/17] xsm: add a default policy to .init.data

[Julien Grall]

Hello Daniel,

Please try to CC relevant maintainers on your patch. I would have missed 
it if Andrew did not ping me on IRC.


On 20/06/16 15:04, Daniel De Graaf wrote:

This adds a Kconfig option and support for including the XSM policy from
tools/flask/policy in the hypervisor so that the bootloader does not
need to provide a policy to get sane behavior from an XSM-enabled
hypervisor.  The policy provided by the bootloader, if present, will
override the built-in policy.

Enabling this option only builds the policy if checkpolicy is available
during compilation of the hypervisor; otherwise, it does nothing.  The
XSM policy is not moved out of tools because that remains the primary
location for installing and configuring the policy.

Signed-off-by: Daniel De Graaf 
Reviewed-by: Konrad Rzeszutek Wilk 


For ARM bits:

Acked-by: Julien Grall 

Although, I one a question below.

[...]


diff --git a/xen/xsm/flask/Makefile b/xen/xsm/flask/Makefile
index 12fc3a9..eefd37c 100644
--- a/xen/xsm/flask/Makefile
+++ b/xen/xsm/flask/Makefile
@@ -27,6 +27,23 @@ $(FLASK_H_FILES): $(FLASK_H_DEPEND)
  $(AV_H_FILES): $(AV_H_DEPEND)
$(CONFIG_SHELL) policy/mkaccess_vector.sh $(AWK) $(AV_H_DEPEND)

+ifeq ($(CONFIG_XSM_POLICY),y)
+HAS_CHECKPOLICY := $(shell checkpolicy -h 2>&1 | grep -q xen && echo y || echo 
n)
+
+obj-$(HAS_CHECKPOLICY) += policy.o


I would have expect a warning (if not an error) here to tell the user 
that checkpolicy is not available. Otherwise it may take some time to 
the user to understand why the policy is not loaded/present. Because if 
you enable XSM, you don't necessarily check which other options have 
been enabled by default.



+endif


Regards,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v6 5/5] x86/vm_event: Add HVM debug exception vm_events

[Tamas K Lengyel]

On Fri, Jun 24, 2016 at 5:24 AM, Jan Beulich  wrote:
 On 24.06.16 at 13:20,  wrote:
>>>  From: Tamas K Lengyel [mailto:ta...@tklengyel.com]
>>> Sent: Friday, June 24, 2016 1:07 AM
>>>
>>> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
>>> index 03fcba7..4b69ca6 100644
>>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>>> @@ -3373,10 +3373,39 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
>>>  HVMTRACE_1D(TRAP_DEBUG, exit_qualification);
>>>  write_debugreg(6, exit_qualification | DR_STATUS_RESERVED_ONE);
>>>  if ( !v->domain->debugger_attached )
>>> -vmx_propagate_intr(intr_info);
>>> +{
>>> +unsigned long insn_len = 0;
>>> +int rc;
>>> +unsigned long trap_type = MASK_EXTR(intr_info,
>>> +
>>> INTR_INFO_INTR_TYPE_MASK);
>>> +
>>> +if ( trap_type >= X86_EVENTTYPE_SW_INTERRUPT )
>>> +__vmread(VM_EXIT_INSTRUCTION_LEN, &insn_len);
>>> +
>>> +rc = hvm_monitor_debug(regs->eip,
>>> +   HVM_MONITOR_DEBUG_EXCEPTION,
>>> +   trap_type, insn_len);
>>> +
>>> +/*
>>> + * !rc  continue normally
>>> + * rc > paused waiting for response, work here is done
>>
>> Did you mean "rc > 0, vcpu is paused and waiting for response..."? Please
>> make the comment clear to understand.

Yes, sure.

>>
>>> + * rc < error, fall-through to exit_and_crash
>>> + */
>>> +if ( !rc )
>>> +{
>>> +vmx_propagate_intr(intr_info);
>>> +break;
>>> +}
>>> +if ( rc > 0 )
>>> +break;
>>> +}
>>>  else
>>> +{
>>>  domain_pause_for_debugger();
>>> -break;
>>> +break;
>>> +}
>>> +
>>> +goto exit_and_crash;
>>
>> Putting goto as the last line within a 'case' looks a bit strange. What
>> about putting goto directly under a "if ( rc < 0 )" check earlier?
>>
>>   if ( !rc )
>>   ...
>>   if ( rc < 0 )
>>   goto exit_and_crash;
>>   }
>>   else
>>   domain_pause_for_debugger();
>>   break;
>
> Thanks, Kevin - indeed that's exactly what I had asked for already
> on the previous iteration.
>

I'm OK with adding the rc < 0 case, it's just that the fall-through
style is already used for handling the int3 events for example. Should
I fix that too while I'm at it so the code is consistent?

Tamas

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 17/17] xsm: add a default policy to .init.data

[Konrad Rzeszutek Wilk]

On Fri, Jun 24, 2016 at 05:30:32PM +0100, Julien Grall wrote:
> Hello Daniel,
> 
> Please try to CC relevant maintainers on your patch. I would have missed it
> if Andrew did not ping me on IRC.
> 
> On 20/06/16 15:04, Daniel De Graaf wrote:
> >This adds a Kconfig option and support for including the XSM policy from
> >tools/flask/policy in the hypervisor so that the bootloader does not
> >need to provide a policy to get sane behavior from an XSM-enabled
> >hypervisor.  The policy provided by the bootloader, if present, will
> >override the built-in policy.
> >
> >Enabling this option only builds the policy if checkpolicy is available
> >during compilation of the hypervisor; otherwise, it does nothing.  The
> >XSM policy is not moved out of tools because that remains the primary
> >location for installing and configuring the policy.
> >
> >Signed-off-by: Daniel De Graaf 
> >Reviewed-by: Konrad Rzeszutek Wilk 
> 
> For ARM bits:
> 
> Acked-by: Julien Grall 
> 
> Although, I one a question below.
> 
> [...]
> 
> >diff --git a/xen/xsm/flask/Makefile b/xen/xsm/flask/Makefile
> >index 12fc3a9..eefd37c 100644
> >--- a/xen/xsm/flask/Makefile
> >+++ b/xen/xsm/flask/Makefile
> >@@ -27,6 +27,23 @@ $(FLASK_H_FILES): $(FLASK_H_DEPEND)
> >  $(AV_H_FILES): $(AV_H_DEPEND)
> > $(CONFIG_SHELL) policy/mkaccess_vector.sh $(AWK) $(AV_H_DEPEND)
> >
> >+ifeq ($(CONFIG_XSM_POLICY),y)
> >+HAS_CHECKPOLICY := $(shell checkpolicy -h 2>&1 | grep -q xen && echo y || 
> >echo n)
> >+
> >+obj-$(HAS_CHECKPOLICY) += policy.o
> 
> I would have expect a warning (if not an error) here to tell the user that
> checkpolicy is not available. Otherwise it may take some time to the user to
> understand why the policy is not loaded/present. Because if you enable XSM,
> you don't necessarily check which other options have been enabled by
> default.

Good point! And we should probably update the INSTALL document too to mention
that you need checkpoint tool!

> 
> >+endif
> 
> Regards,
> 
> -- 
> Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 08/14] hvmloader: Locate the BIOS blob

[Anthony PERARD]

On Fri, Jun 24, 2016 at 01:33:45AM -0600, Jan Beulich wrote:
> >>> On 22.06.16 at 19:15,  wrote:
> > --- a/tools/firmware/hvmloader/hvmloader.c
> > +++ b/tools/firmware/hvmloader/hvmloader.c
> > @@ -253,10 +253,51 @@ static void acpi_enable_sci(void)
> >  BUG_ON(!(pm1a_cnt_val & ACPI_PM1C_SCI_EN));
> >  }
> >  
> > +const struct hvm_modlist_entry *get_module_entry(
> > +const struct hvm_start_info *info,
> > +const char *name)
> > +{
> > +const struct hvm_modlist_entry *modlist =
> > +(struct hvm_modlist_entry *)(uint32_t)info->modlist_paddr;
> > +unsigned int i;
> > +
> > +if ( !modlist || info->modlist_paddr > UINT_MAX)
> > +return NULL;
> 
> How about info->modlist_paddr + info->nr_modules * sizeof()?
> You check for overflow below, but not here. I think you should
> either consistently rely on there being something right below 4Gb
> which makes this impossible (and then say so in a comment), or
> do full checks everywhere.

I'll do the full checks.

> > +for ( i = 0; i < info->nr_modules; i++ )
> > +{
> > +uint32_t module_name = modlist[i].cmdline_paddr;
> > +
> > +/* Skip if the module or its cmdline is missing. */
> > +if ( !module_name || !modlist[i].paddr )
> > +continue;
> > +
> > +/* Skip if the cmdline can not be read. */
> > +if ( modlist[i].cmdline_paddr > UINT_MAX )
> > +continue;
> 
> Similarly here.

Here, I don't know the size of the cmdline and I don't think calling an
extra strlen() would be usefull. I think that the strcmp() below is going to
be enough for the top bondary check.

Or I could use the size of name.

> > +if ( !strcmp(name, (char*)module_name) )
> 
> Stray cast.

Yes. I'll change the type of module_name and remove the cast here.

> > +{
> > +if ( modlist[i].paddr > UINT_MAX || modlist[i].size > UINT_MAX 
> > ||
> > + (modlist[i].paddr + modlist[i].size) > UINT_MAX )
> 
> I think the last one could be >=.

I think it's valid if addr+size == UINT_MAX. That would means the last
byte of the module would be at 0xFFFE.

-- 
Anthony PERARD

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 09/14] hvmloader: Check modules whereabouts in perform_tests

[Anthony PERARD]

On Fri, Jun 24, 2016 at 01:44:30AM -0600, Jan Beulich wrote:
> >>> On 22.06.16 at 19:15,  wrote:
> > +uint64_t cmdline_paddr = modlist[i].cmdline_paddr;
> > +
> > +if ( check_test_overlap(modlist[i].paddr, modlist[i].size) )
> > +{
> > +printf("Skipping tests due to memory used by module[%d]\n", i);
> > +return;
> > +}
> > +if ( cmdline_paddr && cmdline_paddr < UINT_MAX &&
> > + check_test_overlap(cmdline_paddr,
> > +strlen((char*)(uint32_t)cmdline_paddr)) )
> 
> As said on the previous patch - cmdline_paddr being below 4Gb
> doesn't necessarily mean the string not crossing that boundary.
> Depending on the resolution for that other patch this may need
> adjustment.

I'm not sure how I could handle that, here.

> Also, thinking about it again, the use of UINT_MAX for bounding
> pointers is unfortunate: I think this would better be UINTPTR_MAX.

Well, I do cast every addr to uint32_t, and I had to define UINT_MAX in
the previous patch (hvmloader: Locate the BIOS blob)(I should probably
add a comment about it in the patch description).

I could try to add UINTPTR_MAX instead.

-- 
Anthony PERARD

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] Announcing schedbench, a scheduling micro-benchmark

[George Dunlap]

Hey all,

I think I'm ready to publicly announce a project I've been working on
for a few months called schedbench, a "microbenchmark" for schedulers
(and the Xen scheduler in particular).  You can find the code, along
with a quick README, here:

https://github.com/gwd/schedbench

This is definitely following the "minimal viable product" strategy:
what I have is mostly a framework to build on; but it does actually
function enough that even by itself you should be able to start
exploring things.

There's also a TODO.md if you want any ideas of where to pitch in.

Peace,
 -George

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 17/17] xsm: add a default policy to .init.data

[Daniel De Graaf]

On 06/24/2016 12:50 PM, Konrad Rzeszutek Wilk wrote:

On Fri, Jun 24, 2016 at 05:30:32PM +0100, Julien Grall wrote:

Hello Daniel,

Please try to CC relevant maintainers on your patch. I would have missed it
if Andrew did not ping me on IRC.

On 20/06/16 15:04, Daniel De Graaf wrote:

This adds a Kconfig option and support for including the XSM policy from
tools/flask/policy in the hypervisor so that the bootloader does not
need to provide a policy to get sane behavior from an XSM-enabled
hypervisor.  The policy provided by the bootloader, if present, will
override the built-in policy.

Enabling this option only builds the policy if checkpolicy is available
during compilation of the hypervisor; otherwise, it does nothing.  The
XSM policy is not moved out of tools because that remains the primary
location for installing and configuring the policy.

Signed-off-by: Daniel De Graaf 
Reviewed-by: Konrad Rzeszutek Wilk 


For ARM bits:

Acked-by: Julien Grall 

Although, I one a question below.

[...]


diff --git a/xen/xsm/flask/Makefile b/xen/xsm/flask/Makefile
index 12fc3a9..eefd37c 100644
--- a/xen/xsm/flask/Makefile
+++ b/xen/xsm/flask/Makefile
@@ -27,6 +27,23 @@ $(FLASK_H_FILES): $(FLASK_H_DEPEND)
 $(AV_H_FILES): $(AV_H_DEPEND)
$(CONFIG_SHELL) policy/mkaccess_vector.sh $(AWK) $(AV_H_DEPEND)

+ifeq ($(CONFIG_XSM_POLICY),y)
+HAS_CHECKPOLICY := $(shell checkpolicy -h 2>&1 | grep -q xen && echo y || echo 
n)
+
+obj-$(HAS_CHECKPOLICY) += policy.o


I would have expect a warning (if not an error) here to tell the user that
checkpolicy is not available. Otherwise it may take some time to the user to
understand why the policy is not loaded/present. Because if you enable XSM,
you don't necessarily check which other options have been enabled by
default.


Good point! And we should probably update the INSTALL document too to mention
that you need checkpoint tool!


The dependency on checkpolicy is called out in the Kconfig item that
enables this option.  Are you suggesting I should add a mention below
the instructions on running menuconfig for XSM in INSTALL?

I can remove the HAS_CHECKPOLICY check completely and make the call to
checkpolicy only conditional on the Kconfig option.  I think this is
less complicated than stopping the compile one step above the invocation
of checkpolicy, and probably just as informative (and better, if the
detection heuristic ever breaks).




+endif


Regards,

--
Julien Grall





--
Daniel De Graaf
National Security Agency

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 17/17] xsm: add a default policy to .init.data

[Konrad Rzeszutek Wilk]

On Fri, Jun 24, 2016 at 01:34:29PM -0400, Daniel De Graaf wrote:
> On 06/24/2016 12:50 PM, Konrad Rzeszutek Wilk wrote:
> >On Fri, Jun 24, 2016 at 05:30:32PM +0100, Julien Grall wrote:
> >>Hello Daniel,
> >>
> >>Please try to CC relevant maintainers on your patch. I would have missed it
> >>if Andrew did not ping me on IRC.
> >>
> >>On 20/06/16 15:04, Daniel De Graaf wrote:
> >>>This adds a Kconfig option and support for including the XSM policy from
> >>>tools/flask/policy in the hypervisor so that the bootloader does not
> >>>need to provide a policy to get sane behavior from an XSM-enabled
> >>>hypervisor.  The policy provided by the bootloader, if present, will
> >>>override the built-in policy.
> >>>
> >>>Enabling this option only builds the policy if checkpolicy is available
> >>>during compilation of the hypervisor; otherwise, it does nothing.  The
> >>>XSM policy is not moved out of tools because that remains the primary
> >>>location for installing and configuring the policy.
> >>>
> >>>Signed-off-by: Daniel De Graaf 
> >>>Reviewed-by: Konrad Rzeszutek Wilk 
> >>
> >>For ARM bits:
> >>
> >>Acked-by: Julien Grall 
> >>
> >>Although, I one a question below.
> >>
> >>[...]
> >>
> >>>diff --git a/xen/xsm/flask/Makefile b/xen/xsm/flask/Makefile
> >>>index 12fc3a9..eefd37c 100644
> >>>--- a/xen/xsm/flask/Makefile
> >>>+++ b/xen/xsm/flask/Makefile
> >>>@@ -27,6 +27,23 @@ $(FLASK_H_FILES): $(FLASK_H_DEPEND)
> >>> $(AV_H_FILES): $(AV_H_DEPEND)
> >>>   $(CONFIG_SHELL) policy/mkaccess_vector.sh $(AWK) $(AV_H_DEPEND)
> >>>
> >>>+ifeq ($(CONFIG_XSM_POLICY),y)
> >>>+HAS_CHECKPOLICY := $(shell checkpolicy -h 2>&1 | grep -q xen && echo y || 
> >>>echo n)
> >>>+
> >>>+obj-$(HAS_CHECKPOLICY) += policy.o
> >>
> >>I would have expect a warning (if not an error) here to tell the user that
> >>checkpolicy is not available. Otherwise it may take some time to the user to
> >>understand why the policy is not loaded/present. Because if you enable XSM,
> >>you don't necessarily check which other options have been enabled by
> >>default.
> >
> >Good point! And we should probably update the INSTALL document too to mention
> >that you need checkpoint tool!
> 
> The dependency on checkpolicy is called out in the Kconfig item that
> enables this option.  Are you suggesting I should add a mention below
> the instructions on running menuconfig for XSM in INSTALL?

No, I just thinking that the INSTALL has a list of packages one needs.
And it may be good to have checkpolicy on it.

> 
> I can remove the HAS_CHECKPOLICY check completely and make the call to
> checkpolicy only conditional on the Kconfig option.  I think this is
> less complicated than stopping the compile one step above the invocation
> of checkpolicy, and probably just as informative (and better, if the
> detection heuristic ever breaks).

I actually like the way you have it - with the checkpolicy check determining
whether the Kconfig option for XSM is shown or not.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 17/17] xsm: add a default policy to .init.data

[Konrad Rzeszutek Wilk]

On Fri, Jun 24, 2016 at 01:34:29PM -0400, Daniel De Graaf wrote:
> On 06/24/2016 12:50 PM, Konrad Rzeszutek Wilk wrote:
> >On Fri, Jun 24, 2016 at 05:30:32PM +0100, Julien Grall wrote:
> >>Hello Daniel,
> >>
> >>Please try to CC relevant maintainers on your patch. I would have missed it
> >>if Andrew did not ping me on IRC.
> >>
> >>On 20/06/16 15:04, Daniel De Graaf wrote:
> >>>This adds a Kconfig option and support for including the XSM policy from
> >>>tools/flask/policy in the hypervisor so that the bootloader does not
> >>>need to provide a policy to get sane behavior from an XSM-enabled
> >>>hypervisor.  The policy provided by the bootloader, if present, will
> >>>override the built-in policy.
> >>>
> >>>Enabling this option only builds the policy if checkpolicy is available
> >>>during compilation of the hypervisor; otherwise, it does nothing.  The
> >>>XSM policy is not moved out of tools because that remains the primary
> >>>location for installing and configuring the policy.
> >>>
> >>>Signed-off-by: Daniel De Graaf 
> >>>Reviewed-by: Konrad Rzeszutek Wilk 
> >>
> >>For ARM bits:
> >>
> >>Acked-by: Julien Grall 
> >>
> >>Although, I one a question below.
> >>
> >>[...]
> >>
> >>>diff --git a/xen/xsm/flask/Makefile b/xen/xsm/flask/Makefile
> >>>index 12fc3a9..eefd37c 100644
> >>>--- a/xen/xsm/flask/Makefile
> >>>+++ b/xen/xsm/flask/Makefile
> >>>@@ -27,6 +27,23 @@ $(FLASK_H_FILES): $(FLASK_H_DEPEND)
> >>> $(AV_H_FILES): $(AV_H_DEPEND)
> >>>   $(CONFIG_SHELL) policy/mkaccess_vector.sh $(AWK) $(AV_H_DEPEND)
> >>>
> >>>+ifeq ($(CONFIG_XSM_POLICY),y)
> >>>+HAS_CHECKPOLICY := $(shell checkpolicy -h 2>&1 | grep -q xen && echo y || 
> >>>echo n)
> >>>+
> >>>+obj-$(HAS_CHECKPOLICY) += policy.o
> >>
> >>I would have expect a warning (if not an error) here to tell the user that
> >>checkpolicy is not available. Otherwise it may take some time to the user to
> >>understand why the policy is not loaded/present. Because if you enable XSM,
> >>you don't necessarily check which other options have been enabled by
> >>default.

You won't be able to enable XSM if checkpolicy is not present.
As in, it won't show up as an option in the oldconfig or menuconfig at all.



___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 17/17] xsm: add a default policy to .init.data

[Daniel De Graaf]

On 06/24/2016 01:40 PM, Konrad Rzeszutek Wilk wrote:

On Fri, Jun 24, 2016 at 01:34:29PM -0400, Daniel De Graaf wrote:

On 06/24/2016 12:50 PM, Konrad Rzeszutek Wilk wrote:

On Fri, Jun 24, 2016 at 05:30:32PM +0100, Julien Grall wrote:

Hello Daniel,

Please try to CC relevant maintainers on your patch. I would have missed it
if Andrew did not ping me on IRC.

On 20/06/16 15:04, Daniel De Graaf wrote:

This adds a Kconfig option and support for including the XSM policy from
tools/flask/policy in the hypervisor so that the bootloader does not
need to provide a policy to get sane behavior from an XSM-enabled
hypervisor.  The policy provided by the bootloader, if present, will
override the built-in policy.

Enabling this option only builds the policy if checkpolicy is available
during compilation of the hypervisor; otherwise, it does nothing.  The
XSM policy is not moved out of tools because that remains the primary
location for installing and configuring the policy.

Signed-off-by: Daniel De Graaf 
Reviewed-by: Konrad Rzeszutek Wilk 


For ARM bits:

Acked-by: Julien Grall 

Although, I one a question below.

[...]


diff --git a/xen/xsm/flask/Makefile b/xen/xsm/flask/Makefile
index 12fc3a9..eefd37c 100644
--- a/xen/xsm/flask/Makefile
+++ b/xen/xsm/flask/Makefile
@@ -27,6 +27,23 @@ $(FLASK_H_FILES): $(FLASK_H_DEPEND)
$(AV_H_FILES): $(AV_H_DEPEND)
$(CONFIG_SHELL) policy/mkaccess_vector.sh $(AWK) $(AV_H_DEPEND)

+ifeq ($(CONFIG_XSM_POLICY),y)
+HAS_CHECKPOLICY := $(shell checkpolicy -h 2>&1 | grep -q xen && echo y || echo 
n)
+
+obj-$(HAS_CHECKPOLICY) += policy.o


I would have expect a warning (if not an error) here to tell the user that
checkpolicy is not available. Otherwise it may take some time to the user to
understand why the policy is not loaded/present. Because if you enable XSM,
you don't necessarily check which other options have been enabled by
default.


Good point! And we should probably update the INSTALL document too to mention
that you need checkpoint tool!


The dependency on checkpolicy is called out in the Kconfig item that
enables this option.  Are you suggesting I should add a mention below
the instructions on running menuconfig for XSM in INSTALL?


No, I just thinking that the INSTALL has a list of packages one needs.
And it may be good to have checkpolicy on it.



I can remove the HAS_CHECKPOLICY check completely and make the call to
checkpolicy only conditional on the Kconfig option.  I think this is
less complicated than stopping the compile one step above the invocation
of checkpolicy, and probably just as informative (and better, if the
detection heuristic ever breaks).


I actually like the way you have it - with the checkpolicy check determining
whether the Kconfig option for XSM is shown or not.


Is that possible?  That's not what I have; the check I have only determines
if the Kconfig option does anything or not, it is still visible regardless.

--
Daniel De Graaf
National Security Agency

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel