Re: [Xen-devel] [PATCH for-4.6] xen/arm: vgic-v2: Map the GIC virtual CPU interface with the correct size

[Julien Grall]

Hi Ian,

On 21/09/15 12:54, Ian Campbell wrote:
> On Fri, 2015-09-18 at 09:53 +0100, Ian Campbell wrote:
>> On Thu, 2015-09-17 at 19:00 +0100, Julien Grall wrote:
>>> On GICv2, the GIC virtual CPU interface is at minimum 8KB. Due some to
>>> some necessary quirk for GIC using 64KB stride, we are mapping the
>>> region in 2 time.
>>> The first mapping is 4KB and the second one is 8KB, i.e 12KB in total.
>>> Although the minimum supported size (and widely used) is 8KB. This
>>> means
>>> that we are mapping 4KB more to any guest using GICv2.
>>>
>>> While this looks scary at first glance, the GIC virtual CPU interface
>>> is
>>> most frequently at the end the GIC I/O region. So we will most likely
>>> map an an unused I/O region or a mirrored version of GICV for platform
>>> using 64KB stride.
>>>
>>> Nonetheless, fix the second mapping to only map 4KB.
>>>
>>> Signed-off-by: Julien Grall 
>>
>> Acked-by: Ian Campbell 
>>
>>> ---
>>>
>>> This is a candidate for Xen 4.6 and backport to Xen 4.5.
>>
>> I concur and with Wei not being around I shall plan to apply later today
>> unless there are objections.
> 
> I got distracted. Now applied to staging and staging-4.6 now and noted for
> later application to 4.5

It won't apply cleanly for Xen 4.5, this code was living in the gic
driver. I will send a backport patch for it.

Regards,

-- 
Julien Grall

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

Re: [Xen-devel] AMD Radeon 7970 passthrough on XEN 4.4.3 with an AMD FX-8350/Gigabyte GA-970A-UD3 *HORROR*

[Pasi Kärkkäinen]

Hello,

On Sat, Sep 19, 2015 at 08:21:15PM +0300, NiX wrote:
> After a lot of trial and error I got it working as a secondary
> pass-through. Thanks mainly to bullshit examples around the net. None seem
> to know nothing.
> 
> I though of I am the idiot but I was wrong.
> 

GPU passthru isn't very simple or straight-forward unfortunately..


> Whole system crashes upon shutting down the VM that had the adapter passed
> through. This actually screw up whole pass-through feature. Do that crash
> happen because 7970 does not have device reset feature or whatever it was
> called?
> 

Do you happen to have a serial console available, so you could capture the 
crash/error messages from Xen and/or dom0 Linux kernel? 

SOL (Serial-Over-LAN) works too, if you have Intel AMT, IPMI, or other BMC..


> I got it working only few times and Battlefield 4 started and ran actually
> surprisingly good at 50+ FPS with maxed details at 1600:900 on AMD 7970.
> 
> However the next day immediately after when I attempt to login to VM
> screen goes blank and whole system crashes (power off is required to
> restore). It is also significantly lagged. ie. typing the password has
> around 1 second delay per letter.
> 
> This is unacceptable issue. Anyone else experienced the same horror?
> 
> Thanks anyway for providing XEN but there are a lot to be fixed ...
> 
> I've no issues on that VM when I don't use pass-through expect a
> significantly high CPU usage in HVM mode when I start using the computer
> say IE 11 browser. All cores have a 30-50% CPU usage when I do a small
> tasks such as windows udpate etc.
> 

How do you use the VM? I hope using RDP over the network..


> PS. That VM image is on Samsung 840 PRO SSD and it was loading the game
> really fast when it worked.
> 
> There was no difference to the issue wheter or not CCC was installed.


-- Pasi


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

Re: [Xen-devel] [PATCH v2 1/2] x86/msr: Carry on after a non-"safe" MSR access fails without !panic_on_oops

[Paolo Bonzini]

On 21/09/2015 10:46, Ingo Molnar wrote:
> Or we could extend exception table entry encoding to include a 'warning bit', 
> to 
> not bloat the kernel. If the exception handler code encounters such an 
> exception 
> it would generate a one-time warning for that entry, but otherwise not crash 
> the 
> kernel and continue execution with an all-zeroes result for the MSR read.

The 'warning bit' already exists, it is the opcode that caused the fault. :)

The concern about bloat is a good one.  However, why is it necessary to
keep native_*_msr* inline?  If they are moved out-of-line, using the
exception table becomes the obvious solution and doesn't cause bloat
anymore.

Paolo

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

Re: [Xen-devel] [PATCH v7 15/17] vmx: VT-d posted-interrupt core logic handling

[Wu, Feng]

> -Original Message-
> From: George Dunlap [mailto:george.dun...@citrix.com]
> Sent: Monday, September 21, 2015 5:54 PM
> To: Wu, Feng; George Dunlap; Dario Faggioli
> Cc: Jan Beulich; Tian, Kevin; Keir Fraser; Andrew Cooper;
> xen-devel@lists.xen.org
> Subject: Re: [Xen-devel] [PATCH v7 15/17] vmx: VT-d posted-interrupt core 
> logic
> handling
> 
> On 09/21/2015 06:09 AM, Wu, Feng wrote:
> >
> >
> >> -Original Message-
> >> From: dunl...@gmail.com [mailto:dunl...@gmail.com] On Behalf Of
> George
> >> Dunlap
> >> Sent: Friday, September 18, 2015 10:34 PM
> >> To: Dario Faggioli
> >> Cc: Jan Beulich; George Dunlap; Tian, Kevin; Keir Fraser; Andrew Cooper;
> >> xen-devel@lists.xen.org; Wu, Feng
> >> Subject: Re: [Xen-devel] [PATCH v7 15/17] vmx: VT-d posted-interrupt core
> logic
> >> handling
> >>
> >> On Fri, Sep 18, 2015 at 3:31 PM, George Dunlap
> >>  wrote:
>  As said, me too. Perhaps we can go for option 1, which is simpler,
>  cleaner and more consistent, considering the current status of the
>  code. We can always investigate, in future, whether and how to
>  implement the optimization for all the blockings, if beneficial and fea
>  sible, or have them diverge, if deemed worthwhile.
> >>>
> >>> Sounds like a plan.
> >>
> >> Er, just in case that idiom wasn't clear: Option 1 sounds like a
> >> *good* plan, so unless Feng disagrees, let's go with that. :-)
> >
> > Sorry for the late response, I was on leave last Friday.
> >
> > Thanks for your discussions and suggestions. I have one question about 
> > option
> 1.
> > I find that there are two places where '_VPF_blocked' can get set:
> vcpu_block()
> > and do_poll(). After putting the logic in vcpu_block(), do we need to care
> about
> > do_poll(). I don't know the purpose of do_poll() and the usage case of it.
> > Dario/George, could you please share some knowledge about it? Thanks a lot!
> 
> Yes, you'll need to make the callback everywhere _VPF_blocked is set.
> 
> Normally you'd want to try to refactor both of those to share a commmon
> codepath, but it looks like there are specific reasons why they have to
> be different codepaths; so you'll just have to make the callback in both
> places (after setting VPF_blocked).
> 
> You also need to check that local_events_need_delivery() will return
> "true" if you get an interrupt between that time and entering the
> hypervisor.  Will that happen automatically from
> hvm_local_events_need_delivery() -> hvm_vcpu_has_pending_irq() ->
> vlapic_has_pending_irq()?  Or will you need to add a hook in
> hvm_vcpu_has_pending_irq()?

I think I don't need to add hook in hvm_vcpu_has_pending_irq(), what I need
to do in vcpu_block() and do_poll() is as below:

1. set_bit(_VPF_blocked, &v->pause_flags);

2. ret = v->arch.arch_block(), in this hook, we can re-use the same logic in
vmx_pre_ctx_switch_pi(), and check whether ON bit is set during updating
posted-interrupt descriptor, can return 1 when ON is set, something like the
following:

do {
old.control = new.control = pi_desc->control;

/* Should not block the vCPU if an interrupt was posted for it. */
if ( pi_test_on(&old) )
{
spin_unlock_irqrestore(&v->arch.hvm_vmx.pi_lock, flags);
return 1;
}

/*
 * Change the 'NDST' field to v->arch.hvm_vmx.pi_block_cpu,
 * so when external interrupts from assigned deivces happen,
 * wakeup notifiction event will go to
 * v->arch.hvm_vmx.pi_block_cpu, then in pi_wakeup_interrupt()
 * we can find the vCPU in the right list to wake up.
 */
dest = cpu_physical_id(v->arch.hvm_vmx.pi_block_cpu);

if ( x2apic_enabled )
new.ndst = dest;
else
new.ndst = MASK_INSR(dest, PI_xAPIC_NDST_MASK);

pi_clear_sn(&new);
new.nv = pi_wakeup_vector;
} while ( cmpxchg(&pi_desc->control, old.control, new.control) !=
  old.control );

3. After returning from arch_block() we can simple check:
if ( ret || local_events_need_delivery() )
Don't block the vCPU;

Thanks,
Feng


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

Re: [Xen-devel] Xen Project, QEMU and Linux Coverity defect densities

[Lars Kurth]

As an aside,

xen components in QEMU and Linux Coverity scans seem to be also higher than the 
average for those projects

QEMU: .*/xen.* - density 0.52
LINUX: .*/drivers/xen/.* - density 1.40

Regards
Lars

> On 21 Sep 2015, at 13:10, Lars Kurth  wrote:
> 
> Hi folks,
> 
> as preparation for the xen 4.6 release (and putting together PR), I was 
> looking at Coverity stats. It seems that we made a little progress in terms 
> of defect density, but that we have fallen behind compared to Linux & QEMU.
> 
> Defect Density & Outstanding Defects
> Xen: 0.58 & 678
> Linux: 0.51 & 5248
> QEMU: 0.10 & 109
> 
> I attached the stats of Xen, QEMU and Linux as screenshots.
> 
> I am wondering, whether we should go for a push early in the next release 
> cycle to address these.
> 
> Regards
> Lars
> 
> == QEMU ==
> 
> 
> == Linux ==
> 
> 
> == Xen ==
> 
> 


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

Re: [Xen-devel] [PATCH v6 16/18] vmx: Add some scheduler hooks for VT-d posted interrupts

[Wu, Feng]

> >> Thanks for the comments!
> >>
> >> From my understanding, __sync_local_execstate() can only get called
> >> in the following two cases:
> >> #1) this_cpu(curr_vcpu) == current, in this case, __context_switch() is
> >> not called.
> >> #2) this_cpu(curr_vcpu) != current, and current == idle_vcpu, that means
> >> we just switched from a non-idle vCPU to idle vCPU, so here we need to
> >> call __context_switch() to copy things to the original vcpu struct.
> >>
> >> Please correct me if the above understanding is wrong or incomplete?
> >
> > Hi George / Dario,
> >
> > Could you please confirm the above understanding is correct? (In fact, it is
> > Related to lazy context switch, right?) if so I can continue with the
> > pi_context_switch() way George suggested.
> 
> Yes, that's the general idea.  Normally, you can access the registers of
> a non-running vcpu from the vcpu struct.  But if we've done a lazy
> context switch, that's not true -- so to access those registers properly
> we need to go through and do the full context switch *on that pcpu*.

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 RFC 00/13] VT-d Asynchronous Device-TLB Flush for ATS Device

[Jan Beulich]

>>> On 21.09.15 at 11:46,  wrote:
>>> >>> On 21.09.15 at 16:51, < jbeul...@suse.com > wrote:
>>- Anything else?
> 
> 
> Just test the extreme case. The ATS specification mandates a timeout of 1 
> _minute_ for cache flush, even though it doesn't take so much time for cache 
> flush.
> In my design, if the Device-TLB is not completed, the domain's vCPUs are not 
> allowed entry guest mode (patch #7), otherwise, the logic is not correct.

Hmm, that would be a serious limitation, and I can't immediately
see a reason: Taking this with virtualization removed, I don't
think such an invalidation would stall a physical CPU for a whole
minute. Sadly I also can't immediately think of a solution, but I
guess first of all I'll have to take a look at the series (which
unfortunately may take a few days to get to).

Jan


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

Re: [Xen-devel] [PATCH v7 15/17] vmx: VT-d posted-interrupt core logic handling

[Wu, Feng]

> -Original Message-
> From: George Dunlap [mailto:george.dun...@citrix.com]
> Sent: Monday, September 21, 2015 5:19 PM
> To: Wu, Feng; Dario Faggioli
> Cc: xen-devel@lists.xen.org; Tian, Kevin; Keir Fraser; George Dunlap; Andrew
> Cooper; Jan Beulich
> Subject: Re: [Xen-devel] [PATCH v7 15/17] vmx: VT-d posted-interrupt core 
> logic
> handling
> 
> On 09/21/2015 06:08 AM, Wu, Feng wrote:
> >
> >
> >> -Original Message-
> >> From: George Dunlap [mailto:george.dun...@citrix.com]
> >> Sent: Thursday, September 17, 2015 5:38 PM
> >> To: Dario Faggioli; Wu, Feng
> >> Cc: xen-devel@lists.xen.org; Tian, Kevin; Keir Fraser; George Dunlap;
> Andrew
> >> Cooper; Jan Beulich
> >> Subject: Re: [Xen-devel] [PATCH v7 15/17] vmx: VT-d posted-interrupt core
> logic
> >> handling
> >>
> >> On 09/17/2015 09:48 AM, Dario Faggioli wrote:
> >>> On Thu, 2015-09-17 at 08:00 +, Wu, Feng wrote:
> >>>
> > -Original Message-
> > From: Dario Faggioli [mailto:dario.faggi...@citrix.com]
> >>>
> > So, I guess, first of all, can you confirm whether or not it's exploding
> > in debug builds?
> 
>  Does the following information in Config.mk mean it is a debug build?
> 
>  # A debug build of Xen and tools?
>  debug ?= y
>  debug_symbols ?= $(debug)
> 
> >>> I think so. But as I said in my other email, I was wrong, and this is
> >>> probably not an issue.
> >>>
> > And in either case (just tossing out ideas) would it be
> > possible to deal with the "interrupt already raised when blocking" case:
> 
>  Thanks for the suggestions below!
> 
> >>> :-)
> >>>
> >  - later in the context switching function ?
>  In this case, we might need to set a flag in vmx_pre_ctx_switch_pi()
> instead
>  of calling vcpu_unblock() directly, then when it returns to
> context_switch(),
>  we can check the flag and don't really block the vCPU.
> 
> >>> Yeah, and that would still be rather hard to understand and maintain,
> >>> IMO.
> >>>
>  But I don't have a clear
>  picture about how to archive this, here are some questions from me:
>  - When we are in context_switch(), we have done the following changes to
>  vcpu's state:
>   * sd->curr is set to next
>   * vCPU's running state (both prev and next ) is changed by
> vcpu_runstate_change()
>   * next->is_running is set to 1
>   * periodic timer for prev is stopped
>   * periodic timer for next is setup
>   ..
> 
>  So what point should we perform the action to _unblock_ the vCPU? We
>  Need to roll back the formal changes to the vCPU's state, right?
> 
> >>> Mmm... not really. Not blocking prev does not mean that prev would be
> >>> kept running on the pCPU, and that's true for your current solution as
> >>> well! As you say yourself, you're already in the process of switching
> >>> between prev and next, at a point where it's already a thing that next
> >>> will be the vCPU that will run. Not blocking means that prev is
> >>> reinserted to the runqueue, and a new invocation to the scheduler is
> >>> (potentially) queued as well (via raising SCHEDULE_SOFTIRQ, in
> >>> __runq_tickle()), but it's only when such new scheduling happens that
> >>> prev will (potentially) be selected to run again.
> >>>
> >>> So, no, unless I'm fully missing your point, there wouldn't be no
> >>> rollback required. However, I still would like the other solution (doing
> >>> stuff in vcpu_block()) better (see below).
> >>>
> >  - with another hook, perhaps in vcpu_block() ?
> 
>  We could check this in vcpu_block(), however, the logic here is that 
>  before
>  vCPU is blocked, we need to change the posted-interrupt descriptor,
>  and during changing it, if 'ON' bit is set, which means VT-d hardware
>  issues a notification event because interrupts from the assigned devices
>  is coming, we don't need to block the vCPU and hence no need to update
>  the PI descriptor in this case.
> 
> >>> Yep, I saw that. But could it be possible to do *everything* related to
> >>> blocking, including the update of the descriptor, in vcpu_block(), if no
> >>> interrupt have been raised yet at that time? I mean, would you, if
> >>> updating the descriptor in there, still get the event that allows you to
> >>> call vcpu_wake(), and hence vmx_vcpu_wake_prepare(), which would undo
> >>> the blocking, no matter whether that resulted in an actual context
> >>> switch already or not?
> >>>
> >>> I appreciate that this narrows the window for such an event to happen by
> >>> quite a bit, making the logic itself a little less useful (it makes
> >>> things more similar to "regular" blocking vs. event delivery, though,
> >>> AFAICT), but if it's correct, ad if it allows us to save the ugly
> >>> invocation of vcpu_unblock from context switch context, I'd give it a
> >>> try.
> >>>
> >>> After all, this PI thing requires actions to be taken whe

Re: [Xen-devel] DRAFT XSA 142 - libxl fails to honour readonly flag on disks with qemu-xen

[Ian Campbell]

On Tue, 2015-09-15 at 16:22 +, Xen.org security team wrote:

> VULNERABLE SYSTEMS
> ==
> 
[...]
> Only systems using libxl or libxl-based toolstacks are vulnerable.
> (This includes libvirt with the libxl driver.)

^xl and ... ?

> 
> All versions of libxl which support qemu-xen are vulnerable.  Support
> for qemu-xen was introduced in Xen 4.1.

http://wiki.xenproject.org/wiki/Xen_Project_Release_Features says 4.2 as
preview and 4.3 properly. Which is wrong?

Apart from those minor nits this looks good to me, although I confess I've
not fully been following the discussion.

Ian.

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

Re: [Xen-devel] [PATCH v6 16/18] vmx: Add some scheduler hooks for VT-d posted interrupts

[Jan Beulich]

