Re: [Xen-devel] [libvirt] Fixing libvirt's libxl driver breakage -- where to define LIBXL_API_VERSION?
>>> On 27.06.16 at 18:54, wrote:
> Jim Fehlig writes ("Re: [libvirt] [Xen-devel] Fixing libvirt's libxl driver
> breakage -- where to define LIBXL_API_VERSION?"):
>> On 06/27/2016 10:12 AM, Ian Jackson wrote:
>> > Does libvirt have stable release branches ? One approach would be to
>> > have osstest track a suitable libvirt stable release branche for each
>> > Xen stable release branch.
>>
>> I see Daniel already answered this question.
>>
>> >
>> > That would involve setting up a push gate for each of the chosen
>> > libvirt stable branches. That would be worthwhile if we expect those
>> > stable branches to acquire commits which break Xen, and which we could
>> > like to be told about. But I'm not sure that's the case.
>>
>> I occasionally backport Xen bug fixes to -maint branches. Cole has
>> also grabbed some Xen bug fixes when making a stable release of a
>> -maint branch. But such backports should be trivial and obvious bug
>> fixes that shouldn't cause build or runtime breakage with Xen.
>
> OK. Thanks for the feedback. I'll go ahead with my plan with the
> git commit ids named in my earlier email.
The only (hopefully highly theoretical) problem I see with this is that
we may end up picking a libvirt commit which subsequently (e.g. via
a libxl backport) turns out to have an issue. Such a problem could be
dealt with in the suggested the stable branch tracking model (or any
other model not dealing with something completely frozen).
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v12 6/6] vt-d: fix vt-d Device-TLB flush timeout issue
On June 27, 2016 11:21 PM, Jan Beulich wrote:
> >>> On 27.06.16 at 14:56, wrote:
> > On June 27, 2016 4:24 PM, Jan Beulich wrote:
> >> >>> On 24.06.16 at 07:51, wrote:
> >> > @@ -199,24 +199,73 @@ static int __must_check
> >> queue_invalidate_wait(struct iommu *iommu,
> >> > return -EOPNOTSUPP;
> >> > }
> >> >
> >> > -static int __must_check invalidate_sync(struct iommu *iommu,
> >> > -bool_t flush_dev_iotlb)
> >> > +static int __must_check invalidate_sync(struct iommu *iommu)
> >> > {
> >> > struct qi_ctrl *qi_ctrl = iommu_qi_ctrl(iommu);
> >> >
> >> > ASSERT(qi_ctrl->qinval_maddr);
> >> >
> >> > -return queue_invalidate_wait(iommu, 0, 1, 1, flush_dev_iotlb);
> >> > +return queue_invalidate_wait(iommu, 0, 1, 1, 0); }
> >> > +
> >> > +static void dev_invalidate_iotlb_timeout(struct iommu *iommu, u16 did,
> >> > + struct pci_dev *pdev) {
> >> > +struct domain *d = NULL;
> >> > +
> >> > +if ( test_bit(did, iommu->domid_bitmap) )
> >> > +d = rcu_lock_domain_by_id(iommu->domid_map[did]);
> >> > +
> >> > +/*
> >> > + * In case the domain has been freed or the IOMMU domid bitmap is
> >> > + * not valid, the device no longer belongs to this domain.
> >> > + */
> >> > +if ( d == NULL )
> >> > +return;
> >> > +
> >> > +pcidevs_lock();
> >> > +ASSERT(pdev->domain);
> >> > +list_del(&pdev->domain_list);
> >> > +pdev->domain = NULL;
> >> > +pci_hide_existing_device(pdev);
> >> > +pcidevs_unlock();
> >> > +
> >> > +if ( !d->is_shutting_down && printk_ratelimit() )
> >> > +printk(XENLOG_WARNING VTDPREFIX
> >> > + " dom%d: ATS device %04x:%02x:%02x.%u flush failed\n",
> >> > + d->domain_id, pdev->seg, pdev->bus,
> >> > + PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
> >> > +
> >> > +if ( !is_hardware_domain(d) )
> >> > +domain_crash(d);
> >> > +
> >> > +rcu_unlock_domain(d);
> >> > +}
> >>
> >> So in an earlier patch in this series you (supposedly) moved similar
> >> logic up to the vendor independent layer. I think this then would
> >> better get moved up too, if at all possible.
> >>
> >
> > To be honest, I have not much reason for leaving domain crash here and
> > I was aware of this problem, but crash_domain() here is not harmful
> > (as the 'd->is_shutting_down' is Set when to crash, and once the 'd-
> >is_shutting_down'
> > is Set then return in domain_shutdown() ).
> > In case crash domain directly, it may help us narrow down the 'window'
> > (the domain is still running)..
> >
> > To me, moving the logic up is acceptable.
> >
> > In next version, could I only drop:
> >
> > +if ( !is_hardware_domain(d) )
> > +domain_crash(d);
> >
> > In this patch, and leave the rest as is ?
>
> Not really - the entire function looks like it could move out of vtd/, as I
> can't
> see anything VT-d specific in it.
>
Yes, it could be out of vtd, and then benefit arm/amd IOMMU to hide ATS device.
But 'did' and 'iommu->domid_bitmap' are really vtd specific. Both of them are
to get domain* structure, not a big deal, and then I can use domain_id instead.
IMO, the domain* structure is a must here,
As mentioned, not all of call trees of device iotlb flush are under
pcidevs_lock, (.i.e ...--iommu_iotlb_flush()-- xenmem_add_to_physmap()... )
In extreme cases, the domain may has been freed or the device may has been
detached or even attached to another domain ( I also need to add 'if
(pdev->domain == d )' before to hide device).
the domain* structure can help us check above cases.
Quan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 05/16] xen/mm: Introduce INVALID_GFN_T and INVALID_MFN_T
>>> On 27.06.16 at 18:54, wrote: > The two new defines will be a typesafe version of resp. INVALID_GFN and > INVALID_MFN. > > Signed-off-by: Julien Grall Ultimately we'll likely want it the other way around naming-wise, but I understand that's far beyond what this series can and should do. Acked-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 08/16] xen: Replace _mfn(INVALID_MFN) with MFN_INVALID_T
>>> On 27.06.16 at 18:54, wrote: > This patch is a mechanical replacement. Command used: > > 42sh> ack -l "_mfn\(INVALID_MFN\)" | xargs sed -i -e > 's/_mfn(INVALID_MFN)/INVALID_MFN_T/g' Well, wait - if you do this, then I'm no longer sure my remark just made on patch 2 holds: If you do such a global replacement, then I think I'd prefer you to switch to the long term final name right away, rather than having to touch all that code again later. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v12 6/6] vt-d: fix vt-d Device-TLB flush timeout issue
>>> On 28.06.16 at 09:06, wrote:
> On June 27, 2016 11:21 PM, Jan Beulich wrote:
>> >>> On 27.06.16 at 14:56, wrote:
>> > On June 27, 2016 4:24 PM, Jan Beulich wrote:
>> >> >>> On 24.06.16 at 07:51, wrote:
>> >> > @@ -199,24 +199,73 @@ static int __must_check
>> >> queue_invalidate_wait(struct iommu *iommu,
>> >> > return -EOPNOTSUPP;
>> >> > }
>> >> >
>> >> > -static int __must_check invalidate_sync(struct iommu *iommu,
>> >> > -bool_t flush_dev_iotlb)
>> >> > +static int __must_check invalidate_sync(struct iommu *iommu)
>> >> > {
>> >> > struct qi_ctrl *qi_ctrl = iommu_qi_ctrl(iommu);
>> >> >
>> >> > ASSERT(qi_ctrl->qinval_maddr);
>> >> >
>> >> > -return queue_invalidate_wait(iommu, 0, 1, 1, flush_dev_iotlb);
>> >> > +return queue_invalidate_wait(iommu, 0, 1, 1, 0); }
>> >> > +
>> >> > +static void dev_invalidate_iotlb_timeout(struct iommu *iommu, u16 did,
>> >> > + struct pci_dev *pdev) {
>> >> > +struct domain *d = NULL;
>> >> > +
>> >> > +if ( test_bit(did, iommu->domid_bitmap) )
>> >> > +d = rcu_lock_domain_by_id(iommu->domid_map[did]);
>> >> > +
>> >> > +/*
>> >> > + * In case the domain has been freed or the IOMMU domid bitmap is
>> >> > + * not valid, the device no longer belongs to this domain.
>> >> > + */
>> >> > +if ( d == NULL )
>> >> > +return;
>> >> > +
>> >> > +pcidevs_lock();
>> >> > +ASSERT(pdev->domain);
>> >> > +list_del(&pdev->domain_list);
>> >> > +pdev->domain = NULL;
>> >> > +pci_hide_existing_device(pdev);
>> >> > +pcidevs_unlock();
>> >> > +
>> >> > +if ( !d->is_shutting_down && printk_ratelimit() )
>> >> > +printk(XENLOG_WARNING VTDPREFIX
>> >> > + " dom%d: ATS device %04x:%02x:%02x.%u flush failed\n",
>> >> > + d->domain_id, pdev->seg, pdev->bus,
>> >> > + PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
>> >> > +
>> >> > +if ( !is_hardware_domain(d) )
>> >> > +domain_crash(d);
>> >> > +
>> >> > +rcu_unlock_domain(d);
>> >> > +}
>> >>
>> >> So in an earlier patch in this series you (supposedly) moved similar
>> >> logic up to the vendor independent layer. I think this then would
>> >> better get moved up too, if at all possible.
>> >>
>> >
>> > To be honest, I have not much reason for leaving domain crash here and
>> > I was aware of this problem, but crash_domain() here is not harmful
>> > (as the 'd->is_shutting_down' is Set when to crash, and once the 'd-
>> >is_shutting_down'
>> > is Set then return in domain_shutdown() ).
>> > In case crash domain directly, it may help us narrow down the 'window'
>> > (the domain is still running)..
>> >
>> > To me, moving the logic up is acceptable.
>> >
>> > In next version, could I only drop:
>> >
>> > +if ( !is_hardware_domain(d) )
>> > +domain_crash(d);
>> >
>> > In this patch, and leave the rest as is ?
>>
>> Not really - the entire function looks like it could move out of vtd/, as I
>> can't
>> see anything VT-d specific in it.
>>
>
> Yes, it could be out of vtd, and then benefit arm/amd IOMMU to hide ATS
> device.
>
> But 'did' and 'iommu->domid_bitmap' are really vtd specific. Both of them
> are
> to get domain* structure, not a big deal, and then I can use domain_id
> instead.
>
> IMO, the domain* structure is a must here,
I agree, I did overlook that domain lookup. The common code
function should be passed a struct domain *, as you suggest.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 05/16] xen/mm: Introduce INVALID_GFN_T and INVALID_MFN_T
On 28/06/2016 08:16, Jan Beulich wrote:
On 27.06.16 at 18:54, wrote:
>> The two new defines will be a typesafe version of resp. INVALID_GFN and
>> INVALID_MFN.
>>
>> Signed-off-by: Julien Grall
> Ultimately we'll likely want it the other way around naming-wise,
> but I understand that's far beyond what this series can and should
> do.
There are plenty of uses of INVALID_{M,G}FN which are not part of
{m,g}fn_t, such as in the hypercall API. I am not sure that it is
realistic to change INVALID_{M,G}FN to be boxed types.
~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v7 2/3] x86/vm_event: Add HVM debug exception vm_events
>>> On 27.06.16 at 20:08, wrote:
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -3376,7 +3376,29 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
> HVMTRACE_1D(TRAP_DEBUG, exit_qualification);
> write_debugreg(6, exit_qualification | DR_STATUS_RESERVED_ONE);
> if ( !v->domain->debugger_attached )
> -vmx_propagate_intr(intr_info);
> +{
> +unsigned long insn_len = 0;
> +int rc;
> +unsigned long trap_type = MASK_EXTR(intr_info,
> +
> INTR_INFO_INTR_TYPE_MASK);
> +
> +if ( trap_type >= X86_EVENTTYPE_SW_INTERRUPT )
> +__vmread(VM_EXIT_INSTRUCTION_LEN, &insn_len);
> +
> +rc = hvm_monitor_debug(regs->eip,
> + HVM_MONITOR_DEBUG_EXCEPTION,
> + trap_type, insn_len);
> +
> +/*
> + * !rccontinue normally
> + * rc > 0 paused waiting for response, work here is done
> + * rc < 0 error in monitor/vm_event, crash
> + */
> +if ( !rc )
> +vmx_propagate_intr(intr_info);
> +if ( rc < 0 )
> +goto exit_and_crash;
> +}
As opposed to earlier versions, here omitting the "else" seems
undesirable. Or, perhaps better, simply re-order the two if()-s.
This is to make clear that what is now the second if() does in no
way depend on what the body of the current first if() does.
The same would then apply to patch 3, and I'd be fine doing the
adjustment while committing (provided all necessary acks trickle
in). Feel free to add my ack here for the few changes for which
that's actually relevant.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [xen-4.3-testing test] 96321: regressions - FAIL
flight 96321 xen-4.3-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/96321/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-i386-libvirt5 libvirt-build fail REGR. vs. 87893 build-amd64-libvirt 5 libvirt-build fail REGR. vs. 87893 build-armhf 5 xen-build fail REGR. vs. 87893 Tests which are failing intermittently (not blocking): test-amd64-i386-xend-qemut-winxpsp3 9 windows-install fail pass in 96279 Regressions which are regarded as allowable (not blocking): test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 87893 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 87893 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail like 87893 Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-rumpuserxen-i386 1 build-check(1) blocked n/a test-armhf-armhf-xl-vhd 1 build-check(1) blocked n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit2 1 build-check(1) blocked n/a test-armhf-armhf-xl-cubietruck 1 build-check(1) blocked n/a test-armhf-armhf-xl-arndale 1 build-check(1) blocked n/a test-armhf-armhf-xl 1 build-check(1) blocked n/a test-amd64-amd64-rumpuserxen-amd64 1 build-check(1) blocked n/a build-armhf-libvirt 1 build-check(1) blocked n/a test-amd64-i386-xend-qemut-winxpsp3 20 leak-check/check fail in 96279 never pass test-amd64-i386-xl-qemuu-ovmf-amd64 9 debian-hvm-install fail never pass test-amd64-amd64-xl-qemuu-ovmf-amd64 9 debian-hvm-install fail never pass build-amd64-rumpuserxen 6 xen-buildfail never pass build-i386-rumpuserxen6 xen-buildfail never pass test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail never pass version targeted for testing: xen 0a8c94fae993dd8f2b27fd4cc694f61c21de84bf baseline version: xen 8fa31952e2d08ef63897c43b5e8b33475ebf5d93 Last test of basis87893 2016-03-29 13:49:52 Z 90 days Failing since 92180 2016-04-20 17:49:21 Z 68 days 36 attempts Testing same since96017 2016-06-20 17:22:27 Z7 days 15 attempts People who touched revisions under test: Andrew Cooper Anthony Liguori Anthony PERARD Gerd Hoffmann Ian Jackson Jan Beulich Jim Paris Stefan Hajnoczi Tim Deegan Wei Liu jobs: build-amd64 pass build-armhf fail build-i386 pass build-amd64-libvirt fail build-armhf-libvirt blocked build-i386-libvirt fail build-amd64-pvopspass build-armhf-pvopspass build-i386-pvops pass build-amd64-rumpuserxen fail build-i386-rumpuserxen fail test-amd64-amd64-xl pass test-armhf-armhf-xl blocked test-amd64-i386-xl pass test-amd64-i386-qemut-rhel6hvm-amd pass test-amd64-i386-qemuu-rhel6hvm-amd pass test-amd64-amd64-xl-qemut-debianhvm-amd64pass test-amd64-i386-xl-qemut-debianhvm-amd64 pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-i386-xl-qemuu-debianhvm-amd64 pass test-amd64-i386-freebsd10-amd64 pass test-amd64-amd64-xl-qemuu-ovmf-amd64 fail test-amd64-i386-xl-qemuu-ovmf-amd64 fail test-amd64-amd64-rumpuserxen-amd64 blocked test-amd64-amd64-xl-qemut-win7-amd64 fail test-amd64-i386-
Re: [Xen-devel] SMAP/SMEP issues with 32-bit pv guests
>>> On 28.06.16 at 03:58, wrote: > As you know, SMAP/SMEP may affect the 32-bit pv guests, after discussed > internally, our current idea is that we can just disable this two feature for > Xen hypervisor itself, hence only enable it for HVM guests. Do you think this > is acceptable from your perspective? I think at most we should go as far as making this an option. That's better than requiring people to turn off SMEP/SMAP completely to gain back performance, and better than forcing people to accept this security wise step backwards without any alternative. And once an option, I think I'd still like to have current behavior remain the default; distros could choose to alter that default with - presumably - a one line patch. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/cpuid: AVX-512 Feature Detection
>>> On 28.06.16 at 07:51, wrote:
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -3474,6 +3474,14 @@ void hvm_cpuid(unsigned int input, unsigned int *eax,
> unsigned int *ebx,
>xstate_sizes[_XSTATE_BNDCSR]);
> }
>
> +if ( _ebx & cpufeat_mask(X86_FEATURE_AVX512F) )
> +{
> +xfeature_mask |= XSTATE_OPMASK | XSTATE_ZMM | XSTATE_HI_ZMM;
> +xstate_size = max(xstate_size,
> + xstate_offsets[_XSTATE_HI_ZMM] +
> + xstate_sizes[_XSTATE_HI_ZMM]);
I think this would better be three such statements, one per bit.
Otherwise the goal of not putting in assumptions on the relative
ordering of bits and save area ranges gets undermined.
> @@ -1136,9 +1136,16 @@ void pv_cpuid(struct cpu_user_regs *regs)
> case XSTATE_CPUID:
>
> if ( !is_control_domain(currd) && !is_hardware_domain(currd) )
> +{
> domain_cpuid(currd, 1, 0, &tmp, &tmp, &_ecx, &tmp);
> +domain_cpuid(currd, 0x07, 0, &tmp, &_ebx, &tmp, &tmp);
The neighboring line tells you that this should be 7 instead of 0x07.
> --- a/xen/include/public/arch-x86/cpufeatureset.h
> +++ b/xen/include/public/arch-x86/cpufeatureset.h
> @@ -206,15 +206,24 @@ XEN_CPUFEATURE(PQM, 5*32+12) /* Platform
> QoS Monitoring */
> XEN_CPUFEATURE(NO_FPU_SEL,5*32+13) /*! FPU CS/DS stored as zero */
> XEN_CPUFEATURE(MPX, 5*32+14) /*S Memory Protection Extensions */
> XEN_CPUFEATURE(PQE, 5*32+15) /* Platform QoS Enforcement */
> +XEN_CPUFEATURE(AVX512F, 5*32+16) /*A AVX-512 Foundation Instructions
> */
> +XEN_CPUFEATURE(AVX512DQ, 5*32+17) /*A AVX-512 Doubleword & Quadword
> Instrs */
> XEN_CPUFEATURE(RDSEED,5*32+18) /*A RDSEED instruction */
> XEN_CPUFEATURE(ADX, 5*32+19) /*A ADCX, ADOX instructions */
> XEN_CPUFEATURE(SMAP, 5*32+20) /*S Supervisor Mode Access
> Prevention */
> +XEN_CPUFEATURE(AVX512IFMA,5*32+21) /*A AVX-512 Integer Fused Multiply
> Add */
> XEN_CPUFEATURE(CLFLUSHOPT,5*32+23) /*A CLFLUSHOPT instruction */
> XEN_CPUFEATURE(CLWB, 5*32+24) /*A CLWB instruction */
> +XEN_CPUFEATURE(AVX512PF, 5*32+26) /*A AVX-512 Prefetch Instructions */
> +XEN_CPUFEATURE(AVX512ER, 5*32+27) /*A AVX-512 Exponent & Reciprocal
> Instrs */
> +XEN_CPUFEATURE(AVX512CD, 5*32+28) /*A AVX-512 Conflict Detection
> Instrs */
> XEN_CPUFEATURE(SHA, 5*32+29) /*A SHA1 & SHA256 instructions */
> +XEN_CPUFEATURE(AVX512BW, 5*32+30) /*A AVX-512 Byte and Word
> Instructions */
> +XEN_CPUFEATURE(AVX512VL, 5*32+31) /*A AVX-512 Vector Length Extensions
> */
>
> /* Intel-defined CPU features, CPUID level 0x0007:0.ecx, word 6 */
> XEN_CPUFEATURE(PREFETCHWT1, 6*32+ 0) /*A PREFETCHWT1 instruction */
> +XEN_CPUFEATURE(AVX512VBMI,6*32+ 1) /*A AVX-512 Vector Byte Manipulation
> Instrs */
> XEN_CPUFEATURE(PKU, 6*32+ 3) /*H Protection Keys for Userspace */
> XEN_CPUFEATURE(OSPKE, 6*32+ 4) /*! OS Protection Keys Enable */
This lacks an adjustment to the dependencies between features in
xen/tools/gen-cpuid.py.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 05/16] xen/mm: Introduce INVALID_GFN_T and INVALID_MFN_T
>>> On 28.06.16 at 09:29, wrote:
> On 28/06/2016 08:16, Jan Beulich wrote:
> On 27.06.16 at 18:54, wrote:
>>> The two new defines will be a typesafe version of resp. INVALID_GFN and
>>> INVALID_MFN.
>>>
>>> Signed-off-by: Julien Grall
>> Ultimately we'll likely want it the other way around naming-wise,
>> but I understand that's far beyond what this series can and should
>> do.
>
> There are plenty of uses of INVALID_{M,G}FN which are not part of
> {m,g}fn_t, such as in the hypercall API. I am not sure that it is
> realistic to change INVALID_{M,G}FN to be boxed types.
I can't spot any such use in the public interface. And I also can't
see anything wrong with perhaps a few instances of e.g.
mfn_x(INVALID_MFN) remaining long term.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] Crash in xen 4.7 adding nic during domu startup
>>> On 28.06.16 at 00:28, wrote: > (XEN) [2016-06-27 22:11:41.712] [ Xen-4.7.0 x86_64 debug=n Not tainted > ] > (XEN) [2016-06-27 22:11:41.712] CPU:0 > (XEN) [2016-06-27 22:11:41.712] RIP:e008:[] > domain_relinquish_resources+0x10/0x2f0 > (XEN) [2016-06-27 22:11:41.712] RFLAGS: 00010296 CONTEXT: > hypervisor (d0v0) > (XEN) [2016-06-27 22:11:41.712] rax: rbx: 83020ac4f000 > rcx: 82d0802346bc > (XEN) [2016-06-27 22:11:41.712] rdx: 8300bf8d7fff rsi: > rdi: 83020afde050 > (XEN) [2016-06-27 22:11:41.712] rbp: 8300bf8d7e48 rsp: 8300bf8d7d78 > r8: 0001 > (XEN) [2016-06-27 22:11:41.712] r9: r10: 0003 > r11: 83023d94b1d8 > (XEN) [2016-06-27 22:11:41.712] r12: 7ffe21eb9530 r13: > r14: 83020ac4f000 > (XEN) [2016-06-27 22:11:41.712] r15: 7ffe21eb9530 cr0: 80050033 > cr4: 26e0 > (XEN) [2016-06-27 22:11:41.712] cr3: 000216737000 cr2: > (XEN) [2016-06-27 22:11:41.712] ds: es: fs: gs: > ss: e010 cs: e008 > (XEN) [2016-06-27 22:11:41.712] Xen code around > (domain_relinquish_resources+0x10/0x2f0): > (XEN) [2016-06-27 22:11:41.712] 18 48 8b bf e8 01 00 00 <00> 00 00 00 00 00 > 00 00 00 00 00 00 00 00 00 00 Something very clearly corrupted your hypervisor code, and that's what you'll need to track down. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/cpuid: AVX-512 Feature Detection
Thanks for your advice, I will make a change right now.
-Original Message-
From: Jan Beulich [mailto:jbeul...@suse.com]
Sent: Tuesday, June 28, 2016 3:49 PM
To: Kang, Luwei
Cc: andrew.coop...@citrix.com; Peng, Chao P ; Wang, Yong
Y ; xen-devel@lists.xen.org
Subject: Re: [PATCH] x86/cpuid: AVX-512 Feature Detection
>>> On 28.06.16 at 07:51, wrote:
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -3474,6 +3474,14 @@ void hvm_cpuid(unsigned int input, unsigned int *eax,
> unsigned int *ebx,
>xstate_sizes[_XSTATE_BNDCSR]);
> }
>
> +if ( _ebx & cpufeat_mask(X86_FEATURE_AVX512F) )
> +{
> +xfeature_mask |= XSTATE_OPMASK | XSTATE_ZMM | XSTATE_HI_ZMM;
> +xstate_size = max(xstate_size,
> + xstate_offsets[_XSTATE_HI_ZMM] +
> + xstate_sizes[_XSTATE_HI_ZMM]);
I think this would better be three such statements, one per bit.
Otherwise the goal of not putting in assumptions on the relative ordering of
bits and save area ranges gets undermined.
> @@ -1136,9 +1136,16 @@ void pv_cpuid(struct cpu_user_regs *regs)
> case XSTATE_CPUID:
>
> if ( !is_control_domain(currd) && !is_hardware_domain(currd)
> )
> +{
> domain_cpuid(currd, 1, 0, &tmp, &tmp, &_ecx, &tmp);
> +domain_cpuid(currd, 0x07, 0, &tmp, &_ebx, &tmp, &tmp);
The neighboring line tells you that this should be 7 instead of 0x07.
> --- a/xen/include/public/arch-x86/cpufeatureset.h
> +++ b/xen/include/public/arch-x86/cpufeatureset.h
> @@ -206,15 +206,24 @@ XEN_CPUFEATURE(PQM, 5*32+12) /* Platform
> QoS Monitoring */
> XEN_CPUFEATURE(NO_FPU_SEL,5*32+13) /*! FPU CS/DS stored as zero */
> XEN_CPUFEATURE(MPX, 5*32+14) /*S Memory Protection Extensions */
> XEN_CPUFEATURE(PQE, 5*32+15) /* Platform QoS Enforcement */
> +XEN_CPUFEATURE(AVX512F, 5*32+16) /*A AVX-512 Foundation Instructions
> */
> +XEN_CPUFEATURE(AVX512DQ, 5*32+17) /*A AVX-512 Doubleword & Quadword
> Instrs */
> XEN_CPUFEATURE(RDSEED,5*32+18) /*A RDSEED instruction */
> XEN_CPUFEATURE(ADX, 5*32+19) /*A ADCX, ADOX instructions */
> XEN_CPUFEATURE(SMAP, 5*32+20) /*S Supervisor Mode Access
> Prevention */
> +XEN_CPUFEATURE(AVX512IFMA,5*32+21) /*A AVX-512 Integer Fused Multiply
> Add */
> XEN_CPUFEATURE(CLFLUSHOPT,5*32+23) /*A CLFLUSHOPT instruction */
> XEN_CPUFEATURE(CLWB, 5*32+24) /*A CLWB instruction */
> +XEN_CPUFEATURE(AVX512PF, 5*32+26) /*A AVX-512 Prefetch Instructions */
> +XEN_CPUFEATURE(AVX512ER, 5*32+27) /*A AVX-512 Exponent & Reciprocal
> Instrs */
> +XEN_CPUFEATURE(AVX512CD, 5*32+28) /*A AVX-512 Conflict Detection
> Instrs */
> XEN_CPUFEATURE(SHA, 5*32+29) /*A SHA1 & SHA256 instructions */
> +XEN_CPUFEATURE(AVX512BW, 5*32+30) /*A AVX-512 Byte and Word
> Instructions */
> +XEN_CPUFEATURE(AVX512VL, 5*32+31) /*A AVX-512 Vector Length Extensions
> */
>
> /* Intel-defined CPU features, CPUID level 0x0007:0.ecx, word 6 */
> XEN_CPUFEATURE(PREFETCHWT1, 6*32+ 0) /*A PREFETCHWT1 instruction */
> +XEN_CPUFEATURE(AVX512VBMI,6*32+ 1) /*A AVX-512 Vector Byte Manipulation
> Instrs */
> XEN_CPUFEATURE(PKU, 6*32+ 3) /*H Protection Keys for Userspace */
> XEN_CPUFEATURE(OSPKE, 6*32+ 4) /*! OS Protection Keys Enable */
This lacks an adjustment to the dependencies between features in
xen/tools/gen-cpuid.py.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2] xen: x86: remove duplicated IA32_FEATURE_CONTROL MSR macro
From: Kai Huang
Below commit introduced a new macro MSR_IA32_FEATURE_CONTROL for
IA32_FEATURE_CONTROL MSR but it didn't remove old IA32_FEATURE_CONTROL_MSR
macro. The new one has better naming convention, so remove the old as a
duplication. Also move the macros of bit definition of IA32_FEATURE_CONTROL MSR
down to make them together with the new one. The *_MSR* infix is also removed as
it is pointless.
commit 5a211704e8813c4890c8ce8dc4189d1dfb35ecd0
Author: Len Brown
Date: Fri Apr 8 22:31:47 2016 +0200
mwait-idle: prevent SKL-H boot failure when C8+C9+C10 enabled
Some SKL-H configurations require "max_cstate=7" to boot.
While that is an effective workaround, it disables C10.
..
Above commit also used SGX_ENABLE (bit 18) in IA32_FEATURE_CONTROL MSR without a
macro for it. A new macro IA32_FEATURE_CONTROL_SGX_ENABLE is also added for
better code and future use.
Relevant code that uses those macros are changed accordingly.
Signed-off-by: Kai Huang
---
v1 -> v2:
Moved the macros for bit definition of IA32_FEATURE_CONTROL MSR down to make
them together with the MSR macro. Removed the *_MSR* infix. Also refined commit
message.
---
xen/arch/x86/cpu/mwait-idle.c | 2 +-
xen/arch/x86/hvm/vmx/vmcs.c | 20 ++--
xen/arch/x86/hvm/vmx/vmx.c | 4 ++--
xen/arch/x86/hvm/vmx/vvmx.c | 6 +++---
xen/include/asm-x86/msr-index.h | 14 --
5 files changed, 24 insertions(+), 22 deletions(-)
diff --git a/xen/arch/x86/cpu/mwait-idle.c b/xen/arch/x86/cpu/mwait-idle.c
index e062e21..4b33974 100644
--- a/xen/arch/x86/cpu/mwait-idle.c
+++ b/xen/arch/x86/cpu/mwait-idle.c
@@ -1006,7 +1006,7 @@ static void __init sklh_idle_state_table_update(void)
rdmsrl(MSR_IA32_FEATURE_CONTROL, msr);
/* if SGX is enabled */
- if (msr & (1 << 18))
+ if (msr & IA32_FEATURE_CONTROL_SGX_ENABLE)
return;
}
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 848ac33..46b63b6 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -604,14 +604,14 @@ int vmx_cpu_up(void)
return -EINVAL;
}
-rdmsr(IA32_FEATURE_CONTROL_MSR, eax, edx);
+rdmsr(MSR_IA32_FEATURE_CONTROL, eax, edx);
-bios_locked = !!(eax & IA32_FEATURE_CONTROL_MSR_LOCK);
+bios_locked = !!(eax & IA32_FEATURE_CONTROL_LOCK);
if ( bios_locked )
{
if ( !(eax & (tboot_in_measured_env()
- ? IA32_FEATURE_CONTROL_MSR_ENABLE_VMXON_INSIDE_SMX
- : IA32_FEATURE_CONTROL_MSR_ENABLE_VMXON_OUTSIDE_SMX)) )
+ ? IA32_FEATURE_CONTROL_ENABLE_VMXON_INSIDE_SMX
+ : IA32_FEATURE_CONTROL_ENABLE_VMXON_OUTSIDE_SMX)) )
{
printk("CPU%d: VMX disabled by BIOS.\n", cpu);
return -EINVAL;
@@ -619,11 +619,11 @@ int vmx_cpu_up(void)
}
else
{
-eax = IA32_FEATURE_CONTROL_MSR_LOCK;
-eax |= IA32_FEATURE_CONTROL_MSR_ENABLE_VMXON_OUTSIDE_SMX;
+eax = IA32_FEATURE_CONTROL_LOCK;
+eax |= IA32_FEATURE_CONTROL_ENABLE_VMXON_OUTSIDE_SMX;
if ( test_bit(X86_FEATURE_SMX, &boot_cpu_data.x86_capability) )
-eax |= IA32_FEATURE_CONTROL_MSR_ENABLE_VMXON_INSIDE_SMX;
-wrmsr(IA32_FEATURE_CONTROL_MSR, eax, 0);
+eax |= IA32_FEATURE_CONTROL_ENABLE_VMXON_INSIDE_SMX;
+wrmsr(MSR_IA32_FEATURE_CONTROL, eax, 0);
}
if ( (rc = vmx_init_vmcs_config()) != 0 )
@@ -639,8 +639,8 @@ int vmx_cpu_up(void)
case -2: /* #UD or #GP */
if ( bios_locked &&
test_bit(X86_FEATURE_SMX, &boot_cpu_data.x86_capability) &&
- (!(eax & IA32_FEATURE_CONTROL_MSR_ENABLE_VMXON_OUTSIDE_SMX) ||
- !(eax & IA32_FEATURE_CONTROL_MSR_ENABLE_VMXON_INSIDE_SMX)) )
+ (!(eax & IA32_FEATURE_CONTROL_ENABLE_VMXON_OUTSIDE_SMX) ||
+ !(eax & IA32_FEATURE_CONTROL_ENABLE_VMXON_INSIDE_SMX)) )
{
printk("CPU%d: VMXON failed: perhaps because of TXT settings "
"in your BIOS configuration?\n", cpu);
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 54cdb86..c23b1e9 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2622,7 +2622,7 @@ static int vmx_msr_read_intercept(unsigned int msr,
uint64_t *msr_content)
case MSR_IA32_DEBUGCTLMSR:
__vmread(GUEST_IA32_DEBUGCTL, msr_content);
break;
-case IA32_FEATURE_CONTROL_MSR:
+case MSR_IA32_FEATURE_CONTROL:
case MSR_IA32_VMX_BASIC...MSR_IA32_VMX_VMFUNC:
if ( !nvmx_msr_read_intercept(msr, msr_content) )
goto gp_fault;
@@ -2848,7 +2848,7 @@ static int vmx_msr_write_intercept(unsigned int msr,
uint64_t msr_content)
break;
}
-case IA32_FEATURE_CONTROL_MSR:
+case MSR_IA32_FEATURE_CONTROL:
case MSR_IA32_VMX_BASIC...MSR_IA32_VMX_TRUE_ENTRY_CTLS
Re: [Xen-devel] [PATCH v2] xen: x86: remove duplicated IA32_FEATURE_CONTROL MSR macro
>>> On 28.06.16 at 10:12, wrote: > From: Kai Huang On the 24th I had asked you privately to please follow Xen patch submission rules: Patches get sent _to_ the list, and maintainers get _cc_-ed. People receiving mails may have rules in place in their mail systems to pre-sort incoming traffic accordingly. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/cpuid: AVX-512 Feature Detection
On 28/06/16 06:51, Luwei Kang wrote:
> @@ -1136,9 +1136,16 @@ void pv_cpuid(struct cpu_user_regs *regs)
> case XSTATE_CPUID:
>
> if ( !is_control_domain(currd) && !is_hardware_domain(currd) )
> +{
> domain_cpuid(currd, 1, 0, &tmp, &tmp, &_ecx, &tmp);
> +domain_cpuid(currd, 0x07, 0, &tmp, &_ebx, &tmp, &tmp);
> +}
> else
> +{
> _ecx = cpuid_ecx(1);
> +cpuid_count(0x07, 0, &tmp, &_ebx, &tmp, &tmp);
> +}
> +
In addition to Jan's comments, having _ecx from one leaf and _ebx from a
different leaf collected at the same time is liable to cause confusion.
Please split the cpuid call for leaf 7 out from here, and put it in the
next hunk, just like the way the hvm_cpuid() side works.
~Andrew
> _ecx &= pv_featureset[FEATURESET_1c];
>
> if ( !(_ecx & cpufeat_mask(X86_FEATURE_XSAVE)) || subleaf >= 63 )
> @@ -1157,6 +1164,14 @@ void pv_cpuid(struct cpu_user_regs *regs)
> xstate_sizes[_XSTATE_YMM]);
> }
>
> +if ( _ebx & cpufeat_mask(X86_FEATURE_AVX512F) )
> +{
> +xfeature_mask |= XSTATE_OPMASK | XSTATE_ZMM | XSTATE_HI_ZMM;
> +xstate_size = max(xstate_size,
> + xstate_offsets[_XSTATE_HI_ZMM] +
> + xstate_sizes[_XSTATE_HI_ZMM]);
> +}
> +
> a = (uint32_t)xfeature_mask;
> d = (uint32_t)(xfeature_mask >> 32);
> c = xstate_size;
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/cpuid: AVX-512 Feature Detection
OK, no problem.
-Original Message-
From: Andrew Cooper [mailto:andrew.coop...@citrix.com]
Sent: Tuesday, June 28, 2016 4:47 PM
To: Kang, Luwei ; xen-devel@lists.xen.org
Cc: jbeul...@suse.com; Wang, Yong Y ; Peng, Chao P
Subject: Re: [PATCH] x86/cpuid: AVX-512 Feature Detection
On 28/06/16 06:51, Luwei Kang wrote:
> @@ -1136,9 +1136,16 @@ void pv_cpuid(struct cpu_user_regs *regs)
> case XSTATE_CPUID:
>
> if ( !is_control_domain(currd) && !is_hardware_domain(currd)
> )
> +{
> domain_cpuid(currd, 1, 0, &tmp, &tmp, &_ecx, &tmp);
> +domain_cpuid(currd, 0x07, 0, &tmp, &_ebx, &tmp, &tmp);
> +}
> else
> +{
> _ecx = cpuid_ecx(1);
> +cpuid_count(0x07, 0, &tmp, &_ebx, &tmp, &tmp);
> +}
> +
In addition to Jan's comments, having _ecx from one leaf and _ebx from a
different leaf collected at the same time is liable to cause confusion.
Please split the cpuid call for leaf 7 out from here, and put it in the next
hunk, just like the way the hvm_cpuid() side works.
~Andrew
> _ecx &= pv_featureset[FEATURESET_1c];
>
> if ( !(_ecx & cpufeat_mask(X86_FEATURE_XSAVE)) || subleaf >=
> 63 ) @@ -1157,6 +1164,14 @@ void pv_cpuid(struct cpu_user_regs *regs)
> xstate_sizes[_XSTATE_YMM]);
> }
>
> +if ( _ebx & cpufeat_mask(X86_FEATURE_AVX512F) )
> +{
> +xfeature_mask |= XSTATE_OPMASK | XSTATE_ZMM | XSTATE_HI_ZMM;
> +xstate_size = max(xstate_size,
> + xstate_offsets[_XSTATE_HI_ZMM] +
> + xstate_sizes[_XSTATE_HI_ZMM]);
> +}
> +
> a = (uint32_t)xfeature_mask;
> d = (uint32_t)(xfeature_mask >> 32);
> c = xstate_size;
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] xen: x86: remove duplicated IA32_FEATURE_CONTROL MSR macro
On 6/28/2016 8:37 PM, Jan Beulich wrote: On 28.06.16 at 10:12, wrote: From: Kai Huang On the 24th I had asked you privately to please follow Xen patch submission rules: Patches get sent _to_ the list, and maintainers get _cc_-ed. People receiving mails may have rules in place in their mail systems to pre-sort incoming traffic accordingly. Oh sorry. I checked my mailbox but looks I couldn't find your email. Maybe something wrong happened. Will follow this rule in the future. Thanks for reminding. Thanks, -Kai Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [ovmf test] 96322: regressions - FAIL
flight 96322 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/96322/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-qemuu-ovmf-amd64 17 guest-start/debianhvm.repeat fail REGR. vs. 94748 test-amd64-amd64-xl-qemuu-ovmf-amd64 17 guest-start/debianhvm.repeat fail REGR. vs. 94748 version targeted for testing: ovmf 6b5677e1bb62c289fba7848bbfde08220fc37ba1 baseline version: ovmf dc99315b8732b6e3032d01319d3f534d440b43d0 Last test of basis94748 2016-05-24 22:43:25 Z 34 days Failing since 94750 2016-05-25 03:43:08 Z 34 days 60 attempts Testing same since96322 2016-06-27 17:14:45 Z0 days1 attempts People who touched revisions under test: Ard Biesheuvel Chao Zhang Cinnamon Shia Cohen, Eugene Dandan Bi Darbin Reyes Eric Dong Eugene Cohen Evan Lloyd Fu Siyuan Fu, Siyuan Gary Li Gary Lin Giri P Mudusuru Hao Wu Hegde Nagaraj P hegdenag Heyi Guo Jan D?bro? Jan Dabros Jeff Fan Jiaxin Wu Jiewen Yao Joe Zhou Katie Dellaquila Laszlo Ersek Liming Gao Lu, ShifeiX A lushifex Marcin Wojtas Marvin H?user Marvin Haeuser Maurice Ma Michael Zimmermann Qiu Shumin Ruiyu Ni Ryan Harkin Sami Mujawar Satya Yarlagadda Sriram Subramanian Star Zeng Sunny Wang Tapan Shah Thomas Palmer Yarlagadda, Satya P Yonghong Zhu Zhang Lubo Zhang, Chao B jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 fail test-amd64-i386-xl-qemuu-ovmf-amd64 fail sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Not pushing. (No revision log; it would be 3067 lines long.) ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v1 Altp2m cleanup 2/3] Move altp2m specific functions to altp2m files.
>>> On 21.06.16 at 18:04, wrote:
> @@ -65,6 +66,48 @@ altp2m_vcpu_destroy(struct vcpu *v)
> vcpu_unpause(v);
> }
>
> +int
> +hvm_altp2m_init( struct domain *d) {
Coding style (stray blank and misplaced brace).
> +int rv = 0;
I guess rc or ret would be the more conventional names.
> +unsigned int i = 0;
Pointless initializer.
> +/* Init alternate p2m data */
Missing full stop.
> --- a/xen/arch/x86/mm/p2m-ept.c
> +++ b/xen/arch/x86/mm/p2m-ept.c
> @@ -1329,6 +1329,44 @@ void setup_ept_dump(void)
> register_keyhandler('D', ept_dump_p2m_table, "dump VT-x EPT tables", 0);
> }
>
> +void p2m_init_altp2m_helper( struct domain *d, unsigned int i) {
While moving it here, please adjust the name to make clear this
is EPT specific. Also it looks like the first parameter could become
const.
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -196,8 +196,8 @@ static void p2m_teardown_altp2m(struct domain *d)
> if ( !d->arch.altp2m_p2m[i] )
> continue;
> p2m = d->arch.altp2m_p2m[i];
> -d->arch.altp2m_p2m[i] = NULL;
> p2m_free_one(p2m);
> +d->arch.altp2m_p2m[i] = NULL;
> }
Why, without any other changes (you're only moving code around)?
> @@ -37,5 +36,7 @@ static inline uint16_t altp2m_vcpu_idx(const struct vcpu *v)
> {
> return vcpu_altp2m(v).p2midx;
> }
> +int hvm_altp2m_init(struct domain *d);
> +void hvm_altp2m_teardown(struct domain *d);
Missing separating blank line.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V3 09/10] xen/arm: io: Use binary search for mmio handler lookup
Hi Shanker,
On 27/06/16 21:33, Shanker Donthineni wrote:
> As the number of I/O handlers increase, the overhead associated with
> linear lookup also increases. The system might have maximum of 144
> (assuming CONFIG_NR_CPUS=128) mmio handlers. In worst case scenario,
> it would require 144 iterations for finding a matching handler. Now
> it is time for us to change from linear (complexity O(n)) to a binary
> search (complexity O(log n) for reducing mmio handler lookup overhead.
However, you will add contention because the code is using a spinlock.
I am planning to send the following patch as a prerequisite of this series
to switch from spinlock to read-write lock:
commit b69e975ce25b2c94f7205b0b8329f351327fbcf7
Author: Julien Grall
Date: Tue Jun 28 11:04:11 2016 +0100
xen/arm: io: Protect the handlers with a read-write lock
Currently, accessing the I/O handlers does not require to take a lock
because new handlers are always added at the end of the array. In a
follow-up patch, this array will be sort to optimize the look up.
Given that most of the time the I/O handlers will not be modify,
using a spinlock will add contention when multiple vCPU are accessing
the emulated MMIOs. So use a read-write lock to protected the handlers.
Finally, take the opportunity to re-indent correctly domain_io_init.
Signed-off-by: Julien Grall
diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
index 0156755..5a96836 100644
--- a/xen/arch/arm/io.c
+++ b/xen/arch/arm/io.c
@@ -70,23 +70,39 @@ static int handle_write(const struct mmio_handler *handler,
struct vcpu *v,
handler->priv);
}
-int handle_mmio(mmio_info_t *info)
+static const struct mmio_handler *find_mmio_handler(struct domain *d,
+paddr_t gpa)
{
-struct vcpu *v = current;
-int i;
-const struct mmio_handler *handler = NULL;
-const struct vmmio *vmmio = &v->domain->arch.vmmio;
+const struct mmio_handler *handler;
+unsigned int i;
+struct vmmio *vmmio = &d->arch.vmmio;
+
+read_lock(&vmmio->lock);
for ( i = 0; i < vmmio->num_entries; i++ )
{
handler = &vmmio->handlers[i];
-if ( (info->gpa >= handler->addr) &&
- (info->gpa < (handler->addr + handler->size)) )
+if ( (gpa >= handler->addr) &&
+ (gpa < (handler->addr + handler->size)) )
break;
}
if ( i == vmmio->num_entries )
+handler = NULL;
+
+read_unlock(&vmmio->lock);
+
+return handler;
+}
+
+int handle_mmio(mmio_info_t *info)
+{
+struct vcpu *v = current;
+const struct mmio_handler *handler = NULL;
+
+handler = find_mmio_handler(v->domain, info->gpa);
+if ( !handler )
return 0;
if ( info->dabt.write )
@@ -104,7 +120,7 @@ void register_mmio_handler(struct domain *d,
BUG_ON(vmmio->num_entries >= MAX_IO_HANDLER);
-spin_lock(&vmmio->lock);
+write_lock(&vmmio->lock);
handler = &vmmio->handlers[vmmio->num_entries];
@@ -113,24 +129,17 @@ void register_mmio_handler(struct domain *d,
handler->size = size;
handler->priv = priv;
-/*
- * handle_mmio is not using the lock to avoid contention.
- * Make sure the other processors see the new handler before
- * updating the number of entries
- */
-dsb(ish);
-
vmmio->num_entries++;
-spin_unlock(&vmmio->lock);
+write_unlock(&vmmio->lock);
}
int domain_io_init(struct domain *d)
{
- spin_lock_init(&d->arch.vmmio.lock);
- d->arch.vmmio.num_entries = 0;
+rwlock_init(&d->arch.vmmio.lock);
+d->arch.vmmio.num_entries = 0;
- return 0;
+return 0;
}
/*
diff --git a/xen/include/asm-arm/mmio.h b/xen/include/asm-arm/mmio.h
index da1cc2e..32f10f2 100644
--- a/xen/include/asm-arm/mmio.h
+++ b/xen/include/asm-arm/mmio.h
@@ -20,6 +20,7 @@
#define __ASM_ARM_MMIO_H__
#include
+#include
#include
#include
@@ -51,7 +52,7 @@ struct mmio_handler {
struct vmmio {
int num_entries;
-spinlock_t lock;
+rwlock_t lock;
struct mmio_handler handlers[MAX_IO_HANDLER];
};
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v1 Altp2m cleanup 3/3] Making altp2m struct dynamically allocated.
>>> On 21.06.16 at 18:04, wrote:
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -5228,7 +5228,7 @@ static int do_altp2m_op(
>
> if ( (a.cmd != HVMOP_altp2m_get_domain_state) &&
> (a.cmd != HVMOP_altp2m_set_domain_state) &&
> - !d->arch.altp2m_active )
> + ! altp2m_active(d) )
Stray blank.
> @@ -5262,11 +5262,11 @@ static int do_altp2m_op(
> break;
> }
>
> -ostate = d->arch.altp2m_active;
> -d->arch.altp2m_active = !!a.u.domain_state.state;
> +ostate = altp2m_active(d);
> + set_altp2m_active(d, !!a.u.domain_state.state);
Bogus tab indentation.
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -193,12 +193,15 @@ static void p2m_teardown_altp2m(struct domain *d)
>
> for ( i = 0; i < MAX_ALTP2M; i++ )
> {
> -if ( !d->arch.altp2m_p2m[i] )
> +if ( !d->arch.altp2m->altp2m_p2m[i] )
> continue;
> -p2m = d->arch.altp2m_p2m[i];
> +p2m = d->arch.altp2m->altp2m_p2m[i];
> p2m_free_one(p2m);
> -d->arch.altp2m_p2m[i] = NULL;
> +d->arch.altp2m->altp2m_p2m[i] = NULL;
> }
> +
> +if (d->arch.altp2m)
Missing blanks.
> +xfree(d->arch.altp2m);
But the conditional is pointless anyway.
> @@ -206,10 +209,12 @@ static int p2m_init_altp2m(struct domain *d)
> unsigned int i;
> struct p2m_domain *p2m;
>
> -mm_lock_init(&d->arch.altp2m_list_lock);
> +d->arch.altp2m = xzalloc(struct altp2m_domain);
> +
> +mm_lock_init(&d->arch.altp2m->altp2m_list_lock);
Missing error check.
> --- a/xen/include/asm-x86/domain.h
> +++ b/xen/include/asm-x86/domain.h
> @@ -274,6 +274,13 @@ struct monitor_write_data {
> uint64_t cr4;
> };
>
> +struct altp2m_domain {
> +bool_t altp2m_active;
> +struct p2m_domain *altp2m_p2m[MAX_ALTP2M];
> +mm_lock_t altp2m_list_lock;
> +uint64_t *altp2m_eptp;
> +};
No point prefixing all the fields with altp2m_. And also the structure
now doesn't belong here anymore - it should move to e.g. p2m.h.
> @@ -320,10 +327,13 @@ struct arch_domain
> mm_lock_t nested_p2m_lock;
>
> /* altp2m: allow multiple copies of host p2m */
> +/*
> bool_t altp2m_active;
> struct p2m_domain *altp2m_p2m[MAX_ALTP2M];
> mm_lock_t altp2m_list_lock;
> -uint64_t *altp2m_eptp;
> +uint64_t *altp2m_eptp;
> +*/
What's the purpose of this comment?
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4] xen: arm: Update arm64 image header
Hi Dirk, On 27/06/16 08:53, Dirk Behme wrote: With the Linux kernel commits https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/Documentation/arm64/booting.txt?id=4370eec05a887b0cd4392cd5dc5b2713174745c0 https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/Documentation/arm64/booting.txt?id=a2c1d73b94ed49f5fac12e95052d7b140783f800 the arm64 image header changed. While the size of the header isn't changed, some members have changed their usage. Update Xen to this updated image header. The main changes are that the first magic is gone and that there is an image size, now. In case we read a size != 0, let's use this image size, now. This does allow us to check if the kernel Image is larger than the size given in the device tree, too. Additionally, add an error message if the magic is not found. This might be the case with kernel's < 3.12 prior to https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4370eec05a887b0cd4392cd5dc5b2713174745c0 which introduced the second magic. This is acceptable as the support of Xen for ARM64 in Linux has been added in Linux 3.11 and the number of boards supported by Linux 3.11 on ARM64 is very limited: ARM models and X-gene. And for the latter it was an early support with only the serial and timer upstreamed. Signed-off-by: Dirk Behme 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] [PATCH V3 01/10] arm/gic-v3: Use acpi_table_parse_madt() to parse MADT subtables
Hi Shanker,
On 27/06/16 21:33, Shanker Donthineni wrote:
The function acpi_table_parse_madt() does the same functionality as
function acpi_parse_entries() expect it takes a few arguments.
Signed-off-by: Shanker Donthineni
Reviewed-by: Julien Grall
Regards,
---
xen/arch/arm/gic-v3.c | 27 ++-
1 file changed, 6 insertions(+), 21 deletions(-)
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 8d3f149..166f1c1 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1390,28 +1390,15 @@ gic_acpi_get_madt_redistributor_num(struct
acpi_subtable_header *header,
static void __init gicv3_acpi_init(void)
{
-struct acpi_table_header *table;
struct rdist_region *rdist_regs;
-acpi_status status;
int count, i;
-status = acpi_get_table(ACPI_SIG_MADT, 0, &table);
-
-if ( ACPI_FAILURE(status) )
-{
-const char *msg = acpi_format_exception(status);
-
-panic("GICv3: Failed to get MADT table, %s", msg);
-}
-
/*
* Find distributor base address. We expect one distributor entry since
* ACPI 5.0 spec neither support multi-GIC instances nor GIC cascade.
*/
-count = acpi_parse_entries(ACPI_SIG_MADT, sizeof(struct acpi_table_madt),
- gic_acpi_parse_madt_distributor, table,
- ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR, 0);
-
+count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR,
+ gic_acpi_parse_madt_distributor, 0);
if ( count <= 0 )
panic("GICv3: No valid GICD entries exists");
@@ -1420,9 +1407,8 @@ static void __init gicv3_acpi_init(void)
dbase);
/* Get number of redistributor */
-count = acpi_parse_entries(ACPI_SIG_MADT, sizeof(struct acpi_table_madt),
- gic_acpi_get_madt_redistributor_num, table,
- ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR, 0);
+count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR,
+ gic_acpi_get_madt_redistributor_num, 0);
if ( count <= 0 )
panic("GICv3: No valid GICR entries exists");
@@ -1458,9 +1444,8 @@ static void __init gicv3_acpi_init(void)
gicv3.rdist_regions= rdist_regs;
/* Collect CPU base addresses */
-count = acpi_parse_entries(ACPI_SIG_MADT, sizeof(struct acpi_table_madt),
- gic_acpi_parse_madt_cpu, table,
- ACPI_MADT_TYPE_GENERIC_INTERRUPT, 0);
+count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT,
+ gic_acpi_parse_madt_cpu, 0);
if ( count <= 0 )
panic("GICv3: No valid GICC entries exists");
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] making xenstore domain easy configurable
On 27/06/16 22:59, Doug Goldstein wrote:
> On 6/27/16 7:59 AM, Andrew Cooper wrote:
>> On 27/06/16 13:43, Juergen Gross wrote:
>>> I'm just writing some patches to make it easy to switch between
>>> xenstore daemon and xenstore domain. My plan is to achieve this
>>> by a global configuration file containing configuration options
>>> for the host (e.g. /etc/xen/xen.conf).
>>>
>>> With the current systemd support this is not easy. There are
>>> systemd socket definitions to let systemd create the sockets for
>>> xenstored. As the sockets are not to be created in case xenstore
>>> is running in a xenstore domain things are becoming complicated.
>>>
>>> Today we have the following xenstore related systemd items:
>>>
>>> - xenstored_ro.socket and xenstored.socket
>>> - xenstored.service depending on the sockets
>>> - other services depending on xenstored.service
>>>
>>> A xenstore domain would need:
>>>
>>> - xenstore-domain.service
>>> - other services depending on xenstore-domain.service
>>>
>>> Being able to switch between both schemes just via a config file
>>> seems to be not easy, at least I don't know of any way to do the
>>> socket creation only in case they are required without breaking
>>> the dependency chain.
>>>
>>> So I'd suggest to remove xenstored_ro.socket and xenstored.socket
>>> and let xenstored create the sockets (as it is doing without
>>> systemd). I'm not aware of any disadvantage, as xenstored isn't
>>> restartable and thus can't take advantage of the permanent sockets
>>> offered by systemd.
>>>
>>> This would mean I could rip out the systemd specific stuff from
>>> xenstored and oxenstored. I could create a single xenstore.service
>>> script evaluating the config file and starting the correct xenstore
>>> (xenstored or xenstore domain). The other services would then depend
>>> on xenstore.service. This would remove the need to specify the
>>> type of xenstore daemon/domain (ocaml based or C based) in the systemd
>>> file, too.
>>>
>>> Is there a better way to achieve what I want? Any other opinions?
>>
>> This isn't the only advantage offered by socket activation.
>>
>> As currently configured, every service which depends on xenstored.socket
>> can be started in parallel (as systemd creates the sockets ahead of
>> time), with the dependent services blocking a little on the socket while
>> xenstored starts up sufficiently to service the requests.
>>
>> In the case that xenstored is running in the local domain, socket
>> activation is a useful function to have.
>>
>> OTOH, having anything explicitly depend on xenstored.socket is broken in
>> a model where xenstored might be running in a separate domain. I don't
>> suppose systemd has any way of specifying "conditionally might have a
>> socket"?
>>
>> ~Andrew
>
> How about we take this the other way? Let's go away from using the
> socket and always go through kernel interface. I understand that its
> faster to use sockets than using the interface but does this performance
> difference really affect an actual running system. If we manage to steer
> people towards a stubdom xenstore they won't have the option of using
> the sockets anyway. Just thinking that supporting two different
> interfaces always seems clumsy.
Generally I like the idea.
Just did a basic test and it seems just to work:
diff --git a/tools/xenstore/xs.c b/tools/xenstore/xs.c
index d1e01ba..8d806cb 100644
--- a/tools/xenstore/xs.c
+++ b/tools/xenstore/xs.c
@@ -288,10 +288,12 @@ struct xs_handle *xs_open(unsigned long flags)
{
struct xs_handle *xsh = NULL;
+#if 0
if (flags & XS_OPEN_READONLY)
xsh = get_handle(xs_daemon_socket_ro());
else
xsh = get_handle(xs_daemon_socket());
+#endif
if (!xsh && !(flags & XS_OPEN_SOCKETONLY))
xsh = get_handle(xs_domain_dev());
Doing "strace xenstore-ls" verified the tool is now using the kernel
interface in dom0 with xenstored running. Using "time xenstore-ls"
showed no performance drop (the numbers are rather low; if any
difference is really measurable the socket variant seems to be a
little bit slower).
My conclusion: I'm going this route (thanks, Doug!), patches will
follow soon.
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V3 03/10] arm/gic-v3: Move GICR subtable parsing into a new function
Hi Shanker,
On 27/06/16 21:33, Shanker Donthineni wrote:
Add a new function to parse GICR subtable and move the code that
is specific to GICR table to a new function without changing the
function gicv3_acpi_init() behavior.
Signed-off-by: Shanker Donthineni
Acked-by: Julien Grall
Regards,
---
Changes since v2:
Changed function gic_acpi_add_rdist_region() protoype.
Removed the address validation check in gic_acpi_parse_madt_redistributor().
Edited commit text.
Changes since v1:
Removed the unnecessary GICR ioremap operation inside GICR table parse code.
xen/arch/arm/gic-v3.c | 56 +++
1 file changed, 34 insertions(+), 22 deletions(-)
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 1f8fbc4..efdb56b 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1282,6 +1282,14 @@ static int gicv3_iomem_deny_access(const struct domain
*d)
}
#ifdef CONFIG_ACPI
+static void __init gic_acpi_add_rdist_region(paddr_t base, paddr_t size)
+{
+unsigned int idx = gicv3.rdist_count++;
+
+gicv3.rdist_regions[idx].base = base;
+gicv3.rdist_regions[idx].size = size;
+}
+
static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset)
{
struct acpi_subtable_header *header;
@@ -1387,6 +1395,22 @@ gic_acpi_parse_madt_distributor(struct
acpi_subtable_header *header,
return 0;
}
+
+static int __init
+gic_acpi_parse_madt_redistributor(struct acpi_subtable_header *header,
+ const unsigned long end)
+{
+struct acpi_madt_generic_redistributor *rdist;
+
+rdist = (struct acpi_madt_generic_redistributor *)header;
+if ( BAD_MADT_ENTRY(rdist, end) )
+return -EINVAL;
+
+gic_acpi_add_rdist_region(rdist->base_address, rdist->length);
+
+return 0;
+}
+
static int __init
gic_acpi_get_madt_redistributor_num(struct acpi_subtable_header *header,
const unsigned long end)
@@ -1400,7 +1424,7 @@ gic_acpi_get_madt_redistributor_num(struct
acpi_subtable_header *header,
static void __init gicv3_acpi_init(void)
{
struct rdist_region *rdist_regs;
-int count, i;
+int count;
/*
* Find distributor base address. We expect one distributor entry since
@@ -1419,37 +1443,25 @@ static void __init gicv3_acpi_init(void)
if ( count <= 0 )
panic("GICv3: No valid GICR entries exists");
-gicv3.rdist_count = count;
-
-if ( gicv3.rdist_count > MAX_RDIST_COUNT )
+if ( count > MAX_RDIST_COUNT )
panic("GICv3: Number of redistributor regions is more than"
"%d (Increase MAX_RDIST_COUNT!!)\n", MAX_RDIST_COUNT);
-rdist_regs = xzalloc_array(struct rdist_region, gicv3.rdist_count);
+rdist_regs = xzalloc_array(struct rdist_region, count);
if ( !rdist_regs )
panic("GICv3: Failed to allocate memory for rdist regions\n");
-for ( i = 0; i < gicv3.rdist_count; i++ )
-{
-struct acpi_subtable_header *header;
-struct acpi_madt_generic_redistributor *gic_rdist;
-
-header =
acpi_table_get_entry_madt(ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR,
- i);
-if ( !header )
-panic("GICv3: Can't get GICR entry");
-
-gic_rdist =
- container_of(header, struct acpi_madt_generic_redistributor,
header);
-rdist_regs[i].base = gic_rdist->base_address;
-rdist_regs[i].size = gic_rdist->length;
-}
+gicv3.rdist_regions = rdist_regs;
+
+/* Parse always-on power domain Re-distributor entries */
+count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR,
+ gic_acpi_parse_madt_redistributor, count);
+if ( count <= 0 )
+panic("GICv3: Can't get Redistributor entry");
/* The vGIC code requires the region to be sorted */
sort(rdist_regs, gicv3.rdist_count, sizeof(*rdist_regs), cmp_rdist, NULL);
-gicv3.rdist_regions= rdist_regs;
-
/* Collect CPU base addresses */
count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT,
gic_acpi_parse_madt_cpu, 0);
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] making xenstore domain easy configurable
On 27/06/16 21:59, Doug Goldstein wrote: > On 6/27/16 7:59 AM, Andrew Cooper wrote: >> On 27/06/16 13:43, Juergen Gross wrote: >>> I'm just writing some patches to make it easy to switch between >>> xenstore daemon and xenstore domain. My plan is to achieve this >>> by a global configuration file containing configuration options >>> for the host (e.g. /etc/xen/xen.conf). >>> >>> With the current systemd support this is not easy. There are >>> systemd socket definitions to let systemd create the sockets for >>> xenstored. As the sockets are not to be created in case xenstore >>> is running in a xenstore domain things are becoming complicated. >>> >>> Today we have the following xenstore related systemd items: >>> >>> - xenstored_ro.socket and xenstored.socket >>> - xenstored.service depending on the sockets >>> - other services depending on xenstored.service >>> >>> A xenstore domain would need: >>> >>> - xenstore-domain.service >>> - other services depending on xenstore-domain.service >>> >>> Being able to switch between both schemes just via a config file >>> seems to be not easy, at least I don't know of any way to do the >>> socket creation only in case they are required without breaking >>> the dependency chain. >>> >>> So I'd suggest to remove xenstored_ro.socket and xenstored.socket >>> and let xenstored create the sockets (as it is doing without >>> systemd). I'm not aware of any disadvantage, as xenstored isn't >>> restartable and thus can't take advantage of the permanent sockets >>> offered by systemd. >>> >>> This would mean I could rip out the systemd specific stuff from >>> xenstored and oxenstored. I could create a single xenstore.service >>> script evaluating the config file and starting the correct xenstore >>> (xenstored or xenstore domain). The other services would then depend >>> on xenstore.service. This would remove the need to specify the >>> type of xenstore daemon/domain (ocaml based or C based) in the systemd >>> file, too. >>> >>> Is there a better way to achieve what I want? Any other opinions? >> >> This isn't the only advantage offered by socket activation. >> >> As currently configured, every service which depends on xenstored.socket >> can be started in parallel (as systemd creates the sockets ahead of >> time), with the dependent services blocking a little on the socket while >> xenstored starts up sufficiently to service the requests. >> >> In the case that xenstored is running in the local domain, socket >> activation is a useful function to have. >> >> OTOH, having anything explicitly depend on xenstored.socket is broken in >> a model where xenstored might be running in a separate domain. I don't >> suppose systemd has any way of specifying "conditionally might have a >> socket"? >> >> ~Andrew > > How about we take this the other way? Let's go away from using the > socket and always go through kernel interface. I understand that its > faster to use sockets than using the interface but does this performance > difference really affect an actual running system. If we manage to steer > people towards a stubdom xenstore they won't have the option of using > the sockets anyway. Just thinking that supporting two different > interfaces always seems clumsy. xs_restrict() which QEMU will be making use of requires the unix domain socket. So I don't think we want to go down this route unless xs_restrict() can be made to work via the kernel interface as well. David ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V3 04/10] arm/gic-v3: Parse per-cpu redistributor entry in GICC subtable
Hello Shanker,
On 27/06/16 21:33, Shanker Donthineni wrote:
@@ -1397,6 +1408,36 @@ gic_acpi_parse_madt_distributor(struct
acpi_subtable_header *header,
}
static int __init
+gic_acpi_parse_cpu_redistributor(struct acpi_subtable_header *header,
+ const unsigned long end)
+{
+struct acpi_madt_generic_interrupt *processor;
+u32 size;
+
+processor = (struct acpi_madt_generic_interrupt *)header;
+if ( !(processor->flags & ACPI_MADT_ENABLED) )
+return 0;
You did not answer to my question on previous version of this patch. You
said that "Disabled GICC entries should be skipped because its
Redistributor region is not always-on power domain." However from my
understanding, an usable CPU may have his Redistributor in the not
always-on power domain. So the issue would the same, correct?
Regards,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V3 05/10] xen/arm: vgic: Use dynamic memory allocation for vgic_rdist_region
Hi Shanker,
On 27/06/16 21:33, Shanker Donthineni wrote:
The number of Redistributor regions allowed for dom0 is hardcoded
to a define MAX_RDIST_COUNT which is 4. Some systems, especially
latest server chips, may have more than 4 redistributors. Either we
have to increase MAX_RDIST_COUNT to a bigger number or allocate
memory based on the number of redistributors that are found in MADT
table. In the worst case scenario, the macro MAX_RDIST_COUNT should
be equal to CONFIG_NR_CPUS in order to support per CPU Redistributors.
Increasing MAX_RDIST_COUNT has a effect, it blows 'struct domain'
size and hits BUILD_BUG_ON() in domain build code path.
struct domain *alloc_domain_struct(void)
{
struct domain *d;
BUILD_BUG_ON(sizeof(*d) > PAGE_SIZE);
d = alloc_xenheap_pages(0, 0);
if ( d == NULL )
return NULL;
...
This patch uses the second approach to fix the BUILD_BUG().
Signed-off-by: Shanker Donthineni
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] [libvirt] Fixing libvirt's libxl driver breakage -- where to define LIBXL_API_VERSION?
Jan Beulich writes ("Re: [libvirt] [Xen-devel] Fixing libvirt's libxl driver
breakage -- where to define LIBXL_API_VERSION?"):
> On 27.06.16 at 18:54, wrote:
> > OK. Thanks for the feedback. I'll go ahead with my plan with the
> > git commit ids named in my earlier email.
>
> The only (hopefully highly theoretical) problem I see with this is that
> we may end up picking a libvirt commit which subsequently (e.g. via
> a libxl backport) turns out to have an issue. Such a problem could be
> dealt with in the suggested the stable branch tracking model (or any
> other model not dealing with something completely frozen).
I don't think there is anything stopping us manually updating one of
these frozen "branches", should such a situation occur.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V3 07/10] arm: vgic: Split vgic_domain_init() functionality into two functions
Hi Shanker, On 27/06/16 21:33, Shanker Donthineni wrote: Separate the code logic that does the registration of vgic_v3/v2 ops to a new function domain_vgic_register(). The intention of this separation is to record the required mmio count in vgic_v3/v2_init() and pass it to function domain_io_init() in a follow-up patch patch. Signed-off-by: Shanker Donthineni 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] making xenstore domain easy configurable
David Vrabel writes ("Re: [Xen-devel] making xenstore domain easy
configurable"):
> So I don't think we want to go down this route unless xs_restrict() can
> be made to work via the kernel interface as well.
It could, in principle, but it would have to be implemented in the
xenstore kernel driver in each dom0 kernel, and might involve a
protocol extension to convey the right information to xenstored.
I don't think this is a good direction to be going.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V3 09/10] xen/arm: io: Use binary search for mmio handler lookup
Hi Shanker,
On 27/06/16 21:33, Shanker Donthineni wrote:
As the number of I/O handlers increase, the overhead associated with
linear lookup also increases. The system might have maximum of 144
(assuming CONFIG_NR_CPUS=128) mmio handlers. In worst case scenario,
it would require 144 iterations for finding a matching handler. Now
it is time for us to change from linear (complexity O(n)) to a binary
search (complexity O(log n) for reducing mmio handler lookup overhead.
Signed-off-by: Shanker Donthineni
---
Changes since v2:
Converted mmio lookup code to a critical section.
Copied the function bsreach() from Linux kernel.
xen/arch/arm/io.c | 97 +++
1 file changed, 84 insertions(+), 13 deletions(-)
diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
index a5b2c2d..c31fdf3 100644
--- a/xen/arch/arm/io.c
+++ b/xen/arch/arm/io.c
@@ -20,9 +20,50 @@
#include
#include
#include
+#include
#include
#include
+/*
+ * bsearch - binary search an array of elements
+ * @key: pointer to item being searched for
+ * @base: pointer to first element to search
+ * @num: number of elements
+ * @size: size of each element
+ * @cmp: pointer to comparison function
+ *
+ * This function does a binary search on the given array. The
+ * contents of the array should already be in ascending sorted order
+ * under the provided comparison function.
+ *
+ * Note that the key need not have the same type as the elements in
+ * the array, e.g. key could be a string and the comparison function
+ * could compare the string with the struct's name field. However, if
+ * the key and elements in the array are of the same type, you can use
+ * the same comparison function for both sort() and bsearch().
+ */
+static void *bsearch(const void *key, const void *base, size_t num, size_t
size,
+ int (*cmp)(const void *key, const void *elt))
This function is not specific to I/O handlers. So this should be moved
to common code. Also please mention in the commit message where the code
came from.
+{
+size_t start = 0, end = num;
+int result;
+
+while ( start < end )
+{
+size_t mid = start + (end - start) / 2;
+
+result = cmp(key, base + mid * size);
+if ( result < 0 )
+end = mid;
+else if ( result > 0 )
+start = mid + 1;
+else
+return (void *)base + mid * size;
+}
+
+return NULL;
+}
+
static int handle_read(const struct mmio_handler *handler, struct vcpu *v,
mmio_info_t *info)
{
@@ -70,23 +111,41 @@ static int handle_write(const struct mmio_handler
*handler, struct vcpu *v,
handler->priv);
}
-int handle_mmio(mmio_info_t *info)
+static int match_mmio_handler(const void *key, const void *elem)
{
-struct vcpu *v = current;
-int i;
-const struct mmio_handler *handler = NULL;
-const struct vmmio *vmmio = &v->domain->arch.vmmio;
+const struct mmio_handler *handler = elem;
+paddr_t addr = (paddr_t)key;
-for ( i = 0; i < vmmio->num_entries; i++ )
-{
-handler = &vmmio->handlers[i];
+if ( addr < handler->addr )
+return -1;
-if ( (info->gpa >= handler->addr) &&
- (info->gpa < (handler->addr + handler->size)) )
-break;
-}
+if ( addr > (handler->addr + handler->size) )
+return 1;
+
+return 0;
+}
-if ( i == vmmio->num_entries )
+static const struct mmio_handler *
+find_mmio_handler(struct vcpu *v, paddr_t addr)
+{
+struct vmmio *vmmio = &v->domain->arch.vmmio;
+const struct mmio_handler *handler;
+
+spin_lock(&vmmio->lock);
+handler = bsearch((const void *)addr, vmmio->handlers, vmmio->num_entries,
paddr_t is always 64-bit regardless the architecture (ARM64 vs ARM32).
So the cast will lead to a compilation error on ARM32.
Please try to at least compile test your patch with ARM64, ARM32 and x86
(when you touch common code).
Anyway, I would try to merge the two compare functions
(match_mmio_handler, cmp_mmio_handler) which have very similar behavior.
+ sizeof(*handler), match_mmio_handler);
+spin_unlock(&vmmio->lock);
+
+return handler;
+}
+
+int handle_mmio(mmio_info_t *info)
+{
+const struct mmio_handler *handler;
+struct vcpu *v = current;
+
+handler = find_mmio_handler(v, info->gpa);
+if ( !handler )
return 0;
if ( info->dabt.write )
@@ -95,6 +154,14 @@ int handle_mmio(mmio_info_t *info)
return handle_read(handler, v, info);
}
+static int cmp_mmio_handler(const void *key, const void *elem)
+{
+const struct mmio_handler *handler0 = key;
+const struct mmio_handler *handler1 = elem;
+
+return (handler0->addr < handler1->addr) ? -1 : 0;
+}
+
void register_mmio_handler(struct domain *d,
const struct mmio_handler_ops *ops,
Re: [Xen-devel] making xenstore domain easy configurable
On 28/06/16 12:45, Ian Jackson wrote:
> David Vrabel writes ("Re: [Xen-devel] making xenstore domain easy
> configurable"):
>> So I don't think we want to go down this route unless xs_restrict() can
>> be made to work via the kernel interface as well.
>
> It could, in principle, but it would have to be implemented in the
> xenstore kernel driver in each dom0 kernel, and might involve a
> protocol extension to convey the right information to xenstored.
>
> I don't think this is a good direction to be going.
So you are telling me the xenstore domain won't work for this case?
BTW: I couldn't spot the socket being a prerequisite in the C-variant
of xenstored. Is this an oxenstored only problem?
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V2 10/10] arm/vgic: Change fixed number of mmio handlers to variable number
Hi Shanker, On 27/06/16 16:02, Shanker Donthineni wrote: On 06/27/2016 08:35 AM, Julien Grall wrote: Hi Shanker, On 26/06/16 18:48, Shanker Donthineni wrote: diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h index 29346c6..b205461 100644 --- a/xen/include/asm-arm/domain.h +++ b/xen/include/asm-arm/domain.h @@ -111,6 +111,7 @@ struct arch_domain int nr_regions; /* Number of rdist regions */ uint32_t rdist_stride; /* Re-Distributor stride */ #endif +uint32_t mmio_count;/* Number of mmio handlers */ Is it necessary to have this value part of the arch_domain? I.e Do we need this value after the initialization? If not, then it might be better to add a parameter to domain_vgic_register uint32_t *pointer. Absolutely, we don't need this variable after the domain build process. I have taken this approach to avoid too many code changes. Your suggestion requires changes to functions vgic_v2/v3_init() prototype for adding a new parameter. The internal interface of Xen is not set in stone. We modify quite often to match our needs or for clean-up purpose. I always welcome an interface changes as long as it makes sense. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] making xenstore domain easy configurable
Juergen Gross writes ("Re: [Xen-devel] making xenstore domain easy
configurable"):
> So you are telling me the xenstore domain won't work for this case?
Yes.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 11/17] libxl/arm: Construct ACPI DSDT table
On 2016/6/27 20:05, Boris Ostrovsky wrote:
>
>
> On 06/27/2016 06:29 AM, Julien Grall wrote:
>> (CC Boris and Doug)
>>
>> Hi Shannon,
>>
>> On 27/06/16 07:01, Shannon Zhao wrote:
>>> On 2016/6/24 1:03, Julien Grall wrote:
On 23/06/16 04:16, Shannon Zhao wrote:
[...]
> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
> index 264b6ef..5347480 100644
> --- a/tools/libxl/Makefile
> +++ b/tools/libxl/Makefile
> @@ -77,7 +77,29 @@ endif
>
>LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o libxl_x86.o libxl_psr.o
>LIBXL_OBJS-$(CONFIG_ARM) += libxl_nocpuid.o libxl_arm.o
> libxl_libfdt_compat.o
> -LIBXL_OBJS-$(CONFIG_ARM) += libxl_arm_acpi.o
> +LIBXL_OBJS-$(CONFIG_ARM) += libxl_arm_acpi.o libxl_dsdt_anycpu_arm.o
> +
> +vpath iasl $(PATH)
> +libxl_mk_dsdt_arm: libxl_mk_dsdt_arm.c
> +$(CC) $(CFLAGS) -o $@ libxl_mk_dsdt_arm.c
> +
> +libxl_dsdt_anycpu_arm.asl: libxl_empty_dsdt_arm.asl libxl_mk_dsdt_arm
> +awk 'NR > 1 {print s} {s=$$0}' $< > $@
> +./libxl_mk_dsdt_arm >> $@
> +
> +libxl_dsdt_anycpu_arm.c: %.c: iasl %.asl
> +iasl -vs -p $* -tc $*.asl
> +sed -e 's/AmlCode/$*/g' $*.hex >$@
> +echo "int $*_len=sizeof($*);" >>$@
> +rm -f $*.aml $*.hex
> +
I don't like the idea to add iasl as a dependency for all ARM
platforms.
For instance ARMv7 platform will not use ACPI, but we still ask
users to
install iasl. So I think we should allow the user to opt-in/opt-out for
ACPI.
Any opinions?
>>> I agree. But how to exclude for ARMv7. I notice it only has the option
>>> CONFIG_ARM which doesn't distinguish ARM32 and ARM64.
>>
>> I am not sure if we plan to introduce Kconfig for tools. If not, you
>> can add an option to the configure to enable/disable ACPI for guest.
>>
>> This would be gated by the presence of "iasl".
>>
>> [...]
>>
> diff --git a/tools/libxl/libxl_mk_dsdt_arm.c
> b/tools/libxl/libxl_mk_dsdt_arm.c
> new file mode 100644
> index 000..96fadbd
> --- /dev/null
> +++ b/tools/libxl/libxl_mk_dsdt_arm.c
Can we share the code from tools/firmware/acpi/mk_dsdt.c?
>>> Yeah, we can share push_block(), pop_block() stmt() and indent() but the
>>> main() function is totally different since there are only the processor
>>> device objects for ARM DSDT but there are many other things in x86.
>>>
>>> I think that since Boris will move the codes under
>>> tools/firmware/hvmloader/acpi to other place, after that we could see
>>> how to share codes then.
>>
>> I would prefer if we discuss about it now in order to avoid code
>> duplication (I have CCed Boris).
>>
>> For instance we can create a new directory under tools for mk_dsdt.c.
>> The main could be different, although it might be possible to gate ARM
>> options, and the rest of the code would be shared.
>
>
> So I think we decided earlier to keep ARM and x86 ACPI builders
> separate, at least for now.
I think so as well.
> However, looking at the Makefile and mk_dsdt
> I wonder whether it would make sense to put the builders in the same
> directory (I am currently using tools/libacpi) so that those two files
> can be kept common as much as possible, with the sources being
> different. E.g. something like
>
> tools/libacpi:
> Makefile
> mk_dsdt.c
> acpi_x86.[ch]
> acpi_arm.[ch]
> *asl
> etc.
>
> The objects will be built in tools/libxl (there will be no libacpi.so)
> but the infrastructure and sources will live together.
I'm fine with this. But I think the patch moving the codes into
tools/libacpi should be posted firstly, since this series depend on it.
Boris, could you please send that patch? Then I can add the
corresponding ARM patch on top of that.
Thanks,
--
Shannon
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] xc_domain_maximum_gpfn
Hello, On 27/06/16 14:31, sepanta s wrote: On Sun, Jun 26, 2016 at 5:19 PM, sepanta s mailto:sapanta...@gmail.com>> wrote: Hi, what exactly does this module do? sorry, not module but the function. The function xc_domain_maximum_gpfn returns the highest frame that has ever been mapped in the p2m. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Crash in xen 4.7 adding nic during domu startup
A clue on doing this would be useful, I can't debug what is now release code all day. 4.5 is absolutely fine. 4.6.3 (tried last night) does not fail either but refuses to start my domU that has two nics assigned to it in a vif line. I'm presuming other people are successfully running multiple NICs on one VM (ioemu, e1000) PK On Tuesday, 28 June 2016, Jan Beulich wrote: > >>> On 28.06.16 at 00:28, > > wrote: > > (XEN) [2016-06-27 22:11:41.712] [ Xen-4.7.0 x86_64 debug=n Not > tainted ] > > (XEN) [2016-06-27 22:11:41.712] CPU:0 > > (XEN) [2016-06-27 22:11:41.712] RIP:e008:[] > domain_relinquish_resources+0x10/0x2f0 > > (XEN) [2016-06-27 22:11:41.712] RFLAGS: 00010296 CONTEXT: > hypervisor (d0v0) > > (XEN) [2016-06-27 22:11:41.712] rax: rbx: > 83020ac4f000 rcx: 82d0802346bc > > (XEN) [2016-06-27 22:11:41.712] rdx: 8300bf8d7fff rsi: > rdi: 83020afde050 > > (XEN) [2016-06-27 22:11:41.712] rbp: 8300bf8d7e48 rsp: > 8300bf8d7d78 r8: 0001 > > (XEN) [2016-06-27 22:11:41.712] r9: r10: > 0003 r11: 83023d94b1d8 > > (XEN) [2016-06-27 22:11:41.712] r12: 7ffe21eb9530 r13: > r14: 83020ac4f000 > > (XEN) [2016-06-27 22:11:41.712] r15: 7ffe21eb9530 cr0: > 80050033 cr4: 26e0 > > (XEN) [2016-06-27 22:11:41.712] cr3: 000216737000 cr2: > > > (XEN) [2016-06-27 22:11:41.712] ds: es: fs: gs: > ss: e010 cs: e008 > > (XEN) [2016-06-27 22:11:41.712] Xen code around > (domain_relinquish_resources+0x10/0x2f0): > > (XEN) [2016-06-27 22:11:41.712] 18 48 8b bf e8 01 00 00 <00> 00 00 00 > 00 00 00 00 00 00 00 00 00 00 00 00 > > Something very clearly corrupted your hypervisor code, and that's > what you'll need to track down. > > Jan > > ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH] xen/arm: gic-v3: No need to sort the Redistributor regions
The sorting was required by the vGIC emulation until commit
9b9d51e98edb8c5c731e2d06dfad3633053d88a4 "xen/arm: vgic-v3:
Correctly retrieve the vCPU associated to a re-distributor".
Furthermore, the code is buggy because both local variables 'l' and 'r'
point to the same region.
So drop the code which sort the Redistributors array.
Reported-by: Shanker Donthineni
Signed-off-by: Julien Grall
---
xen/arch/arm/gic-v3.c | 11 ---
1 file changed, 11 deletions(-)
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index dfc62e8..3b02a8c 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1134,14 +1134,6 @@ static const hw_irq_controller gicv3_guest_irq_type = {
.set_affinity = gicv3_irq_set_affinity,
};
-static int __init cmp_rdist(const void *a, const void *b)
-{
-const struct rdist_region *l = a, *r = a;
-
-/* We assume that re-distributor regions can never overlap */
-return ( l->base < r->base) ? -1 : 0;
-}
-
static paddr_t __initdata dbase = INVALID_PADDR;
static paddr_t __initdata vbase = INVALID_PADDR, vsize = 0;
static paddr_t __initdata cbase = INVALID_PADDR, csize = 0;
@@ -1210,9 +1202,6 @@ static void __init gicv3_dt_init(void)
rdist_regs[i].size = rdist_size;
}
-/* The vGIC code requires the region to be sorted */
-sort(rdist_regs, gicv3.rdist_count, sizeof(*rdist_regs), cmp_rdist, NULL);
-
if ( !dt_property_read_u32(node, "redistributor-stride",
&gicv3.rdist_stride) )
gicv3.rdist_stride = 0;
--
1.9.1
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3] ArmVirtPkg/ArmVirtXen: Add ACPI support for Virt Xen ARM
On 25 June 2016 at 09:16, Shannon Zhao wrote:
> From: Shannon Zhao
>
> Add ACPI support for Virt Xen ARM and only for aarch64. It gets the
> ACPI tables through Xen ARM multiboot protocol.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Shannon Zhao
Reviewed-by: Ard Biesheuvel
Committed as 402dde68aff9
Thanks,
Ard.
> ---
> Changes since v2:
> * add gFdtClientProtocolGuid to the [Depex]
> * make functions static
> * move XenAcpiRsdpStructurePtr to InstallXenArmTables ()
> * initialize AcpiTable and FdtClient to NULL
>
> Changes since v1:
> * move the codes into ArmVirtPkg
> * use FdtClient
> * don't rely on OvmfPkg/AcpiPlatformDxe/EntryPoint.c while implement own
> entry point since it's minor
> * use compatible string to find the DT node instead of node path
>
> If you want to test, the corresponding Xen patches can be fetched from:
> https://git.linaro.org/people/shannon.zhao/xen.git domu_acpi_v2
> ---
> ArmVirtPkg/ArmVirtXen.dsc | 8 +
> ArmVirtPkg/ArmVirtXen.fdf | 8 +
> ArmVirtPkg/XenAcpiPlatformDxe/XenAcpiPlatformDxe.c | 244
> +
> .../XenAcpiPlatformDxe/XenAcpiPlatformDxe.inf | 50 +
> 4 files changed, 310 insertions(+)
> create mode 100644 ArmVirtPkg/XenAcpiPlatformDxe/XenAcpiPlatformDxe.c
> create mode 100644 ArmVirtPkg/XenAcpiPlatformDxe/XenAcpiPlatformDxe.inf
>
> diff --git a/ArmVirtPkg/ArmVirtXen.dsc b/ArmVirtPkg/ArmVirtXen.dsc
> index 594ca64..a869986 100644
> --- a/ArmVirtPkg/ArmVirtXen.dsc
> +++ b/ArmVirtPkg/ArmVirtXen.dsc
> @@ -216,3 +216,11 @@
>
>OvmfPkg/XenBusDxe/XenBusDxe.inf
>OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
> +
> + #
> + # ACPI support
> + #
> +!if $(ARCH) == AARCH64
> + MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
> + ArmVirtPkg/XenAcpiPlatformDxe/XenAcpiPlatformDxe.inf
> +!endif
> diff --git a/ArmVirtPkg/ArmVirtXen.fdf b/ArmVirtPkg/ArmVirtXen.fdf
> index 13412f9..b1e00e5 100644
> --- a/ArmVirtPkg/ArmVirtXen.fdf
> +++ b/ArmVirtPkg/ArmVirtXen.fdf
> @@ -179,6 +179,14 @@ READ_LOCK_STATUS = TRUE
>INF OvmfPkg/XenBusDxe/XenBusDxe.inf
>INF OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
>
> + #
> + # ACPI support
> + #
> +!if $(ARCH) == AARCH64
> + INF MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
> + INF ArmVirtPkg/XenAcpiPlatformDxe/XenAcpiPlatformDxe.inf
> +!endif
> +
> [FV.FVMAIN_COMPACT]
> FvAlignment= 16
> ERASE_POLARITY = 1
> diff --git a/ArmVirtPkg/XenAcpiPlatformDxe/XenAcpiPlatformDxe.c
> b/ArmVirtPkg/XenAcpiPlatformDxe/XenAcpiPlatformDxe.c
> new file mode 100644
> index 000..c6912ba
> --- /dev/null
> +++ b/ArmVirtPkg/XenAcpiPlatformDxe/XenAcpiPlatformDxe.c
> @@ -0,0 +1,244 @@
> +/** @file
> + Xen ARM ACPI Platform Driver using Xen ARM multiboot protocol
> +
> + Copyright (C) 2016, Linaro Ltd. All rights reserved.
> +
> + This program and the accompanying materials
> + are licensed and made available under the terms and conditions of the BSD
> License
> + which accompanies this distribution. The full text of the license may be
> found at
> + http://opensource.org/licenses/bsd-license.php
> +
> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
> IMPLIED.
> +
> +**/
> +
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +
> +#include
> +
> +/**
> + Get the address of Xen ACPI Root System Description Pointer (RSDP)
> + structure.
> +
> + @param RsdpStructurePtr Return pointer to RSDP structure
> +
> + @return EFI_SUCCESSFind Xen RSDP structure successfully.
> + @return EFI_NOT_FOUND Don't find Xen RSDP structure.
> + @return EFI_ABORTEDFind Xen RSDP structure, but it's not
> integrated.
> +
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +GetXenArmAcpiRsdp (
> + OUT EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER **RsdpPtr
> + )
> +{
> + EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER *RsdpStructurePtr;
> + EFI_STATUS Status;
> + FDT_CLIENT_PROTOCOL*FdtClient;
> + CONST UINT64 *Reg;
> + UINT32 RegElemSize, RegSize;
> + UINT64 RegBase;
> + UINT8 Sum;
> +
> + RsdpStructurePtr = NULL;
> + FdtClient = NULL;
> + //
> + // Get the RSDP structure address from DeviceTree
> + //
> + Status = gBS->LocateProtocol (&gFdtClientProtocolGuid, NULL,
> + (VOID **)&FdtClient);
> + ASSERT_EFI_ERROR (Status);
> +
> + Status = FdtClient->FindCompatibleNodeReg (FdtClient, "xen,guest-acpi",
> +(CONST VOID **)&Reg, &RegElemSize, &RegSize);
> + if (EFI_ERROR (Status)) {
> +DEBUG ((EFI_D_WARN, "%a: No 'xen,guest-acpi' compatible DT node found\n",
> + __FUNCTION__));
> +retur
Re: [Xen-devel] making xenstore domain easy configurable
On 28/06/16 13:03, Ian Jackson wrote:
> Juergen Gross writes ("Re: [Xen-devel] making xenstore domain easy
> configurable"):
>> So you are telling me the xenstore domain won't work for this case?
>
> Yes.
That's rather unfortunate. So in order to be able to make xenstore
domain a common setup we need to find a solution for support of
xs_restrict() via xenbus, right?
TBH, the way xs_restrict() was introduced is rather weird. It is
completely bound to the socket interface of oxenstored. So anyone
wanting to use xs_restrict() is limited to oxenstored running in
dom0. No way to use xenstored or a xenstore domain. I'm really
disappointed such a design was accepted and is now the reason for
not being able to disaggregate dom0.
I've searched through the xen-devel archives and found a very
interesting mail:
http://lists.xen.org/archives/html/xen-devel/2010-04/msg01318.html
The "restrict" feature was added without any further discussion how
it is implemented and that the C-variant doesn't support it. The
explicit question about non-existing features in the C xenstored was
answered just with "the xenstore wire protocol doesn't change".
With:
http://lists.xen.org/archives/html/xen-devel/2010-07/msg00091.html
the XS_RESTRICT value in xs_wire.h (aah, suddenly it was changed?)
was added. Again no mentioning of the special implementation in
oxenstored.
Really, this is not how open source development should be done!
Maybe I'm just upset now, but I'm in favor of dropping xs_restrict()
support as it has been introduced in a foul way.
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3] ArmVirtPkg/ArmVirtXen: Add ACPI support for Virt Xen ARM
Hi Ard, On 28/06/16 12:39, Ard Biesheuvel wrote: On 25 June 2016 at 09:16, Shannon Zhao wrote: From: Shannon Zhao Add ACPI support for Virt Xen ARM and only for aarch64. It gets the ACPI tables through Xen ARM multiboot protocol. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Shannon Zhao Reviewed-by: Ard Biesheuvel Committed as 402dde68aff9 We have not yet agreed on the bindings between Xen and UEFI (see patch [1]). How EDK2 deal with compatibility if we decide to modify the bindings for whatever reasons? Regards, [1] http://lists.xenproject.org/archives/html/xen-devel/2016-06/msg02943.html -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3] ArmVirtPkg/ArmVirtXen: Add ACPI support for Virt Xen ARM
On 28 June 2016 at 14:06, Julien Grall wrote: > Hi Ard, > > On 28/06/16 12:39, Ard Biesheuvel wrote: >> >> On 25 June 2016 at 09:16, Shannon Zhao wrote: >>> >>> From: Shannon Zhao >>> >>> Add ACPI support for Virt Xen ARM and only for aarch64. It gets the >>> ACPI tables through Xen ARM multiboot protocol. >>> >>> Contributed-under: TianoCore Contribution Agreement 1.0 >>> Signed-off-by: Shannon Zhao >> >> >> Reviewed-by: Ard Biesheuvel >> >> Committed as 402dde68aff9 > > > We have not yet agreed on the bindings between Xen and UEFI (see patch [1]). > How EDK2 deal with compatibility if we decide to modify the bindings for > whatever reasons? > Thanks for emphasizing that. It would have been good to mention it in the commit log. Since this is all under development, I would prefer only the final version of the binding to be supported (if it deviates from the one this patch implements). As soon as anything ends up in a Xen release, we can discuss again whether we need to support different versions of the binding. Thanks, Ard. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 08/16] xen: Replace _mfn(INVALID_MFN) with MFN_INVALID_T
Hi Jan,
On 28/06/16 08:19, Jan Beulich wrote:
On 27.06.16 at 18:54, wrote:
This patch is a mechanical replacement. Command used:
42sh> ack -l "_mfn\(INVALID_MFN\)" | xargs sed -i -e
's/_mfn(INVALID_MFN)/INVALID_MFN_T/g'
Well, wait - if you do this, then I'm no longer sure my remark just
made on patch 2 holds: If you do such a global replacement, then
I think I'd prefer you to switch to the long term final name right
away, rather than having to touch all that code again later.
I will give a look to transform INVALID_{MFN,GFN} into a typesafe.
Cheers,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [xen-unstable-smoke test] 96340: tolerable all pass - PUSHED
flight 96340 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/96340/
Failures :-/ but no regressions.
Tests which did not succeed, but are not blocking:
test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass
test-armhf-armhf-xl 12 migrate-support-checkfail never pass
test-armhf-armhf-xl 13 saverestore-support-checkfail never pass
version targeted for testing:
xen 9b15b2e367a8565c73d5ba975e05c89c99078e60
baseline version:
xen 08cffe6696c047123bd552e095163924c8ef4353
Last test of basis96310 2016-06-27 12:02:08 Z1 days
Testing same since96340 2016-06-28 10:01:49 Z0 days1 attempts
People who touched revisions under test:
Kevin Tian
Quan Xu
Razvan Cojocaru
Tamas K Lengyel
jobs:
build-amd64 pass
build-armhf pass
build-amd64-libvirt pass
test-armhf-armhf-xl pass
test-amd64-amd64-xl-qemuu-debianhvm-i386 pass
test-amd64-amd64-libvirt pass
sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images
Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs
Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master
Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary
Pushing revision :
+ branch=xen-unstable-smoke
+ revision=9b15b2e367a8565c73d5ba975e05c89c99078e60
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
getconfig Repos
perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x '!=' x/home/osstest/repos/lock ']'
++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock
++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push xen-unstable-smoke
9b15b2e367a8565c73d5ba975e05c89c99078e60
+ branch=xen-unstable-smoke
+ revision=9b15b2e367a8565c73d5ba975e05c89c99078e60
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
getconfig Repos
perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']'
+ . ./cri-common
++ . ./cri-getconfig
++ umask 002
+ select_xenbranch
+ case "$branch" in
+ tree=xen
+ xenbranch=xen-unstable-smoke
+ qemuubranch=qemu-upstream-unstable
+ '[' xxen = xlinux ']'
+ linuxbranch=
+ '[' xqemu-upstream-unstable = x ']'
+ select_prevxenbranch
++ ./cri-getprevxenbranch xen-unstable-smoke
+ prevxenbranch=xen-4.7-testing
+ '[' x9b15b2e367a8565c73d5ba975e05c89c99078e60 = x ']'
+ : tested/2.6.39.x
+ . ./ap-common
++ : osst...@xenbits.xen.org
+++ getconfig OsstestUpstream
+++ perl -e '
use Osstest;
readglobalconfig();
print $c{"OsstestUpstream"} or die $!;
'
++ :
++ : git://xenbits.xen.org/xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/xen.git
++ : git://xenbits.xen.org/qemu-xen-traditional.git
++ : git://git.kernel.org
++ : git://git.kernel.org/pub/scm/linux/kernel/git
++ : git
++ : git://xenbits.xen.org/libvirt.git
++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git
++ : git://xenbits.xen.org/libvirt.git
++ : git://xenbits.xen.org/rumpuser-xen.git
++ : git
++ : git://xenbits.xen.org/rumpuser-xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/rumpuser-xen.git
+++ besteffort_repo https://github.com/rumpkernel/rumpkernel-netbsd-src
+++ local repo=https://github.com/rumpkernel/rumpkernel-netbsd-src
+++ cached_repo https://github.com/rumpkernel/rumpkernel-netbsd-src
'[fetch=try]'
+++ local repo=https://github.com/rumpkernel/rumpkernel-netbsd-src
+++ local 'options=[fetch=try]'
getconfig GitCacheProxy
perl -e '
use Osstest;
readglobalconfig();
print $c{"GitCacheProxy"} or die $!;
'
+++ local cache=git://cache:9419/
+++ '[' xgit://cache:9419/ '!=' x
Re: [Xen-devel] making xenstore domain easy configurable
On 28/06/16 12:56, Juergen Gross wrote:
> On 28/06/16 13:03, Ian Jackson wrote:
>> Juergen Gross writes ("Re: [Xen-devel] making xenstore domain easy
>> configurable"):
>>> So you are telling me the xenstore domain won't work for this case?
>> Yes.
> That's rather unfortunate. So in order to be able to make xenstore
> domain a common setup we need to find a solution for support of
> xs_restrict() via xenbus, right?
>
> TBH, the way xs_restrict() was introduced is rather weird. It is
> completely bound to the socket interface of oxenstored. So anyone
> wanting to use xs_restrict() is limited to oxenstored running in
> dom0. No way to use xenstored or a xenstore domain. I'm really
> disappointed such a design was accepted and is now the reason for
> not being able to disaggregate dom0.
>
> I've searched through the xen-devel archives and found a very
> interesting mail:
>
> http://lists.xen.org/archives/html/xen-devel/2010-04/msg01318.html
>
> The "restrict" feature was added without any further discussion how
> it is implemented and that the C-variant doesn't support it. The
> explicit question about non-existing features in the C xenstored was
> answered just with "the xenstore wire protocol doesn't change".
>
> With:
>
> http://lists.xen.org/archives/html/xen-devel/2010-07/msg00091.html
>
> the XS_RESTRICT value in xs_wire.h (aah, suddenly it was changed?)
> was added. Again no mentioning of the special implementation in
> oxenstored.
>
> Really, this is not how open source development should be done!
> Maybe I'm just upset now, but I'm in favor of dropping xs_restrict()
> support as it has been introduced in a foul way.
I don't think the lack of xs_restrict() working over the ring should
preclude these improvements to the configuration of how xenstored starts up.
Ideally, this issue would be listed in an appropriate place in
docs/features/, in the hope that it gets considered and addressed in the
future.
However, the xs_restrict() library function will clearly fail in some
way when cxenstored is in use, and nothing currently uses it, so the
lack of it also working when using a xenstored stubdomain doesn't
constitute a reduction in functionality.
Longterm, a sensible solution would be to make a xenstore protocol
extension to wrap an existing xenstore message in a restrict wrapper,
where the kernel device can apply the appropriate restrict around user
requests. This isn't the only protocol extension required; there is an
existing problem XenServer has hit that a xenstore-ls response when
enough domains are running will exceed XENSTORE_MAX_PAYLOAD, and fail.
Someone is going to have to fix that at some point - might as well do
these both at once.
~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V3 09/10] xen/arm: io: Use binary search for mmio handler lookup
Hi Julien,
On 06/28/2016 05:49 AM, Julien Grall wrote:
Hi Shanker,
On 27/06/16 21:33, Shanker Donthineni wrote:
As the number of I/O handlers increase, the overhead associated with
linear lookup also increases. The system might have maximum of 144
(assuming CONFIG_NR_CPUS=128) mmio handlers. In worst case scenario,
it would require 144 iterations for finding a matching handler. Now
it is time for us to change from linear (complexity O(n)) to a binary
search (complexity O(log n) for reducing mmio handler lookup overhead.
Signed-off-by: Shanker Donthineni
---
Changes since v2:
Converted mmio lookup code to a critical section.
Copied the function bsreach() from Linux kernel.
xen/arch/arm/io.c | 97
+++
1 file changed, 84 insertions(+), 13 deletions(-)
diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
index a5b2c2d..c31fdf3 100644
--- a/xen/arch/arm/io.c
+++ b/xen/arch/arm/io.c
@@ -20,9 +20,50 @@
#include
#include
#include
+#include
#include
#include
+/*
+ * bsearch - binary search an array of elements
+ * @key: pointer to item being searched for
+ * @base: pointer to first element to search
+ * @num: number of elements
+ * @size: size of each element
+ * @cmp: pointer to comparison function
+ *
+ * This function does a binary search on the given array. The
+ * contents of the array should already be in ascending sorted order
+ * under the provided comparison function.
+ *
+ * Note that the key need not have the same type as the elements in
+ * the array, e.g. key could be a string and the comparison function
+ * could compare the string with the struct's name field. However, if
+ * the key and elements in the array are of the same type, you can use
+ * the same comparison function for both sort() and bsearch().
+ */
+static void *bsearch(const void *key, const void *base, size_t num,
size_t size,
+ int (*cmp)(const void *key, const void *elt))
This function is not specific to I/O handlers. So this should be moved
to common code. Also please mention in the commit message where the
code came from.
Should I move to xen/arch/arm folder?
+{
+size_t start = 0, end = num;
+int result;
+
+while ( start < end )
+{
+size_t mid = start + (end - start) / 2;
+
+result = cmp(key, base + mid * size);
+if ( result < 0 )
+end = mid;
+else if ( result > 0 )
+start = mid + 1;
+else
+return (void *)base + mid * size;
+}
+
+return NULL;
+}
+
static int handle_read(const struct mmio_handler *handler, struct vcpu
*v,
mmio_info_t *info)
{
@@ -70,23 +111,41 @@ static int handle_write(const struct mmio_handler
*handler, struct vcpu *v,
handler->priv);
}
-int handle_mmio(mmio_info_t *info)
+static int match_mmio_handler(const void *key, const void *elem)
{
-struct vcpu *v = current;
-int i;
-const struct mmio_handler *handler = NULL;
-const struct vmmio *vmmio = &v->domain->arch.vmmio;
+const struct mmio_handler *handler = elem;
+paddr_t addr = (paddr_t)key;
-for ( i = 0; i < vmmio->num_entries; i++ )
-{
-handler = &vmmio->handlers[i];
+if ( addr < handler->addr )
+return -1;
-if ( (info->gpa >= handler->addr) &&
- (info->gpa < (handler->addr + handler->size)) )
-break;
-}
+if ( addr > (handler->addr + handler->size) )
+return 1;
+
+return 0;
+}
-if ( i == vmmio->num_entries )
+static const struct mmio_handler *
+find_mmio_handler(struct vcpu *v, paddr_t addr)
+{
+struct vmmio *vmmio = &v->domain->arch.vmmio;
+const struct mmio_handler *handler;
+
+spin_lock(&vmmio->lock);
+handler = bsearch((const void *)addr, vmmio->handlers,
vmmio->num_entries,
paddr_t is always 64-bit regardless the architecture (ARM64 vs ARM32).
So the cast will lead to a compilation error on ARM32.
I'll fix.
Please try to at least compile test your patch with ARM64, ARM32 and
x86 (when you touch common code).
Thanks, I'll follow next time.
Anyway, I would try to merge the two compare functions
(match_mmio_handler, cmp_mmio_handler) which have very similar behavior.
Yes, they are not exactly same. One compares only start address and
other one compares the range.
+ sizeof(*handler), match_mmio_handler);
+spin_unlock(&vmmio->lock);
+
+return handler;
+}
+
+int handle_mmio(mmio_info_t *info)
+{
+const struct mmio_handler *handler;
+struct vcpu *v = current;
+
+handler = find_mmio_handler(v, info->gpa);
+if ( !handler )
return 0;
if ( info->dabt.write )
@@ -95,6 +154,14 @@ int handle_mmio(mmio_info_t *info)
return handle_read(handler, v, info);
}
+static int cmp_mmio_handler(const void *key, const void *elem)
+{
+const struct mmio_handler *handler0 = key;
+const struct mmio_handler
Re: [Xen-devel] [PATCH V3 09/10] xen/arm: io: Use binary search for mmio handler lookup
On 28/06/16 14:19, Shanker Donthineni wrote: On 06/28/2016 05:49 AM, Julien Grall wrote: On 27/06/16 21:33, Shanker Donthineni wrote: As the number of I/O handlers increase, the overhead associated with linear lookup also increases. The system might have maximum of 144 (assuming CONFIG_NR_CPUS=128) mmio handlers. In worst case scenario, it would require 144 iterations for finding a matching handler. Now it is time for us to change from linear (complexity O(n)) to a binary search (complexity O(log n) for reducing mmio handler lookup overhead. Signed-off-by: Shanker Donthineni --- Changes since v2: Converted mmio lookup code to a critical section. Copied the function bsreach() from Linux kernel. xen/arch/arm/io.c | 97 +++ 1 file changed, 84 insertions(+), 13 deletions(-) diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c index a5b2c2d..c31fdf3 100644 --- a/xen/arch/arm/io.c +++ b/xen/arch/arm/io.c @@ -20,9 +20,50 @@ #include #include #include +#include #include #include +/* + * bsearch - binary search an array of elements + * @key: pointer to item being searched for + * @base: pointer to first element to search + * @num: number of elements + * @size: size of each element + * @cmp: pointer to comparison function + * + * This function does a binary search on the given array. The + * contents of the array should already be in ascending sorted order + * under the provided comparison function. + * + * Note that the key need not have the same type as the elements in + * the array, e.g. key could be a string and the comparison function + * could compare the string with the struct's name field. However, if + * the key and elements in the array are of the same type, you can use + * the same comparison function for both sort() and bsearch(). + */ +static void *bsearch(const void *key, const void *base, size_t num, size_t size, + int (*cmp)(const void *key, const void *elt)) This function is not specific to I/O handlers. So this should be moved to common code. Also please mention in the commit message where the code came from. Should I move to xen/arch/arm folder? To xen/common/ [...] Anyway, I would try to merge the two compare functions (match_mmio_handler, cmp_mmio_handler) which have very similar behavior. Yes, they are not exactly same. One compares only start address and other one compares the range. I don't think this will be an issue to compare the range for both and it will avoid to duplicate the code. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Crash in xen 4.7 adding nic during domu startup
>>> On 28.06.16 at 13:23, wrote: > A clue on doing this would be useful, I can't debug what is now release > code all day. Well, debugging (released code or not) is what's needed, I'm afraid. A first step would be to find out how far the corruption extends: Considering the non-zero code bytes that got dumped, I would have expected the 0xE8 to be a function call opcode, but then the exception couldn't have occurred on the 4th byte after. So I assume corruption extends beyond the zeroed range, and you could figure this out by comparing with the actual binary. If the range of the corrupted area doesn't provide any clues how or when the corruption occurs, I'd then add debugging code which, at coarse intervals, would inspect the area in question. Of course you need to be prepare for the corruption area to move relative to symbols. But in the end this is a pretty unusual situation, where I don't think I can provide pre-cooked debugging instructions. But of course there are other angles to look at this from: You say you use GPU pass-through. What about the case when you don't? And while you appear to try to make logging more verbose by using a (non-existent) "verbose" command line option, using "loglvl=all guest_loglvl=all iommu=debug" would yield better results. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] making xenstore domain easy configurable
On 28/06/16 14:42, Andrew Cooper wrote:
> On 28/06/16 12:56, Juergen Gross wrote:
>> On 28/06/16 13:03, Ian Jackson wrote:
>>> Juergen Gross writes ("Re: [Xen-devel] making xenstore domain easy
>>> configurable"):
So you are telling me the xenstore domain won't work for this case?
>>> Yes.
>> That's rather unfortunate. So in order to be able to make xenstore
>> domain a common setup we need to find a solution for support of
>> xs_restrict() via xenbus, right?
>>
>> TBH, the way xs_restrict() was introduced is rather weird. It is
>> completely bound to the socket interface of oxenstored. So anyone
>> wanting to use xs_restrict() is limited to oxenstored running in
>> dom0. No way to use xenstored or a xenstore domain. I'm really
>> disappointed such a design was accepted and is now the reason for
>> not being able to disaggregate dom0.
>>
>> I've searched through the xen-devel archives and found a very
>> interesting mail:
>>
>> http://lists.xen.org/archives/html/xen-devel/2010-04/msg01318.html
>>
>> The "restrict" feature was added without any further discussion how
>> it is implemented and that the C-variant doesn't support it. The
>> explicit question about non-existing features in the C xenstored was
>> answered just with "the xenstore wire protocol doesn't change".
>>
>> With:
>>
>> http://lists.xen.org/archives/html/xen-devel/2010-07/msg00091.html
>>
>> the XS_RESTRICT value in xs_wire.h (aah, suddenly it was changed?)
>> was added. Again no mentioning of the special implementation in
>> oxenstored.
>>
>> Really, this is not how open source development should be done!
>> Maybe I'm just upset now, but I'm in favor of dropping xs_restrict()
>> support as it has been introduced in a foul way.
>
> I don't think the lack of xs_restrict() working over the ring should
> preclude these improvements to the configuration of how xenstored starts up.
It is limiting the solution by not allowing me to drop the sockets
completely.
> Longterm, a sensible solution would be to make a xenstore protocol
> extension to wrap an existing xenstore message in a restrict wrapper,
> where the kernel device can apply the appropriate restrict around user
> requests.
I'd rather let xs_restrict() work in a clean way: setup a new ring and
event channel with the appropriate privileges and let the connection
to xenstore run via this ring.
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 11/17] libxl/arm: Construct ACPI DSDT table
On 06/28/2016 07:03 AM, Shannon Zhao wrote:
>
> On 2016/6/27 20:05, Boris Ostrovsky wrote:
>>
>> On 06/27/2016 06:29 AM, Julien Grall wrote:
>>> (CC Boris and Doug)
>>>
>>> Hi Shannon,
>>>
>>> On 27/06/16 07:01, Shannon Zhao wrote:
On 2016/6/24 1:03, Julien Grall wrote:
> On 23/06/16 04:16, Shannon Zhao wrote:
>
> [...]
>
>> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
>> index 264b6ef..5347480 100644
>> --- a/tools/libxl/Makefile
>> +++ b/tools/libxl/Makefile
>> @@ -77,7 +77,29 @@ endif
>>
>>LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o libxl_x86.o libxl_psr.o
>>LIBXL_OBJS-$(CONFIG_ARM) += libxl_nocpuid.o libxl_arm.o
>> libxl_libfdt_compat.o
>> -LIBXL_OBJS-$(CONFIG_ARM) += libxl_arm_acpi.o
>> +LIBXL_OBJS-$(CONFIG_ARM) += libxl_arm_acpi.o libxl_dsdt_anycpu_arm.o
>> +
>> +vpath iasl $(PATH)
>> +libxl_mk_dsdt_arm: libxl_mk_dsdt_arm.c
>> +$(CC) $(CFLAGS) -o $@ libxl_mk_dsdt_arm.c
>> +
>> +libxl_dsdt_anycpu_arm.asl: libxl_empty_dsdt_arm.asl libxl_mk_dsdt_arm
>> +awk 'NR > 1 {print s} {s=$$0}' $< > $@
>> +./libxl_mk_dsdt_arm >> $@
>> +
>> +libxl_dsdt_anycpu_arm.c: %.c: iasl %.asl
>> +iasl -vs -p $* -tc $*.asl
>> +sed -e 's/AmlCode/$*/g' $*.hex >$@
>> +echo "int $*_len=sizeof($*);" >>$@
>> +rm -f $*.aml $*.hex
>> +
> I don't like the idea to add iasl as a dependency for all ARM
> platforms.
> For instance ARMv7 platform will not use ACPI, but we still ask
> users to
> install iasl. So I think we should allow the user to opt-in/opt-out for
> ACPI.
>
> Any opinions?
>
I agree. But how to exclude for ARMv7. I notice it only has the option
CONFIG_ARM which doesn't distinguish ARM32 and ARM64.
>>> I am not sure if we plan to introduce Kconfig for tools. If not, you
>>> can add an option to the configure to enable/disable ACPI for guest.
>>>
>>> This would be gated by the presence of "iasl".
>>>
>>> [...]
>>>
>> diff --git a/tools/libxl/libxl_mk_dsdt_arm.c
>> b/tools/libxl/libxl_mk_dsdt_arm.c
>> new file mode 100644
>> index 000..96fadbd
>> --- /dev/null
>> +++ b/tools/libxl/libxl_mk_dsdt_arm.c
> Can we share the code from tools/firmware/acpi/mk_dsdt.c?
>
Yeah, we can share push_block(), pop_block() stmt() and indent() but the
main() function is totally different since there are only the processor
device objects for ARM DSDT but there are many other things in x86.
I think that since Boris will move the codes under
tools/firmware/hvmloader/acpi to other place, after that we could see
how to share codes then.
>>> I would prefer if we discuss about it now in order to avoid code
>>> duplication (I have CCed Boris).
>>>
>>> For instance we can create a new directory under tools for mk_dsdt.c.
>>> The main could be different, although it might be possible to gate ARM
>>> options, and the rest of the code would be shared.
>>
>> So I think we decided earlier to keep ARM and x86 ACPI builders
>> separate, at least for now.
> I think so as well.
>
>> However, looking at the Makefile and mk_dsdt
>> I wonder whether it would make sense to put the builders in the same
>> directory (I am currently using tools/libacpi) so that those two files
>> can be kept common as much as possible, with the sources being
>> different. E.g. something like
>>
>> tools/libacpi:
>> Makefile
>> mk_dsdt.c
>> acpi_x86.[ch]
>> acpi_arm.[ch]
>> *asl
>> etc.
>>
>> The objects will be built in tools/libxl (there will be no libacpi.so)
>> but the infrastructure and sources will live together.
> I'm fine with this. But I think the patch moving the codes into
> tools/libacpi should be posted firstly, since this series depend on it.
> Boris, could you please send that patch? Then I can add the
> corresponding ARM patch on top of that.
I thought I had it almost ready yesterday but I encountered a problem
that I need to resolve before I post it. If I don't get it fixed in the
next couple of days I will send you a link to my repository so that you
can see what I have.
-boris
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] xen: x86: remove duplicated IA32_FEATURE_CONTROL MSR macro
>>> On 28.06.16 at 10:12, wrote: > From: Kai Huang > > Below commit introduced a new macro MSR_IA32_FEATURE_CONTROL for > IA32_FEATURE_CONTROL MSR but it didn't remove old IA32_FEATURE_CONTROL_MSR > macro. The new one has better naming convention, so remove the old as a > duplication. Also move the macros of bit definition of IA32_FEATURE_CONTROL > MSR > down to make them together with the new one. The *_MSR* infix is also > removed as > it is pointless. > > commit 5a211704e8813c4890c8ce8dc4189d1dfb35ecd0 > Author: Len Brown > Date: Fri Apr 8 22:31:47 2016 +0200 > > mwait-idle: prevent SKL-H boot failure when C8+C9+C10 enabled > > Some SKL-H configurations require "max_cstate=7" to boot. > While that is an effective workaround, it disables C10. > > .. > > Above commit also used SGX_ENABLE (bit 18) in IA32_FEATURE_CONTROL MSR > without a > macro for it. A new macro IA32_FEATURE_CONTROL_SGX_ENABLE is also added for > better code and future use. > > Relevant code that uses those macros are changed accordingly. > > Signed-off-by: Kai Huang Reviewed-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V3 04/10] arm/gic-v3: Parse per-cpu redistributor entry in GICC subtable
Hi Julien,
On 06/28/2016 05:40 AM, Julien Grall wrote:
Hello Shanker,
On 27/06/16 21:33, Shanker Donthineni wrote:
@@ -1397,6 +1408,36 @@ gic_acpi_parse_madt_distributor(struct
acpi_subtable_header *header,
}
static int __init
+gic_acpi_parse_cpu_redistributor(struct acpi_subtable_header *header,
+ const unsigned long end)
+{
+struct acpi_madt_generic_interrupt *processor;
+u32 size;
+
+processor = (struct acpi_madt_generic_interrupt *)header;
+if ( !(processor->flags & ACPI_MADT_ENABLED) )
+return 0;
You did not answer to my question on previous version of this patch.
You said that "Disabled GICC entries should be skipped because its
Redistributor region is not always-on power domain." However from my
understanding, an usable CPU may have his Redistributor in the not
always-on power domain. So the issue would the same, correct?
The gicv3_populate_rdist() is not supposed to read GICR registers if
the the associated hardware GICR block is in power-off state. The CPU
accesses to disabled GICR region leads to either a system hang or an
unexpected behavior.
Regards,
--
Shanker Donthineni
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
Foundation Collaborative Project
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] making xenstore domain easy configurable
On 28/06/16 14:42, Andrew Cooper wrote:
> On 28/06/16 12:56, Juergen Gross wrote:
>> On 28/06/16 13:03, Ian Jackson wrote:
>>> Juergen Gross writes ("Re: [Xen-devel] making xenstore domain easy
>>> configurable"):
So you are telling me the xenstore domain won't work for this case?
>>> Yes.
>> That's rather unfortunate. So in order to be able to make xenstore
>> domain a common setup we need to find a solution for support of
>> xs_restrict() via xenbus, right?
>>
>> TBH, the way xs_restrict() was introduced is rather weird. It is
>> completely bound to the socket interface of oxenstored. So anyone
>> wanting to use xs_restrict() is limited to oxenstored running in
>> dom0. No way to use xenstored or a xenstore domain. I'm really
>> disappointed such a design was accepted and is now the reason for
>> not being able to disaggregate dom0.
>>
>> I've searched through the xen-devel archives and found a very
>> interesting mail:
>>
>> http://lists.xen.org/archives/html/xen-devel/2010-04/msg01318.html
>>
>> The "restrict" feature was added without any further discussion how
>> it is implemented and that the C-variant doesn't support it. The
>> explicit question about non-existing features in the C xenstored was
>> answered just with "the xenstore wire protocol doesn't change".
>>
>> With:
>>
>> http://lists.xen.org/archives/html/xen-devel/2010-07/msg00091.html
>>
>> the XS_RESTRICT value in xs_wire.h (aah, suddenly it was changed?)
>> was added. Again no mentioning of the special implementation in
>> oxenstored.
>>
>> Really, this is not how open source development should be done!
>> Maybe I'm just upset now, but I'm in favor of dropping xs_restrict()
>> support as it has been introduced in a foul way.
>
> I don't think the lack of xs_restrict() working over the ring should
> preclude these improvements to the configuration of how xenstored starts up.
>
> Ideally, this issue would be listed in an appropriate place in
> docs/features/, in the hope that it gets considered and addressed in the
> future.
Digging a little bit deeper I think the current xs_restrict()
implementation renders an oxenstore domain completely useless: as
soon as dom0 tries to use xs_restrict() it will loose its privileges
as the complete dom0 connection will be affected. :-(
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] making xenstore domain easy configurable
On 28/06/16 14:52, Juergen Gross wrote:
> On 28/06/16 14:42, Andrew Cooper wrote:
>> On 28/06/16 12:56, Juergen Gross wrote:
>>> On 28/06/16 13:03, Ian Jackson wrote:
Juergen Gross writes ("Re: [Xen-devel] making xenstore domain easy
configurable"):
> So you are telling me the xenstore domain won't work for this case?
Yes.
>>> That's rather unfortunate. So in order to be able to make xenstore
>>> domain a common setup we need to find a solution for support of
>>> xs_restrict() via xenbus, right?
>>>
>>> TBH, the way xs_restrict() was introduced is rather weird. It is
>>> completely bound to the socket interface of oxenstored. So anyone
>>> wanting to use xs_restrict() is limited to oxenstored running in
>>> dom0. No way to use xenstored or a xenstore domain. I'm really
>>> disappointed such a design was accepted and is now the reason for
>>> not being able to disaggregate dom0.
>>>
>>> I've searched through the xen-devel archives and found a very
>>> interesting mail:
>>>
>>> http://lists.xen.org/archives/html/xen-devel/2010-04/msg01318.html
>>>
>>> The "restrict" feature was added without any further discussion how
>>> it is implemented and that the C-variant doesn't support it. The
>>> explicit question about non-existing features in the C xenstored was
>>> answered just with "the xenstore wire protocol doesn't change".
>>>
>>> With:
>>>
>>> http://lists.xen.org/archives/html/xen-devel/2010-07/msg00091.html
>>>
>>> the XS_RESTRICT value in xs_wire.h (aah, suddenly it was changed?)
>>> was added. Again no mentioning of the special implementation in
>>> oxenstored.
>>>
>>> Really, this is not how open source development should be done!
>>> Maybe I'm just upset now, but I'm in favor of dropping xs_restrict()
>>> support as it has been introduced in a foul way.
>> I don't think the lack of xs_restrict() working over the ring should
>> preclude these improvements to the configuration of how xenstored starts up.
>>
>> Ideally, this issue would be listed in an appropriate place in
>> docs/features/, in the hope that it gets considered and addressed in the
>> future.
> Digging a little bit deeper I think the current xs_restrict()
> implementation renders an oxenstore domain completely useless: as
> soon as dom0 tries to use xs_restrict() it will loose its privileges
> as the complete dom0 connection will be affected. :-(
Can't say I am surprised in the slightest. It is an extension which has
never been used, which invariably means it doesn't work.
(More replies on the other half of this thread, which I am still writing.)
~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] making xenstore domain easy configurable
On 28/06/16 14:36, Juergen Gross wrote:
> On 28/06/16 14:42, Andrew Cooper wrote:
>> On 28/06/16 12:56, Juergen Gross wrote:
>>> On 28/06/16 13:03, Ian Jackson wrote:
Juergen Gross writes ("Re: [Xen-devel] making xenstore domain easy
configurable"):
> So you are telling me the xenstore domain won't work for this case?
Yes.
>>> That's rather unfortunate. So in order to be able to make xenstore
>>> domain a common setup we need to find a solution for support of
>>> xs_restrict() via xenbus, right?
>>>
>>> TBH, the way xs_restrict() was introduced is rather weird. It is
>>> completely bound to the socket interface of oxenstored. So anyone
>>> wanting to use xs_restrict() is limited to oxenstored running in
>>> dom0. No way to use xenstored or a xenstore domain. I'm really
>>> disappointed such a design was accepted and is now the reason for
>>> not being able to disaggregate dom0.
>>>
>>> I've searched through the xen-devel archives and found a very
>>> interesting mail:
>>>
>>> http://lists.xen.org/archives/html/xen-devel/2010-04/msg01318.html
>>>
>>> The "restrict" feature was added without any further discussion how
>>> it is implemented and that the C-variant doesn't support it. The
>>> explicit question about non-existing features in the C xenstored was
>>> answered just with "the xenstore wire protocol doesn't change".
>>>
>>> With:
>>>
>>> http://lists.xen.org/archives/html/xen-devel/2010-07/msg00091.html
>>>
>>> the XS_RESTRICT value in xs_wire.h (aah, suddenly it was changed?)
>>> was added. Again no mentioning of the special implementation in
>>> oxenstored.
>>>
>>> Really, this is not how open source development should be done!
>>> Maybe I'm just upset now, but I'm in favor of dropping xs_restrict()
>>> support as it has been introduced in a foul way.
>> I don't think the lack of xs_restrict() working over the ring should
>> preclude these improvements to the configuration of how xenstored starts up.
> It is limiting the solution by not allowing me to drop the sockets
> completely.
I don't think dropping the sockets completely is a sensible course of
action. I had come the conclusion that you were just not going to use
them, as opposed to removing them entirely.
For xenstored running in the same domain as the toolstack, sockets are
less overhead than the shared memory ring, as no hypercalls are
involved. There is also the unfortunate problem that one of the two
linux devices for xenstored *still* causes deadlocks when used; a
problem which is unresolved from Linux 3.14.
>
>> Longterm, a sensible solution would be to make a xenstore protocol
>> extension to wrap an existing xenstore message in a restrict wrapper,
>> where the kernel device can apply the appropriate restrict around user
>> requests.
> I'd rather let xs_restrict() work in a clean way: setup a new ring and
> event channel with the appropriate privileges and let the connection
> to xenstore run via this ring.
Domains currently don't have any notion of multiple xenstore-rings to
the same domain. Somewhere (i.e. in xenstored itself) there would have
to be some kind of hard upper limit to avoid resource exhaustion, and
guest kernels would still have to have some privileged way of
negotiating the setup. Also, how do you plan to make any of this work
with xenstored not in a subdomain?
It is far easier, and IMO sensible, just to mux something new over the
existing ring.
~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH] x86/EFI + Live Patch: avoid symbol address truncation
ld associates __init_end, placed outside of any section by the linker
script, with the following section, resulting in a huge (wrapped, as it
would be negative) section relative offset. COFF symbol tables store
section relative addresses, and hence the above leads to assembler
truncation warnings when all symbols get included in the symbol table
(for Live Patching code). To overcome this, move __init_end past both
ALIGN() directives. The consuming code (init_done()) is fine with such
an adjustment (the distinction really would only be relevant for the
loop claring the pages, and I think it's acceptable to clear a few
more on - for now - EFI). This effectively results in the
(__init_begin,__init_end) and (__2M_init_start,__2M_init_end) pairs to
become identical, with their different names only serving documentation
purposes now.
Note that moving __init_end and __2M_init_end into .init is not a good
idea, as that would significantly grow xen.efi binary size.
While inspecting symbol table and ld behavior I also noticed that
__2M_text_start gets put at address zero in the EFI case, which hasn't
caused problems solely because we don't actually reference that symbol.
Correct the setting of the initial address, and comment out said symbol
for the time being, as with the initial address correction it would in
turn cause an assembler truncation warning similar to the one mentioned
above.
While checking init_done() for correctness with the above changes I
noticed that code can easily be folded there, at once correcting the
logged amount of memory which has got freed for the 2M-alignment case
(i.e. EFI right now).
Signed-off-by: Jan Beulich
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -515,6 +515,7 @@ static inline bool_t using_2M_mapping(vo
static void noinline init_done(void)
{
void *va;
+unsigned long start, end;
system_state = SYS_STATE_active;
@@ -530,18 +531,18 @@ static void noinline init_done(void)
/* Destroy Xen's mappings, and reuse the pages. */
if ( using_2M_mapping() )
{
-destroy_xen_mappings((unsigned long)&__2M_init_start,
- (unsigned long)&__2M_init_end);
-init_xenheap_pages(__pa(__2M_init_start), __pa(__2M_init_end));
+start = (unsigned long)&__2M_init_start,
+end = (unsigned long)&__2M_init_end;
}
else
{
-destroy_xen_mappings((unsigned long)&__init_begin,
- (unsigned long)&__init_end);
-init_xenheap_pages(__pa(__init_begin), __pa(__init_end));
+start = (unsigned long)&__init_begin;
+end = (unsigned long)&__init_end;
}
-printk("Freed %ldkB init memory.\n", (long)(__init_end-__init_begin)>>10);
+destroy_xen_mappings(start, end);
+init_xenheap_pages(__pa(start), __pa(end));
+printk("Freed %ldkB init memory\n", (end - start) >> 10);
startup_cpu_idle_loop();
}
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -40,9 +40,20 @@ SECTIONS
#if !defined(EFI)
. = __XEN_VIRT_START;
__image_base__ = .;
+#else
+ . = __image_base__;
#endif
+#if 0
+/*
+ * We don't really use this symbol anywhere, and the way it would get defined
+ * here would result in it having a negative (wrapped to huge positive)
+ * offset relative to the .text section. That, in turn, causes an assembler
+ * truncation warning when including all symbols in the symbol table for Live
+ * Patching code.
+ */
__2M_text_start = .; /* Start of 2M superpages, mapped RX. */
+#endif
. = __XEN_VIRT_START + MB(1);
_start = .;
@@ -194,14 +205,13 @@ SECTIONS
*(.ctors)
__ctors_end = .;
} :text
- . = ALIGN(PAGE_SIZE);
- __init_end = .;
#ifdef EFI
. = ALIGN(MB(2));
#else
. = ALIGN(PAGE_SIZE);
#endif
+ __init_end = .;
__2M_init_end = .;
__2M_rwdata_start = .; /* Start of 2M superpages, mapped RW. */
@@ -296,7 +306,6 @@ ASSERT(__image_base__ > XEN_VIRT_START |
ASSERT(kexec_reloc_size - kexec_reloc <= PAGE_SIZE, "kexec_reloc is too large")
#endif
-ASSERT(IS_ALIGNED(__2M_text_start, MB(2)), "__2M_text_start misaligned")
#ifdef EFI
ASSERT(IS_ALIGNED(__2M_text_end, MB(2)), "__2M_text_end misaligned")
ASSERT(IS_ALIGNED(__2M_rodata_start, MB(2)), "__2M_rodata_start misaligned")
x86/EFI + Live Patch: avoid symbol address truncation
ld associates __init_end, placed outside of any section by the linker
script, with the following section, resulting in a huge (wrapped, as it
would be negative) section relative offset. COFF symbol tables store
section relative addresses, and hence the above leads to assembler
truncation warnings when all symbols get included in the symbol table
(for Live Patching code). To overcome this, move __init_end past both
ALIGN() directives. The consuming code (init_done()) is fine with such
an adjustment (the distinction really would only be relevant for the
loop claring the pages, and I think it's acceptable to clear a few
more
Re: [Xen-devel] [PATCH] x86/EFI + Live Patch: avoid symbol address truncation
On 28/06/16 15:03, Jan Beulich wrote:
> ld associates __init_end, placed outside of any section by the linker
> script, with the following section, resulting in a huge (wrapped, as it
> would be negative) section relative offset.
So in this case, the cause of the truncation is due to __init_end being
considered relative to .data.read_mostly?
> COFF symbol tables store
> section relative addresses, and hence the above leads to assembler
> truncation warnings when all symbols get included in the symbol table
> (for Live Patching code). To overcome this, move __init_end past both
> ALIGN() directives. The consuming code (init_done()) is fine with such
> an adjustment (the distinction really would only be relevant for the
> loop claring the pages, and I think it's acceptable to clear a few
> more on - for now - EFI). This effectively results in the
> (__init_begin,__init_end) and (__2M_init_start,__2M_init_end) pairs to
> become identical, with their different names only serving documentation
> purposes now.
>
> Note that moving __init_end and __2M_init_end into .init is not a good
> idea, as that would significantly grow xen.efi binary size.
How about moving just __init_end ? That shouldn't affect the size of
any binary, due to the existing page alignment between sections.
>
> While inspecting symbol table and ld behavior I also noticed that
> __2M_text_start gets put at address zero in the EFI case, which hasn't
> caused problems solely because we don't actually reference that symbol.
The reason that __2M_text_start isn't referenced is because I couldn't
get the EFI build working. It was used in my first prototype.
> Correct the setting of the initial address, and comment out said symbol
> for the time being, as with the initial address correction it would in
> turn cause an assembler truncation warning similar to the one mentioned
> above.
>
> While checking init_done() for correctness with the above changes I
> noticed that code can easily be folded there, at once correcting the
> logged amount of memory which has got freed for the 2M-alignment case
> (i.e. EFI right now).
>
> Signed-off-by: Jan Beulich
>
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -515,6 +515,7 @@ static inline bool_t using_2M_mapping(vo
> static void noinline init_done(void)
> {
> void *va;
> +unsigned long start, end;
>
> system_state = SYS_STATE_active;
>
> @@ -530,18 +531,18 @@ static void noinline init_done(void)
> /* Destroy Xen's mappings, and reuse the pages. */
> if ( using_2M_mapping() )
> {
> -destroy_xen_mappings((unsigned long)&__2M_init_start,
> - (unsigned long)&__2M_init_end);
> -init_xenheap_pages(__pa(__2M_init_start), __pa(__2M_init_end));
> +start = (unsigned long)&__2M_init_start,
> +end = (unsigned long)&__2M_init_end;
> }
> else
> {
> -destroy_xen_mappings((unsigned long)&__init_begin,
> - (unsigned long)&__init_end);
> -init_xenheap_pages(__pa(__init_begin), __pa(__init_end));
> +start = (unsigned long)&__init_begin;
> +end = (unsigned long)&__init_end;
> }
>
> -printk("Freed %ldkB init memory.\n",
> (long)(__init_end-__init_begin)>>10);
> +destroy_xen_mappings(start, end);
> +init_xenheap_pages(__pa(start), __pa(end));
> +printk("Freed %ldkB init memory\n", (end - start) >> 10);
The parameter is now unsigned, so %lu.
>
> startup_cpu_idle_loop();
> }
> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -40,9 +40,20 @@ SECTIONS
> #if !defined(EFI)
>. = __XEN_VIRT_START;
>__image_base__ = .;
> +#else
> + . = __image_base__;
> #endif
>
> +#if 0
> +/*
> + * We don't really use this symbol anywhere, and the way it would get defined
> + * here would result in it having a negative (wrapped to huge positive)
> + * offset relative to the .text section. That, in turn, causes an assembler
> + * truncation warning when including all symbols in the symbol table for Live
> + * Patching code.
> + */
>__2M_text_start = .; /* Start of 2M superpages, mapped RX. */
> +#endif
>
>. = __XEN_VIRT_START + MB(1);
>_start = .;
> @@ -194,14 +205,13 @@ SECTIONS
> *(.ctors)
> __ctors_end = .;
>} :text
> - . = ALIGN(PAGE_SIZE);
> - __init_end = .;
>
> #ifdef EFI
>. = ALIGN(MB(2));
> #else
>. = ALIGN(PAGE_SIZE);
> #endif
> + __init_end = .;
>__2M_init_end = .;
>
>__2M_rwdata_start = .; /* Start of 2M superpages, mapped RW. */
> @@ -296,7 +306,6 @@ ASSERT(__image_base__ > XEN_VIRT_START |
> ASSERT(kexec_reloc_size - kexec_reloc <= PAGE_SIZE, "kexec_reloc is too
> large")
> #endif
>
> -ASSERT(IS_ALIGNED(__2M_text_start, MB(2)), "__2M_text_start misaligned")
If we are #if 0'ing the symbol for documentation purposes, can we #if 0
this as well?
~Andrew
> #ifdef EFI
> ASSERT(IS_ALIGNED(__2M_text_end, MB(
Re: [Xen-devel] [PATCH V3 04/10] arm/gic-v3: Parse per-cpu redistributor entry in GICC subtable
On 06/28/2016 08:51 AM, Shanker Donthineni wrote:
Hi Julien,
On 06/28/2016 05:40 AM, Julien Grall wrote:
Hello Shanker,
On 27/06/16 21:33, Shanker Donthineni wrote:
@@ -1397,6 +1408,36 @@ gic_acpi_parse_madt_distributor(struct
acpi_subtable_header *header,
}
static int __init
+gic_acpi_parse_cpu_redistributor(struct acpi_subtable_header *header,
+ const unsigned long end)
+{
+struct acpi_madt_generic_interrupt *processor;
+u32 size;
+
+processor = (struct acpi_madt_generic_interrupt *)header;
+if ( !(processor->flags & ACPI_MADT_ENABLED) )
+return 0;
You did not answer to my question on previous version of this patch.
You said that "Disabled GICC entries should be skipped because its
Redistributor region is not always-on power domain." However from my
understanding, an usable CPU may have his Redistributor in the not
always-on power domain. So the issue would the same, correct?
The gicv3_populate_rdist() is not supposed to read GICR registers if
the the associated hardware GICR block is in power-off state. The CPU
accesses to disabled GICR region leads to either a system hang or an
unexpected behavior.
The description of flag ACPI_MADT_ENABLED in ACPI-6.1 says "If zero,
this processor in unusable, and the operating system support will not
attempt to use it".
Regards,
--
Shanker Donthineni
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
Foundation Collaborative Project
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] making xenstore domain easy configurable
On 28/06/16 15:59, Andrew Cooper wrote:
> On 28/06/16 14:36, Juergen Gross wrote:
>> On 28/06/16 14:42, Andrew Cooper wrote:
>>> On 28/06/16 12:56, Juergen Gross wrote:
On 28/06/16 13:03, Ian Jackson wrote:
> Juergen Gross writes ("Re: [Xen-devel] making xenstore domain easy
> configurable"):
>> So you are telling me the xenstore domain won't work for this case?
> Yes.
That's rather unfortunate. So in order to be able to make xenstore
domain a common setup we need to find a solution for support of
xs_restrict() via xenbus, right?
TBH, the way xs_restrict() was introduced is rather weird. It is
completely bound to the socket interface of oxenstored. So anyone
wanting to use xs_restrict() is limited to oxenstored running in
dom0. No way to use xenstored or a xenstore domain. I'm really
disappointed such a design was accepted and is now the reason for
not being able to disaggregate dom0.
I've searched through the xen-devel archives and found a very
interesting mail:
http://lists.xen.org/archives/html/xen-devel/2010-04/msg01318.html
The "restrict" feature was added without any further discussion how
it is implemented and that the C-variant doesn't support it. The
explicit question about non-existing features in the C xenstored was
answered just with "the xenstore wire protocol doesn't change".
With:
http://lists.xen.org/archives/html/xen-devel/2010-07/msg00091.html
the XS_RESTRICT value in xs_wire.h (aah, suddenly it was changed?)
was added. Again no mentioning of the special implementation in
oxenstored.
Really, this is not how open source development should be done!
Maybe I'm just upset now, but I'm in favor of dropping xs_restrict()
support as it has been introduced in a foul way.
>>> I don't think the lack of xs_restrict() working over the ring should
>>> preclude these improvements to the configuration of how xenstored starts up.
>> It is limiting the solution by not allowing me to drop the sockets
>> completely.
>
> I don't think dropping the sockets completely is a sensible course of
> action. I had come the conclusion that you were just not going to use
> them, as opposed to removing them entirely.
If they are not going to be used they can be dropped, no?
Again: the main problem with the sockets is their systemd definition in
combination of their existence being used for the connection type with
xenstore (socket vs. kernel).
So either I always connect via the kernel making the sockets useless
(then I can remove them completely) or I have a way creating the
sockets only in case of the daemon case which is currently available
only by removing the systemd definition of the sockets.
> For xenstored running in the same domain as the toolstack, sockets are
> less overhead than the shared memory ring, as no hypercalls are
> involved. There is also the unfortunate problem that one of the two
> linux devices for xenstored *still* causes deadlocks when used; a
> problem which is unresolved from Linux 3.14.
So this would mean we should keep the sockets and just remove their
systemd definition.
>>> Longterm, a sensible solution would be to make a xenstore protocol
>>> extension to wrap an existing xenstore message in a restrict wrapper,
>>> where the kernel device can apply the appropriate restrict around user
>>> requests.
>> I'd rather let xs_restrict() work in a clean way: setup a new ring and
>> event channel with the appropriate privileges and let the connection
>> to xenstore run via this ring.
>
> Domains currently don't have any notion of multiple xenstore-rings to
> the same domain. Somewhere (i.e. in xenstored itself) there would have
> to be some kind of hard upper limit to avoid resource exhaustion, and
> guest kernels would still have to have some privileged way of
> negotiating the setup. Also, how do you plan to make any of this work
> with xenstored not in a subdomain?
The multiple ring setup should be allowed for dom0 (or similar
privileged domains) only in order to avoid resource exhaustion.
xs_restrict() is limited to dom0, too.
I haven't worked on a design for the implementation yet. I've just
presented a rough interface idea making it rather easy to distinguish
multiple instances by xenstore regardless whether those instances are
located in the same domain or not.
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] making xenstore domain easy configurable
On 28/06/16 15:58, Juergen Gross wrote:
> On 28/06/16 15:59, Andrew Cooper wrote:
>> On 28/06/16 14:36, Juergen Gross wrote:
>>> On 28/06/16 14:42, Andrew Cooper wrote:
On 28/06/16 12:56, Juergen Gross wrote:
> On 28/06/16 13:03, Ian Jackson wrote:
>> Juergen Gross writes ("Re: [Xen-devel] making xenstore domain easy
>> configurable"):
>>> So you are telling me the xenstore domain won't work for this case?
>> Yes.
> That's rather unfortunate. So in order to be able to make xenstore
> domain a common setup we need to find a solution for support of
> xs_restrict() via xenbus, right?
>
> TBH, the way xs_restrict() was introduced is rather weird. It is
> completely bound to the socket interface of oxenstored. So anyone
> wanting to use xs_restrict() is limited to oxenstored running in
> dom0. No way to use xenstored or a xenstore domain. I'm really
> disappointed such a design was accepted and is now the reason for
> not being able to disaggregate dom0.
>
> I've searched through the xen-devel archives and found a very
> interesting mail:
>
> http://lists.xen.org/archives/html/xen-devel/2010-04/msg01318.html
>
> The "restrict" feature was added without any further discussion how
> it is implemented and that the C-variant doesn't support it. The
> explicit question about non-existing features in the C xenstored was
> answered just with "the xenstore wire protocol doesn't change".
>
> With:
>
> http://lists.xen.org/archives/html/xen-devel/2010-07/msg00091.html
>
> the XS_RESTRICT value in xs_wire.h (aah, suddenly it was changed?)
> was added. Again no mentioning of the special implementation in
> oxenstored.
>
> Really, this is not how open source development should be done!
> Maybe I'm just upset now, but I'm in favor of dropping xs_restrict()
> support as it has been introduced in a foul way.
I don't think the lack of xs_restrict() working over the ring should
preclude these improvements to the configuration of how xenstored starts
up.
>>> It is limiting the solution by not allowing me to drop the sockets
>>> completely.
>> I don't think dropping the sockets completely is a sensible course of
>> action. I had come the conclusion that you were just not going to use
>> them, as opposed to removing them entirely.
> If they are not going to be used they can be dropped, no?
>
> Again: the main problem with the sockets is their systemd definition in
> combination of their existence being used for the connection type with
> xenstore (socket vs. kernel).
>
> So either I always connect via the kernel making the sockets useless
> (then I can remove them completely) or I have a way creating the
> sockets only in case of the daemon case which is currently available
> only by removing the systemd definition of the sockets.
>
>> For xenstored running in the same domain as the toolstack, sockets are
>> less overhead than the shared memory ring, as no hypercalls are
>> involved. There is also the unfortunate problem that one of the two
>> linux devices for xenstored *still* causes deadlocks when used; a
>> problem which is unresolved from Linux 3.14.
> So this would mean we should keep the sockets and just remove their
> systemd definition.
This seems like the best course of action, especially as it appears that
we don't make use of the systemd sockets in the way systemd likes.
As far as I can tell, this will cause xs_restrict() to work as it
currently does when you use oxenstored locally in dom0, in which case I
am happy that there is no net reduction in functionality.
~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] making xenstore domain easy configurable
On 6/28/16 8:59 AM, Andrew Cooper wrote:
> On 28/06/16 14:36, Juergen Gross wrote:
>> On 28/06/16 14:42, Andrew Cooper wrote:
>>> On 28/06/16 12:56, Juergen Gross wrote:
On 28/06/16 13:03, Ian Jackson wrote:
> Juergen Gross writes ("Re: [Xen-devel] making xenstore domain easy
> configurable"):
>> So you are telling me the xenstore domain won't work for this case?
> Yes.
That's rather unfortunate. So in order to be able to make xenstore
domain a common setup we need to find a solution for support of
xs_restrict() via xenbus, right?
TBH, the way xs_restrict() was introduced is rather weird. It is
completely bound to the socket interface of oxenstored. So anyone
wanting to use xs_restrict() is limited to oxenstored running in
dom0. No way to use xenstored or a xenstore domain. I'm really
disappointed such a design was accepted and is now the reason for
not being able to disaggregate dom0.
I've searched through the xen-devel archives and found a very
interesting mail:
http://lists.xen.org/archives/html/xen-devel/2010-04/msg01318.html
The "restrict" feature was added without any further discussion how
it is implemented and that the C-variant doesn't support it. The
explicit question about non-existing features in the C xenstored was
answered just with "the xenstore wire protocol doesn't change".
With:
http://lists.xen.org/archives/html/xen-devel/2010-07/msg00091.html
the XS_RESTRICT value in xs_wire.h (aah, suddenly it was changed?)
was added. Again no mentioning of the special implementation in
oxenstored.
Really, this is not how open source development should be done!
Maybe I'm just upset now, but I'm in favor of dropping xs_restrict()
support as it has been introduced in a foul way.
>>> I don't think the lack of xs_restrict() working over the ring should
>>> preclude these improvements to the configuration of how xenstored starts up.
>> It is limiting the solution by not allowing me to drop the sockets
>> completely.
>
> I don't think dropping the sockets completely is a sensible course of
> action. I had come the conclusion that you were just not going to use
> them, as opposed to removing them entirely.
>
> For xenstored running in the same domain as the toolstack, sockets are
> less overhead than the shared memory ring, as no hypercalls are
> involved. There is also the unfortunate problem that one of the two
> linux devices for xenstored *still* causes deadlocks when used; a
> problem which is unresolved from Linux 3.14.
Since Xen 4.7 the broken devices won't be used by default. I understand
that the socket interface is faster and less overhead but I guess my
question is how much data is sent over this interface? Does it really
matter the small performance difference to justify having two different
methods and not trimming the code base down to one method?
--
Doug Goldstein
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] making xenstore domain easy configurable
On 28/06/16 16:17, Doug Goldstein wrote:
> On 6/28/16 8:59 AM, Andrew Cooper wrote:
>> On 28/06/16 14:36, Juergen Gross wrote:
>>> On 28/06/16 14:42, Andrew Cooper wrote:
On 28/06/16 12:56, Juergen Gross wrote:
> On 28/06/16 13:03, Ian Jackson wrote:
>> Juergen Gross writes ("Re: [Xen-devel] making xenstore domain easy
>> configurable"):
>>> So you are telling me the xenstore domain won't work for this case?
>> Yes.
> That's rather unfortunate. So in order to be able to make xenstore
> domain a common setup we need to find a solution for support of
> xs_restrict() via xenbus, right?
>
> TBH, the way xs_restrict() was introduced is rather weird. It is
> completely bound to the socket interface of oxenstored. So anyone
> wanting to use xs_restrict() is limited to oxenstored running in
> dom0. No way to use xenstored or a xenstore domain. I'm really
> disappointed such a design was accepted and is now the reason for
> not being able to disaggregate dom0.
>
> I've searched through the xen-devel archives and found a very
> interesting mail:
>
> http://lists.xen.org/archives/html/xen-devel/2010-04/msg01318.html
>
> The "restrict" feature was added without any further discussion how
> it is implemented and that the C-variant doesn't support it. The
> explicit question about non-existing features in the C xenstored was
> answered just with "the xenstore wire protocol doesn't change".
>
> With:
>
> http://lists.xen.org/archives/html/xen-devel/2010-07/msg00091.html
>
> the XS_RESTRICT value in xs_wire.h (aah, suddenly it was changed?)
> was added. Again no mentioning of the special implementation in
> oxenstored.
>
> Really, this is not how open source development should be done!
> Maybe I'm just upset now, but I'm in favor of dropping xs_restrict()
> support as it has been introduced in a foul way.
I don't think the lack of xs_restrict() working over the ring should
preclude these improvements to the configuration of how xenstored starts
up.
>>> It is limiting the solution by not allowing me to drop the sockets
>>> completely.
>> I don't think dropping the sockets completely is a sensible course of
>> action. I had come the conclusion that you were just not going to use
>> them, as opposed to removing them entirely.
>>
>> For xenstored running in the same domain as the toolstack, sockets are
>> less overhead than the shared memory ring, as no hypercalls are
>> involved. There is also the unfortunate problem that one of the two
>> linux devices for xenstored *still* causes deadlocks when used; a
>> problem which is unresolved from Linux 3.14.
> Since Xen 4.7 the broken devices won't be used by default. I understand
> that the socket interface is faster and less overhead but I guess my
> question is how much data is sent over this interface? Does it really
> matter the small performance difference to justify having two different
> methods and not trimming the code base down to one method?
My gut feeling is that the XenServer bootstorm scalability tests will
notice. Our metric of "how fast is it to boot 1000 VMs" is very
important for VDI, as employees tend to get to work and try to log
during the same short time period.
~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] making xenstore domain easy configurable
On 28/06/16 17:23, Andrew Cooper wrote:
> On 28/06/16 16:17, Doug Goldstein wrote:
>> On 6/28/16 8:59 AM, Andrew Cooper wrote:
>>> On 28/06/16 14:36, Juergen Gross wrote:
On 28/06/16 14:42, Andrew Cooper wrote:
> On 28/06/16 12:56, Juergen Gross wrote:
>> On 28/06/16 13:03, Ian Jackson wrote:
>>> Juergen Gross writes ("Re: [Xen-devel] making xenstore domain easy
>>> configurable"):
So you are telling me the xenstore domain won't work for this case?
>>> Yes.
>> That's rather unfortunate. So in order to be able to make xenstore
>> domain a common setup we need to find a solution for support of
>> xs_restrict() via xenbus, right?
>>
>> TBH, the way xs_restrict() was introduced is rather weird. It is
>> completely bound to the socket interface of oxenstored. So anyone
>> wanting to use xs_restrict() is limited to oxenstored running in
>> dom0. No way to use xenstored or a xenstore domain. I'm really
>> disappointed such a design was accepted and is now the reason for
>> not being able to disaggregate dom0.
>>
>> I've searched through the xen-devel archives and found a very
>> interesting mail:
>>
>> http://lists.xen.org/archives/html/xen-devel/2010-04/msg01318.html
>>
>> The "restrict" feature was added without any further discussion how
>> it is implemented and that the C-variant doesn't support it. The
>> explicit question about non-existing features in the C xenstored was
>> answered just with "the xenstore wire protocol doesn't change".
>>
>> With:
>>
>> http://lists.xen.org/archives/html/xen-devel/2010-07/msg00091.html
>>
>> the XS_RESTRICT value in xs_wire.h (aah, suddenly it was changed?)
>> was added. Again no mentioning of the special implementation in
>> oxenstored.
>>
>> Really, this is not how open source development should be done!
>> Maybe I'm just upset now, but I'm in favor of dropping xs_restrict()
>> support as it has been introduced in a foul way.
> I don't think the lack of xs_restrict() working over the ring should
> preclude these improvements to the configuration of how xenstored starts
> up.
It is limiting the solution by not allowing me to drop the sockets
completely.
>>> I don't think dropping the sockets completely is a sensible course of
>>> action. I had come the conclusion that you were just not going to use
>>> them, as opposed to removing them entirely.
>>>
>>> For xenstored running in the same domain as the toolstack, sockets are
>>> less overhead than the shared memory ring, as no hypercalls are
>>> involved. There is also the unfortunate problem that one of the two
>>> linux devices for xenstored *still* causes deadlocks when used; a
>>> problem which is unresolved from Linux 3.14.
>> Since Xen 4.7 the broken devices won't be used by default. I understand
>> that the socket interface is faster and less overhead but I guess my
>> question is how much data is sent over this interface? Does it really
>> matter the small performance difference to justify having two different
>> methods and not trimming the code base down to one method?
>
> My gut feeling is that the XenServer bootstorm scalability tests will
> notice. Our metric of "how fast is it to boot 1000 VMs" is very
> important for VDI, as employees tend to get to work and try to log
> during the same short time period.
I think I'll start with just removing the systemd socket stuff. This is
what I need to make the xenstore domain configurable on the installed
system. Removing of the sockets can be done either later or never.
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [libvirt test] 96333: regressions - FAIL
flight 96333 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/96333/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-armhf-armhf-libvirt 6 xen-boot fail REGR. vs. 96299 Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail never pass test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 guest-saverestorefail never pass test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-i386-libvirt 12 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail never pass version targeted for testing: libvirt a2adcc1b9376beb7f464be64c326923ba6a5f7e7 baseline version: libvirt 0b4645a7e061abc8a4be71fe89865cf248ce6e56 Last test of basis96299 2016-06-27 04:21:09 Z1 days Testing same since96333 2016-06-28 04:21:58 Z0 days1 attempts People who touched revisions under test: Andrea Bolognani Ján Tomko Michal Privoznik Olga Krishtal jobs: build-amd64-xsm pass build-armhf-xsm pass build-i386-xsm pass build-amd64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass test-amd64-amd64-libvirt-xsm pass test-armhf-armhf-libvirt-xsm fail test-amd64-i386-libvirt-xsm pass test-amd64-amd64-libvirt pass test-armhf-armhf-libvirt fail test-amd64-i386-libvirt pass test-amd64-amd64-libvirt-pairpass test-amd64-i386-libvirt-pair pass test-armhf-armhf-libvirt-qcow2 fail test-armhf-armhf-libvirt-raw fail test-amd64-amd64-libvirt-vhd pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Not pushing. commit a2adcc1b9376beb7f464be64c326923ba6a5f7e7 Author: Olga Krishtal Date: Mon Jun 27 21:23:11 2016 +0300 vz: fix build for virNetDev* changes Patch fixes vz build after changes in IP-related netdev functions(cf0568b0, fbc1843d). Signed-off-by: Olga Krishtal commit 05eab47559950403aa67d18b098273269ae6916e Author: Ján Tomko Date: Mon Jun 27 11:56:17 2016 +0200 Revert "util: new function virNetDevIPInfoAddToDev" This reverts commit f1e0d0da11c473905470c28a6488bf57d9d0ae6e. This feature was accidentally pushed in the feature freeze. commit f6acf039f046a3463c52cfff66a52836545300fb Author: Ján Tomko Date: Mon Jun 27 11:56:17
[Xen-devel] [PATCH v2] xen/arm: gic-v3: No need to sort the Redistributor regions
The sorting was required by the vGIC emulation until commit
9b9d51e98edb8c5c731e2d06dfad3633053d88a4 "xen/arm: vgic-v3:
Correctly retrieve the vCPU associated to a re-distributor".
Furthermore, the code is buggy because both local variables 'l' and 'r'
point to the same region.
So drop the code which sort the Redistributors array.
Reported-by: Shanker Donthineni
Signed-off-by: Julien Grall
---
Changes in v2:
- Fix compilation with ACPI
---
xen/arch/arm/gic-v3.c | 14 --
1 file changed, 14 deletions(-)
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index dfc62e8..b8a4bde 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1134,14 +1134,6 @@ static const hw_irq_controller gicv3_guest_irq_type = {
.set_affinity = gicv3_irq_set_affinity,
};
-static int __init cmp_rdist(const void *a, const void *b)
-{
-const struct rdist_region *l = a, *r = a;
-
-/* We assume that re-distributor regions can never overlap */
-return ( l->base < r->base) ? -1 : 0;
-}
-
static paddr_t __initdata dbase = INVALID_PADDR;
static paddr_t __initdata vbase = INVALID_PADDR, vsize = 0;
static paddr_t __initdata cbase = INVALID_PADDR, csize = 0;
@@ -1210,9 +1202,6 @@ static void __init gicv3_dt_init(void)
rdist_regs[i].size = rdist_size;
}
-/* The vGIC code requires the region to be sorted */
-sort(rdist_regs, gicv3.rdist_count, sizeof(*rdist_regs), cmp_rdist, NULL);
-
if ( !dt_property_read_u32(node, "redistributor-stride",
&gicv3.rdist_stride) )
gicv3.rdist_stride = 0;
@@ -1455,9 +1444,6 @@ static void __init gicv3_acpi_init(void)
rdist_regs[i].size = gic_rdist->length;
}
-/* The vGIC code requires the region to be sorted */
-sort(rdist_regs, gicv3.rdist_count, sizeof(*rdist_regs), cmp_rdist, NULL);
-
gicv3.rdist_regions= rdist_regs;
/* Collect CPU base addresses */
--
1.9.1
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/EFI + Live Patch: avoid symbol address truncation
>>> On 28.06.16 at 16:26, wrote:
> On 28/06/16 15:03, Jan Beulich wrote:
>> ld associates __init_end, placed outside of any section by the linker
>> script, with the following section, resulting in a huge (wrapped, as it
>> would be negative) section relative offset.
>
> So in this case, the cause of the truncation is due to __init_end being
> considered relative to .data.read_mostly?
Yes.
>> COFF symbol tables store
>> section relative addresses, and hence the above leads to assembler
>> truncation warnings when all symbols get included in the symbol table
>> (for Live Patching code). To overcome this, move __init_end past both
>> ALIGN() directives. The consuming code (init_done()) is fine with such
>> an adjustment (the distinction really would only be relevant for the
>> loop claring the pages, and I think it's acceptable to clear a few
>> more on - for now - EFI). This effectively results in the
>> (__init_begin,__init_end) and (__2M_init_start,__2M_init_end) pairs to
>> become identical, with their different names only serving documentation
>> purposes now.
>>
>> Note that moving __init_end and __2M_init_end into .init is not a good
>> idea, as that would significantly grow xen.efi binary size.
>
> How about moving just __init_end ? That shouldn't affect the size of
> any binary, due to the existing page alignment between sections.
There's no page alignment between sections in the disk image
representation - we build with a file alignment of 32.
>> While inspecting symbol table and ld behavior I also noticed that
>> __2M_text_start gets put at address zero in the EFI case, which hasn't
>> caused problems solely because we don't actually reference that symbol.
>
> The reason that __2M_text_start isn't referenced is because I couldn't
> get the EFI build working. It was used in my first prototype.
Not surprising with the symbol having ended up at zero.
>> @@ -530,18 +531,18 @@ static void noinline init_done(void)
>> /* Destroy Xen's mappings, and reuse the pages. */
>> if ( using_2M_mapping() )
>> {
>> -destroy_xen_mappings((unsigned long)&__2M_init_start,
>> - (unsigned long)&__2M_init_end);
>> -init_xenheap_pages(__pa(__2M_init_start), __pa(__2M_init_end));
>> +start = (unsigned long)&__2M_init_start,
>> +end = (unsigned long)&__2M_init_end;
>> }
>> else
>> {
>> -destroy_xen_mappings((unsigned long)&__init_begin,
>> - (unsigned long)&__init_end);
>> -init_xenheap_pages(__pa(__init_begin), __pa(__init_end));
>> +start = (unsigned long)&__init_begin;
>> +end = (unsigned long)&__init_end;
>> }
>>
>> -printk("Freed %ldkB init memory.\n",
>> (long)(__init_end-__init_begin)>>10);
>> +destroy_xen_mappings(start, end);
>> +init_xenheap_pages(__pa(start), __pa(end));
>> +printk("Freed %ldkB init memory\n", (end - start) >> 10);
>
> The parameter is now unsigned, so %lu.
Oh, of course - fixed.
>> --- a/xen/arch/x86/xen.lds.S
>> +++ b/xen/arch/x86/xen.lds.S
>> @@ -40,9 +40,20 @@ SECTIONS
>> #if !defined(EFI)
>>. = __XEN_VIRT_START;
>>__image_base__ = .;
>> +#else
>> + . = __image_base__;
>> #endif
>>
>> +#if 0
>> +/*
>> + * We don't really use this symbol anywhere, and the way it would get
>> defined
>> + * here would result in it having a negative (wrapped to huge positive)
>> + * offset relative to the .text section. That, in turn, causes an assembler
>> + * truncation warning when including all symbols in the symbol table for
>> Live
>> + * Patching code.
>> + */
>>__2M_text_start = .; /* Start of 2M superpages, mapped RX. */
>> +#endif
>>
>>. = __XEN_VIRT_START + MB(1);
>>_start = .;
>> @@ -194,14 +205,13 @@ SECTIONS
>> *(.ctors)
>> __ctors_end = .;
>>} :text
>> - . = ALIGN(PAGE_SIZE);
>> - __init_end = .;
>>
>> #ifdef EFI
>>. = ALIGN(MB(2));
>> #else
>>. = ALIGN(PAGE_SIZE);
>> #endif
>> + __init_end = .;
>>__2M_init_end = .;
>>
>>__2M_rwdata_start = .; /* Start of 2M superpages, mapped RW. */
>> @@ -296,7 +306,6 @@ ASSERT(__image_base__ > XEN_VIRT_START |
>> ASSERT(kexec_reloc_size - kexec_reloc <= PAGE_SIZE, "kexec_reloc is too
> large")
>> #endif
>>
>> -ASSERT(IS_ALIGNED(__2M_text_start, MB(2)), "__2M_text_start misaligned")
>
> If we are #if 0'ing the symbol for documentation purposes, can we #if 0
> this as well?
I considered it, but the two #if-s would end up disconnected. And
with the symbol being first thing in the image (plus the fact that so
far the assertion was there _without_ triggering despite there
being a problem - just one it couldn't detect), I think chances are
slim that it getting fully removed would be a significant problem.
I.e. I'd prefer the patch to remain as is in this regard, but if the
only way to get it acked is to do as you suggest, I would
(hesitantly) do so.
Jan
_
[Xen-devel] [PATCH v5 00/17] xen/arm: Use the typesafes gfn and mfn
Hello all,
Some of the ARM functions are mixing gfn vs mfn and even physical vs frame.
To avoid more confusion, this patch series makes use of the terminology
described in xen/include/xen/mm.h and the associated typesafe.
This series requires the patch [1] to be applied beforehand. I pushed a
branch with this patch and this series applied on xenbits:
git://xenbits.xen.org/people/julieng/xen-unstable.git branch typesafe-v4
For all the changes see in each patch.
Yours sincerely,
[1] http://lists.xenproject.org/archives/html/xen-devel/2016-06/msg01744.html
Cc: Andrew Cooper
Cc: Boris Ostrovsky
Cc: Christoph Egger
Cc: Feng Wu
Cc: George Dunlap
Cc: Ian Jackson
Cc: Jan Beulich
Cc: Jun Nakajima
Cc: Kevin Tian
Cc: Konrad Rzeszutek Wilk
Cc: Liu Jinsong
Cc: Mukesh Rathor
Cc: Paul Durrant
Cc: Shannon Zhao
Cc: Stefano Stabellini
Cc: Suravee Suthikulpanit
Cc: Tim Deegan
Cc: Wei Liu
Julien Grall (17):
xen: Use typesafe gfn/mfn in guest_physmap_* helpers
xen: Use typesafe gfn in xenmem_add_to_physmap_one
xen/arm: Rename grant_table_gfpn into grant_table_gfn and use the
typesafe gfn
xen: Use the typesafe mfn and gfn in map_mmio_regions...
xen/passthrough: x86: Use INVALID_GFN rather than INVALID_MFN
xen: Use a typesafe to define INVALID_MFN
xen: Use a typesafe to define INVALID_GFN
xen/arm: Rework the interface of p2m_lookup and use typesafe gfn and
mfn
xen/arm: Rework the interface of p2m_cache_flush and use typesafe gfn
xen/arm: map_regions_rw_cache: Map the region with p2m->default_access
xen/arm: dom0_build: Remove dead code in allocate_memory
xen/arm: p2m: Remove unused operation ALLOCATE
xen/arm: Use the typesafes mfn and gfn in map_dev_mmio_region...
xen/arm: Use the typesafes mfn and gfn in map_regions_rw_cache ...
xen/arm: p2m: Introduce helpers to insert and remove mapping
xen/arm: p2m: Use typesafe gfn for {max,lowest}_mapped_gfn
xen/arm: p2m: Rework the interface of apply_p2m_changes and use
typesafe
xen/arch/arm/domain.c | 4 +-
xen/arch/arm/domain_build.c | 72 ++---
xen/arch/arm/domctl.c | 2 +-
xen/arch/arm/gic-v2.c | 4 +-
xen/arch/arm/mm.c | 20 +--
xen/arch/arm/p2m.c | 269
xen/arch/arm/platforms/exynos5.c| 8 +-
xen/arch/arm/platforms/omap5.c | 16 +-
xen/arch/arm/traps.c| 21 +--
xen/arch/arm/vgic-v2.c | 4 +-
xen/arch/x86/cpu/mcheck/mce.c | 2 +-
xen/arch/x86/debug.c| 64
xen/arch/x86/domain.c | 7 +-
xen/arch/x86/domain_build.c | 6 +-
xen/arch/x86/hvm/emulate.c | 7 +-
xen/arch/x86/hvm/hvm.c | 12 +-
xen/arch/x86/hvm/ioreq.c| 16 +-
xen/arch/x86/hvm/svm/nestedsvm.c| 2 +-
xen/arch/x86/hvm/viridian.c | 6 +-
xen/arch/x86/hvm/vmx/vmx.c | 8 +-
xen/arch/x86/mm.c | 21 +--
xen/arch/x86/mm/altp2m.c| 2 +-
xen/arch/x86/mm/guest_walk.c| 4 +-
xen/arch/x86/mm/hap/guest_walk.c| 10 +-
xen/arch/x86/mm/hap/hap.c | 4 +-
xen/arch/x86/mm/hap/nested_ept.c| 2 +-
xen/arch/x86/mm/p2m-ept.c | 6 +-
xen/arch/x86/mm/p2m-pod.c | 24 +--
xen/arch/x86/mm/p2m-pt.c| 18 +--
xen/arch/x86/mm/p2m.c | 164 ++-
xen/arch/x86/mm/paging.c| 12 +-
xen/arch/x86/mm/shadow/common.c | 46 +++---
xen/arch/x86/mm/shadow/multi.c | 38 ++---
xen/arch/x86/mm/shadow/private.h| 2 +-
xen/common/domain.c | 6 +-
xen/common/domctl.c | 4 +-
xen/common/grant_table.c| 13 +-
xen/common/memory.c | 38 ++---
xen/drivers/passthrough/amd/iommu_map.c | 2 +-
xen/drivers/passthrough/arm/smmu.c | 4 +-
xen/drivers/passthrough/vtd/iommu.c | 4 +-
xen/drivers/passthrough/x86/iommu.c | 2 +-
xen/include/asm-arm/domain.h| 2 +-
xen/include/asm-arm/grant_table.h | 2 +-
xen/include/asm-arm/p2m.h | 44 +++---
xen/include/asm-x86/guest_pt.h | 4 +-
xen/include/asm-x86/p2m.h | 13 +-
xen/include/xen/mm.h| 8 +-
xen/include/xen/p2m-common.h| 8 +-
49 files changed, 481 insertions(+), 576 deletions(-)
--
1.9.1
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v5 09/17] xen/arm: Rework the interface of p2m_cache_flush and use typesafe gfn
p2m_cache_flush is expecting GFNs in parameter and not MFNs. Rename
the variable to *gfn* and use typesafe to avoid possible misusage.
Also, modify the prototype of the function to describe the range
using the start and the number of GFNs. This will avoid to wonder
whether the end if inclusive or exclusive.
Note that the type of the parameters 'start' is changed from xen_pfn_t
(aka uint64_t) to gfn_t (aka unsigned long). This means that a truncation
will occur for ARM32. It is fine because it will always be encoded on 28
bits maximum (40 bits address).
Signed-off-by: Julien Grall
---
Changes in v4:
- This patch was originally called "xen/arm: p2m_cache_flush:
Use the correct terminology and typesafe gfn"
- Describe the range using the start and the number of GFNs.
Changes in v3:
- Add a word in the commit message about the truncation.
Changes in v2:
- Drop _gfn suffix
---
xen/arch/arm/domctl.c | 2 +-
xen/arch/arm/p2m.c| 11 ++-
xen/include/asm-arm/p2m.h | 2 +-
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c
index 30453d8..f61f98a 100644
--- a/xen/arch/arm/domctl.c
+++ b/xen/arch/arm/domctl.c
@@ -30,7 +30,7 @@ long arch_do_domctl(struct xen_domctl *domctl, struct domain
*d,
if ( e < s )
return -EINVAL;
-return p2m_cache_flush(d, s, e);
+return p2m_cache_flush(d, _gfn(s), domctl->u.cacheflush.nr_pfns);
}
case XEN_DOMCTL_bind_pt_irq:
{
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 54a363a..1cfb62b 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1469,16 +1469,17 @@ int relinquish_p2m_mapping(struct domain *d)
d->arch.p2m.default_access);
}
-int p2m_cache_flush(struct domain *d, xen_pfn_t start_mfn, xen_pfn_t end_mfn)
+int p2m_cache_flush(struct domain *d, gfn_t start, unsigned long nr)
{
struct p2m_domain *p2m = &d->arch.p2m;
+gfn_t end = gfn_add(start, nr);
-start_mfn = MAX(start_mfn, p2m->lowest_mapped_gfn);
-end_mfn = MIN(end_mfn, p2m->max_mapped_gfn);
+start = gfn_max(start, _gfn(p2m->lowest_mapped_gfn));
+end = gfn_min(end, _gfn(p2m->max_mapped_gfn));
return apply_p2m_changes(d, CACHEFLUSH,
- pfn_to_paddr(start_mfn),
- pfn_to_paddr(end_mfn),
+ pfn_to_paddr(gfn_x(start)),
+ pfn_to_paddr(gfn_x(end)),
pfn_to_paddr(mfn_x(INVALID_MFN)),
MATTR_MEM, 0, p2m_invalid,
d->arch.p2m.default_access);
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index f204482..8a96e68 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -139,7 +139,7 @@ void p2m_dump_info(struct domain *d);
mfn_t p2m_lookup(struct domain *d, gfn_t gfn, p2m_type_t *t);
/* Clean & invalidate caches corresponding to a region of guest address space
*/
-int p2m_cache_flush(struct domain *d, xen_pfn_t start_mfn, xen_pfn_t end_mfn);
+int p2m_cache_flush(struct domain *d, gfn_t start, unsigned long nr);
/* Setup p2m RAM mapping for domain d from start-end. */
int p2m_populate_ram(struct domain *d, paddr_t start, paddr_t end);
--
1.9.1
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v5 07/17] xen: Use a typesafe to define INVALID_GFN
Also take the opportunity to convert arch/x86/debug.c to the typesafe gfn.
Signed-off-by: Julien Grall
---
Cc: Mukesh Rathor
Cc: Jan Beulich
Cc: Andrew Cooper
Cc: Paul Durrant
Cc: Boris Ostrovsky
Cc: Suravee Suthikulpanit
Cc: Jun Nakajima
Cc: Kevin Tian
Cc: George Dunlap
Cc: Tim Deegan
Cc: Feng Wu
Changes in v5:
- Patch added
---
xen/arch/arm/p2m.c | 4 ++--
xen/arch/x86/debug.c| 18 +-
xen/arch/x86/domain.c | 2 +-
xen/arch/x86/hvm/emulate.c | 7 ---
xen/arch/x86/hvm/hvm.c | 6 +++---
xen/arch/x86/hvm/ioreq.c| 8
xen/arch/x86/hvm/svm/nestedsvm.c| 2 +-
xen/arch/x86/hvm/vmx/vmx.c | 6 +++---
xen/arch/x86/mm/altp2m.c| 2 +-
xen/arch/x86/mm/hap/guest_walk.c| 10 +-
xen/arch/x86/mm/hap/nested_ept.c| 2 +-
xen/arch/x86/mm/p2m-pod.c | 6 +++---
xen/arch/x86/mm/p2m.c | 18 +-
xen/arch/x86/mm/shadow/common.c | 2 +-
xen/arch/x86/mm/shadow/multi.c | 2 +-
xen/arch/x86/mm/shadow/private.h| 2 +-
xen/drivers/passthrough/amd/iommu_map.c | 2 +-
xen/drivers/passthrough/vtd/iommu.c | 4 ++--
xen/drivers/passthrough/x86/iommu.c | 2 +-
xen/include/asm-x86/guest_pt.h | 4 ++--
xen/include/asm-x86/p2m.h | 2 +-
xen/include/xen/mm.h| 2 +-
22 files changed, 57 insertions(+), 56 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index d690602..c938dde 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -479,7 +479,7 @@ static int __p2m_get_mem_access(struct domain *d, gfn_t gfn,
}
/* If request to get default access. */
-if ( gfn_x(gfn) == INVALID_GFN )
+if ( gfn_eq(gfn, INVALID_GFN) )
{
*access = memaccess[p2m->default_access];
return 0;
@@ -1879,7 +1879,7 @@ long p2m_set_mem_access(struct domain *d, gfn_t gfn,
uint32_t nr,
p2m->mem_access_enabled = true;
/* If request to set default access. */
-if ( gfn_x(gfn) == INVALID_GFN )
+if ( gfn_eq(gfn, INVALID_GFN) )
{
p2m->default_access = a;
return 0;
diff --git a/xen/arch/x86/debug.c b/xen/arch/x86/debug.c
index 3479f7c..1ce0e89 100644
--- a/xen/arch/x86/debug.c
+++ b/xen/arch/x86/debug.c
@@ -44,8 +44,7 @@ typedef unsigned char dbgbyte_t;
/* Returns: mfn for the given (hvm guest) vaddr */
static mfn_t
-dbg_hvm_va2mfn(dbgva_t vaddr, struct domain *dp, int toaddr,
-unsigned long *gfn)
+dbg_hvm_va2mfn(dbgva_t vaddr, struct domain *dp, int toaddr, gfn_t *gfn)
{
mfn_t mfn;
uint32_t pfec = PFEC_page_present;
@@ -53,14 +52,14 @@ dbg_hvm_va2mfn(dbgva_t vaddr, struct domain *dp, int toaddr,
DBGP2("vaddr:%lx domid:%d\n", vaddr, dp->domain_id);
-*gfn = paging_gva_to_gfn(dp->vcpu[0], vaddr, &pfec);
-if ( *gfn == INVALID_GFN )
+*gfn = _gfn(paging_gva_to_gfn(dp->vcpu[0], vaddr, &pfec));
+if ( gfn_eq(*gfn, INVALID_GFN) )
{
DBGP2("kdb:bad gfn from gva_to_gfn\n");
return INVALID_MFN;
}
-mfn = get_gfn(dp, *gfn, &gfntype);
+mfn = get_gfn(dp, gfn_x(*gfn), &gfntype);
if ( p2m_is_readonly(gfntype) && toaddr )
{
DBGP2("kdb:p2m_is_readonly: gfntype:%x\n", gfntype);
@@ -72,7 +71,7 @@ dbg_hvm_va2mfn(dbgva_t vaddr, struct domain *dp, int toaddr,
if ( mfn_eq(mfn, INVALID_MFN) )
{
-put_gfn(dp, *gfn);
+put_gfn(dp, gfn_x(*gfn));
*gfn = INVALID_GFN;
}
@@ -165,7 +164,8 @@ unsigned int dbg_rw_guest_mem(struct domain *dp, void *
__user gaddr,
char *va;
unsigned long addr = (unsigned long)gaddr;
mfn_t mfn;
-unsigned long gfn = INVALID_GFN, pagecnt;
+gfn_t gfn = INVALID_GFN;
+unsigned long pagecnt;
pagecnt = min_t(long, PAGE_SIZE - (addr & ~PAGE_MASK), len);
@@ -189,8 +189,8 @@ unsigned int dbg_rw_guest_mem(struct domain *dp, void *
__user gaddr,
}
unmap_domain_page(va);
-if ( gfn != INVALID_GFN )
-put_gfn(dp, gfn);
+if ( !gfn_eq(gfn, INVALID_GFN) )
+put_gfn(dp, gfn_x(gfn));
addr += pagecnt;
buf += pagecnt;
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index bb59247..c8c7e2d 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -783,7 +783,7 @@ int arch_domain_soft_reset(struct domain *d)
* gfn == INVALID_GFN indicates that the shared_info page was never mapped
* to the domain's address space and there is nothing to replace.
*/
-if ( gfn == INVALID_GFN )
+if ( gfn == gfn_x(INVALID_GFN) )
goto exit_put_page;
if ( mfn_x(get_gfn_query(d, gfn, &p2mt)) != mfn )
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index 855af4d..c55ad7b 100644
--- a/xen/arch/x86/hvm/emulat
[Xen-devel] [PATCH v5 15/17] xen/arm: p2m: Introduce helpers to insert and remove mapping
More the half of the arguments of INSERT and REMOVE are the same for
each callers. Simplify the callers of apply_p2m_changes by adding new
helpers which will fill common arguments with default values.
Signed-off-by: Julien Grall
---
Changes in v5:
- Add missing Signed-off-by
Changes in v4:
- Patch added
---
xen/arch/arm/p2m.c | 70 --
1 file changed, 36 insertions(+), 34 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 0fdd11f..a5b584b 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1158,17 +1158,40 @@ out:
return rc;
}
+static inline int p2m_insert_mapping(struct domain *d,
+ gfn_t start_gfn,
+ unsigned long nr,
+ mfn_t mfn,
+ int mattr, p2m_type_t t)
+{
+return apply_p2m_changes(d, INSERT,
+ pfn_to_paddr(gfn_x(start_gfn)),
+ pfn_to_paddr(gfn_x(start_gfn) + nr),
+ pfn_to_paddr(mfn_x(mfn)),
+ mattr, 0, t, d->arch.p2m.default_access);
+}
+
+static inline int p2m_remove_mapping(struct domain *d,
+ gfn_t start_gfn,
+ unsigned long nr,
+ mfn_t mfn)
+{
+return apply_p2m_changes(d, REMOVE,
+ pfn_to_paddr(gfn_x(start_gfn)),
+ pfn_to_paddr(gfn_x(start_gfn) + nr),
+ pfn_to_paddr(mfn_x(mfn)),
+ /* arguments below not used when removing mapping
*/
+ MATTR_MEM, 0, p2m_invalid,
+ d->arch.p2m.default_access);
+}
+
int map_regions_rw_cache(struct domain *d,
gfn_t gfn,
unsigned long nr,
mfn_t mfn)
{
-return apply_p2m_changes(d, INSERT,
- pfn_to_paddr(gfn_x(gfn)),
- pfn_to_paddr(gfn_x(gfn) + nr),
- pfn_to_paddr(mfn_x(mfn)),
- MATTR_MEM, 0, p2m_mmio_direct,
- d->arch.p2m.default_access);
+return p2m_insert_mapping(d, gfn, nr, mfn,
+ MATTR_MEM, p2m_mmio_direct);
}
int unmap_regions_rw_cache(struct domain *d,
@@ -1176,12 +1199,7 @@ int unmap_regions_rw_cache(struct domain *d,
unsigned long nr,
mfn_t mfn)
{
-return apply_p2m_changes(d, REMOVE,
- pfn_to_paddr(gfn_x(gfn)),
- pfn_to_paddr(gfn_x(gfn) + nr),
- pfn_to_paddr(mfn_x(mfn)),
- MATTR_MEM, 0, p2m_invalid,
- d->arch.p2m.default_access);
+return p2m_remove_mapping(d, gfn, nr, mfn);
}
int map_mmio_regions(struct domain *d,
@@ -1189,12 +1207,8 @@ int map_mmio_regions(struct domain *d,
unsigned long nr,
mfn_t mfn)
{
-return apply_p2m_changes(d, INSERT,
- pfn_to_paddr(gfn_x(start_gfn)),
- pfn_to_paddr(gfn_x(start_gfn) + nr),
- pfn_to_paddr(mfn_x(mfn)),
- MATTR_DEV, 0, p2m_mmio_direct,
- d->arch.p2m.default_access);
+return p2m_insert_mapping(d, start_gfn, nr, mfn,
+ MATTR_MEM, p2m_mmio_direct);
}
int unmap_mmio_regions(struct domain *d,
@@ -1202,12 +1216,7 @@ int unmap_mmio_regions(struct domain *d,
unsigned long nr,
mfn_t mfn)
{
-return apply_p2m_changes(d, REMOVE,
- pfn_to_paddr(gfn_x(start_gfn)),
- pfn_to_paddr(gfn_x(start_gfn) + nr),
- pfn_to_paddr(mfn_x(mfn)),
- MATTR_DEV, 0, p2m_invalid,
- d->arch.p2m.default_access);
+return p2m_remove_mapping(d, start_gfn, nr, mfn);
}
int map_dev_mmio_region(struct domain *d,
@@ -1237,22 +1246,15 @@ int guest_physmap_add_entry(struct domain *d,
unsigned long page_order,
p2m_type_t t)
{
-return apply_p2m_changes(d, INSERT,
- pfn_to_paddr(gfn_x(gfn)),
- pfn_to_paddr(gfn_x(gfn) + (1 << page_order)),
- pfn_to_paddr(mfn_x(mfn)), MATTR_MEM, 0, t,
- d->arch.p2m.default_access);
+return p2m_insert_mapping(d, gfn, (1 << page_order), mfn,
+ MATTR_MEM, t);
}
void guest_physmap_remove_page(struct domain *d,
[Xen-devel] [PATCH v5 10/17] xen/arm: map_regions_rw_cache: Map the region with p2m->default_access
The parameter 'access' is used by memaccess to restrict temporarily the permission. This parameter should not be used for other purpose (such as restricting permanently the permission). The type p2m_mmio_direct will map the region Read-Write and non-executable. Note that this is already the current behavior with the combination of the type and the access. So there is no functional change. Signed-off-by: Julien Grall --- Cc: Shannon Zhao This patch is a candidate for Xen 4.7. Currently this function is only used to map ACPI regions. I am wondering if we should introduce a new p2m type for it. And map this region RO (I am not sure why a guest would want to modify this region). Changes in v4: - Patch added --- xen/arch/arm/p2m.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index 1cfb62b..fcc4513 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -1231,7 +1231,7 @@ int map_regions_rw_cache(struct domain *d, pfn_to_paddr(start_gfn + nr), pfn_to_paddr(mfn), MATTR_MEM, 0, p2m_mmio_direct, - p2m_access_rw); + d->arch.p2m.default_access); } int unmap_regions_rw_cache(struct domain *d, @@ -1244,7 +1244,7 @@ int unmap_regions_rw_cache(struct domain *d, pfn_to_paddr(start_gfn + nr), pfn_to_paddr(mfn), MATTR_MEM, 0, p2m_invalid, - p2m_access_rw); + d->arch.p2m.default_access); } int map_mmio_regions(struct domain *d, -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v5 04/17] xen: Use the typesafe mfn and gfn in map_mmio_regions...
to avoid mixing machine frame with guest frame.
Signed-off-by: Julien Grall
Acked-by: Jan Beulich
---
Cc: Stefano Stabellini
Cc: Jan Beulich
Cc: Andrew Cooper
Cc: George Dunlap
Cc: Ian Jackson
Cc: Konrad Rzeszutek Wilk
Cc: Tim Deegan
Cc: Wei Liu
Changes in v3:
- Use mfn_add when it is possible
- Add Jan's acked-by
---
xen/arch/arm/domain_build.c | 4 ++--
xen/arch/arm/gic-v2.c| 4 ++--
xen/arch/arm/p2m.c | 22 +++---
xen/arch/arm/platforms/exynos5.c | 8
xen/arch/arm/platforms/omap5.c | 16
xen/arch/arm/vgic-v2.c | 4 ++--
xen/arch/x86/mm/p2m.c| 18 ++
xen/common/domctl.c | 4 ++--
xen/include/xen/p2m-common.h | 8
9 files changed, 45 insertions(+), 43 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 9035486..49185f0 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1036,9 +1036,9 @@ static int map_range_to_domain(const struct
dt_device_node *dev,
if ( need_mapping )
{
res = map_mmio_regions(d,
- paddr_to_pfn(addr),
+ _gfn(paddr_to_pfn(addr)),
DIV_ROUND_UP(len, PAGE_SIZE),
- paddr_to_pfn(addr));
+ _mfn(paddr_to_pfn(addr)));
if ( res < 0 )
{
printk(XENLOG_ERR "Unable to map 0x%"PRIx64
diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 4e2f4c7..3893ece 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -601,9 +601,9 @@ static int gicv2_map_hwdown_extra_mappings(struct domain *d)
d->domain_id, v2m_data->addr, v2m_data->size,
v2m_data->spi_start, v2m_data->nr_spis);
-ret = map_mmio_regions(d, paddr_to_pfn(v2m_data->addr),
+ret = map_mmio_regions(d, _gfn(paddr_to_pfn(v2m_data->addr)),
DIV_ROUND_UP(v2m_data->size, PAGE_SIZE),
-paddr_to_pfn(v2m_data->addr));
+_mfn(paddr_to_pfn(v2m_data->addr)));
if ( ret )
{
printk(XENLOG_ERR "GICv2: Map v2m frame to d%d failed.\n",
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 0395a40..34563bb 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1245,27 +1245,27 @@ int unmap_regions_rw_cache(struct domain *d,
}
int map_mmio_regions(struct domain *d,
- unsigned long start_gfn,
+ gfn_t start_gfn,
unsigned long nr,
- unsigned long mfn)
+ mfn_t mfn)
{
return apply_p2m_changes(d, INSERT,
- pfn_to_paddr(start_gfn),
- pfn_to_paddr(start_gfn + nr),
- pfn_to_paddr(mfn),
+ pfn_to_paddr(gfn_x(start_gfn)),
+ pfn_to_paddr(gfn_x(start_gfn) + nr),
+ pfn_to_paddr(mfn_x(mfn)),
MATTR_DEV, 0, p2m_mmio_direct,
d->arch.p2m.default_access);
}
int unmap_mmio_regions(struct domain *d,
- unsigned long start_gfn,
+ gfn_t start_gfn,
unsigned long nr,
- unsigned long mfn)
+ mfn_t mfn)
{
return apply_p2m_changes(d, REMOVE,
- pfn_to_paddr(start_gfn),
- pfn_to_paddr(start_gfn + nr),
- pfn_to_paddr(mfn),
+ pfn_to_paddr(gfn_x(start_gfn)),
+ pfn_to_paddr(gfn_x(start_gfn) + nr),
+ pfn_to_paddr(mfn_x(mfn)),
MATTR_DEV, 0, p2m_invalid,
d->arch.p2m.default_access);
}
@@ -1280,7 +1280,7 @@ int map_dev_mmio_region(struct domain *d,
if ( !(nr && iomem_access_permitted(d, mfn, mfn + nr - 1)) )
return 0;
-res = map_mmio_regions(d, start_gfn, nr, mfn);
+res = map_mmio_regions(d, _gfn(start_gfn), nr, _mfn(mfn));
if ( res < 0 )
{
printk(XENLOG_G_ERR "Unable to map [%#lx - %#lx] in Dom%d\n",
diff --git a/xen/arch/arm/platforms/exynos5.c b/xen/arch/arm/platforms/exynos5.c
index bf4964d..c43934f 100644
--- a/xen/arch/arm/platforms/exynos5.c
+++ b/xen/arch/arm/platforms/exynos5.c
@@ -83,12 +83,12 @@ static int exynos5_init_time(void)
static int exynos5250_specific_mapping(struct domain *d)
{
/* Map the chip ID */
-map_mmio_regions(d, paddr_to_pfn(EXYNOS5_PA_CHIPID), 1,
- paddr_to_pfn(EXYNOS5_PA_CHIPID));
+map_mmio_regions(d, _gfn(paddr_to_pfn(EXYNOS5_PA_CHIPID)), 1,
+ _mfn(paddr_to_pfn(EXYNOS5_PA_CH
[Xen-devel] [PATCH v5 17/17] xen/arm: p2m: Rework the interface of apply_p2m_changes and use typesafe
Most of the callers of apply_p2m_changes have a GFN, a MFN and the
number of frame to change in hand.
Rather than asking each caller to convert the frame to an address,
rework the interfaces to pass the GFN, MFN and the number of frame.
Note that it would be possible to do more clean-up in apply_p2m_changes,
but this will be done in a follow-up series.
Signed-off-by: Julien Grall
---
Changes in v4:
- Patch added
---
xen/arch/arm/p2m.c | 62 --
1 file changed, 28 insertions(+), 34 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 9fdc417..bb33a72 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -906,25 +906,26 @@ static void update_reference_mapping(struct page_info
*page,
static int apply_p2m_changes(struct domain *d,
enum p2m_operation op,
- paddr_t start_gpaddr,
- paddr_t end_gpaddr,
- paddr_t maddr,
+ gfn_t sgfn,
+ unsigned long nr,
+ mfn_t smfn,
int mattr,
uint32_t mask,
p2m_type_t t,
p2m_access_t a)
{
+paddr_t start_gpaddr = pfn_to_paddr(gfn_x(sgfn));
+paddr_t end_gpaddr = pfn_to_paddr(gfn_x(sgfn) + nr);
+paddr_t maddr = pfn_to_paddr(mfn_x(smfn));
int rc, ret;
struct p2m_domain *p2m = &d->arch.p2m;
lpae_t *mappings[4] = { NULL, NULL, NULL, NULL };
struct page_info *pages[4] = { NULL, NULL, NULL, NULL };
-paddr_t addr, orig_maddr = maddr;
+paddr_t addr;
unsigned int level = 0;
unsigned int cur_root_table = ~0;
unsigned int cur_offset[4] = { ~0, ~0, ~0, ~0 };
unsigned int count = 0;
-const unsigned long sgfn = paddr_to_pfn(start_gpaddr),
-egfn = paddr_to_pfn(end_gpaddr);
const unsigned int preempt_count_limit = (op == MEMACCESS) ? 1 : 0x2000;
const bool_t preempt = !is_idle_vcpu(current);
bool_t flush = false;
@@ -986,9 +987,9 @@ static int apply_p2m_changes(struct domain *d,
* Preempt setting mem_access permissions as required by
XSA-89,
* if it's not the last iteration.
*/
-uint32_t progress = paddr_to_pfn(addr) - sgfn + 1;
+uint32_t progress = paddr_to_pfn(addr) - gfn_x(sgfn) + 1;
-if ( (egfn - sgfn) > progress && !(progress & mask) )
+if ( nr > progress && !(progress & mask) )
{
rc = progress;
goto out;
@@ -1117,8 +1118,9 @@ static int apply_p2m_changes(struct domain *d,
if ( op == INSERT )
{
-p2m->max_mapped_gfn = gfn_max(p2m->max_mapped_gfn, _gfn(egfn));
-p2m->lowest_mapped_gfn = gfn_min(p2m->lowest_mapped_gfn, _gfn(sgfn));
+p2m->max_mapped_gfn = gfn_max(p2m->max_mapped_gfn,
+ gfn_add(sgfn, nr));
+p2m->lowest_mapped_gfn = gfn_min(p2m->lowest_mapped_gfn, sgfn);
}
rc = 0;
@@ -1127,7 +1129,7 @@ out:
if ( flush )
{
flush_tlb_domain(d);
-ret = iommu_iotlb_flush(d, sgfn, egfn - sgfn);
+ret = iommu_iotlb_flush(d, gfn_x(sgfn), nr);
if ( !rc )
rc = ret;
}
@@ -1146,12 +1148,14 @@ out:
if ( rc < 0 && ( op == INSERT ) &&
addr != start_gpaddr )
{
+unsigned long gfn = paddr_to_pfn(addr);
+
BUG_ON(addr == end_gpaddr);
/*
* addr keeps the address of the end of the last successfully-inserted
* mapping.
*/
-apply_p2m_changes(d, REMOVE, start_gpaddr, addr, orig_maddr,
+apply_p2m_changes(d, REMOVE, sgfn, gfn - gfn_x(sgfn), smfn,
mattr, 0, p2m_invalid, d->arch.p2m.default_access);
}
@@ -1164,10 +1168,7 @@ static inline int p2m_insert_mapping(struct domain *d,
mfn_t mfn,
int mattr, p2m_type_t t)
{
-return apply_p2m_changes(d, INSERT,
- pfn_to_paddr(gfn_x(start_gfn)),
- pfn_to_paddr(gfn_x(start_gfn) + nr),
- pfn_to_paddr(mfn_x(mfn)),
+return apply_p2m_changes(d, INSERT, start_gfn, nr, mfn,
mattr, 0, t, d->arch.p2m.default_access);
}
@@ -1176,10 +1177,7 @@ static inline int p2m_remove_mapping(struct domain *d,
unsigned long nr,
mfn_t mfn)
{
-return apply_p2m_changes(d, REMOVE,
- pfn_to_paddr(gfn_x(start_gfn)),
- pfn_to_paddr(gfn_x(start_gfn) + nr),
- pfn_to_paddr(mfn_x(mfn)),
+return apply_p2m_changes(d, REMOVE, start_gfn, nr, mfn,
/* argumen
[Xen-devel] [PATCH v5 14/17] xen/arm: Use the typesafes mfn and gfn in map_regions_rw_cache ...
to avoid mixing machine frame with guest frame. Also rename the
parameters of the function and drop pointless PAGE_MASK in the caller.
Signed-off-by: Julien Grall
---
Changes in v4:
- Patch added
---
xen/arch/arm/domain_build.c | 8
xen/arch/arm/p2m.c | 20 ++--
xen/include/asm-arm/p2m.h | 12 ++--
3 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 923f48a..60db9e4 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1522,9 +1522,9 @@ static void acpi_map_other_tables(struct domain *d)
addr = acpi_gbl_root_table_list.tables[i].address;
size = acpi_gbl_root_table_list.tables[i].length;
res = map_regions_rw_cache(d,
- paddr_to_pfn(addr & PAGE_MASK),
+ _gfn(paddr_to_pfn(addr)),
DIV_ROUND_UP(size, PAGE_SIZE),
- paddr_to_pfn(addr & PAGE_MASK));
+ _mfn(paddr_to_pfn(addr)));
if ( res )
{
panic(XENLOG_ERR "Unable to map ACPI region 0x%"PRIx64
@@ -1878,9 +1878,9 @@ static int prepare_acpi(struct domain *d, struct
kernel_info *kinfo)
/* Map the EFI and ACPI tables to Dom0 */
rc = map_regions_rw_cache(d,
- paddr_to_pfn(d->arch.efi_acpi_gpa),
+ _gfn(paddr_to_pfn(d->arch.efi_acpi_gpa)),
PFN_UP(d->arch.efi_acpi_len),
-
paddr_to_pfn(virt_to_maddr(d->arch.efi_acpi_table)));
+
_mfn(paddr_to_pfn(virt_to_maddr(d->arch.efi_acpi_table;
if ( rc != 0 )
{
printk(XENLOG_ERR "Unable to map EFI/ACPI table 0x%"PRIx64
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 5ffc3df..0fdd11f 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1159,27 +1159,27 @@ out:
}
int map_regions_rw_cache(struct domain *d,
- unsigned long start_gfn,
+ gfn_t gfn,
unsigned long nr,
- unsigned long mfn)
+ mfn_t mfn)
{
return apply_p2m_changes(d, INSERT,
- pfn_to_paddr(start_gfn),
- pfn_to_paddr(start_gfn + nr),
- pfn_to_paddr(mfn),
+ pfn_to_paddr(gfn_x(gfn)),
+ pfn_to_paddr(gfn_x(gfn) + nr),
+ pfn_to_paddr(mfn_x(mfn)),
MATTR_MEM, 0, p2m_mmio_direct,
d->arch.p2m.default_access);
}
int unmap_regions_rw_cache(struct domain *d,
- unsigned long start_gfn,
+ gfn_t gfn,
unsigned long nr,
- unsigned long mfn)
+ mfn_t mfn)
{
return apply_p2m_changes(d, REMOVE,
- pfn_to_paddr(start_gfn),
- pfn_to_paddr(start_gfn + nr),
- pfn_to_paddr(mfn),
+ pfn_to_paddr(gfn_x(gfn)),
+ pfn_to_paddr(gfn_x(gfn) + nr),
+ pfn_to_paddr(mfn_x(mfn)),
MATTR_MEM, 0, p2m_invalid,
d->arch.p2m.default_access);
}
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 8d29eda..6e258b9 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -142,14 +142,14 @@ mfn_t p2m_lookup(struct domain *d, gfn_t gfn, p2m_type_t
*t);
int p2m_cache_flush(struct domain *d, gfn_t start, unsigned long nr);
int map_regions_rw_cache(struct domain *d,
- unsigned long start_gfn,
- unsigned long nr_mfns,
- unsigned long mfn);
+ gfn_t gfn,
+ unsigned long nr,
+ mfn_t mfn);
int unmap_regions_rw_cache(struct domain *d,
- unsigned long start_gfn,
- unsigned long nr_mfns,
- unsigned long mfn);
+ gfn_t gfn,
+ unsigned long nr,
+ mfn_t mfn);
int map_dev_mmio_region(struct domain *d,
gfn_t gfn,
--
1.9.1
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v5 03/17] xen/arm: Rename grant_table_gfpn into grant_table_gfn and use the typesafe gfn
The correct acronym for a guest physical frame is gfn. Also use
the typesafe gfn to ensure that a guest frame is effectively used.
Signed-off-by: Julien Grall
Acked-by: Stefano Stabellini
---
Changes in v4:
- Add Stefano's acked-by
Changes in v2:
- Remove extra pair of brackets.
---
xen/arch/arm/domain.c | 4 ++--
xen/arch/arm/mm.c | 2 +-
xen/include/asm-arm/domain.h | 2 +-
xen/include/asm-arm/grant_table.h | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index d8a804c..6ce4645 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -464,13 +464,13 @@ struct domain *alloc_domain_struct(void)
return NULL;
clear_page(d);
-d->arch.grant_table_gpfn = xzalloc_array(xen_pfn_t, max_grant_frames);
+d->arch.grant_table_gfn = xzalloc_array(gfn_t, max_grant_frames);
return d;
}
void free_domain_struct(struct domain *d)
{
-xfree(d->arch.grant_table_gpfn);
+xfree(d->arch.grant_table_gfn);
free_xenheap_page(d);
}
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 6882d54..0e408f8 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1082,7 +1082,7 @@ int xenmem_add_to_physmap_one(
return -EINVAL;
}
-d->arch.grant_table_gpfn[idx] = gfn_x(gfn);
+d->arch.grant_table_gfn[idx] = gfn;
t = p2m_ram_rw;
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 370cdeb..979f7de 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -51,7 +51,7 @@ struct arch_domain
uint64_t vttbr;
struct hvm_domain hvm_domain;
-xen_pfn_t *grant_table_gpfn;
+gfn_t *grant_table_gfn;
struct vmmio vmmio;
diff --git a/xen/include/asm-arm/grant_table.h
b/xen/include/asm-arm/grant_table.h
index 5e076cc..eb02423 100644
--- a/xen/include/asm-arm/grant_table.h
+++ b/xen/include/asm-arm/grant_table.h
@@ -30,7 +30,7 @@ static inline int replace_grant_supported(void)
#define gnttab_shared_gmfn(d, t, i) \
( ((i >= nr_grant_frames(d->grant_table)) && \
- (i < max_grant_frames)) ? 0 : (d->arch.grant_table_gpfn[i]))
+ (i < max_grant_frames)) ? 0 : gfn_x(d->arch.grant_table_gfn[i]))
#define gnttab_need_iommu_mapping(d)\
(is_domain_direct_mapped(d) && need_iommu(d))
--
1.9.1
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v5 02/17] xen: Use typesafe gfn in xenmem_add_to_physmap_one
The x86 version of the function xenmem_add_to_physmap_one contains
variable name gpfn and gfn which make the code very confusing.
I have left unchanged for now.
Also, rename gpfn to gfn in the ARM version as the latter is the correct
acronym for a guest physical frame.
Finally, remove the trailing whitespace around the changes.
Signed-off-by: Julien Grall
Acked-by: Jan Beulich
Acked-by: Stefano Stabellini
---
Cc: Stefano Stabellini
Cc: Jan Beulich
Cc: Andrew Cooper
Cc: George Dunlap
Cc: Ian Jackson
Cc: Konrad Rzeszutek Wilk
Cc: Tim Deegan
Cc: Wei Liu
Changes in v4:
- Add Stefano's Acked-by
Changes in v3:
- Add Jan's Acked-by for non-ARM bits
---
xen/arch/arm/mm.c| 10 +-
xen/arch/x86/mm.c| 15 +++
xen/common/memory.c | 6 +++---
xen/include/xen/mm.h | 2 +-
4 files changed, 16 insertions(+), 17 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 5ab9b75..6882d54 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1046,7 +1046,7 @@ int xenmem_add_to_physmap_one(
unsigned int space,
union xen_add_to_physmap_batch_extra extra,
unsigned long idx,
-xen_pfn_t gpfn)
+gfn_t gfn)
{
unsigned long mfn = 0;
int rc;
@@ -1081,8 +1081,8 @@ int xenmem_add_to_physmap_one(
else
return -EINVAL;
}
-
-d->arch.grant_table_gpfn[idx] = gpfn;
+
+d->arch.grant_table_gpfn[idx] = gfn_x(gfn);
t = p2m_ram_rw;
@@ -1145,7 +1145,7 @@ int xenmem_add_to_physmap_one(
if ( extra.res0 )
return -EOPNOTSUPP;
-rc = map_dev_mmio_region(d, gpfn, 1, idx);
+rc = map_dev_mmio_region(d, gfn_x(gfn), 1, idx);
return rc;
default:
@@ -1153,7 +1153,7 @@ int xenmem_add_to_physmap_one(
}
/* Map at new location. */
-rc = guest_physmap_add_entry(d, _gfn(gpfn), _mfn(mfn), 0, t);
+rc = guest_physmap_add_entry(d, gfn, _mfn(mfn), 0, t);
/* If we fail to add the mapping, we need to drop the reference we
* took earlier on foreign pages */
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 7fbc94e..dbcf6cb 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4775,7 +4775,7 @@ int xenmem_add_to_physmap_one(
unsigned int space,
union xen_add_to_physmap_batch_extra extra,
unsigned long idx,
-xen_pfn_t gpfn)
+gfn_t gpfn)
{
struct page_info *page = NULL;
unsigned long gfn = 0; /* gcc ... */
@@ -4834,7 +4834,7 @@ int xenmem_add_to_physmap_one(
break;
}
case XENMAPSPACE_gmfn_foreign:
-return p2m_add_foreign(d, idx, gpfn, extra.foreign_domid);
+return p2m_add_foreign(d, idx, gfn_x(gpfn), extra.foreign_domid);
default:
break;
}
@@ -4849,19 +4849,18 @@ int xenmem_add_to_physmap_one(
}
/* Remove previously mapped page if it was present. */
-prev_mfn = mfn_x(get_gfn(d, gpfn, &p2mt));
+prev_mfn = mfn_x(get_gfn(d, gfn_x(gpfn), &p2mt));
if ( mfn_valid(prev_mfn) )
{
if ( is_xen_heap_mfn(prev_mfn) )
/* Xen heap frames are simply unhooked from this phys slot. */
-guest_physmap_remove_page(d, _gfn(gpfn), _mfn(prev_mfn),
- PAGE_ORDER_4K);
+guest_physmap_remove_page(d, gpfn, _mfn(prev_mfn), PAGE_ORDER_4K);
else
/* Normal domain memory is freed, to avoid leaking memory. */
-guest_remove_page(d, gpfn);
+guest_remove_page(d, gfn_x(gpfn));
}
/* In the XENMAPSPACE_gmfn case we still hold a ref on the old page. */
-put_gfn(d, gpfn);
+put_gfn(d, gfn_x(gpfn));
/* Unmap from old location, if any. */
old_gpfn = get_gpfn_from_mfn(mfn);
@@ -4872,7 +4871,7 @@ int xenmem_add_to_physmap_one(
guest_physmap_remove_page(d, _gfn(old_gpfn), _mfn(mfn), PAGE_ORDER_4K);
/* Map at new location. */
-rc = guest_physmap_add_page(d, _gfn(gpfn), _mfn(mfn), PAGE_ORDER_4K);
+rc = guest_physmap_add_page(d, gpfn, _mfn(mfn), PAGE_ORDER_4K);
/* In the XENMAPSPACE_gmfn, we took a ref of the gfn at the top */
if ( space == XENMAPSPACE_gmfn || space == XENMAPSPACE_gmfn_range )
diff --git a/xen/common/memory.c b/xen/common/memory.c
index a8a75e0..812334b 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -649,7 +649,7 @@ static int xenmem_add_to_physmap(struct domain *d,
if ( xatp->space != XENMAPSPACE_gmfn_range )
return xenmem_add_to_physmap_one(d, xatp->space, extra,
- xatp->idx, xatp->gpfn);
+ xatp->idx, _gfn(xatp->gpfn));
if ( xatp->size < start )
return -EILSEQ;
@@ -666,7 +666,7 @@ static int xenmem_add_to_physmap(struct domain *d,
while ( xatp->size > done )
{
rc = xenmem_add_to_physmap_one(d, xatp->space, extra,
-
[Xen-devel] [PATCH v5 05/17] xen/passthrough: x86: Use INVALID_GFN rather than INVALID_MFN
A variable containing a guest frame should be compared to INVALID_GFN
and not INVALID_GFN.
Signed-off-by: Julien Grall
---
Cc: Suravee Suthikulpanit
Cc: Jan Beulich
Changes in v5:
- Patch added
---
xen/drivers/passthrough/amd/iommu_map.c | 2 +-
xen/drivers/passthrough/x86/iommu.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/xen/drivers/passthrough/amd/iommu_map.c
b/xen/drivers/passthrough/amd/iommu_map.c
index 1b914ba..c758459 100644
--- a/xen/drivers/passthrough/amd/iommu_map.c
+++ b/xen/drivers/passthrough/amd/iommu_map.c
@@ -555,7 +555,7 @@ static int update_paging_mode(struct domain *d, unsigned
long gfn)
unsigned long old_root_mfn;
struct domain_iommu *hd = dom_iommu(d);
-if ( gfn == INVALID_MFN )
+if ( gfn == INVALID_GFN )
return -EADDRNOTAVAIL;
ASSERT(!(gfn >> DEFAULT_DOMAIN_ADDRESS_WIDTH));
diff --git a/xen/drivers/passthrough/x86/iommu.c
b/xen/drivers/passthrough/x86/iommu.c
index a18a608..cd435d7 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -61,7 +61,7 @@ int arch_iommu_populate_page_table(struct domain *d)
unsigned long mfn = page_to_mfn(page);
unsigned long gfn = mfn_to_gmfn(d, mfn);
-if ( gfn != INVALID_MFN )
+if ( gfn != INVALID_GFN )
{
ASSERT(!(gfn >> DEFAULT_DOMAIN_ADDRESS_WIDTH));
BUG_ON(SHARED_M2P(gfn));
--
1.9.1
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v5 01/17] xen: Use typesafe gfn/mfn in guest_physmap_* helpers
Also rename some variables to gfn or mfn when it does not require much
rework.
Finally replace %hu with %d when printing the domain id in
guest_physmap_add_entry (arch/x86/mm/p2m.c).
Signed-off-by: Julien Grall
Acked-by: Jan Beulich
Acked-by: Stefano Stabellini
---
Cc: Stefano Stabellini
Cc: Jan Beulich
Cc: Andrew Cooper
Cc: Paul Durrant
Cc: George Dunlap
Cc: Ian Jackson
Cc: Konrad Rzeszutek Wilk
Cc: Tim Deegan
Cc: Wei Liu
Changes in v4:
- Add Stefano's Acked-by
Changes in v3:
- Use %d to print the domain id rather than %hu
- Add Jan's Acked-by for non-ARM bits
Changes in v2:
- Don't use a wrapper for x86. Instead use mfn_* to make
the change simpler.
---
xen/arch/arm/domain_build.c| 2 +-
xen/arch/arm/mm.c | 10 ++---
xen/arch/arm/p2m.c | 20 +-
xen/arch/x86/domain.c | 5 ++-
xen/arch/x86/domain_build.c| 6 +--
xen/arch/x86/hvm/ioreq.c | 8 ++--
xen/arch/x86/mm.c | 12 +++---
xen/arch/x86/mm/p2m.c | 78 --
xen/common/grant_table.c | 7 ++--
xen/common/memory.c| 32
xen/drivers/passthrough/arm/smmu.c | 4 +-
xen/include/asm-arm/p2m.h | 12 +++---
xen/include/asm-x86/p2m.h | 11 +++---
xen/include/xen/mm.h | 2 +-
14 files changed, 110 insertions(+), 99 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 410bb4f..9035486 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -117,7 +117,7 @@ static bool_t insert_11_bank(struct domain *d,
goto fail;
}
-res = guest_physmap_add_page(d, spfn, spfn, order);
+res = guest_physmap_add_page(d, _gfn(spfn), _mfn(spfn), order);
if ( res )
panic("Failed map pages to DOM0: %d", res);
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 2ec211b..5ab9b75 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1153,7 +1153,7 @@ int xenmem_add_to_physmap_one(
}
/* Map at new location. */
-rc = guest_physmap_add_entry(d, gpfn, mfn, 0, t);
+rc = guest_physmap_add_entry(d, _gfn(gpfn), _mfn(mfn), 0, t);
/* If we fail to add the mapping, we need to drop the reference we
* took earlier on foreign pages */
@@ -1282,8 +1282,8 @@ int create_grant_host_mapping(unsigned long addr,
unsigned long frame,
if ( flags & GNTMAP_readonly )
t = p2m_grant_map_ro;
-rc = guest_physmap_add_entry(current->domain, addr >> PAGE_SHIFT,
- frame, 0, t);
+rc = guest_physmap_add_entry(current->domain, _gfn(addr >> PAGE_SHIFT),
+ _mfn(frame), 0, t);
if ( rc )
return GNTST_general_error;
@@ -1294,13 +1294,13 @@ int create_grant_host_mapping(unsigned long addr,
unsigned long frame,
int replace_grant_host_mapping(unsigned long addr, unsigned long mfn,
unsigned long new_addr, unsigned int flags)
{
-unsigned long gfn = (unsigned long)(addr >> PAGE_SHIFT);
+gfn_t gfn = _gfn(addr >> PAGE_SHIFT);
struct domain *d = current->domain;
if ( new_addr != 0 || (flags & GNTMAP_contains_pte) )
return GNTST_general_error;
-guest_physmap_remove_page(d, gfn, mfn, 0);
+guest_physmap_remove_page(d, gfn, _mfn(mfn), 0);
return GNTST_okay;
}
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 5afae1d..0395a40 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1292,26 +1292,26 @@ int map_dev_mmio_region(struct domain *d,
}
int guest_physmap_add_entry(struct domain *d,
-unsigned long gpfn,
-unsigned long mfn,
+gfn_t gfn,
+mfn_t mfn,
unsigned long page_order,
p2m_type_t t)
{
return apply_p2m_changes(d, INSERT,
- pfn_to_paddr(gpfn),
- pfn_to_paddr(gpfn + (1 << page_order)),
- pfn_to_paddr(mfn), MATTR_MEM, 0, t,
+ pfn_to_paddr(gfn_x(gfn)),
+ pfn_to_paddr(gfn_x(gfn) + (1 << page_order)),
+ pfn_to_paddr(mfn_x(mfn)), MATTR_MEM, 0, t,
d->arch.p2m.default_access);
}
void guest_physmap_remove_page(struct domain *d,
- unsigned long gpfn,
- unsigned long mfn, unsigned int page_order)
+ gfn_t gfn,
+ mfn_t mfn, unsigned int page_order)
{
apply_p2m_changes(d, REMOVE,
- pfn_to_paddr(gpfn),
- pfn_to_paddr(gpfn + (1
[Xen-devel] [PATCH v5 11/17] xen/arm: dom0_build: Remove dead code in allocate_memory
The code to allocate memory when dom0 does not use direct mapping is
relying on the presence of memory node in the DT.
However, they are not present when booting using UEFI or when using
ACPI.
Rather than fixing the code, remove it because dom0 is always direct
memory mapped and therefore the code is never tested. Also add a
check to avoid disabling direct memory mapped and not implementing
the associated RAM bank allocation.
Signed-off-by: Julien Grall
---
Changes in v4:
- Patch added
---
xen/arch/arm/domain_build.c | 58 ++---
1 file changed, 7 insertions(+), 51 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 49185f0..923f48a 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -235,7 +235,7 @@ fail:
* (as described above) we allow higher allocations and continue until
* that runs out (or we have allocated sufficient dom0 memory).
*/
-static void allocate_memory_11(struct domain *d, struct kernel_info *kinfo)
+static void allocate_memory(struct domain *d, struct kernel_info *kinfo)
{
const unsigned int min_low_order =
get_order_from_bytes(min_t(paddr_t, dom0_mem, MB(128)));
@@ -247,6 +247,12 @@ static void allocate_memory_11(struct domain *d, struct
kernel_info *kinfo)
bool_t lowmem = is_32bit_domain(d);
unsigned int bits;
+/*
+ * TODO: Implement memory bank allocation when DOM0 is not direct
+ * mapped
+ */
+BUG_ON(!dom0_11_mapping);
+
printk("Allocating 1:1 mappings totalling %ldMB for dom0:\n",
/* Don't want format this as PRIpaddr (16 digit hex) */
(unsigned long)(kinfo->unassigned_mem >> 20));
@@ -343,56 +349,6 @@ static void allocate_memory_11(struct domain *d, struct
kernel_info *kinfo)
}
}
-static void allocate_memory(struct domain *d, struct kernel_info *kinfo)
-{
-
-struct dt_device_node *memory = NULL;
-const void *reg;
-u32 reg_len, reg_size;
-unsigned int bank = 0;
-
-if ( dom0_11_mapping )
-return allocate_memory_11(d, kinfo);
-
-while ( (memory = dt_find_node_by_type(memory, "memory")) )
-{
-int l;
-
-dt_dprintk("memory node\n");
-
-reg_size = dt_cells_to_size(dt_n_addr_cells(memory) +
dt_n_size_cells(memory));
-
-reg = dt_get_property(memory, "reg", ®_len);
-if ( reg == NULL )
-panic("Memory node has no reg property");
-
-for ( l = 0;
- kinfo->unassigned_mem > 0 && l + reg_size <= reg_len
- && kinfo->mem.nr_banks < NR_MEM_BANKS;
- l += reg_size )
-{
-paddr_t start, size;
-
-if ( dt_device_get_address(memory, bank, &start, &size) )
-panic("Unable to retrieve the bank %u for %s",
- bank, dt_node_full_name(memory));
-
-if ( size > kinfo->unassigned_mem )
-size = kinfo->unassigned_mem;
-
-printk("Populate P2M %#"PRIx64"->%#"PRIx64"\n",
- start, start + size);
-if ( p2m_populate_ram(d, start, start + size) < 0 )
-panic("Failed to populate P2M");
-kinfo->mem.bank[kinfo->mem.nr_banks].start = start;
-kinfo->mem.bank[kinfo->mem.nr_banks].size = size;
-kinfo->mem.nr_banks++;
-
-kinfo->unassigned_mem -= size;
-}
-}
-}
-
static int write_properties(struct domain *d, struct kernel_info *kinfo,
const struct dt_device_node *node)
{
--
1.9.1
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v5 06/17] xen: Use a typesafe to define INVALID_MFN
Also take the opportunity to convert arch/x86/debug.c to the typesafe
mfn.
Signed-off-by: Julien Grall
---
Cc: Christoph Egger
Cc: Liu Jinsong
Cc: Jan Beulich
Cc: Andrew Cooper
Cc: Mukesh Rathor
Cc: Paul Durrant
Cc: Jun Nakajima
Cc: Kevin Tian
Cc: George Dunlap
Cc: Tim Deegan
Changes in v5:
- Patch added
---
xen/arch/arm/p2m.c | 4 ++--
xen/arch/x86/cpu/mcheck/mce.c | 2 +-
xen/arch/x86/debug.c| 50 ---
xen/arch/x86/hvm/hvm.c | 6 ++---
xen/arch/x86/hvm/viridian.c | 6 ++---
xen/arch/x86/hvm/vmx/vmx.c | 2 +-
xen/arch/x86/mm/guest_walk.c| 4 ++--
xen/arch/x86/mm/hap/hap.c | 4 ++--
xen/arch/x86/mm/p2m-ept.c | 6 ++---
xen/arch/x86/mm/p2m-pod.c | 18 +++---
xen/arch/x86/mm/p2m-pt.c| 18 +++---
xen/arch/x86/mm/p2m.c | 52 -
xen/arch/x86/mm/paging.c| 12 +-
xen/arch/x86/mm/shadow/common.c | 44 +-
xen/arch/x86/mm/shadow/multi.c | 36 ++--
xen/common/domain.c | 6 ++---
xen/common/grant_table.c| 6 ++---
xen/include/xen/mm.h| 2 +-
18 files changed, 140 insertions(+), 138 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 34563bb..d690602 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1461,7 +1461,7 @@ int relinquish_p2m_mapping(struct domain *d)
return apply_p2m_changes(d, RELINQUISH,
pfn_to_paddr(p2m->lowest_mapped_gfn),
pfn_to_paddr(p2m->max_mapped_gfn),
- pfn_to_paddr(INVALID_MFN),
+ pfn_to_paddr(mfn_x(INVALID_MFN)),
MATTR_MEM, 0, p2m_invalid,
d->arch.p2m.default_access);
}
@@ -1476,7 +1476,7 @@ int p2m_cache_flush(struct domain *d, xen_pfn_t
start_mfn, xen_pfn_t end_mfn)
return apply_p2m_changes(d, CACHEFLUSH,
pfn_to_paddr(start_mfn),
pfn_to_paddr(end_mfn),
- pfn_to_paddr(INVALID_MFN),
+ pfn_to_paddr(mfn_x(INVALID_MFN)),
MATTR_MEM, 0, p2m_invalid,
d->arch.p2m.default_access);
}
diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index edcbe48..2695b0c 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -1455,7 +1455,7 @@ long do_mca(XEN_GUEST_HANDLE_PARAM(xen_mc_t) u_xen_mc)
gfn = PFN_DOWN(gaddr);
mfn = mfn_x(get_gfn(d, gfn, &t));
-if ( mfn == INVALID_MFN )
+if ( mfn == mfn_x(INVALID_MFN) )
{
put_gfn(d, gfn);
put_domain(d);
diff --git a/xen/arch/x86/debug.c b/xen/arch/x86/debug.c
index 58cae22..3479f7c 100644
--- a/xen/arch/x86/debug.c
+++ b/xen/arch/x86/debug.c
@@ -43,11 +43,11 @@ typedef unsigned long dbgva_t;
typedef unsigned char dbgbyte_t;
/* Returns: mfn for the given (hvm guest) vaddr */
-static unsigned long
+static mfn_t
dbg_hvm_va2mfn(dbgva_t vaddr, struct domain *dp, int toaddr,
unsigned long *gfn)
{
-unsigned long mfn;
+mfn_t mfn;
uint32_t pfec = PFEC_page_present;
p2m_type_t gfntype;
@@ -60,16 +60,17 @@ dbg_hvm_va2mfn(dbgva_t vaddr, struct domain *dp, int toaddr,
return INVALID_MFN;
}
-mfn = mfn_x(get_gfn(dp, *gfn, &gfntype));
+mfn = get_gfn(dp, *gfn, &gfntype);
if ( p2m_is_readonly(gfntype) && toaddr )
{
DBGP2("kdb:p2m_is_readonly: gfntype:%x\n", gfntype);
mfn = INVALID_MFN;
}
else
-DBGP2("X: vaddr:%lx domid:%d mfn:%lx\n", vaddr, dp->domain_id, mfn);
+DBGP2("X: vaddr:%lx domid:%d mfn:%lx\n",
+ vaddr, dp->domain_id, mfn_x(mfn));
-if ( mfn == INVALID_MFN )
+if ( mfn_eq(mfn, INVALID_MFN) )
{
put_gfn(dp, *gfn);
*gfn = INVALID_GFN;
@@ -91,7 +92,7 @@ dbg_hvm_va2mfn(dbgva_t vaddr, struct domain *dp, int toaddr,
* mode.
* Returns: mfn for the given (pv guest) vaddr
*/
-static unsigned long
+static mfn_t
dbg_pv_va2mfn(dbgva_t vaddr, struct domain *dp, uint64_t pgd3val)
{
l4_pgentry_t l4e, *l4t;
@@ -99,31 +100,31 @@ dbg_pv_va2mfn(dbgva_t vaddr, struct domain *dp, uint64_t
pgd3val)
l2_pgentry_t l2e, *l2t;
l1_pgentry_t l1e, *l1t;
unsigned long cr3 = (pgd3val ? pgd3val : dp->vcpu[0]->arch.cr3);
-unsigned long mfn = cr3 >> PAGE_SHIFT;
+mfn_t mfn = _mfn(cr3 >> PAGE_SHIFT);
DBGP2("vaddr:%lx domid:%d cr3:%lx pgd3:%lx\n", vaddr, dp->domain_id,
cr3, pgd3val);
if ( pgd3val == 0 )
{
-l4t = map_domain_page(_mfn(mfn));
+l4t = map_domain_page(mfn);
l4e = l4t[l4
[Xen-devel] [PATCH v5 12/17] xen/arm: p2m: Remove unused operation ALLOCATE
The operation ALLOCATE is unused. If we ever need it, it could be
reimplemented with INSERT.
Signed-off-by: Julien Grall
---
Changes in v4:
- Patch added
---
xen/arch/arm/p2m.c| 67 ++-
xen/include/asm-arm/p2m.h | 3 ---
2 files changed, 2 insertions(+), 68 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index fcc4513..f11094e 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -547,7 +547,6 @@ static int p2m_mem_access_radix_set(struct p2m_domain *p2m,
unsigned long pfn,
enum p2m_operation {
INSERT,
-ALLOCATE,
REMOVE,
RELINQUISH,
CACHEFLUSH,
@@ -667,7 +666,6 @@ static int apply_one_level(struct domain *d,
{
const paddr_t level_size = level_sizes[level];
const paddr_t level_mask = level_masks[level];
-const paddr_t level_shift = level_shifts[level];
struct p2m_domain *p2m = &d->arch.p2m;
lpae_t pte;
@@ -678,58 +676,6 @@ static int apply_one_level(struct domain *d,
switch ( op )
{
-case ALLOCATE:
-ASSERT(level < 3 || !p2m_valid(orig_pte));
-ASSERT(*maddr == 0);
-
-if ( p2m_valid(orig_pte) )
-return P2M_ONE_DESCEND;
-
-if ( is_mapping_aligned(*addr, end_gpaddr, 0, level_size) &&
- /* We only create superpages when mem_access is not in use. */
- (level == 3 || (level < 3 && !p2m->mem_access_enabled)) )
-{
-struct page_info *page;
-
-page = alloc_domheap_pages(d, level_shift - PAGE_SHIFT, 0);
-if ( page )
-{
-rc = p2m_mem_access_radix_set(p2m, paddr_to_pfn(*addr), a);
-if ( rc < 0 )
-{
-free_domheap_page(page);
-return rc;
-}
-
-pte = mfn_to_p2m_entry(page_to_mfn(page), mattr, t, a);
-if ( level < 3 )
-pte.p2m.table = 0;
-p2m_write_pte(entry, pte, flush_cache);
-p2m->stats.mappings[level]++;
-
-*addr += level_size;
-
-return P2M_ONE_PROGRESS;
-}
-else if ( level == 3 )
-return -ENOMEM;
-}
-
-/* L3 is always suitably aligned for mapping (handled, above) */
-BUG_ON(level == 3);
-
-/*
- * If we get here then we failed to allocate a sufficiently
- * large contiguous region for this level (which can't be
- * L3) or mem_access is in use. Create a page table and
- * continue to descend so we try smaller allocations.
- */
-rc = p2m_create_table(d, entry, 0, flush_cache);
-if ( rc < 0 )
-return rc;
-
-return P2M_ONE_DESCEND;
-
case INSERT:
if ( is_mapping_aligned(*addr, end_gpaddr, *maddr, level_size) &&
/*
@@ -1169,7 +1115,7 @@ static int apply_p2m_changes(struct domain *d,
}
}
-if ( op == ALLOCATE || op == INSERT )
+if ( op == INSERT )
{
p2m->max_mapped_gfn = max(p2m->max_mapped_gfn, egfn);
p2m->lowest_mapped_gfn = min(p2m->lowest_mapped_gfn, sgfn);
@@ -1197,7 +1143,7 @@ out:
spin_unlock(&p2m->lock);
-if ( rc < 0 && ( op == INSERT || op == ALLOCATE ) &&
+if ( rc < 0 && ( op == INSERT ) &&
addr != start_gpaddr )
{
BUG_ON(addr == end_gpaddr);
@@ -1212,15 +1158,6 @@ out:
return rc;
}
-int p2m_populate_ram(struct domain *d,
- paddr_t start,
- paddr_t end)
-{
-return apply_p2m_changes(d, ALLOCATE, start, end,
- 0, MATTR_MEM, 0, p2m_ram_rw,
- d->arch.p2m.default_access);
-}
-
int map_regions_rw_cache(struct domain *d,
unsigned long start_gfn,
unsigned long nr,
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 8a96e68..4752161 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -141,9 +141,6 @@ mfn_t p2m_lookup(struct domain *d, gfn_t gfn, p2m_type_t
*t);
/* Clean & invalidate caches corresponding to a region of guest address space
*/
int p2m_cache_flush(struct domain *d, gfn_t start, unsigned long nr);
-/* Setup p2m RAM mapping for domain d from start-end. */
-int p2m_populate_ram(struct domain *d, paddr_t start, paddr_t end);
-
int map_regions_rw_cache(struct domain *d,
unsigned long start_gfn,
unsigned long nr_mfns,
--
1.9.1
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v5 08/17] xen/arm: Rework the interface of p2m_lookup and use typesafe gfn and mfn
The prototype and the declaration of p2m_lookup disagree on how the
function should be used. One expect a frame number whilst the other
an address.
Thankfully, everyone is using with an address today. However, most of
the callers have to convert a guest frame to an address. Modify
the interface to take a guest physical frame in parameter and return
a machine frame.
Whilst modifying the interface, use typesafe gfn and mfn for clarity
and catching possible misusage.
Signed-off-by: Julien Grall
---
Changes in v4:
- Use INVALID_MFN_T when possible
---
xen/arch/arm/p2m.c| 43 +++
xen/arch/arm/traps.c | 21 +++--
xen/include/asm-arm/p2m.h | 7 +++
3 files changed, 37 insertions(+), 34 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index c938dde..54a363a 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -140,14 +140,15 @@ void flush_tlb_domain(struct domain *d)
}
/*
- * Lookup the MFN corresponding to a domain's PFN.
+ * Lookup the MFN corresponding to a domain's GFN.
*
* There are no processor functions to do a stage 2 only lookup therefore we
* do a a software walk.
*/
-static paddr_t __p2m_lookup(struct domain *d, paddr_t paddr, p2m_type_t *t)
+static mfn_t __p2m_lookup(struct domain *d, gfn_t gfn, p2m_type_t *t)
{
struct p2m_domain *p2m = &d->arch.p2m;
+const paddr_t paddr = pfn_to_paddr(gfn_x(gfn));
const unsigned int offsets[4] = {
zeroeth_table_offset(paddr),
first_table_offset(paddr),
@@ -158,7 +159,7 @@ static paddr_t __p2m_lookup(struct domain *d, paddr_t
paddr, p2m_type_t *t)
ZEROETH_MASK, FIRST_MASK, SECOND_MASK, THIRD_MASK
};
lpae_t pte, *map;
-paddr_t maddr = INVALID_PADDR;
+mfn_t mfn = INVALID_MFN;
paddr_t mask = 0;
p2m_type_t _t;
unsigned int level, root_table;
@@ -216,21 +217,22 @@ static paddr_t __p2m_lookup(struct domain *d, paddr_t
paddr, p2m_type_t *t)
{
ASSERT(mask);
ASSERT(pte.p2m.type != p2m_invalid);
-maddr = (pte.bits & PADDR_MASK & mask) | (paddr & ~mask);
+mfn = _mfn(paddr_to_pfn((pte.bits & PADDR_MASK & mask) |
+(paddr & ~mask)));
*t = pte.p2m.type;
}
err:
-return maddr;
+return mfn;
}
-paddr_t p2m_lookup(struct domain *d, paddr_t paddr, p2m_type_t *t)
+mfn_t p2m_lookup(struct domain *d, gfn_t gfn, p2m_type_t *t)
{
-paddr_t ret;
+mfn_t ret;
struct p2m_domain *p2m = &d->arch.p2m;
spin_lock(&p2m->lock);
-ret = __p2m_lookup(d, paddr, t);
+ret = __p2m_lookup(d, gfn, t);
spin_unlock(&p2m->lock);
return ret;
@@ -493,8 +495,9 @@ static int __p2m_get_mem_access(struct domain *d, gfn_t gfn,
* No setting was found in the Radix tree. Check if the
* entry exists in the page-tables.
*/
-paddr_t maddr = __p2m_lookup(d, gfn_x(gfn) << PAGE_SHIFT, NULL);
-if ( INVALID_PADDR == maddr )
+mfn_t mfn = __p2m_lookup(d, gfn, NULL);
+
+if ( mfn_eq(mfn, INVALID_MFN) )
return -ESRCH;
/* If entry exists then its rwx. */
@@ -1483,8 +1486,7 @@ int p2m_cache_flush(struct domain *d, xen_pfn_t
start_mfn, xen_pfn_t end_mfn)
mfn_t gfn_to_mfn(struct domain *d, gfn_t gfn)
{
-paddr_t p = p2m_lookup(d, pfn_to_paddr(gfn_x(gfn)), NULL);
-return _mfn(p >> PAGE_SHIFT);
+return p2m_lookup(d, gfn, NULL);
}
/*
@@ -1498,8 +1500,8 @@ p2m_mem_access_check_and_get_page(vaddr_t gva, unsigned
long flag)
{
long rc;
paddr_t ipa;
-unsigned long maddr;
-unsigned long mfn;
+gfn_t gfn;
+mfn_t mfn;
xenmem_access_t xma;
p2m_type_t t;
struct page_info *page = NULL;
@@ -1508,11 +1510,13 @@ p2m_mem_access_check_and_get_page(vaddr_t gva, unsigned
long flag)
if ( rc < 0 )
goto err;
+gfn = _gfn(paddr_to_pfn(ipa));
+
/*
* We do this first as this is faster in the default case when no
* permission is set on the page.
*/
-rc = __p2m_get_mem_access(current->domain, _gfn(paddr_to_pfn(ipa)), &xma);
+rc = __p2m_get_mem_access(current->domain, gfn, &xma);
if ( rc < 0 )
goto err;
@@ -1561,12 +1565,11 @@ p2m_mem_access_check_and_get_page(vaddr_t gva, unsigned
long flag)
* We had a mem_access permission limiting the access, but the page type
* could also be limiting, so we need to check that as well.
*/
-maddr = __p2m_lookup(current->domain, ipa, &t);
-if ( maddr == INVALID_PADDR )
+mfn = __p2m_lookup(current->domain, gfn, &t);
+if ( mfn_eq(mfn, INVALID_MFN) )
goto err;
-mfn = maddr >> PAGE_SHIFT;
-if ( !mfn_valid(mfn) )
+if ( !mfn_valid(mfn_x(mfn)) )
goto err;
/*
@@ -1575,7 +1578,7 @@ p2m_mem_access_check_and_get_page(vaddr_t gva, unsigned
long flag)
if ( t != p2m_ram_rw )
goto err;
-page = mfn_to_page(mfn);
[Xen-devel] [PATCH v5 16/17] xen/arm: p2m: Use typesafe gfn for {max, lowest}_mapped_gfn
Signed-off-by: Julien Grall
---
Changes in v4:
- Patch added
---
xen/arch/arm/mm.c | 2 +-
xen/arch/arm/p2m.c| 18 +-
xen/include/asm-arm/p2m.h | 4 ++--
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index b5fc034..4e256c2 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1004,7 +1004,7 @@ int page_is_ram_type(unsigned long mfn, unsigned long
mem_type)
unsigned long domain_get_maximum_gpfn(struct domain *d)
{
-return d->arch.p2m.max_mapped_gfn;
+return gfn_x(d->arch.p2m.max_mapped_gfn);
}
void share_xen_page_with_guest(struct page_info *page,
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index a5b584b..9fdc417 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -976,7 +976,7 @@ static int apply_p2m_changes(struct domain *d,
* This is set in preempt_count_limit.
*
*/
-p2m->lowest_mapped_gfn = addr >> PAGE_SHIFT;
+p2m->lowest_mapped_gfn = _gfn(addr >> PAGE_SHIFT);
rc = -ERESTART;
goto out;
@@ -1117,8 +1117,8 @@ static int apply_p2m_changes(struct domain *d,
if ( op == INSERT )
{
-p2m->max_mapped_gfn = max(p2m->max_mapped_gfn, egfn);
-p2m->lowest_mapped_gfn = min(p2m->lowest_mapped_gfn, sgfn);
+p2m->max_mapped_gfn = gfn_max(p2m->max_mapped_gfn, _gfn(egfn));
+p2m->lowest_mapped_gfn = gfn_min(p2m->lowest_mapped_gfn, _gfn(sgfn));
}
rc = 0;
@@ -1383,8 +1383,8 @@ int p2m_init(struct domain *d)
p2m->root = NULL;
-p2m->max_mapped_gfn = 0;
-p2m->lowest_mapped_gfn = ULONG_MAX;
+p2m->max_mapped_gfn = _gfn(0);
+p2m->lowest_mapped_gfn = _gfn(ULONG_MAX);
p2m->default_access = p2m_access_rwx;
p2m->mem_access_enabled = false;
@@ -1401,8 +1401,8 @@ int relinquish_p2m_mapping(struct domain *d)
struct p2m_domain *p2m = &d->arch.p2m;
return apply_p2m_changes(d, RELINQUISH,
- pfn_to_paddr(p2m->lowest_mapped_gfn),
- pfn_to_paddr(p2m->max_mapped_gfn),
+ pfn_to_paddr(gfn_x(p2m->lowest_mapped_gfn)),
+ pfn_to_paddr(gfn_x(p2m->max_mapped_gfn)),
pfn_to_paddr(mfn_x(INVALID_MFN)),
MATTR_MEM, 0, p2m_invalid,
d->arch.p2m.default_access);
@@ -1413,8 +1413,8 @@ int p2m_cache_flush(struct domain *d, gfn_t start,
unsigned long nr)
struct p2m_domain *p2m = &d->arch.p2m;
gfn_t end = gfn_add(start, nr);
-start = gfn_max(start, _gfn(p2m->lowest_mapped_gfn));
-end = gfn_min(end, _gfn(p2m->max_mapped_gfn));
+start = gfn_max(start, p2m->lowest_mapped_gfn);
+end = gfn_min(end, p2m->max_mapped_gfn);
return apply_p2m_changes(d, CACHEFLUSH,
pfn_to_paddr(gfn_x(start)),
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 6e258b9..34096bc 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -34,13 +34,13 @@ struct p2m_domain {
/* Highest guest frame that's ever been mapped in the p2m
* Only takes into account ram and foreign mapping
*/
-unsigned long max_mapped_gfn;
+gfn_t max_mapped_gfn;
/* Lowest mapped gfn in the p2m. When releasing mapped gfn's in a
* preemptible manner this is update to track recall where to
* resume the search. Apart from during teardown this can only
* decrease. */
-unsigned long lowest_mapped_gfn;
+gfn_t lowest_mapped_gfn;
/* Gather some statistics for information purposes only */
struct {
--
1.9.1
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v5 13/17] xen/arm: Use the typesafes mfn and gfn in map_dev_mmio_region...
to avoid mixing machine frame with guest frame. Also drop the prefix start_.
Signed-off-by: Julien Grall
---
Changes in v4:
- Patch added
---
xen/arch/arm/mm.c | 2 +-
xen/arch/arm/p2m.c| 10 +-
xen/include/asm-arm/p2m.h | 4 ++--
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 0e408f8..b5fc034 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1145,7 +1145,7 @@ int xenmem_add_to_physmap_one(
if ( extra.res0 )
return -EOPNOTSUPP;
-rc = map_dev_mmio_region(d, gfn_x(gfn), 1, idx);
+rc = map_dev_mmio_region(d, gfn, 1, _mfn(idx));
return rc;
default:
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index f11094e..5ffc3df 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1211,20 +1211,20 @@ int unmap_mmio_regions(struct domain *d,
}
int map_dev_mmio_region(struct domain *d,
-unsigned long start_gfn,
+gfn_t gfn,
unsigned long nr,
-unsigned long mfn)
+mfn_t mfn)
{
int res;
-if ( !(nr && iomem_access_permitted(d, mfn, mfn + nr - 1)) )
+if ( !(nr && iomem_access_permitted(d, mfn_x(mfn), mfn_x(mfn) + nr - 1)) )
return 0;
-res = map_mmio_regions(d, _gfn(start_gfn), nr, _mfn(mfn));
+res = map_mmio_regions(d, gfn, nr, mfn);
if ( res < 0 )
{
printk(XENLOG_G_ERR "Unable to map [%#lx - %#lx] in Dom%d\n",
- mfn, mfn + nr - 1, d->domain_id);
+ mfn_x(mfn), mfn_x(mfn) + nr - 1, d->domain_id);
return res;
}
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 4752161..8d29eda 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -152,9 +152,9 @@ int unmap_regions_rw_cache(struct domain *d,
unsigned long mfn);
int map_dev_mmio_region(struct domain *d,
-unsigned long start_gfn,
+gfn_t gfn,
unsigned long nr,
-unsigned long mfn);
+mfn_t mfn);
int guest_physmap_add_entry(struct domain *d,
gfn_t gfn,
--
1.9.1
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] making xenstore domain easy configurable
>>> On 28.06.16 at 15:59, wrote: > For xenstored running in the same domain as the toolstack, sockets are > less overhead than the shared memory ring, as no hypercalls are > involved. There is also the unfortunate problem that one of the two > linux devices for xenstored *still* causes deadlocks when used; a > problem which is unresolved from Linux 3.14. And these deadlocks aren't possibly related to the introduction of FMODE_ATOMIC_POS, and hence would be resolved by https://patchwork.kernel.org/patch/8752411/(or something along those lines, if nonseekable_open() isn't used on that code path)? I ask because we had a similar report, but when I put together the (refused upstream) patch I assumed the files under /dev wouldn't have the same issue (and I still didn't check whether they would). Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] GDBSX Maintainer
Hi,
I had to modify some code in arch/x86/debug.c and noticed that Mukesh is
still the maintainer. IIRC he left Oracle quite a while ago, so my
e-mail was bounced by the server.
Do we have a new e-mail address for me? If not, does anyone plan to
maintain this code? Shall we mark the code as "Orphan"?
Cheers,
Forwarded Message
Subject: Undelivered Mail Returned to Sender
Date: Tue, 28 Jun 2016 09:18:43 -0700
From: Mail Delivery System
To: julien.gr...@arm.com
This is the mail system at host usa-sjc-mx-foss1.foss.arm.com.
I'm sorry to have to inform you that your message could not
be delivered to one or more recipients. It's attached below.
For further assistance, please send mail to postmaster.
If you do so, please include this problem report. You can
delete your own text from the attached returned message.
The mail system
: host userp2040.oracle.com[156.151.31.90]
said: 550
5.1.1 User Unknown (in reply to RCPT TO command)
Reporting-MTA: dns; usa-sjc-mx-foss1.foss.arm.com
X-Postfix-Queue-ID: DFADD2F
X-Postfix-Sender: rfc822; julien.grall@arm.com
Arrival-Date: Tue, 28 Jun 2016 09:18:26 -0700 (PDT)
Final-Recipient: rfc822; mukesh.rathor@oracle.com
Original-Recipient: rfc822;mukesh.rathor@oracle.com
Action: failed
Status: 5.1.1
Remote-MTA: dns; userp2040.oracle.com
Diagnostic-Code: smtp; 550 5.1.1 User Unknown
--- Begin Message ---
Hello all,
Some of the ARM functions are mixing gfn vs mfn and even physical vs frame.
To avoid more confusion, this patch series makes use of the terminology
described in xen/include/xen/mm.h and the associated typesafe.
This series requires the patch [1] to be applied beforehand. I pushed a
branch with this patch and this series applied on xenbits:
git://xenbits.xen.org/people/julieng/xen-unstable.git branch typesafe-v4
For all the changes see in each patch.
Yours sincerely,
[1] http://lists.xenproject.org/archives/html/xen-devel/2016-06/msg01744.html
Cc: Andrew Cooper
Cc: Boris Ostrovsky
Cc: Christoph Egger
Cc: Feng Wu
Cc: George Dunlap
Cc: Ian Jackson
Cc: Jan Beulich
Cc: Jun Nakajima
Cc: Kevin Tian
Cc: Konrad Rzeszutek Wilk
Cc: Liu Jinsong
Cc: Mukesh Rathor
Cc: Paul Durrant
Cc: Shannon Zhao
Cc: Stefano Stabellini
Cc: Suravee Suthikulpanit
Cc: Tim Deegan
Cc: Wei Liu
Julien Grall (17):
xen: Use typesafe gfn/mfn in guest_physmap_* helpers
xen: Use typesafe gfn in xenmem_add_to_physmap_one
xen/arm: Rename grant_table_gfpn into grant_table_gfn and use the
typesafe gfn
xen: Use the typesafe mfn and gfn in map_mmio_regions...
xen/passthrough: x86: Use INVALID_GFN rather than INVALID_MFN
xen: Use a typesafe to define INVALID_MFN
xen: Use a typesafe to define INVALID_GFN
xen/arm: Rework the interface of p2m_lookup and use typesafe gfn and
mfn
xen/arm: Rework the interface of p2m_cache_flush and use typesafe gfn
xen/arm: map_regions_rw_cache: Map the region with p2m->default_access
xen/arm: dom0_build: Remove dead code in allocate_memory
xen/arm: p2m: Remove unused operation ALLOCATE
xen/arm: Use the typesafes mfn and gfn in map_dev_mmio_region...
xen/arm: Use the typesafes mfn and gfn in map_regions_rw_cache ...
xen/arm: p2m: Introduce helpers to insert and remove mapping
xen/arm: p2m: Use typesafe gfn for {max,lowest}_mapped_gfn
xen/arm: p2m: Rework the interface of apply_p2m_changes and use
typesafe
xen/arch/arm/domain.c | 4 +-
xen/arch/arm/domain_build.c | 72 ++---
xen/arch/arm/domctl.c | 2 +-
xen/arch/arm/gic-v2.c | 4 +-
xen/arch/arm/mm.c | 20 +--
xen/arch/arm/p2m.c | 269
xen/arch/arm/platforms/exynos5.c| 8 +-
xen/arch/arm/platforms/omap5.c | 16 +-
xen/arch/arm/traps.c| 21 +--
xen/arch/arm/vgic-v2.c | 4 +-
xen/arch/x86/cpu/mcheck/mce.c | 2 +-
xen/arch/x86/debug.c| 64
xen/arch/x86/domain.c | 7 +-
xen/arch/x86/domain_build.c | 6 +-
xen/arch/x86/hvm/emulate.c | 7 +-
xen/arch/x86/hvm/hvm.c | 12 +-
xen/arch/x86/hvm/ioreq.c| 16 +-
xen/arch/x86/hvm/svm/nestedsvm.c| 2 +-
xen/arch/x86/hvm/viridian.c | 6 +-
xen/arch/x86/hvm/vmx/vmx.c | 8 +-
xen/arch/x86/mm.c | 21 +--
xen/arch/x86/mm/altp2m.c| 2 +-
xen/arch/x86/mm/guest_walk.c| 4 +-
xen/arch/x86/mm/hap/guest_walk.c| 10 +-
xen/arch/x86/mm/hap/hap.c | 4 +-
xen/arch/x86/mm/hap/nested_ept.c| 2 +-
xen/arch/x86/mm/p2m-ept.c | 6 +-
xen/arch/x86/mm/p2m-pod.c | 24 +--
xen/arch/x86/mm/p2m-pt.c| 18 +--
xen/arch/x86/mm/p2m.c | 164 ++-
xen/arch/x86/mm
[Xen-devel] [PATCH] xen/arm: io: Protect the handlers with a read-write lock
Currently, accessing the I/O handlers does not require to take a lock
because new handlers are always added at the end of the array. In a
follow-up patch, this array will be sort to optimize the look up.
Given that most of the time the I/O handlers will not be modify,
using a spinlock will add contention when multiple vCPU are accessing
the emulated MMIOs. So use a read-write lock to protected the handlers.
Finally, take the opportunity to re-indent correctly domain_io_init.
Signed-off-by: Julien Grall
---
xen/arch/arm/io.c | 47 +++---
xen/include/asm-arm/mmio.h | 3 ++-
2 files changed, 30 insertions(+), 20 deletions(-)
diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
index 0156755..5a96836 100644
--- a/xen/arch/arm/io.c
+++ b/xen/arch/arm/io.c
@@ -70,23 +70,39 @@ static int handle_write(const struct mmio_handler *handler,
struct vcpu *v,
handler->priv);
}
-int handle_mmio(mmio_info_t *info)
+static const struct mmio_handler *find_mmio_handler(struct domain *d,
+paddr_t gpa)
{
-struct vcpu *v = current;
-int i;
-const struct mmio_handler *handler = NULL;
-const struct vmmio *vmmio = &v->domain->arch.vmmio;
+const struct mmio_handler *handler;
+unsigned int i;
+struct vmmio *vmmio = &d->arch.vmmio;
+
+read_lock(&vmmio->lock);
for ( i = 0; i < vmmio->num_entries; i++ )
{
handler = &vmmio->handlers[i];
-if ( (info->gpa >= handler->addr) &&
- (info->gpa < (handler->addr + handler->size)) )
+if ( (gpa >= handler->addr) &&
+ (gpa < (handler->addr + handler->size)) )
break;
}
if ( i == vmmio->num_entries )
+handler = NULL;
+
+read_unlock(&vmmio->lock);
+
+return handler;
+}
+
+int handle_mmio(mmio_info_t *info)
+{
+struct vcpu *v = current;
+const struct mmio_handler *handler = NULL;
+
+handler = find_mmio_handler(v->domain, info->gpa);
+if ( !handler )
return 0;
if ( info->dabt.write )
@@ -104,7 +120,7 @@ void register_mmio_handler(struct domain *d,
BUG_ON(vmmio->num_entries >= MAX_IO_HANDLER);
-spin_lock(&vmmio->lock);
+write_lock(&vmmio->lock);
handler = &vmmio->handlers[vmmio->num_entries];
@@ -113,24 +129,17 @@ void register_mmio_handler(struct domain *d,
handler->size = size;
handler->priv = priv;
-/*
- * handle_mmio is not using the lock to avoid contention.
- * Make sure the other processors see the new handler before
- * updating the number of entries
- */
-dsb(ish);
-
vmmio->num_entries++;
-spin_unlock(&vmmio->lock);
+write_unlock(&vmmio->lock);
}
int domain_io_init(struct domain *d)
{
- spin_lock_init(&d->arch.vmmio.lock);
- d->arch.vmmio.num_entries = 0;
+rwlock_init(&d->arch.vmmio.lock);
+d->arch.vmmio.num_entries = 0;
- return 0;
+return 0;
}
/*
diff --git a/xen/include/asm-arm/mmio.h b/xen/include/asm-arm/mmio.h
index da1cc2e..32f10f2 100644
--- a/xen/include/asm-arm/mmio.h
+++ b/xen/include/asm-arm/mmio.h
@@ -20,6 +20,7 @@
#define __ASM_ARM_MMIO_H__
#include
+#include
#include
#include
@@ -51,7 +52,7 @@ struct mmio_handler {
struct vmmio {
int num_entries;
-spinlock_t lock;
+rwlock_t lock;
struct mmio_handler handlers[MAX_IO_HANDLER];
};
--
1.9.1
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] making xenstore domain easy configurable
On 6/28/16 11:27 AM, Jan Beulich wrote: On 28.06.16 at 15:59, wrote: >> For xenstored running in the same domain as the toolstack, sockets are >> less overhead than the shared memory ring, as no hypercalls are >> involved. There is also the unfortunate problem that one of the two >> linux devices for xenstored *still* causes deadlocks when used; a >> problem which is unresolved from Linux 3.14. > > And these deadlocks aren't possibly related to the introduction > of FMODE_ATOMIC_POS, and hence would be resolved by > https://patchwork.kernel.org/patch/8752411/(or something along > those lines, if nonseekable_open() isn't used on that code path)? > I ask because we had a similar report, but when I put together the > (refused upstream) patch I assumed the files under /dev wouldn't > have the same issue (and I still didn't check whether they would). > > Jan > They are related to FMODE_ATOMIC_POS. The nodes under /dev don't experience the same issues. Jonathan Creekmore and I submitted patches to make /dev/xen/xenbus and /dev/xen/privcmd the preferred interfaces used by the Xen tools over their /proc/xen/ counterparts in an attempt to prevent people from being bitten by that. One of the patches made it into 4.6 and the other didn't make it in until 4.7. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/EFI + Live Patch: avoid symbol address truncation
On 28/06/16 17:11, Jan Beulich wrote: >>> --- a/xen/arch/x86/xen.lds.S >>> +++ b/xen/arch/x86/xen.lds.S >>> @@ -40,9 +40,20 @@ SECTIONS >>> #if !defined(EFI) >>>. = __XEN_VIRT_START; >>>__image_base__ = .; >>> +#else >>> + . = __image_base__; >>> #endif >>> >>> +#if 0 >>> +/* >>> + * We don't really use this symbol anywhere, and the way it would get >>> defined >>> + * here would result in it having a negative (wrapped to huge positive) >>> + * offset relative to the .text section. That, in turn, causes an assembler >>> + * truncation warning when including all symbols in the symbol table for >>> Live >>> + * Patching code. >>> + */ >>>__2M_text_start = .; /* Start of 2M superpages, mapped RX. */ >>> +#endif >>> >>>. = __XEN_VIRT_START + MB(1); >>>_start = .; >>> @@ -194,14 +205,13 @@ SECTIONS >>> *(.ctors) >>> __ctors_end = .; >>>} :text >>> - . = ALIGN(PAGE_SIZE); >>> - __init_end = .; >>> >>> #ifdef EFI >>>. = ALIGN(MB(2)); >>> #else >>>. = ALIGN(PAGE_SIZE); >>> #endif >>> + __init_end = .; >>>__2M_init_end = .; >>> >>>__2M_rwdata_start = .; /* Start of 2M superpages, mapped RW. */ >>> @@ -296,7 +306,6 @@ ASSERT(__image_base__ > XEN_VIRT_START | >>> ASSERT(kexec_reloc_size - kexec_reloc <= PAGE_SIZE, "kexec_reloc is too >> large") >>> #endif >>> >>> -ASSERT(IS_ALIGNED(__2M_text_start, MB(2)), "__2M_text_start misaligned") >> If we are #if 0'ing the symbol for documentation purposes, can we #if 0 >> this as well? > I considered it, but the two #if-s would end up disconnected. And > with the symbol being first thing in the image (plus the fact that so > far the assertion was there _without_ triggering despite there > being a problem - just one it couldn't detect), I think chances are > slim that it getting fully removed would be a significant problem. > I.e. I'd prefer the patch to remain as is in this regard, but if the > only way to get it acked is to do as you suggest, I would > (hesitantly) do so. Ok. I suppose it is sufficiently well documented because of the other alignment assertions. With the %lu fix, Reviewed-by: Andrew Cooper ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v5 05/17] xen/passthrough: x86: Use INVALID_GFN rather than INVALID_MFN
On 28/06/16 17:17, Julien Grall wrote: > A variable containing a guest frame should be compared to INVALID_GFN > and not INVALID_GFN. > > Signed-off-by: Julien Grall Reviewed-by: Andrew Cooper I suspect these (mis)uses predate my movement of INVALID_GFN from x86 paging code to common code. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] GDBSX Maintainer
On 28/06/16 17:31, Julien Grall wrote: > Hi, > > I had to modify some code in arch/x86/debug.c and noticed that Mukesh > is still the maintainer. IIRC he left Oracle quite a while ago, so my > e-mail was bounced by the server. > > Do we have a new e-mail address for me? If not, does anyone plan to > maintain this code? Shall we mark the code as "Orphan"? If noone explicitly wishes to maintain it, then it should be subsumed into general x86. Its not like its a large or complicated area of code. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH linux 0/8] xen: pvhvm: support bootup on secondary vCPUs
It may happen that Xen's and Linux's ideas of vCPU id diverge. In particular, when we crash on a secondary vCPU we may want to do kdump and unlike plain kexec where we do migrate_to_reboot_cpu() we try booting on the vCPU which crashed. This doesn't work very well for PVHVM guests as we have a number of hypercalls where we pass vCPU id as a parameter. These hypercalls either fail or do something unexpected. To solve the issue we need to have a mapping between Linux's and Xen's vCPU ids. This series solves the issue for x86 PVHVM guests. PV guests don't (and probably won't) support kdump so I always assume Xen's vCPU id == Linux's vCPU id. ARM guests will probably need to get proper mapping once we start supporting kexec/kdump there. Vitaly Kuznetsov (8): x86/xen: update cpuid.h from Xen-4.7 xen: introduce xen_vcpu_id mapping x86/xen: use xen_vcpu_id mapping for HYPERVISOR_vcpu_op x86/xen: use xen_vcpu_id mapping when pointing vcpu_info to the shared_info page xen/events: use xen_vcpu_id mapping in events_base xen/events: fifo: use xen_vcpu_id mapping xen/evtchn: use xen_vcpu_id mapping xen/pvhvm: run xen_vcpu_setup() for the boot CPU arch/arm/xen/enlighten.c | 10 ++ arch/x86/include/asm/xen/cpuid.h | 5 - arch/x86/include/asm/xen/hypercall.h | 11 ++- arch/x86/xen/enlighten.c | 35 --- arch/x86/xen/smp.c | 7 +++ arch/x86/xen/xen-ops.h | 1 + drivers/xen/events/events_base.c | 10 +- drivers/xen/events/events_fifo.c | 2 +- drivers/xen/evtchn.c | 2 +- include/xen/xen-ops.h| 1 + 10 files changed, 68 insertions(+), 16 deletions(-) -- 2.5.5 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH linux 3/8] x86/xen: use xen_vcpu_id mapping for HYPERVISOR_vcpu_op
HYPERVISOR_vcpu_op passes Linux's idea of vCPU id as a parameter while
Xen's idea is expected. In some cases these ideas diverge so we need to
do remapping.
There is an issue, however. PV guests do VCPUOP_is_up very early
(see xen_fill_possible_map() and xen_filter_cpu_maps()) when we don't have
perpu areas initialized. While it could be solved with switching to
early_percpu for xen_vcpu_id I think it's not worth it: PV guests will
probably never get to the point where their idea of vCPU id diverges from
Xen's.
Signed-off-by: Vitaly Kuznetsov
---
arch/x86/include/asm/xen/hypercall.h | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/xen/hypercall.h
b/arch/x86/include/asm/xen/hypercall.h
index a12a047..20750c8 100644
--- a/arch/x86/include/asm/xen/hypercall.h
+++ b/arch/x86/include/asm/xen/hypercall.h
@@ -49,6 +49,7 @@
#include
#include
#include
+#include
/*
* The hypercall asms have to meet several constraints:
@@ -422,7 +423,15 @@ HYPERVISOR_vm_assist(unsigned int cmd, unsigned int type)
static inline int
HYPERVISOR_vcpu_op(int cmd, int vcpuid, void *extra_args)
{
- return _hypercall3(int, vcpu_op, cmd, vcpuid, extra_args);
+ /*
+* PV guests call HYPERVISOR_vcpu_op before percpu areas are
+* initialized. As we always use direct mapping for vCPU ids
+* for them we can simply use Linux vcpuid here.
+*/
+ return _hypercall3(int, vcpu_op, cmd,
+ per_cpu(xen_vcpu_id, vcpuid) != -1 ?
+ per_cpu(xen_vcpu_id, vcpuid) : vcpuid,
+ extra_args);
}
#ifdef CONFIG_X86_64
--
2.5.5
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH linux 1/8] x86/xen: update cpuid.h from Xen-4.7
Update cpuid.h header from xen hypervisor tree to get XEN_HVM_CPUID_VCPU_ID_PRESENT definition. Signed-off-by: Vitaly Kuznetsov --- arch/x86/include/asm/xen/cpuid.h | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/arch/x86/include/asm/xen/cpuid.h b/arch/x86/include/asm/xen/cpuid.h index 0d809e9..3bdd10d 100644 --- a/arch/x86/include/asm/xen/cpuid.h +++ b/arch/x86/include/asm/xen/cpuid.h @@ -76,15 +76,18 @@ /* * Leaf 5 (0x4x04) * HVM-specific features + * EAX: Features + * EBX: vcpu id (iff EAX has XEN_HVM_CPUID_VCPU_ID_PRESENT flag) */ -/* EAX Features */ /* Virtualized APIC registers */ #define XEN_HVM_CPUID_APIC_ACCESS_VIRT (1u << 0) /* Virtualized x2APIC accesses */ #define XEN_HVM_CPUID_X2APIC_VIRT (1u << 1) /* Memory mapped from other domains has valid IOMMU entries */ #define XEN_HVM_CPUID_IOMMU_MAPPINGS (1u << 2) +/* vcpu id is present in EBX */ +#define XEN_HVM_CPUID_VCPU_ID_PRESENT (1u << 3) #define XEN_CPUID_MAX_NUM_LEAVES 4 -- 2.5.5 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH linux 2/8] xen: introduce xen_vcpu_id mapping
It may happen that Xen's and Linux's ideas of vCPU id diverge. In
particular, when we crash on a secondary vCPU we may want to do kdump
and unlike plain kexec where we do migrate_to_reboot_cpu() we try booting
on the vCPU which crashed. This doesn't work very well for PVHVM guests as
we have a number of hypercalls where we pass vCPU id as a parameter. These
hypercalls either fail or do something unexpected. To solve the issue
introduce percpu xen_vcpu_id mapping. ARM and PV guests get direct mapping
for now. Boot CPU for PVHVM guest gets its id from CPUID. With secondary
CPUs it is a bit more trickier. Currently, we initialize IPI vectors
before these CPUs boot so we can't use CPUID. However, we know that
physical CPU id (vLAPIC id) is Xen's vCPU id * 2, we can piggyback on
that. Alternatively, we could have disabled all secondary CPUs once we
detect that Xen's and Linux's ideas of vCPU id diverged.
Signed-off-by: Vitaly Kuznetsov
---
arch/arm/xen/enlighten.c | 10 ++
arch/x86/xen/enlighten.c | 18 +-
include/xen/xen-ops.h| 1 +
3 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 75cd734..ea99ca2 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -46,6 +46,10 @@ struct shared_info *HYPERVISOR_shared_info = (void
*)&xen_dummy_shared_info;
DEFINE_PER_CPU(struct vcpu_info *, xen_vcpu);
static struct vcpu_info __percpu *xen_vcpu_info;
+/* Linux <-> Xen vCPU id mapping */
+DEFINE_PER_CPU(int, xen_vcpu_id) = -1;
+EXPORT_SYMBOL_GPL(xen_vcpu_id);
+
/* These are unused until we support booting "pre-ballooned" */
unsigned long xen_released_pages;
struct xen_memory_region xen_extra_mem[XEN_EXTRA_MEM_MAX_REGIONS] __initdata;
@@ -179,6 +183,9 @@ static void xen_percpu_init(void)
pr_info("Xen: initializing cpu%d\n", cpu);
vcpup = per_cpu_ptr(xen_vcpu_info, cpu);
+ /* Direct vCPU id mapping for ARM guests. */
+ per_cpu(xen_vcpu_id, cpu) = cpu;
+
info.mfn = virt_to_gfn(vcpup);
info.offset = xen_offset_in_page(vcpup);
@@ -328,6 +335,9 @@ static int __init xen_guest_init(void)
if (xen_vcpu_info == NULL)
return -ENOMEM;
+ /* Direct vCPU id mapping for ARM guests. */
+ per_cpu(xen_vcpu_id, 0) = 0;
+
if (gnttab_setup_auto_xlat_frames(grant_frames)) {
free_percpu(xen_vcpu_info);
return -ENOMEM;
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 760789a..69f4c0c 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -59,6 +59,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -118,6 +119,10 @@ DEFINE_PER_CPU(struct vcpu_info *, xen_vcpu);
*/
DEFINE_PER_CPU(struct vcpu_info, xen_vcpu_info);
+/* Linux <-> Xen vCPU id mapping */
+DEFINE_PER_CPU(int, xen_vcpu_id) = -1;
+EXPORT_SYMBOL_GPL(xen_vcpu_id);
+
enum xen_domain_type xen_domain_type = XEN_NATIVE;
EXPORT_SYMBOL_GPL(xen_domain_type);
@@ -1137,8 +1142,11 @@ void xen_setup_vcpu_info_placement(void)
{
int cpu;
- for_each_possible_cpu(cpu)
+ for_each_possible_cpu(cpu) {
+ /* Set up direct vCPU id mapping for PV guests. */
+ per_cpu(xen_vcpu_id, cpu) = cpu;
xen_vcpu_setup(cpu);
+ }
/* xen_vcpu_setup managed to place the vcpu_info within the
* percpu area for all cpus, so make use of it. Note that for
@@ -1797,6 +1805,12 @@ static void __init init_hvm_pv_info(void)
xen_setup_features();
+ cpuid(base + 4, &eax, &ebx, &ecx, &edx);
+ if (eax & XEN_HVM_CPUID_VCPU_ID_PRESENT)
+ this_cpu_write(xen_vcpu_id, ebx);
+ else
+ this_cpu_write(xen_vcpu_id, smp_processor_id());
+
pv_info.name = "Xen HVM";
xen_domain_type = XEN_HVM_DOMAIN;
@@ -1808,6 +1822,8 @@ static int xen_hvm_cpu_notify(struct notifier_block
*self, unsigned long action,
int cpu = (long)hcpu;
switch (action) {
case CPU_UP_PREPARE:
+ /* vLAPIC_ID == Xen's vCPU_ID * 2 for HVM guests */
+ per_cpu(xen_vcpu_id, cpu) = cpu_physical_id(cpu) / 2;
xen_vcpu_setup(cpu);
if (xen_have_vector_callback) {
if (xen_feature(XENFEAT_hvm_safe_pvclock))
diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
index 86abe07..b02a343 100644
--- a/include/xen/xen-ops.h
+++ b/include/xen/xen-ops.h
@@ -8,6 +8,7 @@
#include
DECLARE_PER_CPU(struct vcpu_info *, xen_vcpu);
+DECLARE_PER_CPU(int, xen_vcpu_id);
void xen_arch_pre_suspend(void);
void xen_arch_post_suspend(int suspend_cancelled);
--
2.5.5
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH linux 5/8] xen/events: use xen_vcpu_id mapping in events_base
EVTCHNOP_bind_ipi and EVTCHNOP_bind_virq pass vCPU id as a parameter and Xen's idea of vCPU id should be used. Use the newly introduced xen_vcpu_id mapping to convert it from Linux's id. Signed-off-by: Vitaly Kuznetsov --- drivers/xen/events/events_base.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c index 71d49a9..73b8b65 100644 --- a/drivers/xen/events/events_base.c +++ b/drivers/xen/events/events_base.c @@ -895,7 +895,7 @@ static int bind_ipi_to_irq(unsigned int ipi, unsigned int cpu) irq_set_chip_and_handler_name(irq, &xen_percpu_chip, handle_percpu_irq, "ipi"); - bind_ipi.vcpu = cpu; + bind_ipi.vcpu = per_cpu(xen_vcpu_id, cpu); if (HYPERVISOR_event_channel_op(EVTCHNOP_bind_ipi, &bind_ipi) != 0) BUG(); @@ -991,7 +991,7 @@ int bind_virq_to_irq(unsigned int virq, unsigned int cpu, bool percpu) handle_edge_irq, "virq"); bind_virq.virq = virq; - bind_virq.vcpu = cpu; + bind_virq.vcpu = per_cpu(xen_vcpu_id, cpu); ret = HYPERVISOR_event_channel_op(EVTCHNOP_bind_virq, &bind_virq); if (ret == 0) @@ -1318,7 +1318,7 @@ static int rebind_irq_to_cpu(unsigned irq, unsigned tcpu) /* Send future instances of this interrupt to other vcpu. */ bind_vcpu.port = evtchn; - bind_vcpu.vcpu = tcpu; + bind_vcpu.vcpu = per_cpu(xen_vcpu_id, tcpu); /* * Mask the event while changing the VCPU binding to prevent @@ -1458,7 +1458,7 @@ static void restore_cpu_virqs(unsigned int cpu) /* Get a new binding from Xen. */ bind_virq.virq = virq; - bind_virq.vcpu = cpu; + bind_virq.vcpu = per_cpu(xen_vcpu_id, cpu); if (HYPERVISOR_event_channel_op(EVTCHNOP_bind_virq, &bind_virq) != 0) BUG(); @@ -1482,7 +1482,7 @@ static void restore_cpu_ipis(unsigned int cpu) BUG_ON(ipi_from_irq(irq) != ipi); /* Get a new binding from Xen. */ - bind_ipi.vcpu = cpu; + bind_ipi.vcpu = per_cpu(xen_vcpu_id, cpu); if (HYPERVISOR_event_channel_op(EVTCHNOP_bind_ipi, &bind_ipi) != 0) BUG(); -- 2.5.5 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v5 05/17] xen/passthrough: x86: Use INVALID_GFN rather than INVALID_MFN
On 28/06/16 18:43, Andrew Cooper wrote: > On 28/06/16 17:17, Julien Grall wrote: >> A variable containing a guest frame should be compared to INVALID_GFN >> and not INVALID_GFN. I think the text should be changed? I'd expect one 'G' being replaced bay 'M'. :-) Juergen >> >> Signed-off-by: Julien Grall > > Reviewed-by: Andrew Cooper > > I suspect these (mis)uses predate my movement of INVALID_GFN from x86 > paging code to common code. > > ___ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel > ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH linux 6/8] xen/events: fifo: use xen_vcpu_id mapping
EVTCHNOP_init_control has vCPU id as a parameter and Xen's idea of vCPU id should be used. Use the newly introduced xen_vcpu_id mapping to convert it from Linux's id. Signed-off-by: Vitaly Kuznetsov --- drivers/xen/events/events_fifo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/xen/events/events_fifo.c b/drivers/xen/events/events_fifo.c index 9289a17..e3406cd 100644 --- a/drivers/xen/events/events_fifo.c +++ b/drivers/xen/events/events_fifo.c @@ -113,7 +113,7 @@ static int init_control_block(int cpu, init_control.control_gfn = virt_to_gfn(control_block); init_control.offset = 0; - init_control.vcpu= cpu; + init_control.vcpu= per_cpu(xen_vcpu_id, cpu); return HYPERVISOR_event_channel_op(EVTCHNOP_init_control, &init_control); } -- 2.5.5 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