>>> On 21.09.15 at 11:28,  wrote:
> On 09/21/2015 09:23 AM, Jan Beulich wrote:
> On 16.09.15 at 18:56,  wrote:
>>> On Mon, Sep 7, 2015 at 1:54 PM, Jan Beulich  wrote:
>>> On 25.08.15 at 03:57,  wrote:
> @@ -1605,9 +1621,12 @@ void context_switch(struct vcpu *prev, struct vcpu 
> *next)
>
>  set_current(next);
>
> +pi_ctxt_switch_from(prev);
> +
>  if ( (per_cpu(curr_vcpu, cpu) == next) ||
>   (is_idle_domain(nextd) && cpu_online(cpu)) )
>  {
> +pi_ctxt_switch_to(next);
>  local_irq_enable();

 This placement, if really intended that way, needs explanation (in a
 comment) and perhaps even renaming of the involved symbols, as
 looking at it from a general perspective it seems wrong (with
 pi_ctxt_switch_to() excluding idle vCPU-s it effectively means you
 want this only when switching back to what got switched out lazily
 before, i.e. this would be not something to take place on an arbitrary
 context switch). As to possible alternative names - maybe make the
 hooks ctxt_switch_prepare() and ctxt_switch_cancel()?
>>>
>>> Why on earth is this more clear than what he had before?
>>>
>>> In the first call, he's not "preparing" anything -- he's actually
>>> switching the PI context out for prev.  And in the second call, he's
>>> not "cancelling" anything -- he's actually switching the PI context in
>>> for next.  The names you suggest are actively confusing, not helpful.
>> 
>> While I think later discussion on this thread moved in a good direction,
>> I still think I should reply here (even if late): To me, the use of
>> pi_ctxt_switch_to() in the patch fragment still seen above is very
>> much the cancellation of the immediately preceding pi_ctxt_switch_from(),
>> as it's the "we don't want to do anything else" path that it gets put
>> into.
> 
> Either we have different understandings about what the code does, or I
> don't understand what you're saying here.
> 
> The codepath in question will only be called if we're switching *into*
> or *out of* the "lazy context swtich" -- i.e., switching from a vcpu to
> the idle vcpu, but not saving or restoring state.

Oh, I'm sorry - you're right.

Jan


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

Re: [Xen-devel] [PATCH for-4.6] tools/libxc: arm: Check the index before accessing the bank

[Ian Campbell]

On Fri, 2015-09-18 at 11:30 +0100, Wei Liu wrote:

> > > H I forgot my Signed-off-by :(.
> > > 
> > > Signed-off-by: Julien Grall 
> > 
> > Acked-by: Ian Campbell 
> > 
> > This is a no brainer for 4.6 (and further) IMHO and with Wei not being
> > around I shall plan to apply later today unless there are objections.
> > 
> 
> No objection from me of course.

Applied to staging and staging-4.6.

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

Re: [Xen-devel] [PATCH for-4.6] xen/arm: vgic-v2: Map the GIC virtual CPU interface with the correct size

[Ian Campbell]

On Fri, 2015-09-18 at 09:53 +0100, Ian Campbell wrote:
> On Thu, 2015-09-17 at 19:00 +0100, Julien Grall wrote:
> > On GICv2, the GIC virtual CPU interface is at minimum 8KB. Due some to
> > some necessary quirk for GIC using 64KB stride, we are mapping the
> > region in 2 time.
> > The first mapping is 4KB and the second one is 8KB, i.e 12KB in total.
> > Although the minimum supported size (and widely used) is 8KB. This
> > means
> > that we are mapping 4KB more to any guest using GICv2.
> > 
> > While this looks scary at first glance, the GIC virtual CPU interface
> > is
> > most frequently at the end the GIC I/O region. So we will most likely
> > map an an unused I/O region or a mirrored version of GICV for platform
> > using 64KB stride.
> > 
> > Nonetheless, fix the second mapping to only map 4KB.
> > 
> > Signed-off-by: Julien Grall 
> 
> Acked-by: Ian Campbell 
> 
> > ---
> > 
> > This is a candidate for Xen 4.6 and backport to Xen 4.5.
> 
> I concur and with Wei not being around I shall plan to apply later today
> unless there are objections.

I got distracted. Now applied to staging and staging-4.6 now and noted for
later application to 4.5

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

Re: [Xen-devel] [PATCH v2 for-4.6] libxl: handle read-only drives with qemu-xen

[Ian Campbell]

On Tue, 2015-09-15 at 17:22 +0100, Ian Jackson wrote:
> Stefano Stabellini writes ("[PATCH v2 for-4.6] libxl: handle read-only
> drives with qemu-xen"):
> > The current libxl code doesn't deal with read-only drives at all.
> > 
> > Upstream QEMU and qemu-xen only support read-only cdrom drives: make
> > sure to specify "readonly=on" for cdrom drives and return error in case
> > the user requested a non-cdrom read-only drive.
> 
> Acked-by: Ian Jackson 

I applied to staging and staging-4.6 with this Ack and Wei's say so
elsewhere.


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

Re: [Xen-devel] [PATCH for-4.6] INSTALL: Mention MINIOS_UPSTREAM_URL

[Ian Campbell]

On Thu, 2015-09-17 at 17:32 +0100, Wei Liu wrote:
> On Thu, Sep 17, 2015 at 05:30:50PM +0100, Ian Campbell wrote:
> > All the other ones seem to be there.
> > 
> > Signed-off-by: Ian Campbell 
> > ---
> > For 4.6: trivially
> 
> Documentation update is safe to go in.
> 
> Release-acked-by: Wei Liu 

Since this is such a trivial patch I assumed this included an effective
Acked-by as well and applied to both branches. I hope that was OK.


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

Re: [Xen-devel] [PATCH 1/2] docs: Template for feature documents

[Ian Campbell]

On Tue, 2015-09-15 at 14:54 +0100, Andrew Cooper wrote:
> +# Basics
> +
> +A table with an overview of the support status and applicability.
> +
> + 
> + Status: e.g. **Supported**/**Tech Preview**/**Experimental**

My main concern is still the lack of a precise meaning for these terms.
However this is not a problem with this series per-se and we can always
update the template once we know what they mean.

So I've acked and applied both patches to staging and staging-4.6.


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

Re: [Xen-devel] [V5 4/4] libxc: expose xsaves/xgetbv1/xsavec to hvm guest

[Jan Beulich]

>>> On 21.09.15 at 13:34,  wrote:
> @@ -255,8 +260,9 @@ static void xc_cpuid_config_xsave(
>  regs[0] = regs[1] = regs[2] = regs[3] = 0;
>  break;
>  }
> -/* Don't touch EAX, EBX. Also cleanup ECX and EDX */
> -regs[2] = regs[3] = 0;
> +/* Don't touch EAX, EBX. Also cleanup EDX. Cleanup bits 01-32 of 
> ECX*/

This comment is off by one, and it will become stale the moment
XSS_SUPPORT gets added to. Better to write this in a more
generic way.

Jan

> +regs[2] &= XSS_SUPPORT;
> +regs[3] = 0;
>  break;
>  }
>  }



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

[Xen-devel] [V5 1/4] x86/xsaves: add basic definitions/helpers to support xsaves

[Shuai Ruan]

This patch add basic definitions/helpers which will be used in
later patches.

Signed-off-by: Shuai Ruan 
---
 xen/arch/x86/xstate.c| 168 +++
 xen/include/asm-x86/cpufeature.h |   6 +-
 xen/include/asm-x86/hvm/vcpu.h   |   1 +
 xen/include/asm-x86/msr-index.h  |   2 +
 xen/include/asm-x86/xstate.h |  14 +++-
 5 files changed, 189 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
index d5f5e3b..ff03b31 100644
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -29,12 +29,21 @@ static u32 __read_mostly xsave_cntxt_size;
 /* A 64-bit bitmask of the XSAVE/XRSTOR features supported by processor. */
 u64 __read_mostly xfeature_mask;
 
+unsigned int * __read_mostly xstate_offsets;
+unsigned int * __read_mostly xstate_sizes;
+static unsigned int __read_mostly xstate_features;
+static unsigned int __read_mostly xstate_comp_offsets[sizeof(xfeature_mask)*8];
+
+/* Cached msr_xss for fast read */
+static DEFINE_PER_CPU(uint64_t, msr_xss);
+
 /* Cached xcr0 for fast read */
 static DEFINE_PER_CPU(uint64_t, xcr0);
 
 /* Because XCR0 is cached for each CPU, xsetbv() is not exposed. Users should 
  * use set_xcr0() instead.
  */
+
 static inline bool_t xsetbv(u32 index, u64 xfeatures)
 {
 u32 hi = xfeatures >> 32;
@@ -65,6 +74,165 @@ uint64_t get_xcr0(void)
 return this_cpu(xcr0);
 }
 
+void set_msr_xss(u64 xss)
+{
+wrmsrl(MSR_IA32_XSS, xss);
+this_cpu(msr_xss) = xss;
+}
+
+uint64_t get_msr_xss(void)
+{
+return this_cpu(msr_xss);
+}
+
+bool_t xsave_area_compressed(const struct xsave_struct *xsave_area)
+{
+if ( xsave_area && (xsave_area->xsave_hdr.xcomp_bv
+  & XSTATE_COMPACTION_ENABLED))
+   return 1;
+return 0;
+}
+
+static int setup_xstate_features(bool_t bsp)
+{
+unsigned int leaf, tmp, eax, ebx;
+
+if ( bsp )
+xstate_features = fls(xfeature_mask);
+
+if ( bsp )
+{
+xstate_offsets = xzalloc_array(unsigned int, xstate_features);
+if( !xstate_offsets )
+return -ENOMEM;
+
+xstate_sizes = xzalloc_array(unsigned int, xstate_features);
+if( !xstate_sizes )
+return -ENOMEM;
+}
+
+for (leaf = 2; leaf < xstate_features; leaf++)
+{
+if( bsp )
+cpuid_count(XSTATE_CPUID, leaf, &xstate_sizes[leaf],
+&xstate_offsets[leaf], &tmp, &tmp);
+else
+{
+ cpuid_count(XSTATE_CPUID, leaf, &eax,
+&ebx, &tmp, &tmp);
+ BUG_ON(eax != xstate_sizes[leaf]);
+ BUG_ON(ebx != xstate_offsets[leaf]);
+}
+ }
+ return 0;
+}
+
+static void setup_xstate_comp(void)
+{
+unsigned int i;
+
+/*
+ * The FP xstates and SSE xstates are legacy states. They are always
+ * in the fixed offsets in the xsave area in either compacted form
+ * or standard form.
+ */
+xstate_comp_offsets[0] = 0;
+xstate_comp_offsets[1] = XSAVE_SSE_OFFSET;
+
+xstate_comp_offsets[2] = FXSAVE_SIZE + XSAVE_HDR_SIZE;
+
+for (i = 3; i < xstate_features; i++)
+{
+xstate_comp_offsets[i] = xstate_comp_offsets[i-1] + (((1ul << i)
+ & xfeature_mask) ? xstate_sizes[i-1] : 0);
+ASSERT(xstate_comp_offsets[i] <= xsave_cntxt_size);
+}
+}
+
+static void *get_xsave_addr(struct xsave_struct *xsave, unsigned int 
xfeature_idx)
+{
+if ( !((1ul << xfeature_idx) & xfeature_mask) )
+return NULL;
+
+return (void *)xsave + xstate_comp_offsets[xfeature_idx];
+}
+
+void save_xsave_states(struct vcpu *v, void *dest, unsigned int size)
+{
+struct xsave_struct *xsave = v->arch.xsave_area;
+u64 xstate_bv = xsave->xsave_hdr.xstate_bv;
+u64 valid;
+
+ASSERT(xsave_area_compressed(xsave));
+/*
+ * Copy legacy XSAVE area, to avoid complications with CPUID
+ * leaves 0 and 1 in the loop below.
+ */
+memcpy(dest, xsave, FXSAVE_SIZE);
+
+/* Set XSTATE_BV */
+*(u64 *)(dest + XSAVE_HDR_OFFSET) = xstate_bv;
+
+/*
+ * Copy each region from the possibly compacted offset to the
+ * non-compacted offset.
+ */
+valid = xstate_bv & ~XSTATE_FP_SSE;
+while ( valid )
+{
+u64 feature = valid & -valid;
+int index = fls(feature) - 1;
+void *src = get_xsave_addr(xsave, index);
+
+if ( src )
+{
+ASSERT((xstate_offsets[index] + xstate_sizes[index]) <= size);
+memcpy(dest + xstate_offsets[index], src, xstate_sizes[index]);
+}
+
+valid -= feature;
+}
+
+}
+
+void load_xsave_states(struct vcpu *v, const void *src, unsigned int size)
+{
+struct xsave_struct *xsave = v->arch.xsave_area;
+u64 xstate_bv = *(u64 *)(src + XSAVE_HDR_OFFSET);
+u64 valid;
+
+ASSERT(!xsave_area_compressed((struct xsave_struct *)src));
+/*
+ * Copy legacy XSAVE area, to avoid complications with CPUID
+ * lea

[Xen-devel] [V5 2/4] x86/xsaves: enable xsaves/xrstors/xsavec in xen

[Shuai Ruan]

This patch uses xsaves/xrstors instead of xsaveopt/xrstor
to perform the xsave_area switching so that xen itself
can benefit from them when available.

For xsaves/xrstors only use compact format. Add format conversion
support when perform guest os migration.

Signed-off-by: Shuai Ruan 
---
 xen/arch/x86/domain.c|   3 +
 xen/arch/x86/domctl.c|  38 +++--
 xen/arch/x86/hvm/hvm.c   |  21 +--
 xen/arch/x86/i387.c  |   4 ++
 xen/arch/x86/traps.c |   7 +--
 xen/arch/x86/xstate.c| 132 ++-
 xen/include/asm-x86/xstate.h |   4 --
 7 files changed, 151 insertions(+), 58 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 045f6ff..b25094b 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1529,6 +1529,9 @@ static void __context_switch(void)
 if ( xcr0 != get_xcr0() && !set_xcr0(xcr0) )
 BUG();
 }
+if ( cpu_has_xsaves )
+if ( is_hvm_vcpu(n) )
+set_msr_xss(n->arch.hvm_vcpu.msr_xss);
 vcpu_restore_fpu_eager(n);
 n->arch.ctxt_switch_to(n);
 }
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index bf62a88..e2cd0d4 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -867,7 +867,7 @@ long arch_do_domctl(
 if ( domctl->cmd == XEN_DOMCTL_getvcpuextstate )
 {
 unsigned int size;
-
+void * xsave_area;
 ret = 0;
 vcpu_pause(v);
 
@@ -896,9 +896,30 @@ long arch_do_domctl(
 ret = -EFAULT;
 
 offset += sizeof(v->arch.xcr0_accum);
-if ( !ret && copy_to_guest_offset(evc->buffer, offset,
-  (void *)v->arch.xsave_area,
-  size - 2 * sizeof(uint64_t)) )
+
+if ( !ret && (cpu_has_xsaves || cpu_has_xsavec) &&
+ xsave_area_compressed(v->arch.xsave_area) )
+{
+xsave_area = xmalloc_bytes(size);
+if ( !xsave_area )
+{
+ret = -ENOMEM;
+vcpu_unpause(v);
+goto vcpuextstate_out;
+}
+
+save_xsave_states(v, xsave_area,
+  evc->size - 2*sizeof(uint64_t));
+
+if ( !ret && copy_to_guest_offset(evc->buffer, offset,
+ xsave_area, size -
+ 2 * sizeof(uint64_t)) )
+ ret = -EFAULT;
+ xfree(xsave_area);
+   }
+   else if ( !ret && copy_to_guest_offset(evc->buffer, offset,
+ (void *)v->arch.xsave_area,
+ size - 2 * sizeof(uint64_t)) )
 ret = -EFAULT;
 
 vcpu_unpause(v);
@@ -954,8 +975,13 @@ long arch_do_domctl(
 v->arch.xcr0_accum = _xcr0_accum;
 if ( _xcr0_accum & XSTATE_NONLAZY )
 v->arch.nonlazy_xstate_used = 1;
-memcpy(v->arch.xsave_area, _xsave_area,
-   evc->size - 2 * sizeof(uint64_t));
+if ( (cpu_has_xsaves || cpu_has_xsavec) &&
+!xsave_area_compressed(_xsave_area) )
+load_xsave_states(v, _xsave_area,
+  evc->size - 2*sizeof(uint64_t));
+else
+memcpy(v->arch.xsave_area, (void *)_xsave_area,
+   evc->size - 2 * sizeof(uint64_t));
 vcpu_unpause(v);
 }
 else
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 615fa89..ad0a53b 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -2148,8 +2148,13 @@ static int hvm_save_cpu_xsave_states(struct domain *d, 
hvm_domain_context_t *h)
 ctxt->xfeature_mask = xfeature_mask;
 ctxt->xcr0 = v->arch.xcr0;
 ctxt->xcr0_accum = v->arch.xcr0_accum;
-memcpy(&ctxt->save_area, v->arch.xsave_area,
-   size - offsetof(struct hvm_hw_cpu_xsave, save_area));
+   if ( (cpu_has_xsaves || cpu_has_xsavec) &&
+ (xsave_area_compressed(v->arch.xsave_area)) )
+save_xsave_states(v, &ctxt->save_area,
+  size - offsetof(typeof(*ctxt), save_area));
+else
+memcpy(&ctxt->save_area, v->arch.xsave_area,
+   size - offsetof(struct hvm_hw_cpu_xsave, save_area));
 }
 
 return 0;
@@ -2248,9 +2253,15 @@ static int hvm_load_cpu_xsave_states(struct domain *d, 
hvm_domain_context_t *h)
 v->arch.xcr0_accum = ctxt->xcr0_accum;
 if ( ctxt->xcr0_accum & XSTATE_NONLAZY )
 v->arch.nonlazy_xstate_used = 1;
-memcpy(v->arch.xsave_area, &ctxt->save_area,
-   min(d

[Xen-devel] [V5 4/4] libxc: expose xsaves/xgetbv1/xsavec to hvm guest

[Shuai Ruan]

This patch exposes xsaves/xgetbv1/xsavec to hvm guest.
The reserved bits of eax/ebx/ecx/edx must be cleaned up
when call cpuid(0dh) with leaf 1 or 2..63.

According to the spec the following bits must be reserved:
For leaf 1, bits 03-04/08-31 of ecx is reserved. Edx is reserved.
For leaf 2...63, bits 01-31 of ecx is reserved. Edx is reserved.

Acked-by: Ian Campbell 
Signed-off-by: Shuai Ruan 
---
 tools/libxc/xc_cpuid_x86.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
index e146a3e..538d356 100644
--- a/tools/libxc/xc_cpuid_x86.c
+++ b/tools/libxc/xc_cpuid_x86.c
@@ -210,6 +210,10 @@ static void intel_xc_cpuid_policy(
 }
 
 #define XSAVEOPT(1 << 0)
+#define XSAVEC  (1 << 1)
+#define XGETBV1 (1 << 2)
+#define XSAVES  (1 << 3)
+#define XSS_SUPPORT (1 << 0)
 /* Configure extended state enumeration leaves (0x000D for xsave) */
 static void xc_cpuid_config_xsave(
 xc_interface *xch, domid_t domid, uint64_t xfeature_mask,
@@ -246,8 +250,9 @@ static void xc_cpuid_config_xsave(
 regs[1] = 512 + 64; /* FP/SSE + XSAVE.HEADER */
 break;
 case 1: /* leaf 1 */
-regs[0] &= XSAVEOPT;
-regs[1] = regs[2] = regs[3] = 0;
+regs[0] &= (XSAVEOPT | XSAVEC | XGETBV1 | XSAVES);
+regs[2] &= xfeature_mask;
+regs[3] = 0;
 break;
 case 2 ... 63: /* sub-leaves */
 if ( !(xfeature_mask & (1ULL << input[1])) )
@@ -255,8 +260,9 @@ static void xc_cpuid_config_xsave(
 regs[0] = regs[1] = regs[2] = regs[3] = 0;
 break;
 }
-/* Don't touch EAX, EBX. Also cleanup ECX and EDX */
-regs[2] = regs[3] = 0;
+/* Don't touch EAX, EBX. Also cleanup EDX. Cleanup bits 01-32 of ECX*/
+regs[2] &= XSS_SUPPORT;
+regs[3] = 0;
 break;
 }
 }
-- 
1.9.1


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

[Xen-devel] [V5 0/4] add xsaves/xrstors support

[Shuai Ruan]

Changes in v5:
*Address comments from Andrew/Jan,mainly:
*Add lazy writes of the xss_msr.
*Add xsave_area check when save/restore guest.
*Add xsavec support.
*Use word 2 in xen/include/asm-x86/cpufeature.h.
*Fix some code errors.

Changes in v4:
* Address comments from Andrew, mainly:
* No xsaves suporting for pv guest.
* Using xstate_sizes instead of domain_cpuid in hvm_cupid in patch 3.
* Add support for format translation when perform pv guest migration in patch 
2. 
* Fix some code errors.

Changes in v3:
* Address comments from Jan/Konrad
* Change the bits of eax/ebx/ecx/edx passed to guest in patch 4.
* Add code to verify whether host supports xsaves or not in patch 1.

Changes in v2:
* Address comments from Andrew/chao/Jan, mainly:
* Add details information of xsaves/xrstors feature.
* Test migration between xsaves-support machine and none-xsaves-support machine 
* Remove XGETBV1/XSAVEC/XSAVEOPT out of 'else' in patch 3.
* Change macro name XGETBV to XGETBV1 in patch 4.

This patchset enable xsaves/xrstors feature which will be available on 
Intel Skylake and later platform. Like xsave/xrstor, xsaves/xrstors 
feature will save and load processor state from a region of memory called 
XSAVE area. While unlike xsave/xrstor, xsaves/xrstors:

a) use the compacted format only for the extended region 
   of the XSAVE area which saves memory for you;
b) can operate on supervisor state components so the feature
   is prerequisite to support new supervisor state components;
c) execute only if CPL=0. 

Detail hardware spec can be found in chapter 13 (section 13.11 13.12) of the 
Intel SDM [1].

patch1: add basic definition/function to support xsaves
patch2: add xsaves/xrstors support for xen
patch3-4: add xsaves/xrstors support for hvm guest


[1] Intel SDM 
(http://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-32-architectures-software-developer-manual-325462.pdf)


Shuai Ruan (4):
  x86/xsaves: add basic definitions/helpers to support xsaves
  x86/xsaves: enable xsaves/xrstors/xsavec in xen
  x86/xsaves: enable xsaves/xrstors for hvm guest
  libxc: expose xsaves/xgetbv1/xsavec to hvm guest

 tools/libxc/xc_cpuid_x86.c |  14 +-
 xen/arch/x86/domain.c  |   3 +
 xen/arch/x86/domctl.c  |  38 -
 xen/arch/x86/hvm/hvm.c |  51 ++-
 xen/arch/x86/hvm/vmx/vmcs.c|   6 +-
 xen/arch/x86/hvm/vmx/vmx.c |  20 +++
 xen/arch/x86/i387.c|   4 +
 xen/arch/x86/traps.c   |   7 +-
 xen/arch/x86/xstate.c  | 304 -
 xen/include/asm-x86/cpufeature.h   |   6 +-
 xen/include/asm-x86/hvm/vcpu.h |   1 +
 xen/include/asm-x86/hvm/vmx/vmcs.h |   6 +
 xen/include/asm-x86/hvm/vmx/vmx.h  |   2 +
 xen/include/asm-x86/msr-index.h|   2 +
 xen/include/asm-x86/xstate.h   |  19 ++-
 15 files changed, 415 insertions(+), 68 deletions(-)

-- 
1.9.1


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

[Xen-devel] [V5 3/4] x86/xsaves: enable xsaves/xrstors for hvm guest

[Shuai Ruan]

This patch enables xsaves for hvm guest, includes:
1.handle xsaves vmcs init and vmexit.
2.add logic to write/read the XSS msr.

Reviewed-by: Andrew Cooper 
Signed-off-by: Shuai Ruan 
---
 xen/arch/x86/hvm/hvm.c | 30 ++
 xen/arch/x86/hvm/vmx/vmcs.c|  6 --
 xen/arch/x86/hvm/vmx/vmx.c | 20 
 xen/arch/x86/xstate.c  |  4 ++--
 xen/include/asm-x86/hvm/vmx/vmcs.h |  6 ++
 xen/include/asm-x86/hvm/vmx/vmx.h  |  2 ++
 xen/include/asm-x86/xstate.h   |  1 +
 7 files changed, 65 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index ad0a53b..ad2d572 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4550,6 +4550,23 @@ void hvm_cpuid(unsigned int input, unsigned int *eax, 
unsigned int *ebx,
 *ebx = _eax + _ebx;
 }
 }
+if ( count == 1 )
+{
+if ( cpu_has_xsaves )
+{
+*ebx = XSTATE_AREA_MIN_SIZE;
+if ( v->arch.xcr0 | v->arch.hvm_vcpu.msr_xss )
+for ( sub_leaf = 2; sub_leaf < 63; sub_leaf++ )
+{
+if ( !((v->arch.xcr0 | v->arch.hvm_vcpu.msr_xss)
+   & (1ULL << sub_leaf)) )
+continue;
+*ebx += xstate_sizes[sub_leaf];
+}
+}
+else
+*ebx = *ecx = *edx = 0;
+}
 break;
 
 case 0x8001:
@@ -4649,6 +4666,12 @@ int hvm_msr_read_intercept(unsigned int msr, uint64_t 
*msr_content)
 *msr_content = v->arch.hvm_vcpu.guest_efer;
 break;
 
+case MSR_IA32_XSS:
+if ( !cpu_has_xsaves )
+goto gp_fault;
+*msr_content = v->arch.hvm_vcpu.msr_xss;
+break;
+
 case MSR_IA32_TSC:
 *msr_content = _hvm_rdtsc_intercept();
 break;
@@ -4781,6 +4804,13 @@ int hvm_msr_write_intercept(unsigned int msr, uint64_t 
msr_content,
return X86EMUL_EXCEPTION;
 break;
 
+case MSR_IA32_XSS:
+/* No XSS features currently supported for guests. */
+if ( !cpu_has_xsaves || msr_content != 0 )
+goto gp_fault;
+v->arch.hvm_vcpu.msr_xss = msr_content;
+break;
+
 case MSR_IA32_TSC:
 hvm_set_guest_tsc(v, msr_content);
 break;
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index a0a97e7..258cf17 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -236,7 +236,8 @@ static int vmx_init_vmcs_config(void)
SECONDARY_EXEC_PAUSE_LOOP_EXITING |
SECONDARY_EXEC_ENABLE_INVPCID |
SECONDARY_EXEC_ENABLE_VM_FUNCTIONS |
-   SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS);
+   SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS |
+   SECONDARY_EXEC_XSAVES);
 rdmsrl(MSR_IA32_VMX_MISC, _vmx_misc_cap);
 if ( _vmx_misc_cap & VMX_MISC_VMWRITE_ALL )
 opt |= SECONDARY_EXEC_ENABLE_VMCS_SHADOWING;
@@ -1238,7 +1239,8 @@ static int construct_vmcs(struct vcpu *v)
 __vmwrite(HOST_PAT, host_pat);
 __vmwrite(GUEST_PAT, guest_pat);
 }
-
+if ( cpu_has_vmx_xsaves )
+__vmwrite(XSS_EXIT_BITMAP, 0);
 vmx_vmcs_exit(v);
 
 /* PVH: paging mode is updated by arch_set_info_guest(). */
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 2582cdd..b07e1d2 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2833,6 +2833,18 @@ static void vmx_idtv_reinject(unsigned long idtv_info)
 }
 }
 
+static void vmx_handle_xsaves(void)
+{
+gdprintk(XENLOG_ERR, "xsaves should not cause vmexit\n");
+domain_crash(current->domain);
+}
+
+static void vmx_handle_xrstors(void)
+{
+gdprintk(XENLOG_ERR, "xrstors should not cause vmexit\n");
+domain_crash(current->domain);
+}
+
 static int vmx_handle_apic_write(void)
 {
 unsigned long exit_qualification;
@@ -3404,6 +3416,14 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
 vmx_vcpu_flush_pml_buffer(v);
 break;
 
+case EXIT_REASON_XSAVES:
+vmx_handle_xsaves();
+break;
+
+case EXIT_REASON_XRSTORS:
+vmx_handle_xrstors();
+break;
+
 case EXIT_REASON_ACCESS_GDTR_OR_IDTR:
 case EXIT_REASON_ACCESS_LDTR_OR_TR:
 case EXIT_REASON_VMX_PREEMPTION_TIMER_EXPIRED:
diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
index ae59a60..5940acd 100644
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -14,8 +14,8 @@
 #include 
 #include 
 
-static bool_t __read_mostly cpu_has_xsaveopt;
-static bool_t __read_mostly cpu_has_xsavec;
+bool_t __read_mostly cpu_has_xsaveopt;
+bool_t __read_mostly cpu_has_xsavec;
 bool_t __read_mostly cpu_has_xgetbv1;
 bool_t __read_mostly cpu_has_xsaves;
 
diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h 
b/xen/

Re: [Xen-devel] [PATCH v7 06/28] xen/arm: ITS: Add helper functions to manage its_devices

[Julien Grall]

Hi Vijay,

On 18/09/15 14:08, vijay.kil...@gmail.com wrote:
> +static void its_remove_device(struct its_device *dev)
> +{
> +if ( dev )
> +rb_erase(&dev->node, &rb_its_dev);

Either the caller or this function has to take the lock which protect
the RB-tree.

If it's the caller, please add an ASSERT to check the lock is taken. If
not, you know what to do ;).

Regards,

> +}
> +

Regards,

-- 
Julien Grall

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

Re: [Xen-devel] [PATCH v7 05/28] xen/arm: ITS: Port ITS driver to Xen

[Julien Grall]

Hi Vijay,

The only things I haven't check on this patch was the ITS command structure.

On 18/09/15 14:08, vijay.kil...@gmail.com wrote:
> +/* ITS command structure */
> +typedef union {

Can you please sort this union by command name in alphabetical order.
It's way easier to find a command in the list.

> +u64 bits[4];
> +struct __packed {
> +uint8_t cmd;

NIT: Please stay consistent with usage of the type. You are using u8
everywhere within this union except here.

> +uint8_t pad[7];

Why a padding of only 56 bits? Shouldn't it be 248 bits (i.e 31 * 8
bits) to fit all the command?

> +} hdr;
> +struct __packed {
> +u8 cmd;
> +u8 res1[3];
> +u32 devid;
> +u64 size:5;
> +u64 res2:59;
> +/* XXX: Though itt is 40 bit. Keep it 48 to avoid shift */
> +u64 res3:8;

It's very confusing for the reviewer to see a mix of usage (u8 res1[n],
u64 res3:8) within the same structure.

The later (i.e u64 field:n) is the best to use because we can match
quickly the size with the spec.

> +u64 itt:40;
> +u64 res4:15;
> +u64 valid:1;
> +u64 res5;
> +} mapd;

[..]

> +struct __packed {
> +u8 cmd;
> +u8 res1[3];
> +u32 devid;
> +u32 event;
> +u32 res2;
> +u64 res3;
> +u64 res4;
> +} int_cmd;

Maybe you want to add the suffix _cmd to everyone to avoid having only
one because of C spec issue.

> +struct __packed {
> +u8 cmd;
> +u8 res1[3];
> +u32 devid;
> +u32 event;
> +u32 res2;
> +u64 res3;
> +u64 res4;
> +} clear;
> +struct __packed {
> +u8 cmd;
> +u8 res1[7];
> +u64 res2;
> +u16 res3;
> +u32 ta;

It would have been better to have a full name or a description rather
than only a 2 letter field "ta". I won't ask for a documentation of this
field right now, but it would be a nice follow-up of this series.

> +u16 res4;
> +u64 res5;
> +} sync;
> +} its_cmd_block;


Regards,

-- 
Julien Grall

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

Re: [Xen-devel] [PATCH OSSTEST] Correct html syntax error color=="#xxxxxx" => color="#xxxxxx"

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST] Correct html syntax error 
color=="#xx" => color="#xx""):
> Strangely the effect of this (with iceweasel) was that everything was
> cyan (#00fff0?) instead of the intended black or white.

This will make some of the output black when previously it was cyan,
of course.  I don't particularly mind.

Acked-by: Ian Jackson 

Ian.

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

Re: [Xen-devel] [OSSTEST PATCH 15/26] cr*: Support xen-unstable-smoke

[Ian Jackson]

Ian Campbell writes ("Re: [OSSTEST PATCH 15/26] cr*: Support 
xen-unstable-smoke"):
> On Mon, 2015-09-21 at 11:21 +0100, Ian Jackson wrote:
> > if [ "x$branch" != "xxen-unstable" ]; then
> > export REVISION_LINUX_OLD=disable
> > fi
> 
> That's the one which didn't need adjusting I think.

Yes.

> > I'm not sure what the qemu revisions thing is you're thinking of but I
> > think all the places where the source code matches `xen-unstable[^-]',
> > the actual effect is to treat `xen-unstable-smoke' the same way as
> > `xen-unstable'.
> 
> "qemu revisions thing" is:
> 
> @@ -340,7 +340,7 @@ case "$NEW_REVISION/$OLD_REVISION" in
>  "$treeurl#$OLD_REVISION-$NEW_REVISION" \
>  
>  case "$realtree" in
> -xen-4*|xen-unstable)
> +xen-4*|xen-unstable*)
>  oldqemu=`./ap-qemu-revision $realtree $OLD_REVISION`
>  newqemu=`./ap-qemu-revision $realtree $NEW_REVISION`
>  if [ "$oldqemu" ] && [ "$newqemu" ]; then

Ah, OK.  Yes.  We agree.

Thanks,
Ian.

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

Re: [Xen-devel] [qemu-mainline bisection] complete test-armhf-armhf-xl-vhd

[Ian Jackson]

Ian Campbell writes ("Re: [qemu-mainline bisection] complete 
test-armhf-armhf-xl-vhd"):
> This looks to be the issue which Anthony investigated last week and for
> which the responsible patch has been reverted.
> 
> I think this bisection just hadn't caught up with the fact that the issue
> is fixed yet.

That is plausible.  The bisector will keep going until a full flight
has picked up and tested the new input version.

(I haven't checked the timeline for this.)

Ian.

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

[Xen-devel] [qemu-upstream-unstable baseline-only test] 37993: regressions - trouble: broken/fail/pass

[Platform Team regression test user]

This run is configured for baseline tests only.

flight 37993 qemu-upstream-unstable real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/37993/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-qemuu-rhel6hvm-amd  3 host-install(3)   broken REGR. vs. 37965
 test-amd64-amd64-xl-qemuu-win7-amd64  9 windows-install   fail REGR. vs. 37965
 test-amd64-amd64-xl-qemuu-winxpsp3  9 windows-install fail REGR. vs. 37965

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl-vhd   9 debian-di-installfail   never pass
 test-armhf-armhf-xl-qcow2 9 debian-di-installfail   never pass
 test-armhf-armhf-libvirt-qcow2  9 debian-di-installfail never pass
 test-armhf-armhf-libvirt-raw  9 debian-di-installfail   never pass
 test-armhf-armhf-xl-raw   9 debian-di-installfail   never pass
 test-armhf-armhf-libvirt-vhd  9 debian-di-installfail   never pass
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-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-midway   13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-midway   12 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-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-rtds 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   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-xsm 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  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-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-libvirt-pair 21 guest-migrate/src_host/dst_host fail never 
pass
 test-amd64-i386-libvirt-raw  11 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-pair 21 guest-migrate/src_host/dst_host fail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qcow2 11 migrate-support-checkfail  never pass
 test-amd64-amd64-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-amd64-amd64-libvirt-raw 11 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-vhd  11 migrate-support-checkfail   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-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail never pass

version targeted for testing:
 qemuu8ad9e71fc937439730fa68e82d6da11a50eb5c04
baseline version:
 qemuub05befcbea71a979509ce04f02929969a790c923

Last test of basis37965  2015-09-18 06:50:04 Z3 days
Testing same since37993  2015-09-20 01:57:57 Z1 days1 attempts


People who touched revisions under test:
  P J P 
  Stefan Hajnoczi 
  Stefano Stabellini 

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 

Re: [Xen-devel] [PATCH v7 04/28] xen/arm: Rename NR_IRQs and vgic_num_irqs helper function

[Julien Grall]

Hi Vijay,

On 18/09/15 14:08, vijay.kil...@gmail.com wrote:
> From: Vijaya Kumar K 
> 
> NR_IRQS represents the number of lines (i.e SGIs, PPIs and SPIs).
> With the introduction of LPIs, NR_IRQs is renamed to NR_ITLINES.
> Similarly vgic_num_irqs() is renamed as vgic_num_irq_lines().
> 
> Signed-off-by: Vijaya Kumar K 
Reviewed-by: Julien Grall 

Regards,

-- 
Julien Grall

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

Re: [Xen-devel] [OSSTEST PATCH 21/26] ts-debian-hvm-install: Do not create EFI partition if EFI not in use

[Ian Jackson]

Ian Campbell writes ("Re: [OSSTEST PATCH 21/26] ts-debian-hvm-install: Do not 
create EFI partition if EFI not in use"):
> On Fri, 2015-09-18 at 18:50 +0100, Ian Jackson wrote:
> > If we are booting our install ISO using a non-EFI executable, don't
> > try to provide an EFI for the installed system either.
> > 
> > Signed-off-by: Ian Jackson 
> 
> FWIW the default recipes used by d-i use "$iflabel{ gpt }" to achieve the
> same thing. Actually I'm a bit surprised that this isn't the affect of
> "method { efi }" too.
...  
> > -$preseed_file .= (< > +$preseed_file .= (

[Xen-devel] [PATCH OSSTEST] Correct html syntax error color=="#xxxxxx" => color="#xxxxxx"

[Ian Campbell]

Strangely the effect of this (with iceweasel) was that everything was
cyan (#00fff0?) instead of the intended black or white.

Signed-off-by: Ian Campbell 
---
 ms-planner | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ms-planner b/ms-planner
index 1f996a6..cab32c9 100755
--- a/ms-planner
+++ b/ms-planner
@@ -727,7 +727,7 @@ sub cmd_show_html () {
 my $fgcolour=
 $avail ? "#00" : "#ff";
printf "%s %s",
+" bgcolor=\"%s\">%s %s",
$content->{Rowspan}, $bgcolour, $fgcolour, $show;
$content->{Printed}= 1;
}
-- 
2.5.1


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

Re: [Xen-devel] [OSSTEST PATCH 22/26] ts-debian-hvm-install: Use /dev/sda for i386, not /dev/xvda

[Ian Jackson]

Ian Campbell writes ("Re: [OSSTEST PATCH 22/26] ts-debian-hvm-install: Use 
/dev/sda for i386, not /dev/xvda"):
> Because of this the i386 install ends up using the emulated sda device
> instead of switching to the pv devices as amd64 does.

Indeed.

> I think it's probably worth explaining some of this in the commit log
> rather than "Empirically".

Fair enough.

> IIRC someone was working on enabling PVHVM even for non-PAE i386 kernels,
> not sure what the status of that is, but clearly it isn't going to change
> things for Wheezy.
> 
> If you wanted instead to enable PV support for these installs then the
> multi-arch (i386+amd64) isoimages contain the Xen enabled kernels for i386
> (as /install.386/xen/*, n.b. /install.amd/xen/* are suitable symlinks, so
> you can aoid special cases.

I think testing both the emulated and PVHVM devices, in a single test
job, is quite nice, actually.  And it doesn't seem too slow.  So I
will keep it this way.

> Anyway, that's clearly an "if you can be bothered thing" so with an
> expanded commit message:
> Acked-by: Ian Campbell 

Thanks.

Ian.

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

Re: [Xen-devel] [OSSTEST PATCH 21/26] ts-debian-hvm-install: Do not create EFI partition if EFI not in use

[Ian Jackson]

Ian Campbell writes ("Re: [OSSTEST PATCH 21/26] ts-debian-hvm-install: Do not 
create EFI partition if EFI not in use"):
> On Fri, 2015-09-18 at 18:50 +0100, Ian Jackson wrote:
> > If we are booting our install ISO using a non-EFI executable, don't
> > try to provide an EFI for the installed system either.
...
> FWIW the default recipes used by d-i use "$iflabel{ gpt }" to achieve the
> same thing. Actually I'm a bit surprised that this isn't the affect of
> "method { efi }" too.

I'm happy to do this some other way but the way I did it avoided me
having to look up how d-i partman-auto works...

> Anyway, this approach is good enough for our purposes so:
> 
> Acked-by: Ian Campbell 

Thanks,
Ian.

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

Re: [Xen-devel] [OSSTEST PATCH 19/26] ts-debian-hvm-install: Cope with images containing only isolinux

[Ian Jackson]

Ian Campbell writes ("Re: [OSSTEST PATCH 19/26] ts-debian-hvm-install: Cope 
with images containing only isolinux"):
> On Fri, 2015-09-18 at 18:50 +0100, Ian Jackson wrote:
> > debian-7.2.0-i386-CD-1.iso contains no grub, only isolinux.
...
> I'm happy to determine experimentally (i.e. by pushing to pretest) if there
> is any meaning to the order of these (the wiki has them the new way around,
> so I presume not).

My tests (which I left running) have determined that this patch was
wrong in the amd64 case: it would work, but by falling back to
isolinux.

This is because $newiso is not populated at the time $bootfile is
determined.  I think this is fairly easily fixed by reordering things
and I will send an update when I have got something that actually
works in both cases.

(This affects the EFI partition creation too, but shouldn't involve
any change to that patch except maybe to context.)

> > +my $bootfile = 'boot/grub/efi.img';
> > +if (!target_file_exists($ho, "$newiso/$bootfile")) {
> > +   $bootfile = "isolinux/isolinux.bin";
> > +   push @isogen_extra, qw(-c isolinux/boot.cat);
> > +}
> 
> My preference would have been to produce an iso which was bootable either
> via EFI (grub) or legacy (isolinux), but that would require more complex
> command lines and I'm sure neither of us wants to figure out what those
> are. So:

Yes.

> Acked-by: Ian Campbell 

Thanks, but given the circmstances I am not going to apply this ack to
whatever revised version I produce.

Ian.

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

Re: [Xen-devel] [PATCH 2/2] xen: arm: traps: correct cond

[Ian Campbell]

On Mon, 2015-09-21 at 17:08 +0800, Peng Fan wrote:
> On Mon, Sep 21, 2015 at 11:10:11AM +0100, Julien Grall wrote:
> > Hi Peng,
> > 
> > On 21/09/15 08:07, Peng Fan wrote:
> > > From "G6.2.29 CPSR, Current Program Status Register" of Aarch64 ARM
> > > and "B1.3.3 Program Status Registers (PSRs)" of ARMv7-A ARM:
> > > "
> > 
> > The section number may change between the different version of the
> > spec.
> > Can you also precise the spec version?
> > 
> > For instance on my ARM64 spec (ARM DDI 0497A.d) the section G6.2.29
> > points to "CSSELR" and not "CPSR".
> > 
> > > IT[7:5] holds the base condition for the IT block. The base condition
> > > is
> > > the top 3 bits of the condition code specified by the first
> > >   condition field of the IT instruction.
> > > IT[4:0] encodes the size of the IT block, which is the number of
> > > instructions that are to be conditionally executed, by the
> > >   position of the least significant 1 in this field. It also
> > >   encodes the value of the least significant bit of the condition
> > >   code for each instruction in the block.
> > > "
> > > So should be "cond = ( it >> 5 );" but not "cond = ( it >> 4 );"
> > 
> > IT[7:5] encodes the top 3 bits of the condition code and one bit of
> > IT[4:0] will contain the least significant bit of the condition code.
> > 
> > In order to get the full condition code you have to use IT[7:4] which
> > the current code does (see A2.5.2 ARM DDI 0406C.b).
> > 
> > So the current code looks valid to me. Did I miss something?
> 
> No, you are correct.

Were these two patches motivated by an actual issue you were seeing? Or
just from code inspection?


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

Re: [Xen-devel] [OSSTEST PATCH 15/26] cr*: Support xen-unstable-smoke

[Ian Campbell]

On Mon, 2015-09-21 at 11:21 +0100, Ian Jackson wrote:
> Ian Campbell writes ("Re: [OSSTEST PATCH 15/26] cr*: Support xen-unstable
> -smoke"):
> > On Fri, 2015-09-18 at 18:50 +0100, Ian Jackson wrote:
> > > Add this branch to select_xenbranch.  This works like xen-unstable in
> > > most respects.
> > > 
> > > There are only two places in osstest where xenbranch `xen-unstable'
> > > is
> > > treated specially and only one of them needs adjusting to match
> > > xen-unstable-smoke too.
> > 
> > The included one is the one which adds the qemu revisions?
> 
> No, this in cr-daily-branch:
> 
> if [ "x$branch" != "xxen-unstable" ]; then
> export REVISION_LINUX_OLD=disable
> fi

That's the one which didn't need adjusting I think.

> > And the other one is, I think, the thing which sets
> >  REVISION_LINUX_OLD=disable which we want (or maybe don't care about)
> > for
> > the smoked tests. So good.
> 
> Indeed.
> 
> I'm not sure what the qemu revisions thing is you're thinking of but I
> think all the places where the source code matches `xen-unstable[^-]',
> the actual effect is to treat `xen-unstable-smoke' the same way as
> `xen-unstable'.

"qemu revisions thing" is:

@@ -340,7 +340,7 @@ case "$NEW_REVISION/$OLD_REVISION" in
 "$treeurl#$OLD_REVISION-$NEW_REVISION" \
 
 case "$realtree" in
-xen-4*|xen-unstable)
+xen-4*|xen-unstable*)
 oldqemu=`./ap-qemu-revision $realtree $OLD_REVISION`
 newqemu=`./ap-qemu-revision $realtree $NEW_REVISION`
 if [ "$oldqemu" ] && [ "$newqemu" ]; then

> 
> > Acked-by: Ian Campbell 
> 
> Thanks,
> Ian.

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

Re: [Xen-devel] [PATCH 2/2] xen: arm: traps: correct cond

[Peng Fan]

On Mon, Sep 21, 2015 at 11:10:11AM +0100, Julien Grall wrote:
>Hi Peng,
>
>On 21/09/15 08:07, Peng Fan wrote:
>> From "G6.2.29 CPSR, Current Program Status Register" of Aarch64 ARM
>> and "B1.3.3 Program Status Registers (PSRs)" of ARMv7-A ARM:
>> "
>
>The section number may change between the different version of the spec.
>Can you also precise the spec version?
>
>For instance on my ARM64 spec (ARM DDI 0497A.d) the section G6.2.29
>points to "CSSELR" and not "CPSR".
>
>> IT[7:5] holds the base condition for the IT block. The base condition is
>> the top 3 bits of the condition code specified by the first
>>  condition field of the IT instruction.
>> IT[4:0] encodes the size of the IT block, which is the number of
>> instructions that are to be conditionally executed, by the
>>  position of the least significant 1 in this field. It also
>>  encodes the value of the least significant bit of the condition
>>  code for each instruction in the block.
>> "
>> So should be "cond = ( it >> 5 );" but not "cond = ( it >> 4 );"
>
>IT[7:5] encodes the top 3 bits of the condition code and one bit of
>IT[4:0] will contain the least significant bit of the condition code.
>
>In order to get the full condition code you have to use IT[7:4] which
>the current code does (see A2.5.2 ARM DDI 0406C.b).
>
>So the current code looks valid to me. Did I miss something?

No, you are correct.

Thanks,
Peng.

>
>Regards,
>
>> Signed-off-by: Peng Fan 
>> Cc: Ian Campbell 
>> Cc: Stefano Stabellini 
>> Cc: Julien Grall 
>> ---
>>  xen/arch/arm/traps.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
>> index 2e2b1f2..b2879b7 100644
>> --- a/xen/arch/arm/traps.c
>> +++ b/xen/arch/arm/traps.c
>> @@ -1561,8 +1561,8 @@ static int check_conditional_instr(struct 
>> cpu_user_regs *regs,
>>  if ( it == 0 )
>>  return 1;
>>  
>> -/* The cond for this instruction works out as the top 4 bits. */
>> -cond = ( it >> 4 );
>> +/* The cond for this instruction works out as the top 3 bits. */
>> +cond = ( it >> 5 );
>>  }
>>  
>>  cpsr_cond = cpsr >> 28;
>> 
>
>-- 
>Julien Grall

-- 

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

Re: [Xen-devel] [OSSTEST PATCH 15/26] cr*: Support xen-unstable-smoke

[Ian Jackson]

Ian Campbell writes ("Re: [OSSTEST PATCH 15/26] cr*: Support 
xen-unstable-smoke"):
> On Fri, 2015-09-18 at 18:50 +0100, Ian Jackson wrote:
> > Add this branch to select_xenbranch.  This works like xen-unstable in
> > most respects.
> > 
> > There are only two places in osstest where xenbranch `xen-unstable' is
> > treated specially and only one of them needs adjusting to match
> > xen-unstable-smoke too.
> 
> The included one is the one which adds the qemu revisions?

No, this in cr-daily-branch:

if [ "x$branch" != "xxen-unstable" ]; then
export REVISION_LINUX_OLD=disable
fi

> And the other one is, I think, the thing which sets
>  REVISION_LINUX_OLD=disable which we want (or maybe don't care about) for
> the smoked tests. So good.

Indeed.

I'm not sure what the qemu revisions thing is you're thinking of but I
think all the places where the source code matches `xen-unstable[^-]',
the actual effect is to treat `xen-unstable-smoke' the same way as
`xen-unstable'.

> Acked-by: Ian Campbell 

Thanks,
Ian.

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

Re: [Xen-devel] PCI Pass-through in Xen ARM: Draft 4

[Julien Grall]

Hi David,

On 19/09/15 22:51, Daney, David wrote:
> I apologize for the top post, but for the record;
> 
> The Thunder Linux implementation will be obtaining the PCI requester id from 
> the OF device tree, or ACPI IORT table.  It will *not* be derived from any 
> hardware address.

I'm aware about the ACPI IORT table but haven't see anything around a
device tree bindings for the requester ID.

Do you have a link/draft explaining this binding?

Regards,

-- 
Julien Grall

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

Re: [Xen-devel] [PATCH 2/2] xen: arm: traps: correct cond

[Julien Grall]

Hi Peng,

On 21/09/15 08:07, Peng Fan wrote:
> From "G6.2.29 CPSR, Current Program Status Register" of Aarch64 ARM
> and "B1.3.3 Program Status Registers (PSRs)" of ARMv7-A ARM:
> "

The section number may change between the different version of the spec.
Can you also precise the spec version?

For instance on my ARM64 spec (ARM DDI 0497A.d) the section G6.2.29
points to "CSSELR" and not "CPSR".

> IT[7:5] holds the base condition for the IT block. The base condition is
> the top 3 bits of the condition code specified by the first
>   condition field of the IT instruction.
> IT[4:0] encodes the size of the IT block, which is the number of
> instructions that are to be conditionally executed, by the
>   position of the least significant 1 in this field. It also
>   encodes the value of the least significant bit of the condition
>   code for each instruction in the block.
> "
> So should be "cond = ( it >> 5 );" but not "cond = ( it >> 4 );"

IT[7:5] encodes the top 3 bits of the condition code and one bit of
IT[4:0] will contain the least significant bit of the condition code.

In order to get the full condition code you have to use IT[7:4] which
the current code does (see A2.5.2 ARM DDI 0406C.b).

So the current code looks valid to me. Did I miss something?

Regards,

> Signed-off-by: Peng Fan 
> Cc: Ian Campbell 
> Cc: Stefano Stabellini 
> Cc: Julien Grall 
> ---
>  xen/arch/arm/traps.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 2e2b1f2..b2879b7 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -1561,8 +1561,8 @@ static int check_conditional_instr(struct cpu_user_regs 
> *regs,
>  if ( it == 0 )
>  return 1;
>  
> -/* The cond for this instruction works out as the top 4 bits. */
> -cond = ( it >> 4 );
> +/* The cond for this instruction works out as the top 3 bits. */
> +cond = ( it >> 5 );
>  }
>  
>  cpsr_cond = cpsr >> 28;
> 

-- 
Julien Grall

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

[Xen-devel] [PATCH v12 0/1] toolstack-assisted approach to PVHVM guest kexec

[Vitaly Kuznetsov]

This is the only not merged patch from the original 'toolstack-assisted
approach to PVHVM guest kexec' series.

Changes since v11:
- Minor description change in xl.cfg.pod.5 [Wei Liu]
- Goto-style error handling in do_domain_soft_reset() [Wei Liu]

Original description of the series:

The list of currently known issues blocking kexec (and why the series is
required) is:
- Bound event channels.
- Registered vcpu_info.
- PIRQ/emuirq mappings.
- shared_info frame after XENMAPSPACE_shared_info operation.
- Active grant mappings.

Previously there were several attempts to solve these issues in different ways:
- Individual 'shutdown' hypercalls (e.g. VCPUOP_reset_vcpu_info) (we agreed
  that one 'reset everything' hypercall is better).
- Try building new domain reassigning old domain's memory (memory reassignment
  turned out being too cumbersome).
- Toolstack-less 'reset everything' (turned out being impossible because there
  is not enough knowledge in the hypervisor, e.g. interdomain channels are
  being set up by the toolstack).

This series is a mix of the previously sent 'toolstack-based' and
'reset everything' series. Here are some key points:
- No new domain is created.
- Domain is asking for soft reset with SCHEDOP_shutdown with
  SHUTDOWN_soft_reset shutdown reason.
- XEN_DOMCTL_soft_reset is being called by the toolstack.
- Device model is being restarted.

With regards to active grants. In this series we restart domain's device model,
remove it from xenstore and introduce it back. The only 'misbehaving' backend
keeping active mapping I currently observe is xenconsoled: currently it has no
interface to disconnect from a domain (it just periodically scans all domains
to determine if any of them are dead). This is not an issue for us because:
- This matches standard domain startup as this grant mapping is being set up by
  the toolstack.
- Guest domain is aware of this special page.
grant_table_warn_active_grants() is required to find possible misbehaving
backends in future.

v11 of the 'toolstack-assisted approach to pvhvm guest kexec' is available here:
http://lists.xenproject.org/archives/html/xen-devel/2015-09/msg00547.html

Vitaly Kuznetsov (1):
  (lib)xl: soft reset support

 docs/man/xl.cfg.pod.5|   8 +-
 tools/libxl/libxl.c  |  22 -
 tools/libxl/libxl.h  |  15 
 tools/libxl/libxl_create.c   | 196 ++-
 tools/libxl/libxl_internal.h |   4 +
 tools/libxl/libxl_types.idl  |   2 +
 tools/libxl/xl.h |   1 +
 tools/libxl/xl_cmdimpl.c |  25 +-
 8 files changed, 246 insertions(+), 27 deletions(-)

-- 
2.4.3


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

[Xen-devel] [PATCH v12 1/1] (lib)xl: soft reset support

[Vitaly Kuznetsov]

Use existing create/restore path to perform 'soft reset' for HVM
domains. Tear everything down, e.g. destroy domain's device model,
remove the domain from xenstore, save toolstack record and start
over.

Signed-off-by: Vitaly Kuznetsov 
---
Changes since v11:
- Minor description change in xl.cfg.pod.5 [Wei Liu]
- Goto-style error handling in do_domain_soft_reset() [Wei Liu]
---
 docs/man/xl.cfg.pod.5|   8 +-
 tools/libxl/libxl.c  |  22 -
 tools/libxl/libxl.h  |  15 
 tools/libxl/libxl_create.c   | 196 ++-
 tools/libxl/libxl_internal.h |   4 +
 tools/libxl/libxl_types.idl  |   2 +
 tools/libxl/xl.h |   1 +
 tools/libxl/xl_cmdimpl.c |  25 +-
 8 files changed, 246 insertions(+), 27 deletions(-)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index db4a163..3496bdf 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -351,6 +351,12 @@ destroy the domain.
 write a "coredump" of the domain to F and then
 restart the domain.
 
+=item B
+
+Reset all Xen specific interfaces for the Xen-aware HVM domain allowing
+it to reestablish these interfaces and continue executing the domain. PV
+and non-Xen-aware HVM guests are not supported.
+
 =back
 
 The default for C is C.
@@ -372,7 +378,7 @@ Action to take if the domain crashes.  Default is 
C.
 =item B
 
 Action to take if the domain performs 'soft reset' (e.g. does kexec).
-Default is C.
+Default is C.
 
 =back
 
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 4d27891..cb451ff 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1500,6 +1500,7 @@ void libxl__domain_destroy(libxl__egc *egc, 
libxl__domain_destroy_state *dds)
 dds->stubdom.ao = ao;
 dds->stubdom.domid = stubdomid;
 dds->stubdom.callback = stubdom_destroy_callback;
+dds->stubdom.soft_reset = false;
 libxl__destroy_domid(egc, &dds->stubdom);
 } else {
 dds->stubdom_finished = 1;
@@ -1508,6 +1509,7 @@ void libxl__domain_destroy(libxl__egc *egc, 
libxl__domain_destroy_state *dds)
 dds->domain.ao = ao;
 dds->domain.domid = dds->domid;
 dds->domain.callback = domain_destroy_callback;
+dds->domain.soft_reset = dds->soft_reset;
 libxl__destroy_domid(egc, &dds->domain);
 }
 
@@ -1688,10 +1690,14 @@ static void devices_destroy_cb(libxl__egc *egc,
 
 /* Clean up qemu-save and qemu-resume files. They are
  * intermediate files created by libxc. Unfortunately they
- * don't fit in existing userdata scheme very well.
+ * don't fit in existing userdata scheme very well. In soft reset
+ * case we need to keep the file.
  */
-rc = libxl__remove_file(gc, libxl__device_model_savefile(gc, domid));
-if (rc < 0) goto out;
+if (!dis->soft_reset) {
+rc = libxl__remove_file(gc,
+libxl__device_model_savefile(gc, domid));
+if (rc < 0) goto out;
+}
 rc = libxl__remove_file(gc,
  GCSPRINTF(LIBXL_DEVICE_MODEL_RESTORE_FILE".%u", domid));
 if (rc < 0) goto out;
@@ -1702,7 +1708,15 @@ static void devices_destroy_cb(libxl__egc *egc,
 ctx->xch = xc_interface_open(ctx->lg,0,0);
 if (!ctx->xch) goto badchild;
 
-rc = xc_domain_destroy(ctx->xch, domid);
+if (!dis->soft_reset) {
+rc = xc_domain_destroy(ctx->xch, domid);
+} else {
+rc = xc_domain_pause(ctx->xch, domid);
+if (rc < 0) goto badchild;
+rc = xc_domain_soft_reset(ctx->xch, domid);
+if (rc < 0) goto badchild;
+rc = xc_domain_unpause(ctx->xch, domid);
+}
 if (rc < 0) goto badchild;
 _exit(0);
 
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 5f9047c..4ee9fbf 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -205,6 +205,13 @@
 #define LIBXL_HAVE_BUILDINFO_ARM_GIC_VERSION 1
 
 /*
+ * LIBXL_HAVE_SOFT_RESET indicates that libxl supports performing
+ * 'soft reset' for domains and there is 'soft_reset' shutdown reason
+ * in enum libxl_shutdown_reason.
+ */
+#define LIBXL_HAVE_SOFT_RESET 1
+
+/*
  * libxl ABI compatibility
  *
  * The only guarantee which libxl makes regarding ABI compatibility
@@ -1132,6 +1139,14 @@ int static inline libxl_domain_create_restore_0x040200(
 
 #endif
 
+int libxl_domain_soft_reset(libxl_ctx *ctx,
+libxl_domain_config *d_config,
+uint32_t domid,
+const libxl_asyncop_how *ao_how,
+const libxl_asyncprogress_how
+*aop_console_how)
+LIBXL_EXTERNAL_CALLERS_ONLY;
+
   /* A progress report will be made via ao_console_how, of type
* domain_create_console_available, when the domain's primary
* console is available and can be connected to.
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index f5771da..b

Re: [Xen-devel] [PATCH v7 15/17] vmx: VT-d posted-interrupt core logic handling

[George Dunlap]

On 09/21/2015 06:09 AM, Wu, Feng wrote:
> 
> 
>> -Original Message-
>> From: dunl...@gmail.com [mailto:dunl...@gmail.com] On Behalf Of George
>> Dunlap
>> Sent: Friday, September 18, 2015 10:34 PM
>> To: Dario Faggioli
>> Cc: Jan Beulich; George Dunlap; Tian, Kevin; Keir Fraser; Andrew Cooper;
>> xen-devel@lists.xen.org; Wu, Feng
>> Subject: Re: [Xen-devel] [PATCH v7 15/17] vmx: VT-d posted-interrupt core 
>> logic
>> handling
>>
>> On Fri, Sep 18, 2015 at 3:31 PM, George Dunlap
>>  wrote:
 As said, me too. Perhaps we can go for option 1, which is simpler,
 cleaner and more consistent, considering the current status of the
 code. We can always investigate, in future, whether and how to
 implement the optimization for all the blockings, if beneficial and fea
 sible, or have them diverge, if deemed worthwhile.
>>>
>>> Sounds like a plan.
>>
>> Er, just in case that idiom wasn't clear: Option 1 sounds like a
>> *good* plan, so unless Feng disagrees, let's go with that. :-)
> 
> Sorry for the late response, I was on leave last Friday.
> 
> Thanks for your discussions and suggestions. I have one question about option 
> 1.
> I find that there are two places where '_VPF_blocked' can get set: 
> vcpu_block()
> and do_poll(). After putting the logic in vcpu_block(), do we need to care 
> about
> do_poll(). I don't know the purpose of do_poll() and the usage case of it.
> Dario/George, could you please share some knowledge about it? Thanks a lot!

Yes, you'll need to make the callback everywhere _VPF_blocked is set.

Normally you'd want to try to refactor both of those to share a commmon
codepath, but it looks like there are specific reasons why they have to
be different codepaths; so you'll just have to make the callback in both
places (after setting VPF_blocked).

You also need to check that local_events_need_delivery() will return
"true" if you get an interrupt between that time and entering the
hypervisor.  Will that happen automatically from
hvm_local_events_need_delivery() -> hvm_vcpu_has_pending_irq() ->
vlapic_has_pending_irq()?  Or will you need to add a hook in
hvm_vcpu_has_pending_irq()?

 -George

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

Re: [Xen-devel] [PATCH 1/2] xen: arm: traps: check hsr.ec for ARM32

[Peng Fan]

Hi Julien,
On Mon, Sep 21, 2015 at 10:47:23AM +0100, Julien Grall wrote:
>Hi Peng,
>
>On 21/09/15 08:07, Peng Fan wrote:
>> To ARM64, "if ( hsr.ec >= 0x10 ) return 1;" is ok for unconditional
>> check, but to ARM32, we need to use 'hsr.ec >> 30' to check.
>
>hsr.ec is encoded on 5 bits, therefore the shift you suggest is wrong.
>Maybe you wanted to use (hsr.ec >> 4)?
Thanks for correcting me. 0x10 can handle hsr.ec >> 4.
My bad, this patch is wrong. 

Regards,
Peng.
>
>Although, can you explain why you need a different check for ARM32?
>
>Regards,
>
>> Signed-off-by: Peng Fan 
>> Cc: Ian Campbell 
>> Cc: Stefano Stabellini 
>> Cc: Julien Grall 
>> ---
>>  xen/arch/arm/traps.c | 5 +
>>  1 file changed, 5 insertions(+)
>> 
>> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
>> index 9d2bd6a..2e2b1f2 100644
>> --- a/xen/arch/arm/traps.c
>> +++ b/xen/arch/arm/traps.c
>> @@ -1531,8 +1531,13 @@ static int check_conditional_instr(struct 
>> cpu_user_regs *regs,
>>  int cond;
>>  
>>  /* Unconditional Exception classes */
>> +#ifdef CONFIG_ARM_64
>>  if ( hsr.ec >= 0x10 )
>>  return 1;
>> +#else
>> +if ( hsr.ec >> 30 )
>> +return 1;
>> +#endif
>>  
>>  /* Check for valid condition in hsr */
>>  cond = hsr.cond.ccvalid ? hsr.cond.cc : -1;
>> 
>
>
>-- 
>Julien Grall

-- 

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

Re: [Xen-devel] [PATCH 1/2] xen: arm: traps: check hsr.ec for ARM32

[Julien Grall]

Hi Peng,

On 21/09/15 08:07, Peng Fan wrote:
> To ARM64, "if ( hsr.ec >= 0x10 ) return 1;" is ok for unconditional
> check, but to ARM32, we need to use 'hsr.ec >> 30' to check.

hsr.ec is encoded on 5 bits, therefore the shift you suggest is wrong.
Maybe you wanted to use (hsr.ec >> 4)?

Although, can you explain why you need a different check for ARM32?

Regards,

> Signed-off-by: Peng Fan 
> Cc: Ian Campbell 
> Cc: Stefano Stabellini 
> Cc: Julien Grall 
> ---
>  xen/arch/arm/traps.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 9d2bd6a..2e2b1f2 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -1531,8 +1531,13 @@ static int check_conditional_instr(struct 
> cpu_user_regs *regs,
>  int cond;
>  
>  /* Unconditional Exception classes */
> +#ifdef CONFIG_ARM_64
>  if ( hsr.ec >= 0x10 )
>  return 1;
> +#else
> +if ( hsr.ec >> 30 )
> +return 1;
> +#endif
>  
>  /* Check for valid condition in hsr */
>  cond = hsr.cond.ccvalid ? hsr.cond.cc : -1;
> 


-- 
Julien Grall

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

Re: [Xen-devel] [Patch RFC 00/13] VT-d Asynchronous Device-TLB Flush for ATS Device

[Xu, Quan]

Thanks Jan.

>> >>> On 21.09.15 at 16:51, < jbeul...@suse.com > wrote:
>>> On 17.09.15 at 05:26,  wrote:
> Much more information:
>If I run a service in this domain and tested this waitqueue case. 
> The domain is still working after 60s, but It prints out Call Trace with 
> $dmesg:
> 
> [  161.978599] BUG: soft lockup - CPU#0 stuck for 57s! 
> [kworker/0:1:272]

>Not sure what you meant to tell us with that (it clearly means there's a bug 
>somewhere in the series you're testing):
>- Drop the current series and wait for an update?

No.

>- A request for help? If so, I don't think there's much to be said
  based on just the kernel soft lockup detector output.
>- Anything else?


Just test the extreme case. The ATS specification mandates a timeout of 1 
_minute_ for cache flush, even though it doesn't take so much time for cache 
flush.
In my design, if the Device-TLB is not completed, the domain's vCPUs are not 
allowed entry guest mode (patch #7), otherwise, the logic is not correct.



~Quan


>Jan


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

Re: [Xen-devel] [PATCH v6 16/18] vmx: Add some scheduler hooks for VT-d posted interrupts

[George Dunlap]

On 09/21/2015 06:07 AM, Wu, Feng wrote:
> 
> 
>> -Original Message-
>> From: Wu, Feng
>> Sent: Thursday, September 17, 2015 2:16 PM
>> To: George Dunlap; Jan Beulich
>> Cc: Tian, Kevin; Keir Fraser; Andrew Cooper; Dario Faggioli;
>> xen-devel@lists.xen.org; Wu, Feng
>> Subject: RE: [Xen-devel] [PATCH v6 16/18] vmx: Add some scheduler hooks for
>> VT-d posted interrupts
>>
>>
>>
>>> -Original Message-
>>> From: dunl...@gmail.com [mailto:dunl...@gmail.com] On Behalf Of
>> George
>>> Dunlap
>>> Sent: Thursday, September 17, 2015 12:57 AM
>>> To: Jan Beulich
>>> Cc: Wu, Feng; Tian, Kevin; Keir Fraser; Andrew Cooper; Dario Faggioli;
>>> xen-devel@lists.xen.org
>>> Subject: Re: [Xen-devel] [PATCH v6 16/18] vmx: Add some scheduler hooks for
>>> VT-d posted interrupts
>>>
>>> On Mon, Sep 7, 2015 at 1:54 PM, Jan Beulich  wrote:
>>> On 25.08.15 at 03:57,  wrote:
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -1573,6 +1573,22 @@ static void __context_switch(void)
>  per_cpu(curr_vcpu, cpu) = n;
>  }
>
> +static inline void pi_ctxt_switch_from(struct vcpu *prev)
> +{
> +/*
> + * When switching from non-idle to idle, we only do a lazy context
>>> switch.
> + * However, in order for posted interrupt (if available and enabled)
>> to
> + * work properly, we at least need to update the descriptors.
> + */
> +if ( prev->arch.pi_ctxt_switch_from && !is_idle_vcpu(prev) )
> +prev->arch.pi_ctxt_switch_from(prev);
> +}
> +
> +static inline void pi_ctxt_switch_to(struct vcpu *next)
> +{
> +if ( next->arch.pi_ctxt_switch_to && !is_idle_vcpu(next) )
> +next->arch.pi_ctxt_switch_to(next);
> +}
>
>  void context_switch(struct vcpu *prev, struct vcpu *next)
>  {
> @@ -1605,9 +1621,12 @@ void context_switch(struct vcpu *prev, struct
>>> vcpu *next)
>
>  set_current(next);
>
> +pi_ctxt_switch_from(prev);
> +
>  if ( (per_cpu(curr_vcpu, cpu) == next) ||
>   (is_idle_domain(nextd) && cpu_online(cpu)) )
>  {
> +pi_ctxt_switch_to(next);
>  local_irq_enable();

 This placement, if really intended that way, needs explanation (in a
 comment) and perhaps even renaming of the involved symbols, as
 looking at it from a general perspective it seems wrong (with
 pi_ctxt_switch_to() excluding idle vCPU-s it effectively means you
 want this only when switching back to what got switched out lazily
 before, i.e. this would be not something to take place on an arbitrary
 context switch). As to possible alternative names - maybe make the
 hooks ctxt_switch_prepare() and ctxt_switch_cancel()?
>>>
>>> Why on earth is this more clear than what he had before?
>>>
>>> In the first call, he's not "preparing" anything -- he's actually
>>> switching the PI context out for prev.  And in the second call, he's
>>> not "cancelling" anything -- he's actually switching the PI context in
>>> for next.  The names you suggest are actively confusing, not helpful.
>>>
>>> But before talking about how to make things more clear, one side
>>> question -- do we need to actually call pi_ctxt_switch_to() in
>>> __context_switch()?
>>>
>>> The only other place __context_switch() is called is
>>> from__sync_local_execstate().  But the only reason that needs to be
>>> called is because sometimes we *don't* call __context_switch(), and so
>>> there are things on the cpu that aren't copied into the vcpu struct.
>>
>> Thanks for the comments!
>>
>> From my understanding, __sync_local_execstate() can only get called
>> in the following two cases:
>> #1) this_cpu(curr_vcpu) == current, in this case, __context_switch() is
>> not called.
>> #2) this_cpu(curr_vcpu) != current, and current == idle_vcpu, that means
>> we just switched from a non-idle vCPU to idle vCPU, so here we need to
>> call __context_switch() to copy things to the original vcpu struct.
>>
>> Please correct me if the above understanding is wrong or incomplete?
> 
> Hi George / Dario,
> 
> Could you please confirm the above understanding is correct? (In fact, it is
> Related to lazy context switch, right?) if so I can continue with the
> pi_context_switch() way George suggested.

Yes, that's the general idea.  Normally, you can access the registers of
a non-running vcpu from the vcpu struct.  But if we've done a lazy
context switch, that's not true -- so to access those registers properly
we need to go through and do the full context switch *on that pcpu*.

Since we need to do the full context switch for PI every time, there
should never be any "local" state which needs to be synced.

I think at this point you should probably just give it a try. :-)

 -George


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

Re: [Xen-devel] [OSSTEST PATCH 25/26] Debian i386 HVM tests: Increase installation timeout

[Ian Campbell]

On Fri, 2015-09-18 at 18:50 +0100, Ian Jackson wrote:
> The Debian i386 image boots a 32-bit non-PAE kernel which therefore
> cannot have any PV drivers on our 64-bit hypervisors.  This makes it a
> bit slow: in my test on a machine under my desk it took 1400s out of
> the allowed 2000s.
> 
> With this change the timeout is 3000s instead.
> 
> Signed-off-by: Ian Jackson 

Acked-by: Ian Campbell 

(although see comments earlier regarding how you could enable PV drivers
here...)


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

Re: [Xen-devel] [OSSTEST PATCH 26/26] cri-common: Add a missing semicolon

[Ian Campbell]

On Fri, 2015-09-18 at 18:50 +0100, Ian Jackson wrote:
> This is not technically needed as bash interprets `a=1 b=2' as
> settings of both a and b.  But it's not idiomatic sh within osstest to
> use this syntax.
> 
> No functional change.
> 
> Signed-off-by: Ian Jackson 

Acked-by: Ian Campbell 


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

Re: [Xen-devel] [OSSTEST PATCH 24/26] Timeouts: Honour guest-related timeout-adjustment runvars

[Ian Campbell]

On Fri, 2015-09-18 at 18:50 +0100, Ian Jackson wrote:
> We look up a guest runvar GUEST_CONTEXT_timeoutfactor (according to
> the usual rules for guest runvars).
> 
> Here CONTEXT can currently be `general' or `install'.  (`install'
> applies when the guest is being installed.)  If the runvar exists, all
> timeouts relating to that guest in that context are increased by the
> specified factor.
> 
> Signed-off-by: Ian Jackson 

Acked-by: Ian Campbell 


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

Re: [Xen-devel] [OSSTEST PATCH 23/26] Timeouts: Introduce target_adjust_timeout

[Ian Campbell]

On Fri, 2015-09-18 at 18:50 +0100, Ian Jackson wrote:
> This function is now called for almost every timeout.
> 
> Specifically, it is called in the cases in TestSupport where a guest-
> or host-related timeout is passed from code which has a $ho or $gho,
> to code which does not: all callers of poll_loop, and tcmd.
> 
> Currently the function is a no-op.  Its existence and use will allow
> us to introduce various provocations for adjusting timeouts, of which
> I have one planned.
> 
> Signed-off-by: Ian Jackson 

Acked-by: Ian Campbell 


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

Re: [Xen-devel] [OSSTEST PATCH 22/26] ts-debian-hvm-install: Use /dev/sda for i386, not /dev/xvda

[Ian Campbell]

On Fri, 2015-09-18 at 18:50 +0100, Ian Jackson wrote:
> Empirically, the device shows up as /dev/sda.

This is because the plain i386 kernel used by the installer does not
support Xen (which requires PAE) and this affects even the options needed
for PVHVM. amd64 on the other hand doesn't have this restriction and PVHVM
works with the installer kernel.

Because of this the i386 install ends up using the emulated sda device
instead of switching to the pv devices as amd64 does.

I think it's probably worth explaining some of this in the commit log
rather than "Empirically".

IIRC someone was working on enabling PVHVM even for non-PAE i386 kernels,
not sure what the status of that is, but clearly it isn't going to change
things for Wheezy.

If you wanted instead to enable PV support for these installs then the
multi-arch (i386+amd64) isoimages contain the Xen enabled kernels for i386
(as /install.386/xen/*, n.b. /install.amd/xen/* are suitable symlinks, so
you can aoid special cases.

e.g. this one:

http://cdimage.debian.org/cdimage/archive/7.9.0/multi-arch/iso-cd/

(sorry for forgetting about this when we spoke IRL last week).

Anyway, that's clearly an "if you can be bothered thing" so with an
expanded commit message:
Acked-by: Ian Campbell 


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

Re: [Xen-devel] [OSSTEST PATCH 21/26] ts-debian-hvm-install: Do not create EFI partition if EFI not in use

[Ian Campbell]

On Fri, 2015-09-18 at 18:50 +0100, Ian Jackson wrote:
> If we are booting our install ISO using a non-EFI executable, don't
> try to provide an EFI for the installed system either.
> 
> Signed-off-by: Ian Jackson 

FWIW the default recipes used by d-i use "$iflabel{ gpt }" to achieve the
same thing. Actually I'm a bit surprised that this isn't the affect of
"method { efi }" too.

Anyway, this approach is good enough for our purposes so:

Acked-by: Ian Campbell 


> ---
>  ts-debian-hvm-install |7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/ts-debian-hvm-install b/ts-debian-hvm-install
> index fb07293..9bc6cc8 100755
> --- a/ts-debian-hvm-install
> +++ b/ts-debian-hvm-install
> @@ -49,6 +49,7 @@ our $guesthost= "$gn.guest.osstest";
>  our $gho;
>  
>  our ($kernel, $ramdisk);
> +our $bootfile;
>  
>  our $gsuite;
>  
> @@ -56,7 +57,7 @@ sub preseed () {
>  
>  my $preseed_file = preseed_base($gho,$gsuite,'','',());
>  
> -$preseed_file .= (< +$preseed_file .= (<  d-i netcfg/get_hostname string $gn
>  
>  d-i partman-auto/disk string /dev/xvda
> @@ -64,12 +65,14 @@ d-i partman-auto/method string  regular
>  
>  d-i partman-auto/expert_recipe string \\
>  boot-root :: \\
> +END
>  512 50 512 vfat \\
>  \$primary{ } \$bootable{ } \\
>  method{ efi } format{ } \\
>  use_filesystem{ } filesystem{ vfat } \\
>  mountpoint{ /boot/efi } \\
>  . \\
> +END
>  5000 50 5000 ext4 \\
>  method{ format } format{ } \\
>  use_filesystem{ } filesystem{ ext4 } \\
> @@ -201,7 +204,7 @@ sub prep () {
>-no-emul-boot
>-r);
>  
> -my $bootfile = 'boot/grub/efi.img';
> +$bootfile = 'boot/grub/efi.img';
>  if (!target_file_exists($ho, "$newiso/$bootfile")) {
>   $bootfile = "isolinux/isolinux.bin";
>   push @isogen_extra, qw(-c isolinux/boot.cat);

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

Re: [Xen-devel] [OSSTEST PATCH 20/26] ts-debian-hvm-install: Set $gsuite after $gho

[Ian Campbell]

On Fri, 2015-09-18 at 18:50 +0100, Ian Jackson wrote:
> $gsuite was set from guest_var, but before $gho was set, leading to an
> undefined value warning from Perl.
> 
> This would ignore any guest-specific suite runvars.  AFAICT these are
> set by some of the jobs in make-distros-flight.  I think the effect of
> this change is to apply workarounds for the intended suite, rather
> than for wheezy.
> 
> (Although there is another assignment to $gho later in
> ts-debian-hvm-install, for stage 2, the stage 2 code does some trivial
> TestSupport calls and does not need $gsuite.  So there is no need to
> make arrangements to assign to $gsuite - or, for that matter, $kernel
> or $ramdisk, in that path.)
> 
> Signed-off-by: Ian Jackson 

Acked-by: Ian Campbell 


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

Re: [Xen-devel] [OSSTEST PATCH 19/26] ts-debian-hvm-install: Cope with images containing only isolinux

[Ian Campbell]

On Fri, 2015-09-18 at 18:50 +0100, Ian Jackson wrote:
> debian-7.2.0-i386-CD-1.iso contains no grub, only isolinux.
> 
> If the specified EFI grub file does not exist, fall back to isolinux.
> This requires a -c option as well, according to
>   https://wiki.debian.org/DebianInstaller/Modify/CD
> 
> Only try to set up a grub config if we are booting grub.  (The i386
> image in question does not contain a [debian]/boot/grub directory.)
> 
> If boot/grub/efi.img _does_ exist (ie, for other existing tests), the
> only difference in behaviour is to reorder slightly the options to
> genisoimage: `-b boot/grub/efi.img' now occurs after `-no-emul-boot
> -r' rather than before.

I'm happy to determine experimentally (i.e. by pushing to pretest) if there
is any meaning to the order of these (the wiki has them the new way around,
so I presume not).

> Signed-off-by: Ian Jackson 
> ---
>  ts-debian-hvm-install |   13 +++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/ts-debian-hvm-install b/ts-debian-hvm-install
> index 3b93ebd..71ab1a5 100755
> --- a/ts-debian-hvm-install
> +++ b/ts-debian-hvm-install
> @@ -197,9 +197,16 @@ sub prep () {
>  my $preseed_file_path = $base . "preseed";
>  
>  my @isogen_extra = qw(-eltorito-alt-boot
> -  -b boot/grub/efi.img
>-no-emul-boot
>-r);
> +
> +my $bootfile = 'boot/grub/efi.img';
> +if (!target_file_exists($ho, "$newiso/$bootfile")) {
> + $bootfile = "isolinux/isolinux.bin";
> + push @isogen_extra, qw(-c isolinux/boot.cat);
> +}

My preference would have been to produce an iso which was bootable either
via EFI (grub) or legacy (isolinux), but that would require more complex
command lines and I'm sure neither of us wants to figure out what those
are. So:

Acked-by: Ian Campbell 


> +push @isogen_extra, '-b', $bootfile;
> +
>  my @isogen_opts = (iso_gen_flags_basic(), @isogen_extra);
>  
>  iso_create_empty($ho, $emptyiso, $emptydir);
> @@ -226,8 +233,10 @@ sub prep () {
>  my $cmds = iso_copy_content_from_image($gho, $newiso);
>  $cmds .= prepare_initrd($initrddir,$newiso,$preseed_file_path);
>  target_cmd_root($ho, $cmds, $isotimeout);
> +
>  target_putfilecontents_root_stash($ho, 10, grub_cfg(),
> - 
>  "$newiso/debian/boot/grub/grub.cfg");
> + 
>  "$newiso/debian/boot/grub/grub.cfg")
> + if $bootfile =~ m/grub/;
>  
>  target_putfilecontents_root_stash($ho, 10, isolinux_cfg(),
>   
>  "$newiso/isolinux/isolinux.cfg");

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

Re: [Xen-devel] [PATCH v6 16/18] vmx: Add some scheduler hooks for VT-d posted interrupts

[George Dunlap]

On 09/21/2015 09:23 AM, Jan Beulich wrote:
 On 16.09.15 at 18:56,  wrote:
>> On Mon, Sep 7, 2015 at 1:54 PM, Jan Beulich  wrote:
>> On 25.08.15 at 03:57,  wrote:
 --- a/xen/arch/x86/domain.c
 +++ b/xen/arch/x86/domain.c
 @@ -1573,6 +1573,22 @@ static void __context_switch(void)
  per_cpu(curr_vcpu, cpu) = n;
  }

 +static inline void pi_ctxt_switch_from(struct vcpu *prev)
 +{
 +/*
 + * When switching from non-idle to idle, we only do a lazy context 
 switch.
 + * However, in order for posted interrupt (if available and enabled) 
 to
 + * work properly, we at least need to update the descriptors.
 + */
 +if ( prev->arch.pi_ctxt_switch_from && !is_idle_vcpu(prev) )
 +prev->arch.pi_ctxt_switch_from(prev);
 +}
 +
 +static inline void pi_ctxt_switch_to(struct vcpu *next)
 +{
 +if ( next->arch.pi_ctxt_switch_to && !is_idle_vcpu(next) )
 +next->arch.pi_ctxt_switch_to(next);
 +}

  void context_switch(struct vcpu *prev, struct vcpu *next)
  {
 @@ -1605,9 +1621,12 @@ void context_switch(struct vcpu *prev, struct vcpu 
 *next)

  set_current(next);

 +pi_ctxt_switch_from(prev);
 +
  if ( (per_cpu(curr_vcpu, cpu) == next) ||
   (is_idle_domain(nextd) && cpu_online(cpu)) )
  {
 +pi_ctxt_switch_to(next);
  local_irq_enable();
>>>
>>> This placement, if really intended that way, needs explanation (in a
>>> comment) and perhaps even renaming of the involved symbols, as
>>> looking at it from a general perspective it seems wrong (with
>>> pi_ctxt_switch_to() excluding idle vCPU-s it effectively means you
>>> want this only when switching back to what got switched out lazily
>>> before, i.e. this would be not something to take place on an arbitrary
>>> context switch). As to possible alternative names - maybe make the
>>> hooks ctxt_switch_prepare() and ctxt_switch_cancel()?
>>
>> Why on earth is this more clear than what he had before?
>>
>> In the first call, he's not "preparing" anything -- he's actually
>> switching the PI context out for prev.  And in the second call, he's
>> not "cancelling" anything -- he's actually switching the PI context in
>> for next.  The names you suggest are actively confusing, not helpful.
> 
> While I think later discussion on this thread moved in a good direction,
> I still think I should reply here (even if late): To me, the use of
> pi_ctxt_switch_to() in the patch fragment still seen above is very
> much the cancellation of the immediately preceding pi_ctxt_switch_from(),
> as it's the "we don't want to do anything else" path that it gets put
> into.

Either we have different understandings about what the code does, or I
don't understand what you're saying here.

The codepath in question will only be called if we're switching *into*
or *out of* the "lazy context swtich" -- i.e., switching from a vcpu to
the idle vcpu, but not saving or restoring state.

For the "switch into" case:
* prev will be a domain vcpu, next will be the idle vcpu
* pi_ctxt_switch_from(prev) will either change NV to 'pi_wake' or set SN
for prev's PI vectors (depending on whether prev is blocked or offline)
* pi_ctxt_switch_to(next) will do nothing, since next is the idle vcpu

For the "switching out" case:
* prev will be the idle vcpu, next will be a domain vcpu
* pi_ctxt_switch_from(prev) will do nothing, since prev is the idle vcpu
* pi_ctxt_switch_to(next) will clear SN and change the vector to
'posted_intr'

So there is no situation in which pi_ctxt_switch_to() "cancels" the
immediately preceding pi_ctxt_switch_from().  Either the preceding
from() did something, in which case to() does nothing; or the preceding
from() did nothing, in which case to() does something.

 -George

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

Re: [Xen-devel] [OSSTEST PATCH 18/26] ts-debian-hvm-install, etc.: Do not hardcode in-iso path

[Ian Campbell]

On Fri, 2015-09-18 at 18:50 +0100, Ian Jackson wrote:
> ts-debian-hvm-install hardcoded `install.amd' as the directory in the
> .iso in which to find the kernel and initrd.  This is wrong for
> architectures other than amd64.
> 
> Instead, pass this information in runvars (as is done for the netinst
> installs in make-distros-flight), and honour it in
> ts-debian-hvm-install.
> 
> If the runvars are not set, default to the previous hardcoded values.
> (This arranges that clones of old flights still work with new osstest,
> eg for bisection.)
> 
> Signed-off-by: Ian Jackson 

Acked-by: Ian Campbell 


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

Re: [Xen-devel] [PATCH v7 15/17] vmx: VT-d posted-interrupt core logic handling

[George Dunlap]

On 09/21/2015 06:08 AM, Wu, Feng wrote:
> 
> 
>> -Original Message-
>> From: George Dunlap [mailto:george.dun...@citrix.com]
>> Sent: Thursday, September 17, 2015 5:38 PM
>> To: Dario Faggioli; Wu, Feng
>> Cc: xen-devel@lists.xen.org; Tian, Kevin; Keir Fraser; George Dunlap; Andrew
>> Cooper; Jan Beulich
>> Subject: Re: [Xen-devel] [PATCH v7 15/17] vmx: VT-d posted-interrupt core 
>> logic
>> handling
>>
>> On 09/17/2015 09:48 AM, Dario Faggioli wrote:
>>> On Thu, 2015-09-17 at 08:00 +, Wu, Feng wrote:
>>>
> -Original Message-
> From: Dario Faggioli [mailto:dario.faggi...@citrix.com]
>>>
> So, I guess, first of all, can you confirm whether or not it's exploding
> in debug builds?

 Does the following information in Config.mk mean it is a debug build?

 # A debug build of Xen and tools?
 debug ?= y
 debug_symbols ?= $(debug)

>>> I think so. But as I said in my other email, I was wrong, and this is
>>> probably not an issue.
>>>
> And in either case (just tossing out ideas) would it be
> possible to deal with the "interrupt already raised when blocking" case:

 Thanks for the suggestions below!

>>> :-)
>>>
>  - later in the context switching function ?
 In this case, we might need to set a flag in vmx_pre_ctx_switch_pi() 
 instead
 of calling vcpu_unblock() directly, then when it returns to 
 context_switch(),
 we can check the flag and don't really block the vCPU.

>>> Yeah, and that would still be rather hard to understand and maintain,
>>> IMO.
>>>
 But I don't have a clear
 picture about how to archive this, here are some questions from me:
 - When we are in context_switch(), we have done the following changes to
 vcpu's state:
* sd->curr is set to next
* vCPU's running state (both prev and next ) is changed by
  vcpu_runstate_change()
* next->is_running is set to 1
* periodic timer for prev is stopped
* periodic timer for next is setup
..

 So what point should we perform the action to _unblock_ the vCPU? We
 Need to roll back the formal changes to the vCPU's state, right?

>>> Mmm... not really. Not blocking prev does not mean that prev would be
>>> kept running on the pCPU, and that's true for your current solution as
>>> well! As you say yourself, you're already in the process of switching
>>> between prev and next, at a point where it's already a thing that next
>>> will be the vCPU that will run. Not blocking means that prev is
>>> reinserted to the runqueue, and a new invocation to the scheduler is
>>> (potentially) queued as well (via raising SCHEDULE_SOFTIRQ, in
>>> __runq_tickle()), but it's only when such new scheduling happens that
>>> prev will (potentially) be selected to run again.
>>>
>>> So, no, unless I'm fully missing your point, there wouldn't be no
>>> rollback required. However, I still would like the other solution (doing
>>> stuff in vcpu_block()) better (see below).
>>>
>  - with another hook, perhaps in vcpu_block() ?

 We could check this in vcpu_block(), however, the logic here is that before
 vCPU is blocked, we need to change the posted-interrupt descriptor,
 and during changing it, if 'ON' bit is set, which means VT-d hardware
 issues a notification event because interrupts from the assigned devices
 is coming, we don't need to block the vCPU and hence no need to update
 the PI descriptor in this case.

>>> Yep, I saw that. But could it be possible to do *everything* related to
>>> blocking, including the update of the descriptor, in vcpu_block(), if no
>>> interrupt have been raised yet at that time? I mean, would you, if
>>> updating the descriptor in there, still get the event that allows you to
>>> call vcpu_wake(), and hence vmx_vcpu_wake_prepare(), which would undo
>>> the blocking, no matter whether that resulted in an actual context
>>> switch already or not?
>>>
>>> I appreciate that this narrows the window for such an event to happen by
>>> quite a bit, making the logic itself a little less useful (it makes
>>> things more similar to "regular" blocking vs. event delivery, though,
>>> AFAICT), but if it's correct, ad if it allows us to save the ugly
>>> invocation of vcpu_unblock from context switch context, I'd give it a
>>> try.
>>>
>>> After all, this PI thing requires actions to be taken when a vCPU is
>>> scheduled or descheduled because of blocking, unblocking and
>>> preemptions, and it would seem natural to me to:
>>>  - deal with blockings in vcpu_block()
>>>  - deal with unblockings in vcpu_wake()
>>>  - deal with preemptions in context_switch()
>>>
>>> This does not mean being able to consolidate some of the cases (like
>>> blockings and preemptions, in the current version of the code) were not
>>> a nice thing... But we don't want it at all costs . :-)
>>
>> So just to clarify the situation...
>>
>> If a vcpu config

Re: [Xen-devel] [OSSTEST PATCH 15/26] cr*: Support xen-unstable-smoke

[Ian Campbell]

On Fri, 2015-09-18 at 18:50 +0100, Ian Jackson wrote:
> Add this branch to select_xenbranch.  This works like xen-unstable in
> most respects.
> 
> There are only two places in osstest where xenbranch `xen-unstable' is
> treated specially and only one of them needs adjusting to match
> xen-unstable-smoke too.

The included one is the one which adds the qemu revisions?

And the other one is, I think, the thing which sets
 REVISION_LINUX_OLD=disable which we want (or maybe don't care about) for
the smoked tests. So good.

> The new branch `xen-unstable-smoke' has a `prev' branch of
> `xen-unstable' according to cri-getprevxenbranch, which is technically
> wrong, but this is not important because xen-unstable-smoke has no
> prev tests.
> 
> We are going to sort out the push gate ref plumbing in xen.git in the
> next osstest patch.
> 
> Also, use a branch-settings file to set the new branch's resource
> priority to -20 to make it run ahead of anything else automatic.
> 
> Signed-off-by: Ian Jackson 

Acked-by: Ian Campbell 


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

Re: [Xen-devel] [PATCH 2/2] xen: Introduce VM_EVENT_FLAG_SET_EIP

[Razvan Cojocaru]

On 09/21/2015 11:53 AM, Jan Beulich wrote:
 On 18.09.15 at 21:19,  wrote:
>> On Wed, Sep 16, 2015 at 12:12 PM, Razvan Cojocaru >> wrote:
>>> I have nothing in principle against having a SET_REGISTERS flag instead
>>> of a SET_EIP one, but I am unsure of how (and where) that would be best
>>> implemented. What do you have in mind? A handler similar to void
>>> vm_event_register_write_resume() where we set these registers for the
>>> respective vcpu? Is this even possible at vm_event response handling time?
>>>
>>
>> No, that function falls under a switch on rsp.reason, for which we have a
>> 1:1 unofficial and not really enforced rule to match the type of event that
>> was sent. This should fall under a flag on rsp.flags and be handled similar
>> to how vm_event_toggle_singlestep is.
> 
> I.e. I take this to mean that we should wait for a new patch
> rather than further looking at the current one.

Yes, I've already modified the first patch on Andrew Cooper's suggestion
(to switch from xc_domain_emulate_each_rep() to
xc_monitor_emulate_each_rep(), and gate the emulation disable condition
on mem_access_emulate_enable as well as mem_access_emulate_each_rep),
and I'm working on switching from SET_EIP to SET_REGISTERS as we speak,
after which I'll do a test run and send a new version, hopefully no
later than tomorrow.

For this patch, I'm slightly unsure if I should expect trouble for
trying to do it this way (I know I have to abstract that raw code away
for x86 and ARM in their respective functions, but let's just assume x86
for the example):

418 if ( rsp.flags & VM_EVENT_FLAG_VCPU_PAUSED )
419 {
420 if ( rsp.flags & VM_EVENT_FLAG_SET_REGISTERS )
421 v->arch.user_regs.eip = rsp.data.regs.x86.rip;
422
423 if ( rsp.flags & VM_EVENT_FLAG_TOGGLE_SINGLESTEP )
424 vm_event_toggle_singlestep(d, v);
425
426 vm_event_vcpu_unpause(v);
427 }

at the end of vm_event_resume() in common/vm_event.c. Looks like it
should be safe, but I'm not sure.


Thanks,
Razvan

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

Re: [Xen-devel] [PATCH 2/2] xen: Introduce VM_EVENT_FLAG_SET_EIP

[Jan Beulich]

>>> On 18.09.15 at 21:19,  wrote:
> On Wed, Sep 16, 2015 at 12:12 PM, Razvan Cojocaru > wrote:
>> I have nothing in principle against having a SET_REGISTERS flag instead
>> of a SET_EIP one, but I am unsure of how (and where) that would be best
>> implemented. What do you have in mind? A handler similar to void
>> vm_event_register_write_resume() where we set these registers for the
>> respective vcpu? Is this even possible at vm_event response handling time?
>>
> 
> No, that function falls under a switch on rsp.reason, for which we have a
> 1:1 unofficial and not really enforced rule to match the type of event that
> was sent. This should fall under a flag on rsp.flags and be handled similar
> to how vm_event_toggle_singlestep is.

I.e. I take this to mean that we should wait for a new patch
rather than further looking at the current one.

Jan


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

Re: [Xen-devel] [Patch RFC 00/13] VT-d Asynchronous Device-TLB Flush for ATS Device

[Jan Beulich]

>>> On 17.09.15 at 05:26,  wrote:
> Much more information:
>If I run a service in this domain and tested this waitqueue case. The 
> domain is still working after 60s, but It prints out Call Trace with $dmesg:
> 
> [  161.978599] BUG: soft lockup - CPU#0 stuck for 57s! [kworker/0:1:272]

Not sure what you meant to tell us with that (it clearly means
there's a bug somewhere in the series you're testing):
- Drop the current series and wait for an update?
- A request for help? If so, I don't think there's much to be said
  based on just the kernel soft lockup detector output.
- Anything else?

Jan


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

Re: [Xen-devel] [PATCH v2 1/2] x86/msr: Carry on after a non-"safe" MSR access fails without !panic_on_oops

[Ingo Molnar]

* Andy Lutomirski  wrote:

> On Sep 20, 2015 5:15 PM, "Linus Torvalds"  
> wrote:
> >
> > On Sun, Sep 20, 2015 at 5:02 PM, Andy Lutomirski  wrote:
> > > This demotes an OOPS and likely panic due to a failed non-"safe" MSR
> > > access to a WARN_ON_ONCE and a return of zero (in the RDMSR case).
> > > We still write a pr_info entry unconditionally for debugging.
> >
> > No, this is wrong.
> >
> > If you really want to do something like this, then just make all MSR reads 
> > safe. So the only difference between "safe" and "unsafe" is that the unsafe 
> > version just doesn't check the return value, and silently just returns zero 
> > for reads (or writes nothing).
> >
> > To quote Obi-Wan: "Use the exception table, Luke".
> >
> > Because decoding instructions is just too ugly. We'll do it for CPU errata 
> > where we might have to do it for user space code too (ie the AMD prefetch 
> > mess), but for code that _we_ control? Hell no.
> >
> > So NAK on this.
> 
> My personal preference is to just not do this at all.  A couple people 
> disagree.  
> If we make the unsafe variants not oops, then I think we want to have the 
> nice 
> loud warning, since these issues are bugs if they happen.
> 
> We could certainly use the exception table for this, but it'll result in 
> bigger 
> core, since each MSR access will need an exception table entry and an 
> associated 
> fixup to call some helper that warns and sets the result to zero.
> 
> I'd be happy to implement that, but only if it'll be applied. Otherwise I'd 
> rather just drop this patch and keep the rest of the series.

Linus, what's your preference?

Due to the bug mentioned earlier in this thread all MSR reads are currently 
'safe' 
on all the major Linux distros (which all have CONFIG_PARAVIRT=y), i.e. by 
'fixing' them we'd reintroduce random crashes into various fragile pieces of 
code...

To add insult to injury, the current 'silently safe by accident' MSR code isn't 
so 
safe: because it leaves the result of the read uninitialized...

To fix this all I'd really like to have:

 - safe MSR reads by default (i.e. never boot crash the kernel on some rare 
   condition - which to most users is either a silent boot hang or an instant 
   restart). Historicaly we had a stream of 'silly boot crashes' due to MSR 
reads 
   that generate a #GPF. They make Linux less usable around the edges, 
especially 
   in the x86 non-server (desktop) space where most hardware vendors are either 
   openly Linux hostile, or, at best, Linux oblivious.

 - proper result-zeroing behavior on exceptions

 - and we should also generate _some_ sort of warning when MSR exceptions happen
   in an 'unintended' fashion.

Maybe the warning could be put under a (default-enabled) config option for the 
size conscious.

Or we could extend exception table entry encoding to include a 'warning bit', 
to 
not bloat the kernel. If the exception handler code encounters such an 
exception 
it would generate a one-time warning for that entry, but otherwise not crash 
the 
kernel and continue execution with an all-zeroes result for the MSR read.

Thanks,

Ingo

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

Re: [Xen-devel] [qemu-mainline bisection] complete test-armhf-armhf-xl-vhd

[Ian Campbell]

This looks to be the issue which Anthony investigated last week and for
which the responsible patch has been reverted.

I think this bisection just hadn't caught up with the fact that the issue
is fixed yet.

Ian.


On Mon, 2015-09-21 at 05:36 +, osstest service owner wrote:
> branch xen-unstable
> xen branch xen-unstable
> job test-armhf-armhf-xl-vhd
> test xen-boot
> 
> Tree: linux git://xenbits.xen.org/linux-pvops.git
> Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
> Tree: qemuu git://git.qemu.org/qemu.git
> Tree: xen git://xenbits.xen.org/xen.git
> 
> *** Found and reproduced problem changeset ***
> 
>   Bug is in tree:  qemuu git://git.qemu.org/qemu.git
>   Bug introduced:  a2aa09e18186801931763fbd40a751fa39971b18
>   Bug not present: 7e4804dafd4689312ef1172b549927a973bb5414
> 
> 
>   commit a2aa09e18186801931763fbd40a751fa39971b18
>   Merge: 7e4804d 47d4be1
>   Author: Peter Maydell 
>   Date:   Mon Sep 14 16:13:16 2015 +0100
>   
>   Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream'
> into staging
>   
>   * Support for jemalloc
>   * qemu_mutex_lock_iothread "No such process" fix
>   * cutils: qemu_strto* wrappers
>   * iohandler.c simplification
>   * Many other fixes and misc patches.
>   
>   And some MTTCG work (with Emilio's fixes squashed):
>   * Signal-free TCG kick
>   * Removing spinlock in favor of QemuMutex
>   * User-mode emulation multi-threading fixes/docs
>   
>   # gpg: Signature made Thu 10 Sep 2015 09:03:07 BST using RSA key ID
> 78C7AE83
>   # gpg: Good signature from "Paolo Bonzini "
>   # gpg: aka "Paolo Bonzini "
>   
>   * remotes/bonzini/tags/for-upstream: (44 commits)
> cutils: work around platform differences in strto{l,ul,ll,ull}
> cpu-exec: fix lock hierarchy for user-mode emulation
> exec: make mmap_lock/mmap_unlock globally available
> tcg: comment on which functions have to be called with mmap_lock
> held
> tcg: add memory barriers in page_find_alloc accesses
> remove unused spinlock.
> replace spinlock by QemuMutex.
> cpus: remove tcg_halt_cond and tcg_cpu_thread globals
> cpus: protect work list with work_mutex
> scripts/dump-guest-memory.py: fix after RAMBlock change
> configure: Add support for jemalloc
> add macro file for coccinelle
> configure: factor out adding disas configure
> vhost-scsi: fix wrong vhost-scsi firmware path
> checkpatch: remove tests that are not relevant outside the kernel
> checkpatch: adapt some tests to QEMU
> CODING_STYLE: update mixed declaration rules
> qmp: Add example usage of strto*l() qemu wrapper
> cutils: Add qemu_strtoull() wrapper
> cutils: Add qemu_strtoll() wrapper
> ...
>   
>   Signed-off-by: Peter Maydell 
>   
>   commit 47d4be12c3997343e436c6cca89aefbbbeb70863
>   Author: Paolo Bonzini 
>   Date:   Thu Sep 10 10:02:00 2015 +0200
>   
>   cutils: work around platform differences in strto{l,ul,ll,ull}
>   
>   Linux returns 0 if no conversion was made, while OS X and
> presumably
>   the BSDs return EINVAL.  The OS X convention rejects more invalid
>   inputs, so convert to it and adjust the test case.
>   
>   Windows returns 1 from strtoul and strtoull (instead of -1) for
>   negative out-of-range input; fix it up.
>   
>   Reported-by: Peter Maydell 
>   Signed-off-by: Paolo Bonzini 
>   
>   commit 9fd1a94888cd6a559f95c3596ec1ac28b74838c1
>   Author: Paolo Bonzini 
>   Date:   Tue Aug 11 11:33:24 2015 +0200
>   
>   cpu-exec: fix lock hierarchy for user-mode emulation
>   
>   tb_lock has to be taken inside the mmap_lock (example:
>   tb_invalidate_phys_range is called by target_mmap), but
>   tb_link_page is taking the mmap_lock and it is called
>   with the tb_lock held.
>   
>   To fix this, take the mmap_lock in tb_find_slow, not
>   in tb_link_page.
>   
>   Reviewed-by: Peter Maydell 
>   Signed-off-by: Paolo Bonzini 
>   
>   commit 8fd19e6cfd5b6cdf028c6ac2ff4157ed831ea3a6
>   Author: Paolo Bonzini 
>   Date:   Tue Aug 11 10:57:52 2015 +0200
>   
>   exec: make mmap_lock/mmap_unlock globally available
>   
>   There is some iffy lock hierarchy going on in translate-all.c.  To
>   fix it, we need to take the mmap_lock in cpu-exec.c.  Make the
>   functions globally available.
>   
>   Reviewed-by: Peter Maydell 
>   Signed-off-by: Paolo Bonzini 
>   
>   commit 756920876f60829fad0d15df4f3fa205077a8131
>   Author: Paolo Bonzini 
>   Date:   Tue Aug 11 10:59:50 2015 +0200
>   
>   tcg: comment on which functions have to be called with mmap_lock
> held
>   
>   Reviewed-by: Peter Maydell 
>   Signed-off-by: Paolo Bonzini 
>   
>   commit 6940fab84b826175cf90d48d0e3da1b76518f5b4

Re: [Xen-devel] libxen-light

[Ian Campbell]

On Mon, 2015-09-21 at 11:56 +0530, kumara rathnavel wrote:
> Hello,
> 
> I do not find any proper documentation regarding API provided by the
> Libxenlight. All I find is the documentation for the xl that in turn uses
> Libxl Library. But I would like to use the library calls directly, I am
> able to basic operations like listing vm, reboot and all those . But when
> it comes to creation of domain or attaching device , I am filling the
> structures the same way as xl is doing. But I get a Seg Fault. Can you
> share some documentation on the Libxenlight API if possible or guidance
> would do,,.

The documentation is mainly in the headers, starting with libxl.h.

If you have specific questions then please provide the code as well as the
error (e.g. the stack trace).

Ian.

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

Re: [Xen-devel] [PATCH v6 16/18] vmx: Add some scheduler hooks for VT-d posted interrupts

[Jan Beulich]

>>> On 16.09.15 at 18:56,  wrote:
> On Mon, Sep 7, 2015 at 1:54 PM, Jan Beulich  wrote:
> On 25.08.15 at 03:57,  wrote:
>>> --- a/xen/arch/x86/domain.c
>>> +++ b/xen/arch/x86/domain.c
>>> @@ -1573,6 +1573,22 @@ static void __context_switch(void)
>>>  per_cpu(curr_vcpu, cpu) = n;
>>>  }
>>>
>>> +static inline void pi_ctxt_switch_from(struct vcpu *prev)
>>> +{
>>> +/*
>>> + * When switching from non-idle to idle, we only do a lazy context 
>>> switch.
>>> + * However, in order for posted interrupt (if available and enabled) to
>>> + * work properly, we at least need to update the descriptors.
>>> + */
>>> +if ( prev->arch.pi_ctxt_switch_from && !is_idle_vcpu(prev) )
>>> +prev->arch.pi_ctxt_switch_from(prev);
>>> +}
>>> +
>>> +static inline void pi_ctxt_switch_to(struct vcpu *next)
>>> +{
>>> +if ( next->arch.pi_ctxt_switch_to && !is_idle_vcpu(next) )
>>> +next->arch.pi_ctxt_switch_to(next);
>>> +}
>>>
>>>  void context_switch(struct vcpu *prev, struct vcpu *next)
>>>  {
>>> @@ -1605,9 +1621,12 @@ void context_switch(struct vcpu *prev, struct vcpu 
>>> *next)
>>>
>>>  set_current(next);
>>>
>>> +pi_ctxt_switch_from(prev);
>>> +
>>>  if ( (per_cpu(curr_vcpu, cpu) == next) ||
>>>   (is_idle_domain(nextd) && cpu_online(cpu)) )
>>>  {
>>> +pi_ctxt_switch_to(next);
>>>  local_irq_enable();
>>
>> This placement, if really intended that way, needs explanation (in a
>> comment) and perhaps even renaming of the involved symbols, as
>> looking at it from a general perspective it seems wrong (with
>> pi_ctxt_switch_to() excluding idle vCPU-s it effectively means you
>> want this only when switching back to what got switched out lazily
>> before, i.e. this would be not something to take place on an arbitrary
>> context switch). As to possible alternative names - maybe make the
>> hooks ctxt_switch_prepare() and ctxt_switch_cancel()?
> 
> Why on earth is this more clear than what he had before?
> 
> In the first call, he's not "preparing" anything -- he's actually
> switching the PI context out for prev.  And in the second call, he's
> not "cancelling" anything -- he's actually switching the PI context in
> for next.  The names you suggest are actively confusing, not helpful.

While I think later discussion on this thread moved in a good direction,
I still think I should reply here (even if late): To me, the use of
pi_ctxt_switch_to() in the patch fragment still seen above is very
much the cancellation of the immediately preceding pi_ctxt_switch_from(),
as it's the "we don't want to do anything else" path that it gets put
into.

Jan


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

[Xen-devel] [qemu-upstream-4.4-testing test] 62142: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-vhd9 debian-di-install fail REGR. vs. 60565
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-localmigrate/x10 fail REGR. vs. 
60565
 test-amd64-i386-xl-qcow2  9 debian-di-install fail REGR. vs. 60565

Regressions which are regarded as allowable (not blocking):
 test-amd64-i386-libvirt-raw   9 debian-di-install fail REGR. vs. 60565
 test-amd64-i386-libvirt  11 guest-start  fail   like 60565
 test-amd64-amd64-libvirt 11 guest-start  fail   like 60565

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-raw   9 debian-di-installfail   never pass
 test-amd64-i386-libvirt-qcow2  9 debian-di-installfail  never pass
 test-amd64-i386-libvirt-pair 20 guest-start/debian   fail   never pass
 test-amd64-amd64-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-amd64-i386-libvirt-vhd  11 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-raw 11 migrate-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass

version targeted for testing:
 qemuu5fe74249f5ab528fe84a26fa60438a6de4c787f0
baseline version:
 qemuu0fc147387f0b683d2dfefec7b1af569f17b72e9c

Last test of basis60565  2015-08-04 01:59:38 Z   48 days
Failing since 61617  2015-09-08 12:10:54 Z   12 days6 attempts
Testing same since62062  2015-09-16 11:15:04 Z4 days2 attempts


People who touched revisions under test:
  Gerd Hoffmann 
  P J P 
  Peter Lieven 
  Stefan Hajnoczi 
  Stefano Stabellini 

jobs:
 build-amd64-xend pass
 build-i386-xend  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  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 pass
 test-amd64-i386-xl-qemuu-ovmf-amd64  pass
 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 fail
 test-amd64-i386-libvirt  fail
 test-amd64-amd64-xl-multivcpupass
 test-amd64-amd64-pairpass
 test-amd64-i386-pair pass
 test-amd64-amd64-libvirt-pairpass
 test-amd64-i386-libvirt-pair fail
 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-libvirt-qcow2   pass
 test-amd64-i386-libvirt-qcow2fail
 test-amd64-amd64-xl-qcow2pass
 test-amd64-i386-xl-qcow2 fail
 test-amd64-amd64-libvirt-raw pass
 test-amd64-i386-libvirt-raw  fail
 test-amd64-amd64-xl-raw  fail
 test-amd64-i386-xl-raw   pass
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 

Re: [Xen-devel] Kbuild and Kconfig

[Jan Beulich]

>>> On 18.09.15 at 21:31,  wrote:
> So locally I have in the xen/ (the hypervisor) "make menuconfig" working
> but I'd likely address any concerns you might have upfront in the
> patches. Effectively the xen/ directory now when built runs "make
> defconfig" before it builds to generate the configuration. The default
> configs I've used match what the tree currently contains. However you
> expressed interest in having from the top level certain variables
> controlled from that level like debug. I can pretty easily make it that
> the effective command line is "make debug=y/1/n/0 defconfig" and that
> would set CONFIG_DEBUG appropriately. If that's desired I can also
> include that. Are there other flags that it would be desirable to have
> them controllable from the top level?

First of all I think top level settings should only be used to set values
that currently have no setting at all in .config (or however you intend
to name that file). And then I think all top level setting currently
effectable via make command line option and affecting the hypervisor
build) should be treated the same.

Jan


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

[Xen-devel] [distros-debian-snapshot test] 37991: regressions - FAIL

[Platform Team regression test user]

flight 37991 distros-debian-snapshot real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/37991/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-amd64-daily-netboot-pvgrub  6 xen-boot   fail REGR. vs. 37923
 test-amd64-amd64-i386-daily-netboot-pygrub 9 debian-di-install fail REGR. vs. 
37923

Regressions which are regarded as allowable (not blocking):
 test-amd64-i386-amd64-current-netinst-pygrub 9 debian-di-install fail like 
37923
 test-amd64-i386-amd64-daily-netboot-pygrub 13 guest-saverestore fail like 37923
 test-amd64-i386-amd64-weekly-netinst-pygrub 13 guest-saverestore fail like 
37923
 test-amd64-amd64-amd64-weekly-netinst-pygrub 13 guest-saverestore fail like 
37923
 test-amd64-amd64-i386-current-netinst-pygrub 9 debian-di-install fail like 
37923
 test-amd64-i386-i386-current-netinst-pygrub 9 debian-di-install fail like 37923
 test-amd64-amd64-amd64-current-netinst-pygrub 9 debian-di-install fail like 
37923

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-armhf-daily-netboot-pygrub 9 debian-di-install fail never pass

baseline version:
 flight   37923

jobs:
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-amd64-daily-netboot-pvgrub  fail
 test-amd64-i386-i386-daily-netboot-pvgrubpass
 test-amd64-i386-amd64-daily-netboot-pygrub   fail
 test-armhf-armhf-armhf-daily-netboot-pygrub  fail
 test-amd64-amd64-i386-daily-netboot-pygrub   fail
 test-amd64-amd64-amd64-current-netinst-pygrubfail
 test-amd64-i386-amd64-current-netinst-pygrub fail
 test-amd64-amd64-i386-current-netinst-pygrub fail
 test-amd64-i386-i386-current-netinst-pygrub  fail
 test-amd64-amd64-amd64-weekly-netinst-pygrub fail
 test-amd64-i386-amd64-weekly-netinst-pygrub  fail
 test-amd64-amd64-i386-weekly-netinst-pygrub  pass
 test-amd64-i386-i386-weekly-netinst-pygrub   pass



sg-report-flight on osstest.xs.citrite.net
logs: /home/osstest/logs
images: /home/osstest/images

Logs, config files, etc. are available at
http://osstest.xs.citrite.net/~osstest/testlogs/logs

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


Push not applicable.


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

[Xen-devel] [linux-3.10 test] 62141: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qcow2 9 debian-di-install fail REGR. vs. 60670
 test-amd64-amd64-xl-vhd   9 debian-di-install fail REGR. vs. 60670
 test-amd64-amd64-xl-qemut-debianhvm-amd64 9 debian-hvm-install fail REGR. vs. 
60670
 test-amd64-i386-freebsd10-amd64 14 guest-localmigrate fail REGR. vs. 60670
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 9 debian-hvm-install 
fail REGR. vs. 60670
 test-amd64-amd64-xl-qemuu-debianhvm-amd64 9 debian-hvm-install fail REGR. vs. 
60670
 test-amd64-i386-qemuu-rhel6hvm-intel 10 guest-stopfail REGR. vs. 60670
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail REGR. 
vs. 60670
 test-amd64-i386-qemut-rhel6hvm-amd  9 redhat-install  fail REGR. vs. 60670
 test-amd64-i386-qemut-rhel6hvm-intel 12 guest-start/redhat.repeat fail REGR. 
vs. 60670
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 9 debian-hvm-install fail REGR. 
vs. 60670
 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm 9 debian-hvm-install fail REGR. 
vs. 60670
 test-amd64-i386-xl-qemut-debianhvm-amd64 13 guest-localmigrate fail REGR. vs. 
60670
 test-amd64-i386-xl-qcow2  9 debian-di-install fail REGR. vs. 60670
 test-amd64-amd64-xl-qemuu-ovmf-amd64 9 debian-hvm-install fail REGR. vs. 60670
 test-amd64-amd64-xl-qemut-winxpsp3  9 windows-install fail REGR. vs. 60670
 test-amd64-i386-xl-qemuu-winxpsp3  9 windows-install  fail REGR. vs. 60670
 test-amd64-amd64-xl-qemuu-winxpsp3  9 windows-install fail REGR. vs. 60670
 test-amd64-i386-xl-qemut-winxpsp3  9 windows-install  fail REGR. vs. 60670
 test-amd64-amd64-xl-qemuu-win7-amd64  9 windows-install   fail REGR. vs. 60670
 test-amd64-i386-xl-qemuu-win7-amd64  9 windows-installfail REGR. vs. 60670
 test-amd64-i386-xl-qemut-winxpsp3-vcpus1 9 windows-install fail REGR. vs. 60670
 test-amd64-amd64-xl-qemut-win7-amd64  9 windows-install   fail REGR. vs. 60670
 test-amd64-i386-xl-qemut-win7-amd64  9 windows-installfail REGR. vs. 60670
 test-amd64-i386-xl-vhd   10 guest-start  fail in 61983 REGR. vs. 60670
 test-amd64-i386-xl-qemuu-ovmf-amd64 12 guest-saverestore fail in 61983 REGR. 
vs. 60670
 test-amd64-i386-qemuu-rhel6hvm-amd 11 guest-start.2 fail in 61983 REGR. vs. 
60670
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 12 guest-saverestore fail in 
61983 REGR. vs. 60670
 test-amd64-i386-xl-qemuu-debianhvm-amd64 14 guest-saverestore.2 fail in 61983 
REGR. vs. 60670
 test-amd64-i386-freebsd10-i386 14 guest-localmigrate fail in 62060 REGR. vs. 
60670
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 14 guest-saverestore.2 fail in 62060 
REGR. vs. 60670

Tests which are failing intermittently (not blocking):
 test-amd64-i386-freebsd10-amd64 13 guest-saverestore fail in 61983 pass in 
62141
 test-amd64-amd64-libvirt-xsm 14 guest-saverestore  fail in 61983 pass in 62141
 test-amd64-i386-libvirt 15 guest-saverestore.2 fail in 61983 pass in 62141
 test-amd64-i386-qemut-rhel6hvm-intel 10 guest-stop fail in 61983 pass in 62141
 test-amd64-i386-xl-qemut-debianhvm-amd64 9 debian-hvm-install fail in 61983 
pass in 62141
 test-amd64-i386-libvirt-xsm 15 guest-saverestore.2 fail in 61983 pass in 62141
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 9 debian-hvm-install fail 
in 61983 pass in 62141
 test-amd64-i386-libvirt-raw   9 debian-di-install  fail in 61983 pass in 62141
 test-amd64-i386-xl-qemuu-debianhvm-amd64 12 guest-saverestore fail in 62060 
pass in 61983
 test-amd64-i386-qemuu-rhel6hvm-intel 9 redhat-install fail in 62060 pass in 
62141
 test-amd64-i386-qemuu-rhel6hvm-amd 9 redhat-install fail in 62060 pass in 62141
 test-amd64-i386-qemut-rhel6hvm-intel 9 redhat-install fail in 62060 pass in 
62141
 test-amd64-i386-xl-qemut-debianhvm-amd64 12 guest-saverestore fail in 62060 
pass in 62141
 test-amd64-i386-xl-vhd9 debian-di-install   fail pass in 61983
 test-amd64-i386-xl-qemuu-ovmf-amd64  9 debian-hvm-install   fail pass in 61983
 test-amd64-i386-qemuu-rhel6hvm-amd 10 guest-stopfail pass in 61983
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail pass in 
61983
 test-amd64-i386-libvirt-qcow2  9 debian-di-install  fail pass in 61983
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail 
pass in 61983
 test-amd64-i386-libvirt-vhd   9 debian-di-install   fail pass in 61983
 test-amd64-i386-freebsd10-i386 13 guest-saverestore fail pass in 62060
 test-amd64-i386-xl-qemuu-debianhvm-amd64 9 debian-hvm-install fail pass in 
62060
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1  9 windows-install fail pass in 62060

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-libvirt-raw  9 debian-di-install fail REGR. vs. 60670
 test-amd64-amd64-libvirt-qcow2  9 debian-di-inst

[Xen-devel] [PATCH 2/2] xen: arm: traps: correct cond

[Peng Fan]

>From "G6.2.29 CPSR, Current Program Status Register" of Aarch64 ARM
and "B1.3.3 Program Status Registers (PSRs)" of ARMv7-A ARM:
"
IT[7:5] holds the base condition for the IT block. The base condition is
the top 3 bits of the condition code specified by the first
condition field of the IT instruction.
IT[4:0] encodes the size of the IT block, which is the number of
instructions that are to be conditionally executed, by the
position of the least significant 1 in this field. It also
encodes the value of the least significant bit of the condition
code for each instruction in the block.
"

So should be "cond = ( it >> 5 );" but not "cond = ( it >> 4 );"

Signed-off-by: Peng Fan 
Cc: Ian Campbell 
Cc: Stefano Stabellini 
Cc: Julien Grall 
---
 xen/arch/arm/traps.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 2e2b1f2..b2879b7 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1561,8 +1561,8 @@ static int check_conditional_instr(struct cpu_user_regs 
*regs,
 if ( it == 0 )
 return 1;
 
-/* The cond for this instruction works out as the top 4 bits. */
-cond = ( it >> 4 );
+/* The cond for this instruction works out as the top 3 bits. */
+cond = ( it >> 5 );
 }
 
 cpsr_cond = cpsr >> 28;
-- 
1.8.4



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

[Xen-devel] [PATCH 1/2] xen: arm: traps: check hsr.ec for ARM32

[Peng Fan]

To ARM64, "if ( hsr.ec >= 0x10 ) return 1;" is ok for unconditional
check, but to ARM32, we need to use 'hsr.ec >> 30' to check.

Signed-off-by: Peng Fan 
Cc: Ian Campbell 
Cc: Stefano Stabellini 
Cc: Julien Grall 
---
 xen/arch/arm/traps.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 9d2bd6a..2e2b1f2 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1531,8 +1531,13 @@ static int check_conditional_instr(struct cpu_user_regs 
*regs,
 int cond;
 
 /* Unconditional Exception classes */
+#ifdef CONFIG_ARM_64
 if ( hsr.ec >= 0x10 )
 return 1;
+#else
+if ( hsr.ec >> 30 )
+return 1;
+#endif
 
 /* Check for valid condition in hsr */
 cond = hsr.cond.ccvalid ? hsr.cond.cc : -1;
-- 
1.8.4



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

Re: [Xen-devel] PCI Pass-through in Xen ARM: Draft 4

[Daney, David]

I apologize for the top post, but for the record;

The Thunder Linux implementation will be obtaining the PCI requester id from 
the OF device tree, or ACPI IORT table.  It will *not* be derived from any 
hardware address.

It may make sense to use the same technique to get the PCI requester id in the 
XEN environment too.

David Daney.

From: Julien Grall 
Sent: Saturday, September 19, 2015 1:48:43 PM
To: Jaggi, Manish; Jaggi, Manish; Xen Devel
Cc: Konrad Rzeszutek Wilk; ★ Stefano Stabellini; Ian Campbell; Daney, David; 
prasun.kap...@cavium.com; Kumar, Vijaya
Subject: Re: [Xen-devel] PCI Pass-through in Xen ARM: Draft 4

Hi Manish,

On 19/09/2015 21:24, Manish Jaggi wrote:
> On Wednesday 16 September 2015 06:28 PM, Julien Grall wrote:
>> On 15/09/15 19:58, Jaggi, Manish wrote:
 I can see 2 different solutions:
  1) Let DOM0 pass the first requester ID when registering the bus
 Pros:
  * Less per-platform code in Xen
 Cons:
  * Assume that the requester ID are contiguous. (Is it really a
 cons?)
  * Still require quirk for buggy device (i.e requester ID not
 correct)
  2) Do it in Xen
 Pros:
  * We are not relying on DOM0 giving the requester ID
  => Not assuming contiguous requester ID
  Cons:
  * Per PCI bridge code to handle the mapping

>>>We can have (3) that when PHYSDEVOP_pci_add_device is called the sbdf
>>> and requesterID both are passed in hypercall.
>> The name of the physdev operation is PHYSDEVOP_pci_device_add and not
>> PHYSDEVOP_pci_add_device. Please rename it all the usage in the design
>> doc.
>>
>> Although, we can't modify PHYSDEVOP_pci_device_add because it's part of
>> the ABI which is stable.
>>
>> Based on David's mail, the requester ID of a given device can be found
>> using base + devfn where base is the first requesterID of the bus.
>>
>> IIRC, this is also match the IORT ACPI spec.
>>
>> So for now, I would extend the physdev you've introduced to add an host
>> bridge (PHYSDEV_pci_host_bridge_add) to pass the base requesterID.
> The requester-ID is derived from the Node# and ECAM# as per David. I
> guess the ECAM and Node# can be derived from
> the cfg_addr.

There is no place for "I guess". Any approximation here would lead to
add more hypercalls because we design the first one on speculation.

> Each Ecam has a cfg_addr in Thunder, which is mentioned in the pci node
> in device tree.

IIUC what you said, you suggest to retrieve the ECAM based on the
cfg_addr in Xen, right?

If so, this is the wrong way to go we should avoid to have platform
specific code in Xen as much as possible and get everything either via
the firmware table (ACPI or DT) or via hypercall.

> For thunder I think we don't need to pass requester-ID in the phydevop.

May I remind you that this design doc is not about Thunder-X but PCI
passthrough in general. If your platform already requires specific code
in Xen to get the requester ID, what prevents other to not do the same?

So it looks like to me that adding the base requester-ID in the
PHYSDEVOP_pci_host_bridge_add is the right way to go.

Regards,

--
Julien Grall

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

[Xen-devel] libxen-light

[kumara rathnavel]

Hello,

I do not find any proper documentation regarding API provided by the
Libxenlight. All I find is the documentation for the xl that in turn uses
Libxl Library. But I would like to use the library calls directly, I am
able to basic operations like listing vm, reboot and all those . But when
it comes to creation of domain or attaching device , I am filling the
structures the same way as xl is doing. But I get a Seg Fault. Can you
share some documentation on the Libxenlight API if possible or guidance
would do,,.

Thank you,
Kumar,...
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel