Re: [Xen-devel] [PATCH 6/7] vm-event/arm: move hvm_event_cr->common vm_event_monitor_cr
On 6/17/2016 11:25 AM, Corneliu ZUZU wrote:
On 6/16/2016 6:16 PM, Jan Beulich wrote:
On 16.06.16 at 16:12, wrote:
Prepare for ARM implementation of control-register write vm-events
by moving
X86-specific hvm_event_cr to the common-side.
Signed-off-by: Corneliu ZUZU
---
xen/arch/x86/hvm/event.c| 30 --
xen/arch/x86/hvm/hvm.c | 2 +-
xen/arch/x86/monitor.c | 37
-
xen/arch/x86/vm_event.c | 2 +-
xen/common/monitor.c| 40
xen/common/vm_event.c | 31 +++
xen/include/asm-x86/hvm/event.h | 13 -
xen/include/asm-x86/monitor.h | 2 --
xen/include/xen/monitor.h | 4
xen/include/xen/vm_event.h | 10 ++
10 files changed, 91 insertions(+), 80 deletions(-)
Considering that there's no ARM file getting altered here at all,
mentioning ARM in the subject is a little odd.
This patch and the following one should be meld together.
I only split them to ease reviewing, sorry I forgot to mention that in
the cover letter.
--- a/xen/common/monitor.c
+++ b/xen/common/monitor.c
@@ -62,6 +62,46 @@ int monitor_domctl(struct domain *d, struct
xen_domctl_monitor_op *mop)
switch ( mop->event )
{
+#if CONFIG_X86
#ifdef please.
Ack.
+case XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG:
+{
+struct arch_domain *ad = &d->arch;
Peeking into the next patch I see that this stays there. Common code,
however, shouldn't access ->arch sub-structures - respective fields
should be moved out.
Then we need to find a resolution that avoids code duplication.
The code is the same, but those bits that are currently on the arch
side (arch.monitor.write_ctrlreg_*) cannot be moved to common as they
are, since their -number- might differ from arch-to-arch.
But we could:
- in public/vm_event.h, besides the VM_EVENT_X86_* and VM_EVENT_ARM_*
defines (wcr index), also have
#define VM_EVENT_X86_CR_COUNT4
#define VM_EVENT_ARM_CR_COUNT 4
- move the 3 write_ctrlreg_{enabled,sync,onchangeonly} fields from
arch_domain to domain (common) and make them 8-bits wide each for now
(widened more in the future if the need arises)
- let monitor_ctrlreg_bitmask() macro to be architecture-dependent and
use the introduced VM_EVENT__CR_COUNT
Tamas, we also talked on this matter @ some point (when I sent the
patches that moved vm-event parts to common). What do you think of the
above?
Tamas, Jan, thoughts on this?
Corneliu.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 0/4] x86: accommodate 32-bit PV guests with SMEP/SMAP handling
>>> On 21.06.16 at 08:19, wrote: >> From: Jan Beulich [mailto:jbeul...@suse.com] >> Sent: Thursday, March 17, 2016 3:51 PM >> >> As has been explained previously[1], SMAP (and with less relevance >> also SMEP) is not compatible with 32-bit PV guests which aren't >> aware/prepared to be run with that feature enabled. Andrew's >> original approach either sacrificed architectural correctness for >> making 32-bit guests work again, or disabled SMAP also for not >> insignificant portions of hypervisor code, both by allowing to control >> the workaround mode via command line option. > > Hi Jan, do you mind sharing more about Andrew's original approach? > And to solve this issue can we just disable SMEP/SMAP for Xen itself, > hence only HVM will benefit from this feature? Did you look at the link still visible below? If you did, please be more precise about the details you need. >> This alternative approach disables SMEP and SMAP only while >> running 32-bit PV guest code plus a few hypervisor instructions >> early after entering hypervisor context from or late before exiting >> to such guests. Those few instructions (in assembly source) are of >> course much easier to prove not to perform undue memory >> accesses than code paths reaching deep into C sources. >> >> The 4th patch really is unrelated except for not applying cleanly >> without the earlier ones, and the potential having been noticed >> while putting together the 2nd one. >> >> 1: move cached CR4 value to struct cpu_info >> 2: suppress SMEP and SMAP while running 32-bit PV guest code >> 3: use optimal NOPs to fill the SMEP/SMAP placeholders >> 4: use 32-bit loads for 32-bit PV guest state reload >> >> Signed-off-by: Jan Beulich >> --- >> v3: New patch 1, as a prereq to changes in patch 2 (previously >> 1). The primary reason for this are performance issues that >> have been found by Andrew with the previous version. >> v2: Various changes to patches 1 and 2 - see there. >> >> [1] >> http://lists.xenproject.org/archives/html/xen-devel/2015-06/msg03888.html ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 6/7] vm-event/arm: move hvm_event_cr->common vm_event_monitor_cr
>>> On 21.06.16 at 09:08, wrote:
> On 6/17/2016 11:25 AM, Corneliu ZUZU wrote:
>> On 6/16/2016 6:16 PM, Jan Beulich wrote:
>> On 16.06.16 at 16:12, wrote:
Prepare for ARM implementation of control-register write vm-events
by moving
X86-specific hvm_event_cr to the common-side.
Signed-off-by: Corneliu ZUZU
---
xen/arch/x86/hvm/event.c| 30 --
xen/arch/x86/hvm/hvm.c | 2 +-
xen/arch/x86/monitor.c | 37
-
xen/arch/x86/vm_event.c | 2 +-
xen/common/monitor.c| 40
xen/common/vm_event.c | 31 +++
xen/include/asm-x86/hvm/event.h | 13 -
xen/include/asm-x86/monitor.h | 2 --
xen/include/xen/monitor.h | 4
xen/include/xen/vm_event.h | 10 ++
10 files changed, 91 insertions(+), 80 deletions(-)
>>> Considering that there's no ARM file getting altered here at all,
>>> mentioning ARM in the subject is a little odd.
>>
>> This patch and the following one should be meld together.
>> I only split them to ease reviewing, sorry I forgot to mention that in
>> the cover letter.
>>
>>>
--- a/xen/common/monitor.c
+++ b/xen/common/monitor.c
@@ -62,6 +62,46 @@ int monitor_domctl(struct domain *d, struct
xen_domctl_monitor_op *mop)
switch ( mop->event )
{
+#if CONFIG_X86
>>> #ifdef please.
>> Ack.
+case XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG:
+{
+struct arch_domain *ad = &d->arch;
>>> Peeking into the next patch I see that this stays there. Common code,
>>> however, shouldn't access ->arch sub-structures - respective fields
>>> should be moved out.
>>
>> Then we need to find a resolution that avoids code duplication.
>> The code is the same, but those bits that are currently on the arch
>> side (arch.monitor.write_ctrlreg_*) cannot be moved to common as they
>> are, since their -number- might differ from arch-to-arch.
>> But we could:
>> - in public/vm_event.h, besides the VM_EVENT_X86_* and VM_EVENT_ARM_*
>> defines (wcr index), also have
>> #define VM_EVENT_X86_CR_COUNT4
>> #define VM_EVENT_ARM_CR_COUNT 4
>> - move the 3 write_ctrlreg_{enabled,sync,onchangeonly} fields from
>> arch_domain to domain (common) and make them 8-bits wide each for now
>> (widened more in the future if the need arises)
>> - let monitor_ctrlreg_bitmask() macro to be architecture-dependent and
>> use the introduced VM_EVENT__CR_COUNT
>>
>> Tamas, we also talked on this matter @ some point (when I sent the
>> patches that moved vm-event parts to common). What do you think of the
>> above?
I don't really care about the specifics, my only request is what I
already voiced: Common code should not access arch-specific
fields. Having the field to hold the control register bits common,
but the definitions for the individual bits arch-specific is perfectly
fine for this (assuming that these per-arch definitions then again
don't get used in common code).
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [xen-unstable test] 96009: regressions - FAIL
flight 96009 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/96009/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-armhf-armhf-xl-multivcpu 15 guest-start/debian.repeat fail REGR. vs. 95980 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 9 debian-hvm-install fail REGR. vs. 95980 Regressions which are regarded as allowable (not blocking): build-amd64-rumpuserxen 6 xen-buildfail like 95980 build-i386-rumpuserxen6 xen-buildfail like 95980 test-amd64-amd64-xl-rtds 9 debian-install fail like 95980 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail like 95980 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 95980 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 95980 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 95980 Tests which did not succeed, but are not blocking: test-amd64-amd64-rumpuserxen-amd64 1 build-check(1) blocked n/a test-amd64-i386-rumpuserxen-i386 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvh-amd 11 guest-start fail never pass test-amd64-amd64-xl-pvh-intel 11 guest-start fail never pass test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail never pass test-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-i386-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-armhf-armhf-libvirt 14 guest-saverestorefail never pass test-armhf-armhf-libvirt 12 migrate-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2 fail never pass test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 12 migrate-support-checkfail never pass test-amd64-i386-libvirt 12 migrate-support-checkfail never pass test-armhf-armhf-xl 12 migrate-support-checkfail never pass test-armhf-armhf-xl 13 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 11 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 12 saverestore-support-checkfail never pass test-armhf-armhf-xl-xsm 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 13 saverestore-support-checkfail never pass version targeted for testing: xen 7fb86e30733f3f4eadb57cbadb241b126639047e baseline version: xen 08754333892407f415045c05659783baeb8fc5d4 Last test of basis95980 2016-06-20 01:59:52 Z1 days Testing same since96009 2016-06-20 13:12:31 Z0 days1 attempts People who touched revisions under test: Julien Grall Shanker Donthineni Stefano Stabellini jobs: build-amd64-xsm pass build-armhf-xsm pass build-i386-xsm pass build-amd64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-armhf-libvirt pas
Re: [Xen-devel] [PATCH v4 3/3] x86/ioreq server: Add HVMOP to map guest ram with p2m_ioreq_server to an ioreq server.
On 6/20/2016 9:13 PM, George Dunlap wrote: On 20/06/16 12:28, Yu Zhang wrote: On 6/20/2016 6:55 PM, Jan Beulich wrote: On 20.06.16 at 12:32, wrote: On 20/06/16 11:25, Jan Beulich wrote: On 20.06.16 at 12:10, wrote: On 20/06/16 10:03, Yu Zhang wrote: However, there are conflicts if we take live migration into account, i.e. if the live migration is triggered by the user(unintentionally maybe) during the gpu emulation process, resolve_misconfig() will set all the outstanding p2m_ioreq_server entries to p2m_log_dirty, which is not what we expected, because our intention is to only reset the outdated p2m_ioreq_server entries back to p2m_ram_rw. Well the real problem in the situation you describe is that a second "lazy" p2m_change_entry_type_global() operation is starting before the first one is finished. All that's needed to resolve the situation is that if you get a second p2m_change_entry_type_global() operation while there are outstanding entries from the first type change, you have to finish the first operation (i.e., go "eagerly" find all the misconfigured entries and change them to the new type) before starting the second one. Eager resolution of outstanding entries can't be the solution here, I think, as that would - afaict - be as time consuming as doing the type change synchronously right away. But isn't it the case that p2m_change_entry_type_global() is only implemented for EPT? Also for NPT, we're using a similar model in p2m-pt.c (see e.g. the uses of RECALC_FLAGS - we're utilizing the _PAGE_USER set unconditionally leads to NPF). And since shadow sits on top of p2m-pt, that should be covered too. So we've been doing the slow method for both shadow and AMD HAP (whatever it's called these days) since the beginning. And in any case we'd only have to go for the "slow" case in circumstances where the 2nd type change happened before the first one had completed. We can't even tell when one have fully finished. I agree, we have no idea if the previous type change is completely done. Besides, IIUC, the p2m_change_entry_type_gobal() is not a quite slow method, because it does not invalidate all the paging structure entries at once, it just writes the upper level ones, others are updated in resolve_misconfig(). p2m_change_entry_type_global(), at least right now, can be invoked freely without prior type changes having fully propagated. The logic resolving mis-configured entries simply needs to be able to know the correct new type. I can't see why this logic shouldn't therefore be extensible to this new type which can be in flight - after we ought to have a way to know what type a particular GFN is supposed to be? Actually, come to think of it -- since the first type change is meant to convert all ioreq_server -> ram_rw, and the second is meant to change all ram_rw -> logdirty, is there any case in which we *wouldn't* want the resulting type to be logdirty? Isn't that exactly what we'd get if we'd done both operations synchronously? I think Yu's concern is for pages which did not get converted back? Or on the restore side? Otherwise - "yes" to both of your questions. Yes. My concern is that resolve_misconfig() can not easily be extended to differentiate the p2m_ioreq_server entries which need to be reset and the normal p2m_ioreq_server entries. Under what circumstance should resolve_misconfig() change a misconfigured entry into a p2m_ioreq_server entry? Oh, I did not mean that. Routine resolve_misconfig() shall not change any entry into a p2m_ioreq_server type. I hoped this routine could be changed to reset outdated p2m_ioreq_server entries(by "outdated" I refer to the entries which are no longer tracked by an ioreq server but remain as p2m_ioreq_server) back to p2m_ram_rw type. Later I realized that we may also change the normal p2m_ioreq_server entries(by "nomal" I mean the gfns which are in emulation process) if live migration is triggered during emulation process. And it's hard to distinguish the outdated p2m_ioreq_server entries and the normal ones. Thanks Yu ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 3/3] x86/ioreq server: Add HVMOP to map guest ram with p2m_ioreq_server to an ioreq server.
On 6/20/2016 9:38 PM, Jan Beulich wrote:
On 20.06.16 at 14:06, wrote:
Suppose resolve_misconfig() is modified to change all p2m_ioreq_server
entries(which also
have e.recalc flag turned on) back to p2m_ram_rw. And suppose we have
ioreq server 1, which
emulates gfn A, and ioreq server 2 which emulates gfn B:
1> At the beginning, ioreq server 1 is attached to p2m_ioreq_server, and
gfn A is write protected
by setting it to p2m_ioreq_server;
2> ioreq server 1 is detached from p2m_ioreq_server, left gfn A's p2m
type unchanged;
3> After the detachment of ioreq server 1,
p2m_change_entry_type_global() is called, all ept
entries are invalidated;
4> Later, ioreq server 2 is attached to p2m_ioreq_server;
5> Gfn B is set to p2m_ioreq_server, although its corresponding ept
entry was invalidated,
ept_set_entry() will trigger resolve_misconfig(), which will set the p2m
type of gfn B back to
p2m_ram_rw;
6> ept_set_entry() will set gfn B's p2m type to p2m_ioreq_server next;
And now, we have two
ept entries with p2m_ioreq_server type - gfn A's and gfn B's.
With no live migration, things could work fine - later accesses to gfn A
will ultimately change
its type back to p2m_ram_rw.
However, if live migration is started(all pte entries invalidated
again), resolve_misconfig() would
change both gfn A's and gfn B's p2m type back to p2m_ram_rw, which means
the emulation of
gfn B would fail.
Why would it? Changes to p2m_ram_logdirty won't alter
p2m_ioreq_server entries, and hence changes from it back to
p2m_ram_rw won't either.
Oh, above example is based on the assumption that resolve_misconfig() is
extended
to handle the p2m_ioreq_server case(see my "Suppose resolve_misconfig()
is modified...").
The code change could be something like below:
@@ -542,10 +542,14 @@ static int resolve_misconfig(struct p2m_domain
*p2m, unsigned long gfn)
-if ( e.recalc && p2m_is_changeable(e.sa_p2mt) )
+ if ( e.recalc )
{
- e.sa_p2mt = p2m_is_logdirty_range(p2m, gfn +
i, gfn + i)
- ? p2m_ram_logdirty : p2m_ram_rw;
+ if ( e.sa_p2mt == p2m_ioreq_server )
+ e.sa_p2mt = p2m_ram_rw;
+ else if ( p2m_is_changeable(e.sa_p2mt) )
+ e.sa_p2mt = p2m_is_logdirty_range(p2m, gfn
+ i, gfn + i)
+ ? p2m_ram_logdirty : p2m_ram_rw;
+
ept_p2m_type_to_flags(p2m, &e, e.sa_p2mt,
e.access);
}
e.recalc = 0;
With changes like this, both p2m types of gfn A and gfn B from above example
would be set to p2m_ram_rw if log dirty is enabled.
So that's what I am worrying - if a user unintentionally typed "xl save"
during
the emulation process , the emulation would fail. We can let the
enable_logdirty()
return false if XenGT is detected, but we still wish to keep the log
dirty feature.
And then - didn't we mean to disable that part of XenGT during
migration, i.e. temporarily accept the higher performance
overhead without the p2m_ioreq_server entries? In which case
flipping everything back to p2m_ram_rw after (completed or
canceled) migration would be exactly what we want. The (new
or previous) ioreq server should attach only afterwards, and
can then freely re-establish any p2m_ioreq_server entries it
deems necessary.
Well, I agree this part of XenGT should be disabled during migration.
But in such
case I think it's device model's job to trigger the p2m type
flipping(i.e. by calling
HVMOP_set_mem_type). And the device model should be notified first when the
migration begins - we may need new patches to do so if XenGT is going to
support
vGPU migration in the future.
Thanks
Yu
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 3/3] x86/ioreq server: Add HVMOP to map guest ram with p2m_ioreq_server to an ioreq server.
>>> On 21.06.16 at 09:45, wrote:
> On 6/20/2016 9:38 PM, Jan Beulich wrote:
> On 20.06.16 at 14:06, wrote:
>>> However, if live migration is started(all pte entries invalidated
>>> again), resolve_misconfig() would
>>> change both gfn A's and gfn B's p2m type back to p2m_ram_rw, which means
>>> the emulation of
>>> gfn B would fail.
>> Why would it? Changes to p2m_ram_logdirty won't alter
>> p2m_ioreq_server entries, and hence changes from it back to
>> p2m_ram_rw won't either.
>
> Oh, above example is based on the assumption that resolve_misconfig() is
> extended
> to handle the p2m_ioreq_server case(see my "Suppose resolve_misconfig()
> is modified...").
> The code change could be something like below:
>
> @@ -542,10 +542,14 @@ static int resolve_misconfig(struct p2m_domain
> *p2m, unsigned long gfn)
>
> -if ( e.recalc && p2m_is_changeable(e.sa_p2mt) )
> + if ( e.recalc )
> {
> - e.sa_p2mt = p2m_is_logdirty_range(p2m, gfn +
> i, gfn + i)
> - ? p2m_ram_logdirty : p2m_ram_rw;
> + if ( e.sa_p2mt == p2m_ioreq_server )
> + e.sa_p2mt = p2m_ram_rw;
> + else if ( p2m_is_changeable(e.sa_p2mt) )
> + e.sa_p2mt = p2m_is_logdirty_range(p2m, gfn
> + i, gfn + i)
> + ? p2m_ram_logdirty : p2m_ram_rw;
> +
>ept_p2m_type_to_flags(p2m, &e, e.sa_p2mt,
> e.access);
> }
> e.recalc = 0;
>
> With changes like this, both p2m types of gfn A and gfn B from above example
> would be set to p2m_ram_rw if log dirty is enabled.
Above modification would convert _all_ p2m_ioreq_server into
p2m_ram_rw, irrespective of log-dirty mode being active. Which
I don't think is what you want.
> So that's what I am worrying - if a user unintentionally typed "xl save"
> during
> the emulation process , the emulation would fail. We can let the
> enable_logdirty()
> return false if XenGT is detected, but we still wish to keep the log
> dirty feature.
Well, enabling log-dirty mode would succeed as soon as all
p2m_ioreq_server pages got converted back to normal ones (by
the device model). So an unintentional "xl save" would simply fail.
Is there any problem with that?
>> And then - didn't we mean to disable that part of XenGT during
>> migration, i.e. temporarily accept the higher performance
>> overhead without the p2m_ioreq_server entries? In which case
>> flipping everything back to p2m_ram_rw after (completed or
>> canceled) migration would be exactly what we want. The (new
>> or previous) ioreq server should attach only afterwards, and
>> can then freely re-establish any p2m_ioreq_server entries it
>> deems necessary.
>>
>
> Well, I agree this part of XenGT should be disabled during migration.
> But in such
> case I think it's device model's job to trigger the p2m type
> flipping(i.e. by calling
> HVMOP_set_mem_type).
I agree - this would seem to be the simpler model here, despite (as
George validly says) the more consistent model would be for the
hypervisor to do the cleanup. Such cleanup would imo be reasonable
only if there was an easy way for the hypervisor to enumerate all
p2m_ioreq_server pages.
> And the device model should be notified first when the
> migration begins - we may need new patches to do so if XenGT is going to
> support
> vGPU migration in the future.
Quite possible.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [ovmf test] 96015: regressions - FAIL
flight 96015 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/96015/ 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 c73cf875524666582343a479665e0469444a38c8 baseline version: ovmf dc99315b8732b6e3032d01319d3f534d440b43d0 Last test of basis94748 2016-05-24 22:43:25 Z 27 days Failing since 94750 2016-05-25 03:43:08 Z 27 days 48 attempts Testing same since96015 2016-06-20 16:22:24 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 Jeff Fan Jiaxin Wu Jiewen Yao Katie Dellaquila Laszlo Ersek Liming Gao lushifex Marvin H?user Marvin Haeuser Maurice Ma Michael Zimmermann Ruiyu Ni Ryan Harkin Sami Mujawar Satya Yarlagadda Sriram Subramanian Star Zeng Tapan Shah Thomas Palmer Yonghong Zhu 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 2253 lines long.) ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] Xen on MinnowBoard Max
hi, i'm looking for a detailed tutorial on how to install Xen on a MinnowBoard Max Best Regards, ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen/pciback: Update data filter intersection logic.
>>> On 20.06.16 at 18:23, wrote: > Here is printouts with applying the new logic > > [ 382.13] xen-pciback: :06:00.0: write request 4 bytes at 0xc = 4000 > [ 382.14] xen-pciback: :06:00.0: read 1 bytes at 0xc > [ 382.28] xen-pciback: :06:00.0: read 1 bytes at 0xc = 0 > [ 382.38] xen-pciback: :06:00.0: read 1 bytes at 0xd > [ 382.81] xen-pciback: :06:00.0: read 1 bytes at 0xd = 0 > [ 382.222313] xen-pciback: :06:00.0: read 1 bytes at 0xf > [ 382.222341] xen-pciback: :06:00.0: read 1 bytes at 0xf = 0 > > So from prints the logic is not equivalent. 0xd is included in this case. > > In original logic field 0xd is excluded because > Not if ((req_start >= field_start && req_start < field_end) > Nor (req_end > field_start && req_end <= field_end)) evaluate to true. > Where request would be 4b segment starting with 0xc Oh, I see - the current expression is screwed in two ways: For one it has req_* and field_* the wrong way round (or should I better say their uses are not symmetric, which for any kind of overlap check they of course should be), and then it uses || instead of && (i.e. kind of, but not really checking that req is contained in field, whereas for the purpose here we'd need the other direction). So yes, your change is not just a simplification, but a correction. But then on top of the previously mentioned change to your patch you should also fix the read path in a similar manner. And the description should of course accurately reflect the change (i.e. no talk of quirks and overlapping filters, and a proper explanation of what's wrong with the current expression). Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH] xen: arm: Update arm64 image header
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 warn if the kernel Image is larger than the size given in
the device tree, too.
Signed-off-by: Dirk Behme
---
xen/arch/arm/kernel.c | 41 -
1 file changed, 24 insertions(+), 17 deletions(-)
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index 3f6cce3..1cfaf02 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -28,8 +28,7 @@
#define ZIMAGE32_MAGIC 0x016f2818
-#define ZIMAGE64_MAGIC_V0 0x1408
-#define ZIMAGE64_MAGIC_V1 0x644d5241 /* "ARM\x64" */
+#define ZIMAGE64_MAGIC 0x644d5241 /* "ARM\x64" */
struct minimal_dtb_header {
uint32_t magic;
@@ -335,17 +334,17 @@ static int kernel_zimage64_probe(struct kernel_info *info,
{
/* linux/Documentation/arm64/booting.txt */
struct {
-uint32_t magic0;
-uint32_t res0;
-uint64_t text_offset; /* Image load offset */
-uint64_t res1;
-uint64_t res2;
+uint32_t code0;
+uint32_t code1;
+uint64_t text_offset; /* Image load offset, little endian */
+uint64_t image_size; /* Effective Image size, little endian */
+uint64_t flags;
/* zImage V1 only from here */
+uint64_t res2;
uint64_t res3;
uint64_t res4;
-uint64_t res5;
-uint32_t magic1;
-uint32_t res6;
+uint32_t magic;/* Magic number, little endian, "ARM\x64" */
+uint32_t res5;
} zimage;
uint64_t start, end;
@@ -354,20 +353,28 @@ static int kernel_zimage64_probe(struct kernel_info *info,
copy_from_paddr(&zimage, addr, sizeof(zimage));
-if ( zimage.magic0 != ZIMAGE64_MAGIC_V0 &&
- zimage.magic1 != ZIMAGE64_MAGIC_V1 )
+if ( zimage.magic != ZIMAGE64_MAGIC )
return -EINVAL;
-/* Currently there is no length in the header, so just use the size */
start = 0;
-end = size;
/*
- * Given the above this check is a bit pointless, but leave it
- * here in case someone adds a length field in the future.
+ * Where image_size is non-zero image_size is little-endian
+ * and must be respected.
*/
-if ( (end - start) > size )
+if ( zimage.image_size )
+end = zimage.image_size;
+else
+end = size;
+
+if ( (end - start) > size ) {
+if ( zimage.image_size ) {
+printk(XENLOG_ERR "Error: Kernel Image size: %lu bytes >
bootmodule size: %lu bytes\n",
+ zimage.image_size, (uint64_t)size);
+printk(XENLOG_ERR "Check the device tree configuration!\n");
+}
return -EINVAL;
+}
info->zimage.kernel_addr = addr;
info->zimage.len = end - start;
--
2.8.0
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH 1/3] xen/arm: drivers: scif: Remove dead code
In scif_uart_init() uart->baud is set to BAUD_AUTO. So its a basic error
if this is different later. Detect this by an ASSERT, but remove the
dead code normally never reached.
Signed-off-by: Dirk Behme
---
xen/drivers/char/scif-uart.c | 23 ++-
1 file changed, 6 insertions(+), 17 deletions(-)
diff --git a/xen/drivers/char/scif-uart.c b/xen/drivers/char/scif-uart.c
index 51a2233..ca88c0f 100644
--- a/xen/drivers/char/scif-uart.c
+++ b/xen/drivers/char/scif-uart.c
@@ -143,23 +143,12 @@ static void __init scif_uart_init_preirq(struct
serial_port *port)
scif_writew(uart, SCIF_SCSMR, val);
ASSERT( uart->clock_hz > 0 );
-if ( uart->baud != BAUD_AUTO )
-{
-/* Setup desired Baud rate */
-divisor = uart->clock_hz / (uart->baud << 4);
-ASSERT( divisor >= 1 && divisor <= (uint16_t)UINT_MAX );
-scif_writew(uart, SCIF_DL, (uint16_t)divisor);
-/* Selects the frequency divided clock (SC_CLK external input) */
-scif_writew(uart, SCIF_CKS, 0);
-udelay(100 / uart->baud + 1);
-}
-else
-{
-/* Read current Baud rate */
-divisor = scif_readw(uart, SCIF_DL);
-ASSERT( divisor >= 1 && divisor <= (uint16_t)UINT_MAX );
-uart->baud = uart->clock_hz / (divisor << 4);
-}
+ASSERT( uart->baud == BAUD_AUTO );
+
+/* Read current Baud rate */
+divisor = scif_readw(uart, SCIF_DL);
+ASSERT( divisor >= 1 && divisor <= (uint16_t)UINT_MAX );
+uart->baud = uart->clock_hz / (divisor << 4);
/* Setup trigger level for TX/RX FIFOs */
scif_writew(uart, SCIF_SCFCR, SCFCR_RTRG11 | SCFCR_TTRG11);
--
2.8.0
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH 2/3] xen/arm: drivers: scif: Remove unused variables
The two struct members baud and clock_hz are in the end read only
variables nowhere used for anything useful. Removing them makes
the code much simpler without changing any functionality.
Signed-off-by: Dirk Behme
---
xen/drivers/char/scif-uart.c| 13 +
xen/include/asm-arm/scif-uart.h | 1 -
2 files changed, 1 insertion(+), 13 deletions(-)
diff --git a/xen/drivers/char/scif-uart.c b/xen/drivers/char/scif-uart.c
index ca88c0f..bc157fe 100644
--- a/xen/drivers/char/scif-uart.c
+++ b/xen/drivers/char/scif-uart.c
@@ -41,7 +41,7 @@
#define scif_writew(uart, off, val)writew((val), (uart)->regs + (off))
static struct scif_uart {
-unsigned int baud, clock_hz, data_bits, parity, stop_bits;
+unsigned int data_bits, parity, stop_bits;
unsigned int irq;
char __iomem *regs;
struct irqaction irqaction;
@@ -87,7 +87,6 @@ static void scif_uart_interrupt(int irq, void *data, struct
cpu_user_regs *regs)
static void __init scif_uart_init_preirq(struct serial_port *port)
{
struct scif_uart *uart = port->uart;
-unsigned int divisor;
uint16_t val;
/*
@@ -142,14 +141,6 @@ static void __init scif_uart_init_preirq(struct
serial_port *port)
}
scif_writew(uart, SCIF_SCSMR, val);
-ASSERT( uart->clock_hz > 0 );
-ASSERT( uart->baud == BAUD_AUTO );
-
-/* Read current Baud rate */
-divisor = scif_readw(uart, SCIF_DL);
-ASSERT( divisor >= 1 && divisor <= (uint16_t)UINT_MAX );
-uart->baud = uart->clock_hz / (divisor << 4);
-
/* Setup trigger level for TX/RX FIFOs */
scif_writew(uart, SCIF_SCFCR, SCFCR_RTRG11 | SCFCR_TTRG11);
@@ -292,8 +283,6 @@ static int __init scif_uart_init(struct dt_device_node *dev,
uart = &scif_com;
-uart->clock_hz = SCIF_CLK_FREQ;
-uart->baud = BAUD_AUTO;
uart->data_bits = 8;
uart->parity= PARITY_NONE;
uart->stop_bits = 1;
diff --git a/xen/include/asm-arm/scif-uart.h b/xen/include/asm-arm/scif-uart.h
index 7a9f639..d030b26 100644
--- a/xen/include/asm-arm/scif-uart.h
+++ b/xen/include/asm-arm/scif-uart.h
@@ -22,7 +22,6 @@
#define __ASM_ARM_SCIF_UART_H
#define SCIF_FIFO_MAX_SIZE16
-#define SCIF_CLK_FREQ 14745600
/* Register offsets */
#define SCIF_SCSMR (0x00)/* Serial mode register */
--
2.8.0
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH 3/3] xen/arm: drivers: scif: Add clock auto detection
Besides the 14MHz external clock, the SCIF might be clocked by an internal 66MHz clock. Detect this clock based on the SCIF_DL register being 0 (internal clock) or != 0 (external clock). Signed-off-by: Dirk Behme --- xen/drivers/char/scif-uart.c | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/xen/drivers/char/scif-uart.c b/xen/drivers/char/scif-uart.c index bc157fe..678f46b 100644 --- a/xen/drivers/char/scif-uart.c +++ b/xen/drivers/char/scif-uart.c @@ -107,8 +107,19 @@ static void __init scif_uart_init_preirq(struct serial_port *port) scif_readw(uart, SCIF_SCLSR); scif_writew(uart, SCIF_SCLSR, 0); -/* Select Baud rate generator output as a clock source */ -scif_writew(uart, SCIF_SCSCR, SCSCR_CKE10); +/* + * Select Baud rate generator output as a clock source + * The clock source can be an internal or external clock. + * Depending on this the DL register is either 0 or contains + * the divisor. I.e. we can use this to detect the clock + * source and based on this can configure the CKE[1:0] bits + * of the SCSCR register. + */ +if ( scif_readw(uart, SCIF_DL) ) +scif_writew(uart, SCIF_SCSCR, SCSCR_CKE10); /* External clk */ +else +scif_writew(uart, SCIF_SCSCR, SCSCR_CKE00); /* Internal clk */ + /* Setup protocol format and Baud rate, select Asynchronous mode */ val = 0; -- 2.8.0 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v5 3/9] monitor: Rename vm_event_monitor_guest_request
Hello Tamas, On 02/06/16 23:52, Tamas K Lengyel wrote: Mechanical renaming and relocation to the monitor subsystem. Signed-off-by: Tamas K Lengyel Acked-by: Razvan Cojocaru Acked-by: Jan Beulich For ARM bits: Acked-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 v5 2/9] monitor: Rename vm_event_monitor_get_capabilities
Hello Tamas, On 02/06/16 23:52, Tamas K Lengyel wrote: The monitor_get_capabilities check actually belongs to the monitor subsystem so relocating and renaming it to sanitize the code's name and location. Mechanical patch, no code changes introduced. Signed-off-by: Tamas K Lengyel Acked-by: Andrew Cooper Acked-by: Razvan Cojocaru For the ARM bits: Acked-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] xen/pciback: Update data filter intersection logic.
>>> On 21.06.16 at 11:04, wrote: On 20.06.16 at 18:23, wrote: >> Here is printouts with applying the new logic >> >> [ 382.13] xen-pciback: :06:00.0: write request 4 bytes at 0xc = > 4000 >> [ 382.14] xen-pciback: :06:00.0: read 1 bytes at 0xc >> [ 382.28] xen-pciback: :06:00.0: read 1 bytes at 0xc = 0 >> [ 382.38] xen-pciback: :06:00.0: read 1 bytes at 0xd >> [ 382.81] xen-pciback: :06:00.0: read 1 bytes at 0xd = 0 >> [ 382.222313] xen-pciback: :06:00.0: read 1 bytes at 0xf >> [ 382.222341] xen-pciback: :06:00.0: read 1 bytes at 0xf = 0 >> >> So from prints the logic is not equivalent. 0xd is included in this case. >> >> In original logic field 0xd is excluded because >> Not if ((req_start >= field_start && req_start < field_end) >> Nor (req_end > field_start && req_end <= field_end)) evaluate to true. >> Where request would be 4b segment starting with 0xc > > Oh, I see - the current expression is screwed in two ways: For one > it has req_* and field_* the wrong way round (or should I better > say their uses are not symmetric, which for any kind of overlap > check they of course should be), and then it uses || instead of && > (i.e. kind of, but not really checking that req is contained in field, > whereas for the purpose here we'd need the other direction). So > yes, your change is not just a simplification, but a correction. > > But then on top of the previously mentioned change to your patch > you should also fix the read path in a similar manner. And the > description should of course accurately reflect the change (i.e. > no talk of quirks and overlapping filters, and a proper explanation > of what's wrong with the current expression). Yet then what I don't understand: How does this change help you? There's still not going to be any actual write to the Latency Timer register, as conf_space_write() - even if now getting called - won't do anything for it. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 3/3] x86/ioreq server: Add HVMOP to map guest ram with p2m_ioreq_server to an ioreq server.
On 6/21/2016 4:22 PM, Jan Beulich wrote:
On 21.06.16 at 09:45, wrote:
On 6/20/2016 9:38 PM, Jan Beulich wrote:
On 20.06.16 at 14:06, wrote:
However, if live migration is started(all pte entries invalidated
again), resolve_misconfig() would
change both gfn A's and gfn B's p2m type back to p2m_ram_rw, which means
the emulation of
gfn B would fail.
Why would it? Changes to p2m_ram_logdirty won't alter
p2m_ioreq_server entries, and hence changes from it back to
p2m_ram_rw won't either.
Oh, above example is based on the assumption that resolve_misconfig() is
extended
to handle the p2m_ioreq_server case(see my "Suppose resolve_misconfig()
is modified...").
The code change could be something like below:
@@ -542,10 +542,14 @@ static int resolve_misconfig(struct p2m_domain
*p2m, unsigned long gfn)
-if ( e.recalc && p2m_is_changeable(e.sa_p2mt) )
+ if ( e.recalc )
{
- e.sa_p2mt = p2m_is_logdirty_range(p2m, gfn +
i, gfn + i)
- ? p2m_ram_logdirty : p2m_ram_rw;
+ if ( e.sa_p2mt == p2m_ioreq_server )
+ e.sa_p2mt = p2m_ram_rw;
+ else if ( p2m_is_changeable(e.sa_p2mt) )
+ e.sa_p2mt = p2m_is_logdirty_range(p2m, gfn
+ i, gfn + i)
+ ? p2m_ram_logdirty : p2m_ram_rw;
+
ept_p2m_type_to_flags(p2m, &e, e.sa_p2mt,
e.access);
}
e.recalc = 0;
With changes like this, both p2m types of gfn A and gfn B from above example
would be set to p2m_ram_rw if log dirty is enabled.
Above modification would convert _all_ p2m_ioreq_server into
p2m_ram_rw, irrespective of log-dirty mode being active. Which
I don't think is what you want.
Well, this is another situation I found very interesting: without log-dirty,
this approach actually works. :) And the reasons are:
- resolve_misconfig() will only recalculate entries which have e.recalc
flag set;
- For the outdated p2m_ioreq_server entries, this routine will reset
them back to p2m_ram_rw;
- For the new p2m_ioreq_server entries, their e.recalc flag will first
be cleared to 0 by ept_set_entry() -> resolve_misconfig() is used to
set the p2m type when hvmop_set_mem_type is invoked);
- Yet live migration will turn on the recalc flag for all entries again...
You can see this in steps 3> to 6> in my previous example. :)
So that's what I am worrying - if a user unintentionally typed "xl save"
during
the emulation process , the emulation would fail. We can let the
enable_logdirty()
return false if XenGT is detected, but we still wish to keep the log
dirty feature.
Well, enabling log-dirty mode would succeed as soon as all
p2m_ioreq_server pages got converted back to normal ones (by
the device model). So an unintentional "xl save" would simply fail.
Is there any problem with that?
Well, I agree.
Since this patchset is only about ioreq server change, my plan is to keep
the log dirty logic as it is for now, and in the future if we are gonna
support
vGPU migration, we can consider to let "xl save" simply fail if the device
model side is not ready(i.e. not having finished its memory type cleaning
tasks).
And then - didn't we mean to disable that part of XenGT during
migration, i.e. temporarily accept the higher performance
overhead without the p2m_ioreq_server entries? In which case
flipping everything back to p2m_ram_rw after (completed or
canceled) migration would be exactly what we want. The (new
or previous) ioreq server should attach only afterwards, and
can then freely re-establish any p2m_ioreq_server entries it
deems necessary.
Well, I agree this part of XenGT should be disabled during migration.
But in such
case I think it's device model's job to trigger the p2m type
flipping(i.e. by calling
HVMOP_set_mem_type).
I agree - this would seem to be the simpler model here, despite (as
George validly says) the more consistent model would be for the
hypervisor to do the cleanup. Such cleanup would imo be reasonable
only if there was an easy way for the hypervisor to enumerate all
p2m_ioreq_server pages.
Well, for me, the "easy way" means we should avoid traversing the whole ept
paging structure all at once, right? I have not figured out any clean
solution
in hypervisor side, that's one reason I'd like to left this job to
device model
side(another reason is that I do think device model should take this
responsibility).
Thanks
Yu
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 11/19] tools: tracing: adapt Credit2 load tracking events to new format
On Sat, Jun 18, 2016 at 01:12:36AM +0200, Dario Faggioli wrote: > in both xenalyze and formats (for xentrace_format). > > In particular, in xenalyze, now that we have the precision > of the fixed point load values in the tracepoint, show both > the raw value and the (easier to interpreet) percentage. > > Signed-off-by: Dario Faggioli FAOD I will leave this patch and the other one to George because he knows better than me about xentrace. Just by skimming the two patches, they look fine to me. Wei. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 0/8] Add support for parsing per CPU Redistributor entry
Hello Shanker, On 19/06/16 00:45, Shanker Donthineni wrote: The current driver doesn't support parsing Redistributor entries that are described in the MADT GICC table. Not all the GIC implementors places the Redistributor regions in the always-on power domain. On systems, the UEFI firmware should describe Redistributor base address in the associated GIC CPU Interface (GICC) instead of GIC Redistributor (GICR) table. The maximum number of mmio handlers and struct vgic_rdist_region that holds Redistributor addresses are allocated through a static array with hardcoded size. I don't think this is the right approach and is not scalable for implementing features like this. I have decided to convert static to dynamic allocation based on comments from the below link. https://patchwork.kernel.org/patch/9163435/ You addressed only one part of my comment. This series increases the number of I/O handlers but the lookup is still linear (see handle_mmio in arch/arm/io.c). After this series, the maximum number of I/O handlers is 160. So in the worst case, we have to do 160 iterations before finding an handler or concluding the I/O cannot be emulated. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/8] arm/gic-v3: Add a separate function for mapping GICD region
Hello Shanker,
On 19/06/16 00:45, Shanker Donthineni wrote:
Move the code that validates base address and does ioremap of GIC
distributor region to a separate function. Later patches need to
access the GICD region inside function gicv3_acpi_init() for
finding per CPU Redistributor size.
This patch contains two things:
- A clean up: I.e moving the validation/ioremap in a function
- Calling ioremap map earlier
The latter is the most important point of this patch, not the clean up.
However based on your commit message/title, only the clean up matter.
Please reword the commit message/title to make clear what matters.
Signed-off-by: Shanker Donthineni
---
xen/arch/arm/gic-v3.c | 23 +--
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 8d3f149..ab1f380 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1169,6 +1169,17 @@ static void __init gicv3_init_v2(void)
vgic_v2_setup_hw(dbase, cbase, csize, vbase, 0);
}
+static void __init gicv3_ioremap_distributor(paddr_t dist_paddr)
+{
+if ( (dist_paddr & ~PAGE_MASK) )
The second pair of brackets is pointless.
+panic("GICv3: Found unaligned distributor address %"PRIpaddr"",
+ dbase);
+
+gicv3.map_dbase = ioremap_nocache(dist_paddr, SZ_64K);
+if ( !gicv3.map_dbase )
+panic("GICv3: Failed to ioremap for GIC distributor\n");
+}
+
Regards,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 3/3] x86/ioreq server: Add HVMOP to map guest ram with p2m_ioreq_server to an ioreq server.
>>> On 21.06.16 at 11:16, wrote:
>
> On 6/21/2016 4:22 PM, Jan Beulich wrote:
> On 21.06.16 at 09:45, wrote:
>>> On 6/20/2016 9:38 PM, Jan Beulich wrote:
>>> On 20.06.16 at 14:06, wrote:
> However, if live migration is started(all pte entries invalidated
> again), resolve_misconfig() would
> change both gfn A's and gfn B's p2m type back to p2m_ram_rw, which means
> the emulation of
> gfn B would fail.
Why would it? Changes to p2m_ram_logdirty won't alter
p2m_ioreq_server entries, and hence changes from it back to
p2m_ram_rw won't either.
>>> Oh, above example is based on the assumption that resolve_misconfig() is
>>> extended
>>> to handle the p2m_ioreq_server case(see my "Suppose resolve_misconfig()
>>> is modified...").
>>> The code change could be something like below:
>>>
>>> @@ -542,10 +542,14 @@ static int resolve_misconfig(struct p2m_domain
>>> *p2m, unsigned long gfn)
>>>
>>> -if ( e.recalc && p2m_is_changeable(e.sa_p2mt) )
>>> + if ( e.recalc )
>>>{
>>> - e.sa_p2mt = p2m_is_logdirty_range(p2m, gfn +
>>> i, gfn + i)
>>> - ? p2m_ram_logdirty : p2m_ram_rw;
>>> + if ( e.sa_p2mt == p2m_ioreq_server )
>>> + e.sa_p2mt = p2m_ram_rw;
>>> + else if ( p2m_is_changeable(e.sa_p2mt) )
>>> + e.sa_p2mt = p2m_is_logdirty_range(p2m, gfn
>>> + i, gfn + i)
>>> + ? p2m_ram_logdirty : p2m_ram_rw;
>>> +
>>> ept_p2m_type_to_flags(p2m, &e, e.sa_p2mt,
>>> e.access);
>>>}
>>>e.recalc = 0;
>>>
>>> With changes like this, both p2m types of gfn A and gfn B from above example
>>> would be set to p2m_ram_rw if log dirty is enabled.
>> Above modification would convert _all_ p2m_ioreq_server into
>> p2m_ram_rw, irrespective of log-dirty mode being active. Which
>> I don't think is what you want.
>
> Well, this is another situation I found very interesting: without log-dirty,
> this approach actually works. :)
And what if the recalc flag gets set for some other reason?
And then - didn't we mean to disable that part of XenGT during
migration, i.e. temporarily accept the higher performance
overhead without the p2m_ioreq_server entries? In which case
flipping everything back to p2m_ram_rw after (completed or
canceled) migration would be exactly what we want. The (new
or previous) ioreq server should attach only afterwards, and
can then freely re-establish any p2m_ioreq_server entries it
deems necessary.
>>> Well, I agree this part of XenGT should be disabled during migration.
>>> But in such
>>> case I think it's device model's job to trigger the p2m type
>>> flipping(i.e. by calling
>>> HVMOP_set_mem_type).
>> I agree - this would seem to be the simpler model here, despite (as
>> George validly says) the more consistent model would be for the
>> hypervisor to do the cleanup. Such cleanup would imo be reasonable
>> only if there was an easy way for the hypervisor to enumerate all
>> p2m_ioreq_server pages.
>
> Well, for me, the "easy way" means we should avoid traversing the whole ept
> paging structure all at once, right?
Yes.
> I have not figured out any clean
> solution
> in hypervisor side, that's one reason I'd like to left this job to
> device model
> side(another reason is that I do think device model should take this
> responsibility).
Let's see if we can get George to agree.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [qemu-upstream-4.3-testing test] 96026: regressions - FAIL
flight 96026 qemu-upstream-4.3-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/96026/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-libvirt 5 libvirt-build fail REGR. vs. 80927 build-i386-libvirt5 libvirt-build fail REGR. vs. 80927 Tests which are failing intermittently (not blocking): test-amd64-amd64-xl-qemuu-win7-amd64 15 guest-localmigrate/x10 fail pass in 96003 Regressions which are regarded as allowable (not blocking): test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stopfail in 96003 like 80927 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 80927 Tests which did not succeed, but are not blocking: test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-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 version targeted for testing: qemuu12e8fccf5b5460be7aecddc71d27eceaba6e1f15 baseline version: qemuu10c1b763c26feb645627a1639e722515f3e1e876 Last test of basis80927 2016-02-06 13:30:02 Z 135 days Failing since 93977 2016-05-10 11:09:16 Z 41 days 137 attempts Testing same since95534 2016-06-11 00:59:46 Z 10 days 17 attempts People who touched revisions under test: Anthony PERARD Gerd Hoffmann Ian Jackson Stefano Stabellini Wei Liu jobs: build-amd64 pass build-i386 pass build-amd64-libvirt fail build-i386-libvirt fail build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl pass test-amd64-i386-xl pass test-amd64-i386-qemuu-rhel6hvm-amd pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-i386-xl-qemuu-debianhvm-amd64 pass test-amd64-i386-freebsd10-amd64 pass test-amd64-amd64-xl-qemuu-ovmf-amd64 fail test-amd64-i386-xl-qemuu-ovmf-amd64 fail test-amd64-amd64-xl-qemuu-win7-amd64 fail test-amd64-i386-xl-qemuu-win7-amd64 fail test-amd64-amd64-xl-credit2 pass test-amd64-i386-freebsd10-i386 pass test-amd64-i386-qemuu-rhel6hvm-intel pass test-amd64-amd64-libvirt blocked test-amd64-i386-libvirt blocked test-amd64-amd64-xl-multivcpupass test-amd64-amd64-pairpass test-amd64-i386-pair pass test-amd64-amd64-pv pass test-amd64-i386-pv pass test-amd64-amd64-amd64-pvgrubpass test-amd64-amd64-i386-pvgrub pass test-amd64-amd64-pygrub pass test-amd64-amd64-xl-qcow2pass test-amd64-i386-xl-raw pass test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 pass test-amd64-amd64-libvirt-vhd blocked test-amd64-amd64-xl-qemuu-winxpsp3 pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Not pushing. commit 12e8fccf5b5460be7aecddc71d27eceaba6e1f15 Author: Ian Jackson Date: Thu May 26 16:21:56 2016 +0100 main loop: Big hammer to fix logfile disk DoS in Xen setups Each
Re: [Xen-devel] [PATCH v4 3/3] x86/ioreq server: Add HVMOP to map guest ram with p2m_ioreq_server to an ioreq server.
On 6/21/2016 5:47 PM, Jan Beulich wrote: On 6/21/2016 4:22 PM, Jan Beulich wrote: Above modification would convert _all_ p2m_ioreq_server into p2m_ram_rw, irrespective of log-dirty mode being active. Which I don't think is what you want. Well, this is another situation I found very interesting: without log-dirty, this approach actually works. :) And what if the recalc flag gets set for some other reason? Then the previous assumption will not hold. :) But for now, the log dirty code is the only place I have found in hypervisor which will turn on the recalc flag. I agree - this would seem to be the simpler model here, despite (as George validly says) the more consistent model would be for the hypervisor to do the cleanup. Such cleanup would imo be reasonable only if there was an easy way for the hypervisor to enumerate all p2m_ioreq_server pages. Well, for me, the "easy way" means we should avoid traversing the whole ept paging structure all at once, right? Yes. I have not figured out any clean solution in hypervisor side, that's one reason I'd like to left this job to device model side(another reason is that I do think device model should take this responsibility). Let's see if we can get George to agree. OK. Thanks! Yu ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 09/11] hvmctl: convert HVMOP_inject_msi
On Mon, Jun 20, 2016 at 06:57:11AM -0600, Jan Beulich wrote: > Signed-off-by: Jan Beulich > Reviewed-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 07/11] hvmctl: convert HVMOP_set_mem_type
On Mon, Jun 20, 2016 at 06:56:14AM -0600, Jan Beulich wrote: > This allows elimination of the (ab)use of the high operation number > bits for encoding continuations. > > Also limiting "nr" at the libxc level to 32 bits (the high 32 bits of > the previous 64-bit parameter got ignore so far). > > Signed-off-by: Jan Beulich Reviewed-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 06/11] hvmctl: convert HVMOP_modified_memory
On Mon, Jun 20, 2016 at 06:55:43AM -0600, Jan Beulich wrote: > Also limiting "nr" at the libxc level to 32 bits (the high 32 bits of > the previous 64-bit parameter got ignore so far). > > Signed-off-by: Jan Beulich > Reviewed-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 03/11] hvmctl: convert HVMOP_set_isa_irq_level
On Mon, Jun 20, 2016 at 06:53:53AM -0600, Jan Beulich wrote: > Note that this retains the hvmop interface definitions as those had > (wrongly) been exposed to non-tool stack consumers (albeit the > operation wouldn't have succeeded when requested by a domain for > itself). > > Signed-off-by: Jan Beulich > Reviewed-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 08/11] hvmctl: convert HVMOP_inject_trap
On Mon, Jun 20, 2016 at 06:56:41AM -0600, Jan Beulich wrote: > Signed-off-by: Jan Beulich Reviewed-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 02/11] hvmctl: convert HVMOP_set_pci_intx_level
On Mon, Jun 20, 2016 at 06:53:23AM -0600, Jan Beulich wrote:
> Note that this adds validation of the "domain" interface structure
> field, which previously got ignored.
>
> Note further that this retains the hvmop interface definitions as those
> had (wrongly) been exposed to non-tool stack consumers (albeit the
> operation wouldn't have succeeded when requested by a domain for
> itself).
>
> Signed-off-by: Jan Beulich
> ---
> TBD: xen/xsm/flask/policy/access_vectors says "also needs hvmctl", but
> I don't see how this has been done so far. With the change here,
> doing two checks in flask_hvm_control() (the generic one always
> and a specific one if needed) would of course be simple, but it's
> unclear how subsequently added sub-ops should then be dealt with
> (which don't have a similar remark).
>
> --- a/tools/libxc/xc_misc.c
> +++ b/tools/libxc/xc_misc.c
> @@ -473,30 +473,14 @@ int xc_hvm_set_pci_intx_level(
> uint8_t domain, uint8_t bus, uint8_t device, uint8_t intx,
> unsigned int level)
> {
> -DECLARE_HYPERCALL_BUFFER(struct xen_hvm_set_pci_intx_level, arg);
> -int rc;
> +DECLARE_HVMCTL(set_pci_intx_level, dom,
> + .domain = domain,
> + .bus= bus,
> + .device = device,
> + .intx = intx,
> + .level = level);
Minor nit: the "=" is not aligned.
For tool and hypervisor code changes, sans the XSM changes:
Reviewed-by: Wei Liu
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 04/11] hvmctl: convert HVMOP_set_pci_link_route
On Mon, Jun 20, 2016 at 06:54:24AM -0600, Jan Beulich wrote: > Note that this retains the hvmop interface definitions as those had > (wrongly) been exposed to non-tool stack consumers (albeit the > operation wouldn't have succeeded when requested by a domain for > itself). > > Signed-off-by: Jan Beulich > Reviewed-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 05/11] hvmctl: convert HVMOP_track_dirty_vram
On Mon, Jun 20, 2016 at 06:54:57AM -0600, Jan Beulich wrote: > Also limiting "nr" at the libxc level to 32 bits (the high 32 bits of > the previous 64-bit parameter got ignore so far). > > Signed-off-by: Jan Beulich > Reviewed-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 10/11] hvmctl: convert HVMOP_*ioreq_server*
On Mon, Jun 20, 2016 at 06:57:47AM -0600, Jan Beulich wrote: > Note that we can't adjust HVM_IOREQSRV_BUFIOREQ_* to properly obey > name space rules, as these constants as in use by callers of the libxc > interface. > > Signed-off-by: Jan Beulich > Reviewed-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 01/11] public / x86: introduce hvmctl hypercall
On Mon, Jun 20, 2016 at 06:52:41AM -0600, Jan Beulich wrote: > ... as a means to replace all HVMOP_* which a domain can't issue on > itself (i.e. intended for use by only the control domain or device > model). > > Signed-off-by: Jan Beulich Reviewed-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 3/8] arm/gic-v3: Parse per-cpu redistributor entry in GICC subtable
Hello Shanker,
On 19/06/16 00:45, Shanker Donthineni wrote:
The redistributor address can be specified either as part of GICC or
GICR subtable depending on the power domain. The current driver
doesn't support parsing redistributor entry that is defined in GICC
subtable. The GIC CPU subtable entry holds the associated Redistributor
base address if it is not on always-on power domain.
This patch adds necessary code to handle both types of Redistributors
base addresses.
Signed-off-by: Shanker Donthineni
---
xen/arch/arm/gic-v3.c | 97 ---
xen/include/asm-arm/gic.h | 2 +
xen/include/asm-arm/gic_v3_defs.h | 1 +
3 files changed, 83 insertions(+), 17 deletions(-)
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index af12ebc..42cf848 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -659,6 +659,10 @@ static int __init gicv3_populate_rdist(void)
smp_processor_id(), i, ptr);
return 0;
}
+
+if ( gicv3.rdist_regions[i].single_rdist )
+break;
+
if ( gicv3.rdist_stride )
ptr += gicv3.rdist_stride;
else
@@ -1282,6 +1286,11 @@ static int gicv3_iomem_deny_access(const struct domain
*d)
}
#ifdef CONFIG_ACPI
+static bool gic_dist_supports_dvis(void)
static inline and please use bool_t here.
+{
+return !!(readl_relaxed(GICD + GICD_TYPER) & GICD_TYPER_DVIS);
+}
+
static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset)
{
struct acpi_subtable_header *header;
@@ -1393,18 +1402,39 @@ gic_acpi_parse_madt_redistributor(struct
acpi_subtable_header *header,
const unsigned long end)
{
struct acpi_madt_generic_redistributor *rdist;
+struct acpi_madt_generic_interrupt *processor;
struct rdist_region *region;
region = gicv3.rdist_regions + gicv3.rdist_count;
-rdist = (struct acpi_madt_generic_redistributor *)header;
-if ( BAD_MADT_ENTRY(rdist, end) )
-return -EINVAL;
+if ( header->type == ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR )
+{
+rdist = (struct acpi_madt_generic_redistributor *)header;
+if ( BAD_MADT_ENTRY(rdist, end) )
+return -EINVAL;
-if ( !rdist->base_address || !rdist->length )
-return -EINVAL;
+if ( !rdist->base_address || !rdist->length )
+return -EINVAL;
+
+region->base = rdist->base_address;
+region->size = rdist->length;
+}
+else if ( header->type == ACPI_MADT_TYPE_GENERIC_INTERRUPT )
+{
Parsing the GICC and the redistributor is quite different. I would much
prefer a function for parsing each table and an helper to add a new
redistributor.
+processor = (struct acpi_madt_generic_interrupt *)header;
+if ( BAD_MADT_ENTRY(processor, end) )
+return -EINVAL;
+
+if ( !(processor->flags & ACPI_MADT_ENABLED) )
+return 0;
+
+if ( !processor->gicr_base_address )
+return -EINVAL;
+
+region->base = processor->gicr_base_address;
+region->size = gic_dist_supports_dvis() ? SZ_256K : SZ_128K;
Please explain in the commit message how you find the size. I would also
prefer if you use (4 x SZ_64K) and (2 * SZ_64K) as we do in populate_rdist.
+ region->single_rdist = true;
The indentation looks wrong.
+ }
-region->base = rdist->base_address;
-region->size = rdist->length;
region->map_base = ioremap_nocache(region->base, region->size);
if ( !region->map_base )
@@ -1412,6 +1442,7 @@ gic_acpi_parse_madt_redistributor(struct
acpi_subtable_header *header,
printk("Unable to map GICR registers\n");
return -ENOMEM;
}
+
Spurious change.
gicv3.rdist_count++;
return 0;
@@ -1421,9 +1452,22 @@ static int __init
gic_acpi_get_madt_redistributor_num(struct acpi_subtable_header *header,
const unsigned long end)
{
-/* Nothing to do here since it only wants to get the number of GIC
- * redistributors.
- */
+struct acpi_madt_generic_redistributor *rdist;
+struct acpi_madt_generic_interrupt *cpuif;
+
+if ( header->type == ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR )
+{
+rdist = (struct acpi_madt_generic_redistributor *)header;
+if ( BAD_MADT_ENTRY(rdist, end) || !rdist->base_address )
+return -EINVAL;
+}
+else if ( header->type == ACPI_MADT_TYPE_GENERIC_INTERRUPT )
+{
+cpuif = (struct acpi_madt_generic_interrupt *)header;
+if ( BAD_MADT_ENTRY(cpuif, end) || !cpuif->gicr_base_address )
+return -EINVAL;
+}
+
Ditto for the parsing.
return 0;
}
[...]
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 44b9ef6..7f9ad86 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@
[Xen-devel] [PATCH] xen/arm: domain_build: DT: add clocks node to the hypervisor node
Some clocks might be used by Xen (drivers) and not by the Linux kernel. If
these are not registered by the Linux kernel, they might be disabled by the
Linux kernel's clk_disable_unused() as the kernel doesn't know that
they are used (by Xen drivers). The clock of the serial console handled by
Xen is one example for this. It might be disabled by clk_disable_unused() which
stops the whole serial output, even from Xen, then.
Up to now, the workaround for this has been to use the Linux kernel
command line parameter 'clk_ignore_unused'. See Xen bug
http://bugs.xenproject.org/xen/bug/45
too.
To fix this, add the clocks used by Xen to the hypervisor node. The Linux
kernel has to register the clocks from the hypervisor node, then.
Therefore, collect all clocks from nodes used by Xen. These are marked
with DOMID_XEN. Afterwards, add a 'clocks' node to the hypervisor node
containing all these clocks. The Linux kernel can register all these clocks,
preventing them from being disabled, then.
Signed-off-by: Dirk Behme
---
xen/arch/arm/domain_build.c | 20
xen/arch/arm/kernel.h | 7 +++
2 files changed, 27 insertions(+)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 2e4c295..fccf87e 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -657,6 +657,10 @@ static int make_hypervisor_node(const struct kernel_info
*kinfo,
if ( res )
return res;
+res = fdt_property(fdt, "clocks", kinfo->clk.dtclocks, kinfo->clk.cnt);
+if ( res )
+return res;
+
res = fdt_end_node(fdt);
return res;
@@ -1213,9 +1217,11 @@ static int handle_node(struct domain *d, struct
kernel_info *kinfo,
{ /* sentinel */ },
};
struct dt_device_node *child;
+unsigned int len;
int res;
const char *name;
const char *path;
+const char *clocks;
path = dt_node_full_name(node);
@@ -1246,6 +1252,20 @@ static int handle_node(struct domain *d, struct
kernel_info *kinfo,
if ( dt_device_used_by(node) == DOMID_XEN )
{
DPRINT(" Skip it (used by Xen)\n");
+
+/*
+ * Remember the clock used by the skipped node
+ * We add it later to the hypervisor node to make the
+ * Linux kernel aware of its usage
+ */
+clocks = dt_get_property(node, "clocks", &len);
+if ( kinfo->clk.cnt + len >= MAX_DT_CLOCKS ) {
+printk("Failed to remember the clock node of %s\n", path);
+printk("Use the Linux kernel command 'clk_ignore_unused'\n");
+return 0;
+}
+memcpy(&kinfo->clk.dtclocks[kinfo->clk.cnt], clocks, len);
+kinfo->clk.cnt += len;
return 0;
}
diff --git a/xen/arch/arm/kernel.h b/xen/arch/arm/kernel.h
index c1b07d4..527b279 100644
--- a/xen/arch/arm/kernel.h
+++ b/xen/arch/arm/kernel.h
@@ -10,6 +10,12 @@
#include
#include
+#define MAX_DT_CLOCKS 256
+struct dtclk {
+unsigned char dtclocks[MAX_DT_CLOCKS];
+unsigned int cnt;
+};
+
struct kernel_info {
#ifdef CONFIG_ARM_64
enum domain_type type;
@@ -18,6 +24,7 @@ struct kernel_info {
void *fdt; /* flat device tree */
paddr_t unassigned_mem; /* RAM not (yet) assigned to a bank */
struct meminfo mem;
+struct dtclk clk;
/* kernel entry point */
paddr_t entry;
--
2.8.0
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/3] xen/arm: drivers: scif: Remove dead code
Hi, Dirk.
On Tue, Jun 21, 2016 at 12:15 PM, Dirk Behme wrote:
> In scif_uart_init() uart->baud is set to BAUD_AUTO. So its a basic error
> if this is different later. Detect this by an ASSERT, but remove the
> dead code normally never reached.
>
> Signed-off-by: Dirk Behme
> ---
> xen/drivers/char/scif-uart.c | 23 ++-
> 1 file changed, 6 insertions(+), 17 deletions(-)
>
> diff --git a/xen/drivers/char/scif-uart.c b/xen/drivers/char/scif-uart.c
> index 51a2233..ca88c0f 100644
> --- a/xen/drivers/char/scif-uart.c
> +++ b/xen/drivers/char/scif-uart.c
> @@ -143,23 +143,12 @@ static void __init scif_uart_init_preirq(struct
> serial_port *port)
> scif_writew(uart, SCIF_SCSMR, val);
>
> ASSERT( uart->clock_hz > 0 );
> -if ( uart->baud != BAUD_AUTO )
> -{
> -/* Setup desired Baud rate */
> -divisor = uart->clock_hz / (uart->baud << 4);
> -ASSERT( divisor >= 1 && divisor <= (uint16_t)UINT_MAX );
> -scif_writew(uart, SCIF_DL, (uint16_t)divisor);
> -/* Selects the frequency divided clock (SC_CLK external input) */
> -scif_writew(uart, SCIF_CKS, 0);
> -udelay(100 / uart->baud + 1);
This part of code might be used for people who are not satisfied with
default baudrate which has been set in U-Boot.
If we remove this we will take away the opportunity to just change
uart->baud from BAUD_AUTO to desired one.
> -}
> -else
> -{
> -/* Read current Baud rate */
> -divisor = scif_readw(uart, SCIF_DL);
> -ASSERT( divisor >= 1 && divisor <= (uint16_t)UINT_MAX );
> -uart->baud = uart->clock_hz / (divisor << 4);
> -}
> +ASSERT( uart->baud == BAUD_AUTO );
> +
> +/* Read current Baud rate */
> +divisor = scif_readw(uart, SCIF_DL);
> +ASSERT( divisor >= 1 && divisor <= (uint16_t)UINT_MAX );
> +uart->baud = uart->clock_hz / (divisor << 4);
>
> /* Setup trigger level for TX/RX FIFOs */
> scif_writew(uart, SCIF_SCFCR, SCFCR_RTRG11 | SCFCR_TTRG11);
> --
> 2.8.0
>
--
Oleksandr Tyshchenko | Embedded Dev
GlobalLogic
www.globallogic.com
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH] xen/arm: register clocks used by the hypervisor
Some clocks might be used by the Xen hypervisor and not by the Linux
kernel. If these are not registered by the Linux kernel, they might be
disabled by clk_disable_unused() as the kernel doesn't know that they
are used. The clock of the serial console handled by Xen is one
example for this. It might be disabled by clk_disable_unused() which
stops the whole serial output, even from Xen, then.
Up to now, the workaround for this has been to use the Linux kernel
command line parameter 'clk_ignore_unused'. See Xen bug
http://bugs.xenproject.org/xen/bug/45
too.
To fix this, we will add the "unused" clocks in Xen to the hypervisor
node. The Linux kernel has to register the clocks from the hypervisor
node, then.
Therefore, check if there is a "clocks" entry in the hypervisor node
and if so register the given clocks to the Linux kernel clock
framework and with this mark them as used. This prevents the clocks
from being disabled.
Signed-off-by: Dirk Behme
---
arch/arm/xen/enlighten.c | 35 +++
1 file changed, 35 insertions(+)
diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 75cd734..ee6e81f 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -381,6 +382,40 @@ static int __init xen_pm_init(void)
}
late_initcall(xen_pm_init);
+/*
+ * Check if we want to register some clocks, that they
+ * are not freed because unused by clk_disable_unused().
+ * E.g. the serial console clock.
+ */
+static int __init xen_arm_register_clks(void)
+{
+ struct clk *clk;
+ unsigned int i, count;
+ int ret;
+
+ count = of_clk_get_parent_count(xen_node);
+ if (!count)
+ return 0;
+
+ for (i = 0; i < count; i++) {
+ clk = of_clk_get(xen_node, i);
+ if (IS_ERR(clk)) {
+ pr_err("Xen failed to register clock %i. Error: %li\n",
+ i, PTR_ERR(clk));
+ return PTR_ERR(clk);
+ }
+
+ ret = clk_prepare_enable(clk);
+ if (ret < 0) {
+ pr_err("Xen failed to enable clock %i. Error: %i\n",
+ i, ret);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+late_initcall(xen_arm_register_clks);
/* empty stubs */
void xen_arch_pre_suspend(void) { }
--
2.8.0
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] xenbits "official" repo for XTF (was Re: [PATCH 0/2] xtf: add launcher (+1 bugfix)
On Mon, Jun 20, 2016 at 06:03:42PM +0100, Ian Jackson wrote: > We are in danger of getting stuck on this naming question. I would > like everyone to put forward some suggestions for the name of thisr > toplevel epo on xenbits. > > Hopefully we can find one that Andrew likes and that's acceptable to > the committers. > > I suggest > xen-microvm-test-framework Either this or the original name looks ok to me. > xen-microvm-test-suite > xtf-microvm-suite > > Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 2/8] arm/gic-v3: Fold GICR subtable parsing into a new function
Hello Shanker,
On 19/06/16 00:45, Shanker Donthineni wrote:
Add a new function for parsing GICR subtable and move the code
that is specific to GICR table to new function without changing
the function gicv3_acpi_init() behavior.
Signed-off-by: Shanker Donthineni
---
xen/arch/arm/gic-v3.c | 64 +--
1 file changed, 42 insertions(+), 22 deletions(-)
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index ab1f380..af12ebc 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1387,6 +1387,36 @@ 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;
+struct rdist_region *region;
+
+region = gicv3.rdist_regions + gicv3.rdist_count;
+rdist = (struct acpi_madt_generic_redistributor *)header;
+if ( BAD_MADT_ENTRY(rdist, end) )
+return -EINVAL;
+
+if ( !rdist->base_address || !rdist->length )
+return -EINVAL;
+
+region->base = rdist->base_address;
+region->size = rdist->length;
+
+region->map_base = ioremap_nocache(region->base, region->size);
In the commit message you said there is no functional change, however
the remapping is not part of gicv3_acpi_init. So why did you add this
line here?
+if ( !region->map_base )
+{
+printk("Unable to map GICR registers\n");
+return -ENOMEM;
+}
+gicv3.rdist_count++;
+
+return 0;
+}
+
[...]
Regards,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen/arm: domain_build: DT: add clocks node to the hypervisor node
On 21.06.2016 12:15, Dirk Behme wrote:
Some clocks might be used by Xen (drivers) and not by the Linux kernel. If
these are not registered by the Linux kernel, they might be disabled by the
Linux kernel's clk_disable_unused() as the kernel doesn't know that
they are used (by Xen drivers). The clock of the serial console handled by
Xen is one example for this. It might be disabled by clk_disable_unused() which
stops the whole serial output, even from Xen, then.
Up to now, the workaround for this has been to use the Linux kernel
command line parameter 'clk_ignore_unused'. See Xen bug
http://bugs.xenproject.org/xen/bug/45
too.
To fix this, add the clocks used by Xen to the hypervisor node. The Linux
kernel has to register the clocks from the hypervisor node, then.
Therefore, collect all clocks from nodes used by Xen. These are marked
with DOMID_XEN. Afterwards, add a 'clocks' node to the hypervisor node
containing all these clocks. The Linux kernel can register all these clocks,
preventing them from being disabled, then.
Just for the logs, the Linux kernel counterpart is
http://lists.infradead.org/pipermail/linux-arm-kernel/2016-June/438067.html
Dirk
---
xen/arch/arm/domain_build.c | 20
xen/arch/arm/kernel.h | 7 +++
2 files changed, 27 insertions(+)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 2e4c295..fccf87e 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -657,6 +657,10 @@ static int make_hypervisor_node(const struct kernel_info
*kinfo,
if ( res )
return res;
+res = fdt_property(fdt, "clocks", kinfo->clk.dtclocks, kinfo->clk.cnt);
+if ( res )
+return res;
+
res = fdt_end_node(fdt);
return res;
@@ -1213,9 +1217,11 @@ static int handle_node(struct domain *d, struct
kernel_info *kinfo,
{ /* sentinel */ },
};
struct dt_device_node *child;
+unsigned int len;
int res;
const char *name;
const char *path;
+const char *clocks;
path = dt_node_full_name(node);
@@ -1246,6 +1252,20 @@ static int handle_node(struct domain *d, struct
kernel_info *kinfo,
if ( dt_device_used_by(node) == DOMID_XEN )
{
DPRINT(" Skip it (used by Xen)\n");
+
+/*
+ * Remember the clock used by the skipped node
+ * We add it later to the hypervisor node to make the
+ * Linux kernel aware of its usage
+ */
+clocks = dt_get_property(node, "clocks", &len);
+if ( kinfo->clk.cnt + len >= MAX_DT_CLOCKS ) {
+printk("Failed to remember the clock node of %s\n", path);
+printk("Use the Linux kernel command 'clk_ignore_unused'\n");
+return 0;
+}
+memcpy(&kinfo->clk.dtclocks[kinfo->clk.cnt], clocks, len);
+kinfo->clk.cnt += len;
return 0;
}
diff --git a/xen/arch/arm/kernel.h b/xen/arch/arm/kernel.h
index c1b07d4..527b279 100644
--- a/xen/arch/arm/kernel.h
+++ b/xen/arch/arm/kernel.h
@@ -10,6 +10,12 @@
#include
#include
+#define MAX_DT_CLOCKS 256
+struct dtclk {
+unsigned char dtclocks[MAX_DT_CLOCKS];
+unsigned int cnt;
+};
+
struct kernel_info {
#ifdef CONFIG_ARM_64
enum domain_type type;
@@ -18,6 +24,7 @@ struct kernel_info {
void *fdt; /* flat device tree */
paddr_t unassigned_mem; /* RAM not (yet) assigned to a bank */
struct meminfo mem;
+struct dtclk clk;
/* kernel entry point */
paddr_t entry;
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen/arm: register clocks used by the hypervisor
On 21.06.2016 12:16, Dirk Behme wrote:
Some clocks might be used by the Xen hypervisor and not by the Linux
kernel. If these are not registered by the Linux kernel, they might be
disabled by clk_disable_unused() as the kernel doesn't know that they
are used. The clock of the serial console handled by Xen is one
example for this. It might be disabled by clk_disable_unused() which
stops the whole serial output, even from Xen, then.
Up to now, the workaround for this has been to use the Linux kernel
command line parameter 'clk_ignore_unused'. See Xen bug
http://bugs.xenproject.org/xen/bug/45
too.
To fix this, we will add the "unused" clocks in Xen to the hypervisor
node. The Linux kernel has to register the clocks from the hypervisor
node, then.
Therefore, check if there is a "clocks" entry in the hypervisor node
and if so register the given clocks to the Linux kernel clock
framework and with this mark them as used. This prevents the clocks
from being disabled.
Just for the logs, the Xen counterpart is
https://lists.xen.org/archives/html/xen-devel/2016-06/msg02607.html
Dirk
---
arch/arm/xen/enlighten.c | 35 +++
1 file changed, 35 insertions(+)
diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 75cd734..ee6e81f 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -381,6 +382,40 @@ static int __init xen_pm_init(void)
}
late_initcall(xen_pm_init);
+/*
+ * Check if we want to register some clocks, that they
+ * are not freed because unused by clk_disable_unused().
+ * E.g. the serial console clock.
+ */
+static int __init xen_arm_register_clks(void)
+{
+ struct clk *clk;
+ unsigned int i, count;
+ int ret;
+
+ count = of_clk_get_parent_count(xen_node);
+ if (!count)
+ return 0;
+
+ for (i = 0; i < count; i++) {
+ clk = of_clk_get(xen_node, i);
+ if (IS_ERR(clk)) {
+ pr_err("Xen failed to register clock %i. Error: %li\n",
+ i, PTR_ERR(clk));
+ return PTR_ERR(clk);
+ }
+
+ ret = clk_prepare_enable(clk);
+ if (ret < 0) {
+ pr_err("Xen failed to enable clock %i. Error: %i\n",
+ i, ret);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+late_initcall(xen_arm_register_clks);
/* empty stubs */
void xen_arch_pre_suspend(void) { }
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 4/8] xen/arm: vgic: Use dynamic memory allocation for vgic_rdist_region
Hello Shanker,
On 19/06/16 00:45, Shanker Donthineni wrote:
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 370cdeb..9492727 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -40,6 +40,12 @@ struct vtimer {
uint64_t cval;
};
+struct vgic_rdist_region {
+paddr_t base; /* Base address */
+paddr_t size; /* Size */
+unsigned int first_cpu;/* First CPU handled */
+};
+
struct arch_domain
{
#ifdef CONFIG_ARM_64
@@ -103,11 +109,7 @@ struct arch_domain
#ifdef CONFIG_HAS_GICV3
/* GIC V3 addressing */
/* List of contiguous occupied by the redistributors */
-struct vgic_rdist_region {
-paddr_t base; /* Base address */
-paddr_t size; /* Size */
-unsigned int first_cpu; /* First CPU handled */
-} rdist_regions[MAX_RDIST_COUNT];
+struct vgic_rdist_region *rdist_regions;
I don't think it is necessary to move the definition of
vgic_rdist_region outside.
int nr_regions; /* Number of rdist regions */
uint32_t rdist_stride; /* Re-Distributor stride */
#endif
diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
index a2fccc0..fbb763a 100644
--- a/xen/include/asm-arm/vgic.h
+++ b/xen/include/asm-arm/vgic.h
@@ -128,6 +128,8 @@ struct vgic_ops {
int (*vcpu_init)(struct vcpu *v);
/* Domain specific initialization of vGIC */
int (*domain_init)(struct domain *d);
+/* Release resources that are allocated by domain_init */
+void (*domain_free)(struct domain *d);
/* vGIC sysreg emulation */
int (*emulate_sysreg)(struct cpu_user_regs *regs, union hsr hsr);
/* Maximum number of vCPU supported */
Regards,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] ARM Xen Bug #45: Is there a solution?
Subject: Re: ARM Xen Bug #45: Is there a solution? Date: Tue, 31 May 2016 11:44:23 +0100 From: Julien Grall To: Dirk Behme , xen-devel@lists.xen.org CC: Stefano Stabellini , Ian Jackson Hello Dirk, On 27/05/16 13:34, Dirk Behme wrote: On 26.05.2016 11:00, Julien Grall wrote: On 25/05/2016 16:10, Dirk Behme wrote: On 24.05.2016 22:05, Julien Grall wrote: On 24/05/2016 14:39, Dirk Behme wrote: On 23.05.2016 22:15, Julien Grall wrote: All the devices (UART included) used by Xen will return DOMID_XEN when dt_device_used_by is called to the node. You could use it to collect the clocks of all those devices and gather the value in a single property to be created in the hypervisor node. Anything like below (untested) [1]? Yes. I'm unhappy about the global variables and the max clocks, though. How about moving those variables in the structure kernel_info? Just for the logs to finish this thread: https://lists.xen.org/archives/html/xen-devel/2016-06/msg02607.html http://lists.infradead.org/pipermail/linux-arm-kernel/2016-June/438067.html Best regards Dirk ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] xenbits "official" repo for XTF (was Re: [PATCH 0/2] xtf: add launcher (+1 bugfix)
On 20/06/16 18:03, Ian Jackson wrote: > We are in danger of getting stuck on this naming question. I would > like everyone to put forward some suggestions for the name of thisr > toplevel epo on xenbits. > > Hopefully we can find one that Andrew likes and that's acceptable to > the committers. > > I suggest > xen-microvm-test-framework > xen-microvm-test-suite > xtf-microvm-suite "xtf" It seems unfair to give Andrew's project a clunky (repo) name because osstest is not sufficiently discoverable. Perhaps you should consider renaming osstest to xen-ci-system instead? David ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 19/19] xen: credit2: use cpumask_first instead of cpumask_any when choosing cpu
On 18/06/16 00:13, Dario Faggioli wrote: > because it is cheaper, and there is no much point in > randomizing which cpu gets selected anyway, as such > choice will be overridden shortly after, in runq_tickle(). > > If we really feel the need (e.g., we prove it worth with > benchmarking), we can record the last cpu which was used > by csched2_cpu_pick() and migrate() in a per-runq variable, > and then use cpumask_cycle()... but this really does not > look necessary. Isn't this backwards? Surely you should demonstrate that this change is beneficial before proposing it? I don't think any performance related change should be accepted without experimental evidence that it makes something better, especially if it looks like it might have negative consequences (e.g., by favouring low cpus). David ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] xenbits "official" repo for XTF (was Re: [PATCH 0/2] xtf: add launcher (+1 bugfix)
David Vrabel writes ("Re: [Xen-devel] xenbits "official" repo for XTF (was Re:
[PATCH 0/2] xtf: add launcher (+1 bugfix)"):
> It seems unfair to give Andrew's project a clunky (repo) name because
> osstest is not sufficiently discoverable.
>
> Perhaps you should consider renaming osstest to xen-ci-system instead?
Sure, how about "xen-test-framework" ? :-P
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 6/8] arm: vgic: Split vgic_domain_init() functionality into two functions
Hello Shanker,
On 19/06/16 00:45, Shanker Donthineni wrote:
Split code that installs mmio handlers for GICD and Re-distributor
regions to a new function. The intension of this separation is to defer
steps that registers vgic_v2/v3 mmio handlers.
Looking at this patch and the follow-up ones, I don't think this is the
right way to go. You differ the registration of the IO handlers just
because you are unable to find the size of the handlers array.
I am wondering if the array for the handlers is the best solution here.
On another side, it would be possible to find the maximum of handlers
before hand.
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 5df5f01..5b39e0d 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -151,9 +151,12 @@ int domain_vgic_init(struct domain *d, unsigned int
nr_spis)
for ( i = 0; i < NR_GIC_SGI; i++ )
set_bit(i, d->arch.vgic.allocated_irqs);
+d->arch.vgic.handler->domain_register_mmio(d);
+
return 0;
}
+
Spurious change.
void register_vgic_ops(struct domain *d, const struct vgic_ops *ops)
{
d->arch.vgic.handler = ops;
diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
index fbb763a..8fe65b4 100644
--- a/xen/include/asm-arm/vgic.h
+++ b/xen/include/asm-arm/vgic.h
@@ -132,6 +132,8 @@ struct vgic_ops {
void (*domain_free)(struct domain *d);
/* vGIC sysreg emulation */
int (*emulate_sysreg)(struct cpu_user_regs *regs, union hsr hsr);
+/* Register mmio handlers */
+void (*domain_register_mmio)(struct domain *d);
/* Maximum number of vCPU supported */
const unsigned int max_vcpus;
};
Regards,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [qemu-mainline test] 96022: regressions - FAIL
flight 96022 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/96022/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-qemuu-debianhvm-amd64 9 debian-hvm-install fail REGR. vs. 94856 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail REGR. vs. 94856 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail REGR. vs. 94856 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail REGR. vs. 94856 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail REGR. vs. 94856 test-amd64-i386-xl-qemuu-ovmf-amd64 9 debian-hvm-install fail REGR. vs. 94856 test-armhf-armhf-libvirt-raw 9 debian-di-install fail REGR. vs. 94856 test-armhf-armhf-xl-multivcpu 6 xen-boot fail REGR. vs. 94856 Regressions which are regarded as allowable (not blocking): test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 94856 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 94856 test-amd64-amd64-xl-rtds 9 debian-install fail like 94856 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-pvh-amd 11 guest-start fail never pass test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail never pass test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass test-amd64-i386-libvirt 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-xl-arndale 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 13 saverestore-support-checkfail never pass test-armhf-armhf-xl 12 migrate-support-checkfail never pass test-armhf-armhf-xl 13 saverestore-support-checkfail never pass test-amd64-amd64-xl-pvh-intel 11 guest-start fail never pass test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 14 guest-saverestorefail never pass test-armhf-armhf-libvirt 12 migrate-support-checkfail never pass test-armhf-armhf-xl-xsm 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 11 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 12 saverestore-support-checkfail never pass test-amd64-i386-libvirt-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 12 migrate-support-checkfail never pass version targeted for testing: qemuu7e13ea57f47710de2c19f22b27b34ab9fb045700 baseline version: qemuud6550e9ed2e1a60d889dfb721de00d9a4e3bafbe Last test of basis94856 2016-05-27 20:14:49 Z 24 days Failing since 94983 2016-05-31 09:40:12 Z 21 days 28 attempts Testing same since96022 2016-06-20 19:45:06 Z0 days1 attempts People who touched revisions under test: Alberto Garcia Alex Bennée Alex Bligh Alex Williamson Alexander Graf Alexey Kardashevskiy Alistair Francis Amit Shah Andrea Arcangeli Andrew Jeffery Andrew Jones Anthony PERARD Anton Blanchard Ard Biesheuvel Benjamin Herrenschmidt Bharata B Rao Cao jin Changlong Xie Chao Peng Chen Fan Christian Borntraeger Christophe Lyon Cole Robinson Colin Lord Corey Minyard Cornelia Huck Cédric Le Goater Daniel P. Berrange David Gibson David Hildenbrand Denis V. Lunev Dmitry Fleytman Dmitry Fleytman Dmitry Osipenko Dr. David Alan Gilbert Drew Jones Edgar E. Iglesias Eduardo Habkost Emilio G. Cota Eric Blake Fam Zheng Gavin Shan Gerd Hoffmann Greg Kurz Gu Zheng Guillaume Delbergue Halil Pasic Hannes Reinecke Hyun Kwon Igor Mammedov Jakub Horak James Clarke Jan Beulich Jan Vesely Jason Wang Jean-Christophe Dubois Jens Wiklander Jerome Forissier Kevi
Re: [Xen-devel] [PATCH] xen: arm: Update arm64 image header
Hello Dirk,
On 21/06/16 10:08, 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.
Whilst the first magic is gone in the new version of the header, older
kernel will still use it. So we have to support them.
In case we read a size != 0, let's use this image size, now. This does
allow us to warn if the kernel Image is larger than the size given in
the device tree, too.
Based on the code below, you don't warn but return an error.
Signed-off-by: Dirk Behme
---
xen/arch/arm/kernel.c | 41 -
1 file changed, 24 insertions(+), 17 deletions(-)
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index 3f6cce3..1cfaf02 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
[...]
@@ -354,20 +353,28 @@ static int kernel_zimage64_probe(struct kernel_info *info,
copy_from_paddr(&zimage, addr, sizeof(zimage));
-if ( zimage.magic0 != ZIMAGE64_MAGIC_V0 &&
- zimage.magic1 != ZIMAGE64_MAGIC_V1 )
+if ( zimage.magic != ZIMAGE64_MAGIC )
return -EINVAL;
-/* Currently there is no length in the header, so just use the size */
start = 0;
-end = size;
/*
- * Given the above this check is a bit pointless, but leave it
- * here in case someone adds a length field in the future.
+ * Where image_size is non-zero image_size is little-endian
+ * and must be respected.
Can you explain what "must be respected" stands for?
*/
-if ( (end - start) > size )
+if ( zimage.image_size )
+end = zimage.image_size;
+else
+end = size;
+
+if ( (end - start) > size ) {
+if ( zimage.image_size ) {
+printk(XENLOG_ERR "Error: Kernel Image size: %lu bytes > bootmodule
size: %lu bytes\n",
+ zimage.image_size, (uint64_t)size);
+printk(XENLOG_ERR "Check the device tree configuration!\n");
This message is not really helpful when using UEFI. In this case,
multiboot is not used and the kernel image will be loaded by UEFI.
However, the size of the kernel may still mismatch the value in the
field image_size.
+}
return -EINVAL;
+}
info->zimage.kernel_addr = addr;
info->zimage.len = end - start;
Regards,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 2/7] vm-event: VM_EVENT_FLAG_DENY requires VM_EVENT_FLAG_VCPU_PAUSED
On 6/16/2016 7:11 PM, Tamas K Lengyel wrote:
On Thu, Jun 16, 2016 at 8:07 AM, Corneliu ZUZU wrote:
For VM_EVENT_FLAG_DENY to work, the vcpu must be paused (sync = 1) until the
vm-event is handled. A vm-event response having VM_EVENT_FLAG_DENY flag set
should also set the VM_EVENT_FLAG_VCPU_PAUSED flag. Enforce that in
vm_event_register_write_resume().
Well, the problem with this is that the user can set the VCPU_PAUSED
flag any time it wants. It can happen that Xen hasn't paused the vCPU
but the user still sends that flag, in which case the unpause the flag
induces will not actually do anything. You should also check if the
vCPU is in fact paused rather then just relying on this flag.
Tamas
Tamas,
Isn't that also the case with the following block @ vm_event_resume:
if ( rsp.flags & VM_EVENT_FLAG_VCPU_PAUSED )
{
if ( rsp.flags & VM_EVENT_FLAG_SET_REGISTERS )
vm_event_set_registers(v, &rsp);
if ( rsp.flags & VM_EVENT_FLAG_TOGGLE_SINGLESTEP )
vm_event_toggle_singlestep(d, v);
vm_event_vcpu_unpause(v);
}
, i.e. shouldn't it be fixed to:
/* check flags which apply only when the vCPU is paused */
if ( atomic_read(&v->vm_event_pause_count) )
{
if ( rsp.flags & VM_EVENT_FLAG_SET_REGISTERS )
vm_event_set_registers(v, &rsp);
if ( rsp.flags & VM_EVENT_FLAG_TOGGLE_SINGLESTEP )
vm_event_toggle_singlestep(d, v);
if ( rsp.flags & VM_EVENT_FLAG_VCPU_PAUSED )
vm_event_vcpu_unpause(v);
}
?
If this holds, the check for vCPU pause can also be removed from
vm_event_toggle_singlestep (maybe turned into an ASSERT which could also
be added to vm_event_set_registers).
Corneliu.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen: arm: Update arm64 image header
Hi Julien,
On 21.06.2016 13:14, Julien Grall wrote:
Hello Dirk,
On 21/06/16 10:08, 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.
Whilst the first magic is gone in the new version of the header, older
kernel will still use it. So we have to support them.
Hmm, the check for the first magic is dropped. What do you mean with
"support them"? I.e. if we don't check for it, we support kernel images
with and without this magic.
In case we read a size != 0, let's use this image size, now. This does
allow us to warn if the kernel Image is larger than the size given in
the device tree, too.
Based on the code below, you don't warn but return an error.
Yes, I should rephrase the commit message here.
In the device tree case, it's correct to return an error, as the system
doesn't boot if the kernel is larger than the memory reserved for it via
the device tree. Btw, up to now it failed silently, then.
Signed-off-by: Dirk Behme
---
xen/arch/arm/kernel.c | 41 -
1 file changed, 24 insertions(+), 17 deletions(-)
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index 3f6cce3..1cfaf02 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
[...]
@@ -354,20 +353,28 @@ static int kernel_zimage64_probe(struct
kernel_info *info,
copy_from_paddr(&zimage, addr, sizeof(zimage));
-if ( zimage.magic0 != ZIMAGE64_MAGIC_V0 &&
- zimage.magic1 != ZIMAGE64_MAGIC_V1 )
+if ( zimage.magic != ZIMAGE64_MAGIC )
return -EINVAL;
-/* Currently there is no length in the header, so just use the
size */
start = 0;
-end = size;
/*
- * Given the above this check is a bit pointless, but leave it
- * here in case someone adds a length field in the future.
+ * Where image_size is non-zero image_size is little-endian
+ * and must be respected.
Can you explain what "must be respected" stands for?
I copied this comment from
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/Documentation/arm64/booting.txt?id=a2c1d73b94ed49f5fac12e95052d7b140783f800
;)
To my understanding, it wants to say that non-zero image sizes have to
be used instead of "using as much memory as possible" (?)
*/
-if ( (end - start) > size )
+if ( zimage.image_size )
+end = zimage.image_size;
+else
+end = size;
+
+if ( (end - start) > size ) {
+if ( zimage.image_size ) {
+printk(XENLOG_ERR "Error: Kernel Image size: %lu bytes >
bootmodule size: %lu bytes\n",
+ zimage.image_size, (uint64_t)size);
+printk(XENLOG_ERR "Check the device tree configuration!\n");
This message is not really helpful when using UEFI. In this case,
multiboot is not used and the kernel image will be loaded by UEFI.
However, the size of the kernel may still mismatch the value in the
field image_size.
I don't know much about the UEFI boot case. Could you help a little?
What's needed for that case? Just dropping the "Check the device tree"
message? Where does size come from in the UEFI case? Is it set similar
to the device tree case?
+}
return -EINVAL;
+}
info->zimage.kernel_addr = addr;
info->zimage.len = end - start;
Thanks,
Dirk
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen: arm: Update arm64 image header
On 21/06/16 12:40, Dirk Behme wrote:
On 21.06.2016 13:14, Julien Grall wrote:
On 21/06/16 10:08, 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.
Whilst the first magic is gone in the new version of the header, older
kernel will still use it. So we have to support them.
Hmm, the check for the first magic is dropped. What do you mean with
"support them"? I.e. if we don't check for it, we support kernel images
with and without this magic.
Any kernel older than commit 4370eec05a887b0cd4392cd5dc5b2713174745c0
"arm64: Expand arm64 image header" will not have the second magic (i.e
ARM\x64). This commit was introduced in Linux 3.12 whilst Xen support
for ARM64 was added in Linux 3.11.
You can argue that 3.11 will unlikely be used on Xen. However this would
need to be mentioned in the commit message with maybe an error message
in Xen.
In case we read a size != 0, let's use this image size, now. This does
allow us to warn if the kernel Image is larger than the size given in
the device tree, too.
Based on the code below, you don't warn but return an error.
Yes, I should rephrase the commit message here.
In the device tree case, it's correct to return an error, as the system
doesn't boot if the kernel is larger than the memory reserved for it via
the device tree. Btw, up to now it failed silently, then.
Currently it will never failed because 'end' is always set to 'size'.
The check is here in case someone adds a length field in the future (see
comment above the check).
Signed-off-by: Dirk Behme
---
xen/arch/arm/kernel.c | 41 -
1 file changed, 24 insertions(+), 17 deletions(-)
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index 3f6cce3..1cfaf02 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
[...]
@@ -354,20 +353,28 @@ static int kernel_zimage64_probe(struct
kernel_info *info,
copy_from_paddr(&zimage, addr, sizeof(zimage));
-if ( zimage.magic0 != ZIMAGE64_MAGIC_V0 &&
- zimage.magic1 != ZIMAGE64_MAGIC_V1 )
+if ( zimage.magic != ZIMAGE64_MAGIC )
return -EINVAL;
-/* Currently there is no length in the header, so just use the
size */
start = 0;
-end = size;
/*
- * Given the above this check is a bit pointless, but leave it
- * here in case someone adds a length field in the future.
+ * Where image_size is non-zero image_size is little-endian
+ * and must be respected.
Can you explain what "must be respected" stands for?
I copied this comment from
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/Documentation/arm64/booting.txt?id=a2c1d73b94ed49f5fac12e95052d7b140783f800
;)
To my understanding, it wants to say that non-zero image sizes have to
be used instead of "using as much memory as possible" (?)
Sorry I misread the comment. I thought it was said "zero image_size". So
I am fine with it.
*/
-if ( (end - start) > size )
+if ( zimage.image_size )
+end = zimage.image_size;
+else
+end = size;
+
+if ( (end - start) > size ) {
+if ( zimage.image_size ) {
+printk(XENLOG_ERR "Error: Kernel Image size: %lu bytes >
bootmodule size: %lu bytes\n",
+ zimage.image_size, (uint64_t)size);
+printk(XENLOG_ERR "Check the device tree
configuration!\n");
This message is not really helpful when using UEFI. In this case,
multiboot is not used and the kernel image will be loaded by UEFI.
However, the size of the kernel may still mismatch the value in the
field image_size.
I don't know much about the UEFI boot case. Could you help a little?
What's needed for that case? Just dropping the "Check the device tree"
message? Where does size come from in the UEFI case? Is it set similar
to the device tree case?
Sorry my comment was not clear. I meant that the error message should be
more generic. Such as "The field 'size' does not match the size of blob"
or something similar.
+}
return -EINVAL;
+}
info->zimage.kernel_addr = addr;
info->zimage.len = end - start;
Regards,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/8] x86/time: improve cross-CPU clock monotonicity (and more)
On 06/17/2016 08:32 AM, Jan Beulich wrote:
On 16.06.16 at 22:27, wrote:
>>> I.e. my plan was, once the backwards moves are small enough, to maybe
>>> indeed compensate them by maintaining a global variable tracking
>>> the most recently returned value. There are issues with such an
>>> approach too, though: HT effects can result in one hyperthread
>>> making it just past that check of the global, then hardware
>>> switching to the other hyperthread, NOW() producing a slightly
>>> larger value there, and hardware switching back to the first
>>> hyperthread only after the second one consumed the result of
>>> NOW(). Dario's use would be unaffected by this aiui, as his NOW()
>>> invocations are globally serialized through a spinlock, but arbitrary
>>> NOW() invocations on two hyperthreads can't be made such that
>>> the invoking party can be guaranteed to see strictly montonic
>>> values.
>>>
>>> And btw., similar considerations apply for two fully independent
>>> CPUs, if one runs at a much higher P-state than the other (i.e.
>>> the faster one could overtake the slower one between the
>>> montonicity check in NOW() and the callers consuming the returned
>>> values). So in the end I'm not sure it's worth the performance hit
>>> such a global montonicity check would incur, and therefore I didn't
>>> make a respective patch part of this series.
>>>
>>
>> Hm, guests pvclock should have faced similar issues too as their
>> local stamps for each vcpu diverge. Linux commit 489fb49 ("x86, paravirt:
>> Add a
>> global synchronization point for pvclock") depicts a fix to similar
>> situations to the
>> scenarios you just described - which lead to have a global variable to keep
>> track of
>> most recent timestamp. One important chunk of that commit is pasted below
>> for
>> convenience:
>>
>> --
>> /*
>> * Assumption here is that last_value, a global accumulator, always goes
>> * forward. If we are less than that, we should not be much smaller.
>> * We assume there is an error marging we're inside, and then the correction
>> * does not sacrifice accuracy.
>> *
>> * For reads: global may have changed between test and return,
>> * but this means someone else updated poked the clock at a later time.
>> * We just need to make sure we are not seeing a backwards event.
>> *
>> * For updates: last_value = ret is not enough, since two vcpus could be
>> * updating at the same time, and one of them could be slightly behind,
>> * making the assumption that last_value always go forward fail to hold.
>> */
>> last = atomic64_read(&last_value);
>> do {
>> if (ret < last)
>> return last;
>> last = atomic64_cmpxchg(&last_value, last, ret);
>> } while (unlikely(last != ret));
>> --
>
> Meaning they decided it's worth the overhead. But (having read
> through the entire description) they don't even discuss whether this
> indeed eliminates _all_ apparent backward moves due to effects
> like the ones named above.
>
> Plus, the contention they're facing is limited to a single VM, i.e. likely
> much more narrow than that on an entire physical system. So for
> us to do the same in the hypervisor, quite a bit more of win would
> be needed to outweigh the cost.
>
Experimental details look very unclear too - likely running the time
warp test for 5 days would get some of these cases cleared out. But
as you say it should be much more narrow that of an entire system.
BTW It was implicit in the discussion but my apologies for not
formally/explicitly stating. So FWIW:
Tested-by: Joao Martins
This series is certainly a way forward into improving cross-CPU monotonicity,
and I am seeing indeed less occurrences of time going backwards on my systems.
Joao
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/3] xen/arm: drivers: scif: Remove dead code
On 21/06/16 11:16, Oleksandr Tyshchenko wrote:
Hi, Dirk.
Hello Oleksandr,
On Tue, Jun 21, 2016 at 12:15 PM, Dirk Behme wrote:
In scif_uart_init() uart->baud is set to BAUD_AUTO. So its a basic error
if this is different later. Detect this by an ASSERT, but remove the
dead code normally never reached.
Signed-off-by: Dirk Behme
---
xen/drivers/char/scif-uart.c | 23 ++-
1 file changed, 6 insertions(+), 17 deletions(-)
diff --git a/xen/drivers/char/scif-uart.c b/xen/drivers/char/scif-uart.c
index 51a2233..ca88c0f 100644
--- a/xen/drivers/char/scif-uart.c
+++ b/xen/drivers/char/scif-uart.c
@@ -143,23 +143,12 @@ static void __init scif_uart_init_preirq(struct
serial_port *port)
scif_writew(uart, SCIF_SCSMR, val);
ASSERT( uart->clock_hz > 0 );
-if ( uart->baud != BAUD_AUTO )
-{
-/* Setup desired Baud rate */
-divisor = uart->clock_hz / (uart->baud << 4);
-ASSERT( divisor >= 1 && divisor <= (uint16_t)UINT_MAX );
-scif_writew(uart, SCIF_DL, (uint16_t)divisor);
-/* Selects the frequency divided clock (SC_CLK external input) */
-scif_writew(uart, SCIF_CKS, 0);
-udelay(100 / uart->baud + 1);
This part of code might be used for people who are not satisfied with
default baudrate which has been set in U-Boot.
Can you elaborate? If the baud rate is different, it will not be
possible to get output from U-boot.
If we remove this we will take away the opportunity to just change
uart->baud from BAUD_AUTO to desired one.
I have some doubt that the current code is valid. The clock frequency is
hardcoded (see SCIF_CLK_FREQ), so are you saying that the frequency is
always the same across all the platforms?
I would rather avoid to keep dead code (or not accessible without
hacking Xen). For what is worth, we recently removed a similar code from
the PL011 driver as this should be correctly configured by the firmware.
-}
-else
-{
-/* Read current Baud rate */
-divisor = scif_readw(uart, SCIF_DL);
-ASSERT( divisor >= 1 && divisor <= (uint16_t)UINT_MAX );
-uart->baud = uart->clock_hz / (divisor << 4);
-}
+ASSERT( uart->baud == BAUD_AUTO );
+
+/* Read current Baud rate */
+divisor = scif_readw(uart, SCIF_DL);
+ASSERT( divisor >= 1 && divisor <= (uint16_t)UINT_MAX );
+uart->baud = uart->clock_hz / (divisor << 4);
/* Setup trigger level for TX/RX FIFOs */
scif_writew(uart, SCIF_SCFCR, SCFCR_RTRG11 | SCFCR_TTRG11);
--
2.8.0
Regards,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 2/3] xen/arm: drivers: scif: Remove unused variables
Hello Dirk,
On 21/06/16 10:15, Dirk Behme wrote:
The two struct members baud and clock_hz are in the end read only
variables nowhere used for anything useful. Removing them makes
the code much simpler without changing any functionality.
From my understanding, this patch is removing code you just added on
the previous patch. I would prefer if you squash this patch into #1.
Regards,
Signed-off-by: Dirk Behme
---
xen/drivers/char/scif-uart.c| 13 +
xen/include/asm-arm/scif-uart.h | 1 -
2 files changed, 1 insertion(+), 13 deletions(-)
diff --git a/xen/drivers/char/scif-uart.c b/xen/drivers/char/scif-uart.c
index ca88c0f..bc157fe 100644
--- a/xen/drivers/char/scif-uart.c
+++ b/xen/drivers/char/scif-uart.c
@@ -41,7 +41,7 @@
#define scif_writew(uart, off, val)writew((val), (uart)->regs + (off))
static struct scif_uart {
-unsigned int baud, clock_hz, data_bits, parity, stop_bits;
+unsigned int data_bits, parity, stop_bits;
unsigned int irq;
char __iomem *regs;
struct irqaction irqaction;
@@ -87,7 +87,6 @@ static void scif_uart_interrupt(int irq, void *data, struct
cpu_user_regs *regs)
static void __init scif_uart_init_preirq(struct serial_port *port)
{
struct scif_uart *uart = port->uart;
-unsigned int divisor;
uint16_t val;
/*
@@ -142,14 +141,6 @@ static void __init scif_uart_init_preirq(struct
serial_port *port)
}
scif_writew(uart, SCIF_SCSMR, val);
-ASSERT( uart->clock_hz > 0 );
-ASSERT( uart->baud == BAUD_AUTO );
-
-/* Read current Baud rate */
-divisor = scif_readw(uart, SCIF_DL);
-ASSERT( divisor >= 1 && divisor <= (uint16_t)UINT_MAX );
-uart->baud = uart->clock_hz / (divisor << 4);
-
/* Setup trigger level for TX/RX FIFOs */
scif_writew(uart, SCIF_SCFCR, SCFCR_RTRG11 | SCFCR_TTRG11);
@@ -292,8 +283,6 @@ static int __init scif_uart_init(struct dt_device_node *dev,
uart = &scif_com;
-uart->clock_hz = SCIF_CLK_FREQ;
-uart->baud = BAUD_AUTO;
uart->data_bits = 8;
uart->parity= PARITY_NONE;
uart->stop_bits = 1;
diff --git a/xen/include/asm-arm/scif-uart.h b/xen/include/asm-arm/scif-uart.h
index 7a9f639..d030b26 100644
--- a/xen/include/asm-arm/scif-uart.h
+++ b/xen/include/asm-arm/scif-uart.h
@@ -22,7 +22,6 @@
#define __ASM_ARM_SCIF_UART_H
#define SCIF_FIFO_MAX_SIZE16
-#define SCIF_CLK_FREQ 14745600
/* Register offsets */
#define SCIF_SCSMR (0x00)/* Serial mode register */
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 3/3] xen/arm: drivers: scif: Add clock auto detection
Hello Dirk, On 21/06/16 10:15, Dirk Behme wrote: Besides the 14MHz external clock, the SCIF might be clocked by an internal 66MHz clock. Detect this clock based on the SCIF_DL register being 0 (internal clock) or != 0 (external clock). Do you have a public link to the specification? Signed-off-by: Dirk Behme --- xen/drivers/char/scif-uart.c | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/xen/drivers/char/scif-uart.c b/xen/drivers/char/scif-uart.c index bc157fe..678f46b 100644 --- a/xen/drivers/char/scif-uart.c +++ b/xen/drivers/char/scif-uart.c @@ -107,8 +107,19 @@ static void __init scif_uart_init_preirq(struct serial_port *port) scif_readw(uart, SCIF_SCLSR); scif_writew(uart, SCIF_SCLSR, 0); -/* Select Baud rate generator output as a clock source */ -scif_writew(uart, SCIF_SCSCR, SCSCR_CKE10); +/* + * Select Baud rate generator output as a clock source + * The clock source can be an internal or external clock. + * Depending on this the DL register is either 0 or contains + * the divisor. I.e. we can use this to detect the clock + * source and based on this can configure the CKE[1:0] bits + * of the SCSCR register. + */ +if ( scif_readw(uart, SCIF_DL) ) +scif_writew(uart, SCIF_SCSCR, SCSCR_CKE10); /* External clk */ +else +scif_writew(uart, SCIF_SCSCR, SCSCR_CKE00); /* Internal clk */ Why would we need to select the baud rate generator if the baud has been configured by the firmware? + Please drop this newline. /* Setup protocol format and Baud rate, select Asynchronous mode */ val = 0; Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/8] x86/time: improve cross-CPU clock monotonicity (and more)
>>> On 21.06.16 at 14:05, wrote:
>
> On 06/17/2016 08:32 AM, Jan Beulich wrote:
> On 16.06.16 at 22:27, wrote:
I.e. my plan was, once the backwards moves are small enough, to maybe
indeed compensate them by maintaining a global variable tracking
the most recently returned value. There are issues with such an
approach too, though: HT effects can result in one hyperthread
making it just past that check of the global, then hardware
switching to the other hyperthread, NOW() producing a slightly
larger value there, and hardware switching back to the first
hyperthread only after the second one consumed the result of
NOW(). Dario's use would be unaffected by this aiui, as his NOW()
invocations are globally serialized through a spinlock, but arbitrary
NOW() invocations on two hyperthreads can't be made such that
the invoking party can be guaranteed to see strictly montonic
values.
And btw., similar considerations apply for two fully independent
CPUs, if one runs at a much higher P-state than the other (i.e.
the faster one could overtake the slower one between the
montonicity check in NOW() and the callers consuming the returned
values). So in the end I'm not sure it's worth the performance hit
such a global montonicity check would incur, and therefore I didn't
make a respective patch part of this series.
>>>
>>> Hm, guests pvclock should have faced similar issues too as their
>>> local stamps for each vcpu diverge. Linux commit 489fb49 ("x86, paravirt:
>>> Add a
>>> global synchronization point for pvclock") depicts a fix to similar
>>> situations to the
>>> scenarios you just described - which lead to have a global variable to keep
>>> track of
>>> most recent timestamp. One important chunk of that commit is pasted below
>>> for
>>> convenience:
>>>
>>> --
>>> /*
>>> * Assumption here is that last_value, a global accumulator, always goes
>>> * forward. If we are less than that, we should not be much smaller.
>>> * We assume there is an error marging we're inside, and then the correction
>>> * does not sacrifice accuracy.
>>> *
>>> * For reads: global may have changed between test and return,
>>> * but this means someone else updated poked the clock at a later time.
>>> * We just need to make sure we are not seeing a backwards event.
>>> *
>>> * For updates: last_value = ret is not enough, since two vcpus could be
>>> * updating at the same time, and one of them could be slightly behind,
>>> * making the assumption that last_value always go forward fail to hold.
>>> */
>>> last = atomic64_read(&last_value);
>>> do {
>>> if (ret < last)
>>> return last;
>>> last = atomic64_cmpxchg(&last_value, last, ret);
>>> } while (unlikely(last != ret));
>>> --
>>
>> Meaning they decided it's worth the overhead. But (having read
>> through the entire description) they don't even discuss whether this
>> indeed eliminates _all_ apparent backward moves due to effects
>> like the ones named above.
>>
>> Plus, the contention they're facing is limited to a single VM, i.e. likely
>> much more narrow than that on an entire physical system. So for
>> us to do the same in the hypervisor, quite a bit more of win would
>> be needed to outweigh the cost.
>>
> Experimental details look very unclear too - likely running the time
> warp test for 5 days would get some of these cases cleared out. But
> as you say it should be much more narrow that of an entire system.
>
> BTW It was implicit in the discussion but my apologies for not
> formally/explicitly stating. So FWIW:
>
> Tested-by: Joao Martins
Thanks, but this ...
> This series is certainly a way forward into improving cross-CPU monotonicity,
> and I am seeing indeed less occurrences of time going backwards on my
> systems.
... leaves me guessing whether the above was meant for just this
patch, or the entire series.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 3/3] xen/arm: drivers: scif: Add clock auto detection
Hi Julien, On 21.06.2016 14:20, Julien Grall wrote: Hello Dirk, On 21/06/16 10:15, Dirk Behme wrote: Besides the 14MHz external clock, the SCIF might be clocked by an internal 66MHz clock. Detect this clock based on the SCIF_DL register being 0 (internal clock) or != 0 (external clock). Do you have a public link to the specification? I have to check this ;) Signed-off-by: Dirk Behme --- xen/drivers/char/scif-uart.c | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/xen/drivers/char/scif-uart.c b/xen/drivers/char/scif-uart.c index bc157fe..678f46b 100644 --- a/xen/drivers/char/scif-uart.c +++ b/xen/drivers/char/scif-uart.c @@ -107,8 +107,19 @@ static void __init scif_uart_init_preirq(struct serial_port *port) scif_readw(uart, SCIF_SCLSR); scif_writew(uart, SCIF_SCLSR, 0); -/* Select Baud rate generator output as a clock source */ -scif_writew(uart, SCIF_SCSCR, SCSCR_CKE10); +/* + * Select Baud rate generator output as a clock source + * The clock source can be an internal or external clock. + * Depending on this the DL register is either 0 or contains + * the divisor. I.e. we can use this to detect the clock + * source and based on this can configure the CKE[1:0] bits + * of the SCSCR register. + */ +if ( scif_readw(uart, SCIF_DL) ) +scif_writew(uart, SCIF_SCSCR, SCSCR_CKE10); /* External clk */ +else +scif_writew(uart, SCIF_SCSCR, SCSCR_CKE00); /* Internal clk */ Why would we need to select the baud rate generator if the baud has been configured by the firmware? Just to get the correct understanding: The proposal is to just remove the code which (wrongly) overwrites the correct settings done by the firmware? I.e. instead of doing the same thing the firmware is already doing, again (the if .. else ), the proposal is simply dropping the -/* Select Baud rate generator output as a clock source */ -scif_writew(uart, SCIF_SCSCR, SCSCR_CKE10); ? Best regards Dirk ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 3/3] xen/arm: drivers: scif: Add clock auto detection
On 21/06/16 13:30, Dirk Behme wrote: diff --git a/xen/drivers/char/scif-uart.c b/xen/drivers/char/scif-uart.c index bc157fe..678f46b 100644 --- a/xen/drivers/char/scif-uart.c +++ b/xen/drivers/char/scif-uart.c @@ -107,8 +107,19 @@ static void __init scif_uart_init_preirq(struct serial_port *port) scif_readw(uart, SCIF_SCLSR); scif_writew(uart, SCIF_SCLSR, 0); -/* Select Baud rate generator output as a clock source */ -scif_writew(uart, SCIF_SCSCR, SCSCR_CKE10); +/* + * Select Baud rate generator output as a clock source + * The clock source can be an internal or external clock. + * Depending on this the DL register is either 0 or contains + * the divisor. I.e. we can use this to detect the clock + * source and based on this can configure the CKE[1:0] bits + * of the SCSCR register. + */ +if ( scif_readw(uart, SCIF_DL) ) +scif_writew(uart, SCIF_SCSCR, SCSCR_CKE10); /* External clk */ +else +scif_writew(uart, SCIF_SCSCR, SCSCR_CKE00); /* Internal clk */ Why would we need to select the baud rate generator if the baud has been configured by the firmware? Just to get the correct understanding: The proposal is to just remove the code which (wrongly) overwrites the correct settings done by the firmware? I.e. instead of doing the same thing the firmware is already doing, again (the if .. else ), the proposal is simply dropping the -/* Select Baud rate generator output as a clock source */ -scif_writew(uart, SCIF_SCSCR, SCSCR_CKE10); ? Yes. However I don't have any spec in hand so I am not sure if it is correct. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 10/11] hvmctl: convert HVMOP_*ioreq_server*
> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: 20 June 2016 13:58
> To: xen-devel
> Cc: Andrew Cooper; Paul Durrant; Wei Liu; George Dunlap; Ian Jackson;
> Stefano Stabellini; dgde...@tycho.nsa.gov; Tim (Xen.org)
> Subject: [PATCH 10/11] hvmctl: convert HVMOP_*ioreq_server*
>
> Note that we can't adjust HVM_IOREQSRV_BUFIOREQ_* to properly obey
> name space rules, as these constants as in use by callers of the libxc
> interface.
>
> Signed-off-by: Jan Beulich
Reviewed-by: Paul Durrant
>
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -41,6 +41,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> --- a/tools/libxc/xc_domain.c
> +++ b/tools/libxc/xc_domain.c
> @@ -1416,23 +1416,14 @@ int xc_hvm_create_ioreq_server(xc_interf
> int handle_bufioreq,
> ioservid_t *id)
> {
> -DECLARE_HYPERCALL_BUFFER(xen_hvm_create_ioreq_server_t, arg);
> +DECLARE_HVMCTL(create_ioreq_server, domid,
> + .handle_bufioreq = handle_bufioreq);
> int rc;
>
> -arg = xc_hypercall_buffer_alloc(xch, arg, sizeof(*arg));
> -if ( arg == NULL )
> -return -1;
> -
> -arg->domid = domid;
> -arg->handle_bufioreq = handle_bufioreq;
> -
> -rc = xencall2(xch->xcall, __HYPERVISOR_hvm_op,
> - HVMOP_create_ioreq_server,
> - HYPERCALL_BUFFER_AS_ARG(arg));
> +rc = do_hvmctl(xch, &hvmctl);
>
> -*id = arg->id;
> +*id = hvmctl.u.create_ioreq_server.id;
>
> -xc_hypercall_buffer_free(xch, arg);
> return rc;
> }
>
> @@ -1443,84 +1434,52 @@ int xc_hvm_get_ioreq_server_info(xc_inte
> xen_pfn_t *bufioreq_pfn,
> evtchn_port_t *bufioreq_port)
> {
> -DECLARE_HYPERCALL_BUFFER(xen_hvm_get_ioreq_server_info_t, arg);
> +DECLARE_HVMCTL(get_ioreq_server_info, domid,
> + .id = id);
> int rc;
>
> -arg = xc_hypercall_buffer_alloc(xch, arg, sizeof(*arg));
> -if ( arg == NULL )
> -return -1;
> -
> -arg->domid = domid;
> -arg->id = id;
> -
> -rc = xencall2(xch->xcall, __HYPERVISOR_hvm_op,
> - HVMOP_get_ioreq_server_info,
> - HYPERCALL_BUFFER_AS_ARG(arg));
> +rc = do_hvmctl(xch, &hvmctl);
> if ( rc != 0 )
> -goto done;
> +return rc;
>
> if ( ioreq_pfn )
> -*ioreq_pfn = arg->ioreq_pfn;
> +*ioreq_pfn = hvmctl.u.get_ioreq_server_info.ioreq_pfn;
>
> if ( bufioreq_pfn )
> -*bufioreq_pfn = arg->bufioreq_pfn;
> +*bufioreq_pfn = hvmctl.u.get_ioreq_server_info.bufioreq_pfn;
>
> if ( bufioreq_port )
> -*bufioreq_port = arg->bufioreq_port;
> +*bufioreq_port = hvmctl.u.get_ioreq_server_info.bufioreq_port;
>
> -done:
> -xc_hypercall_buffer_free(xch, arg);
> -return rc;
> +return 0;
> }
>
> int xc_hvm_map_io_range_to_ioreq_server(xc_interface *xch, domid_t
> domid,
> ioservid_t id, int is_mmio,
> uint64_t start, uint64_t end)
> {
> -DECLARE_HYPERCALL_BUFFER(xen_hvm_io_range_t, arg);
> -int rc;
> -
> -arg = xc_hypercall_buffer_alloc(xch, arg, sizeof(*arg));
> -if ( arg == NULL )
> -return -1;
> -
> -arg->domid = domid;
> -arg->id = id;
> -arg->type = is_mmio ? HVMOP_IO_RANGE_MEMORY :
> HVMOP_IO_RANGE_PORT;
> -arg->start = start;
> -arg->end = end;
> -
> -rc = xencall2(xch->xcall, __HYPERVISOR_hvm_op,
> - HVMOP_map_io_range_to_ioreq_server,
> - HYPERCALL_BUFFER_AS_ARG(arg));
> +DECLARE_HVMCTL(map_io_range_to_ioreq_server, domid,
> + .id = id,
> + .type = is_mmio ? XEN_HVMCTL_IO_RANGE_MEMORY
> + : XEN_HVMCTL_IO_RANGE_PORT,
> + .start = start,
> + .end = end);
>
> -xc_hypercall_buffer_free(xch, arg);
> -return rc;
> +return do_hvmctl(xch, &hvmctl);
> }
>
> int xc_hvm_unmap_io_range_from_ioreq_server(xc_interface *xch,
> domid_t domid,
> ioservid_t id, int is_mmio,
> uint64_t start, uint64_t end)
> {
> -DECLARE_HYPERCALL_BUFFER(xen_hvm_io_range_t, arg);
> -int rc;
> -
> -arg = xc_hypercall_buffer_alloc(xch, arg, sizeof(*arg));
> -if ( arg == NULL )
> -return -1;
> +DECLARE_HVMCTL(unmap_io_range_from_ioreq_server, domid,
> + .id = id,
> + .type = is_mmio ? XEN_HVMCTL_IO_RANGE_MEMORY
> + : XEN_HVMCTL_IO_RANGE_PORT,
> + .start = start,
> + .end = end);
>
> -arg->domid = domid;
> -arg->id = id;
> -arg->type = is_mmio ?
Re: [Xen-devel] [PATCH 1/3] xen/arm: drivers: scif: Remove dead code
On Tue, Jun 21, 2016 at 3:15 PM, Julien Grall wrote:
>
>
> On 21/06/16 11:16, Oleksandr Tyshchenko wrote:
>>
>> Hi, Dirk.
>
>
> Hello Oleksandr,
Hello Julien.
>
>
>> On Tue, Jun 21, 2016 at 12:15 PM, Dirk Behme
>> wrote:
>>>
>>> In scif_uart_init() uart->baud is set to BAUD_AUTO. So its a basic error
>>> if this is different later. Detect this by an ASSERT, but remove the
>>> dead code normally never reached.
>>>
>>> Signed-off-by: Dirk Behme
>>> ---
>>> xen/drivers/char/scif-uart.c | 23 ++-
>>> 1 file changed, 6 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/xen/drivers/char/scif-uart.c b/xen/drivers/char/scif-uart.c
>>> index 51a2233..ca88c0f 100644
>>> --- a/xen/drivers/char/scif-uart.c
>>> +++ b/xen/drivers/char/scif-uart.c
>>> @@ -143,23 +143,12 @@ static void __init scif_uart_init_preirq(struct
>>> serial_port *port)
>>> scif_writew(uart, SCIF_SCSMR, val);
>>>
>>> ASSERT( uart->clock_hz > 0 );
>>> -if ( uart->baud != BAUD_AUTO )
>>> -{
>>> -/* Setup desired Baud rate */
>>> -divisor = uart->clock_hz / (uart->baud << 4);
>>> -ASSERT( divisor >= 1 && divisor <= (uint16_t)UINT_MAX );
>>> -scif_writew(uart, SCIF_DL, (uint16_t)divisor);
>>> -/* Selects the frequency divided clock (SC_CLK external input)
>>> */
>>> -scif_writew(uart, SCIF_CKS, 0);
>>> -udelay(100 / uart->baud + 1);
>>
>>
>> This part of code might be used for people who are not satisfied with
>> default baudrate which has been set in U-Boot.
>
>
> Can you elaborate? If the baud rate is different, it will not be possible to
> get output from U-boot.
>
>> If we remove this we will take away the opportunity to just change
>> uart->baud from BAUD_AUTO to desired one.
>
>
> I have some doubt that the current code is valid. The clock frequency is
> hardcoded (see SCIF_CLK_FREQ), so are you saying that the frequency is
> always the same across all the platforms?
No.
This driver has been initially written for Renesas Lager board based
on R-Car H2 SoC which
has SCIF compatible UART. And the current code is valid for it. I have
tested both auto and
variable (38400, 115200) baudrates.
But, I am afraid that current code won't be suitable for
all of the boards which have the same UART IP. It depends at least
from clock source
(external/internal) and clock frequency.
>
> I would rather avoid to keep dead code (or not accessible without hacking
> Xen). For what is worth, we recently removed a similar code from the PL011
> driver as this should be correctly configured by the firmware.
>
>>> -}
>>> -else
>>> -{
>>> -/* Read current Baud rate */
>>> -divisor = scif_readw(uart, SCIF_DL);
>>> -ASSERT( divisor >= 1 && divisor <= (uint16_t)UINT_MAX );
>>> -uart->baud = uart->clock_hz / (divisor << 4);
>>> -}
>>> +ASSERT( uart->baud == BAUD_AUTO );
>>> +
>>> +/* Read current Baud rate */
>>> +divisor = scif_readw(uart, SCIF_DL);
>>> +ASSERT( divisor >= 1 && divisor <= (uint16_t)UINT_MAX );
>>> +uart->baud = uart->clock_hz / (divisor << 4);
>>>
>>> /* Setup trigger level for TX/RX FIFOs */
>>> scif_writew(uart, SCIF_SCFCR, SCFCR_RTRG11 | SCFCR_TTRG11);
>>> --
>>> 2.8.0
>>>
>
> Regards,
>
> --
> Julien Grall
--
Oleksandr Tyshchenko | Embedded Dev
GlobalLogic
www.globallogic.com
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/3] xen/arm: drivers: scif: Remove dead code
Hello Oleksandr,
On 21.06.2016 14:54, Oleksandr Tyshchenko wrote:
On Tue, Jun 21, 2016 at 3:15 PM, Julien Grall wrote:
On 21/06/16 11:16, Oleksandr Tyshchenko wrote:
Hi, Dirk.
Hello Oleksandr,
Hello Julien.
On Tue, Jun 21, 2016 at 12:15 PM, Dirk Behme
wrote:
In scif_uart_init() uart->baud is set to BAUD_AUTO. So its a basic error
if this is different later. Detect this by an ASSERT, but remove the
dead code normally never reached.
Signed-off-by: Dirk Behme
---
xen/drivers/char/scif-uart.c | 23 ++-
1 file changed, 6 insertions(+), 17 deletions(-)
diff --git a/xen/drivers/char/scif-uart.c b/xen/drivers/char/scif-uart.c
index 51a2233..ca88c0f 100644
--- a/xen/drivers/char/scif-uart.c
+++ b/xen/drivers/char/scif-uart.c
@@ -143,23 +143,12 @@ static void __init scif_uart_init_preirq(struct
serial_port *port)
scif_writew(uart, SCIF_SCSMR, val);
ASSERT( uart->clock_hz > 0 );
-if ( uart->baud != BAUD_AUTO )
-{
-/* Setup desired Baud rate */
-divisor = uart->clock_hz / (uart->baud << 4);
-ASSERT( divisor >= 1 && divisor <= (uint16_t)UINT_MAX );
-scif_writew(uart, SCIF_DL, (uint16_t)divisor);
-/* Selects the frequency divided clock (SC_CLK external input)
*/
-scif_writew(uart, SCIF_CKS, 0);
-udelay(100 / uart->baud + 1);
This part of code might be used for people who are not satisfied with
default baudrate which has been set in U-Boot.
Can you elaborate? If the baud rate is different, it will not be possible to
get output from U-boot.
If we remove this we will take away the opportunity to just change
uart->baud from BAUD_AUTO to desired one.
I have some doubt that the current code is valid. The clock frequency is
hardcoded (see SCIF_CLK_FREQ), so are you saying that the frequency is
always the same across all the platforms?
No.
This driver has been initially written for Renesas Lager board based
on R-Car H2 SoC which
has SCIF compatible UART. And the current code is valid for it. I have
tested both auto and
variable (38400, 115200) baudrates.
Could you help me a little to understand this?
The driver has
scif_uart_init()
{
...
struct scif_uart *uart;
...
uart->baud = BAUD_AUTO;
...
}
I checked the code for struct scif_uart but it isn't used anywhere
outside this driver. So uart->baud is a driver local variable, which
looks to me isn't used anywhere useful.
What have I missed?
Best regards
Dirk
But, I am afraid that current code won't be suitable for
all of the boards which have the same UART IP. It depends at least
from clock source
(external/internal) and clock frequency.
I would rather avoid to keep dead code (or not accessible without hacking
Xen). For what is worth, we recently removed a similar code from the PL011
driver as this should be correctly configured by the firmware.
-}
-else
-{
-/* Read current Baud rate */
-divisor = scif_readw(uart, SCIF_DL);
-ASSERT( divisor >= 1 && divisor <= (uint16_t)UINT_MAX );
-uart->baud = uart->clock_hz / (divisor << 4);
-}
+ASSERT( uart->baud == BAUD_AUTO );
+
+/* Read current Baud rate */
+divisor = scif_readw(uart, SCIF_DL);
+ASSERT( divisor >= 1 && divisor <= (uint16_t)UINT_MAX );
+uart->baud = uart->clock_hz / (divisor << 4);
/* Setup trigger level for TX/RX FIFOs */
scif_writew(uart, SCIF_SCFCR, SCFCR_RTRG11 | SCFCR_TTRG11);
--
2.8.0
Regards,
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/3] xen/arm: drivers: scif: Remove dead code
On 21/06/16 13:54, Oleksandr Tyshchenko wrote: On Tue, Jun 21, 2016 at 3:15 PM, Julien Grall wrote: On Tue, Jun 21, 2016 at 12:15 PM, Dirk Behme wrote: I have some doubt that the current code is valid. The clock frequency is hardcoded (see SCIF_CLK_FREQ), so are you saying that the frequency is always the same across all the platforms? No. This driver has been initially written for Renesas Lager board based on R-Car H2 SoC which has SCIF compatible UART. And the current code is valid for it. I have tested both auto and variable (38400, 115200) baudrates. But, I am afraid that current code won't be suitable for all of the boards which have the same UART IP. It depends at least from clock source (external/internal) and clock frequency. In this case, the code should either be fixed by reading the clock frequency from the firmware tables or be dropped. I would lean towards the later because the user cannot specify the baud rate to Xen. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/3] xen/arm: drivers: scif: Remove dead code
On Tue, Jun 21, 2016 at 4:07 PM, Julien Grall wrote: > > > On 21/06/16 13:54, Oleksandr Tyshchenko wrote: >> >> On Tue, Jun 21, 2016 at 3:15 PM, Julien Grall >> wrote: On Tue, Jun 21, 2016 at 12:15 PM, Dirk Behme wrote: >>> >>> I have some doubt that the current code is valid. The clock frequency is >>> hardcoded (see SCIF_CLK_FREQ), so are you saying that the frequency is >>> always the same across all the platforms? >> >> >> No. >> >> This driver has been initially written for Renesas Lager board based >> on R-Car H2 SoC which >> has SCIF compatible UART. And the current code is valid for it. I have >> tested both auto and >> variable (38400, 115200) baudrates. >> But, I am afraid that current code won't be suitable for >> all of the boards which have the same UART IP. It depends at least >> from clock source >> (external/internal) and clock frequency. > > > In this case, the code should either be fixed by reading the clock frequency > from the firmware tables or be dropped. > > I would lean towards the later because the user cannot specify the baud rate > to Xen. Agree. > > Regards, > > -- > Julien Grall -- Oleksandr Tyshchenko | Embedded Dev GlobalLogic www.globallogic.com ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 4/8] 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
---
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:
- 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,
- xatp->idx, xatp->gpfn);
+ xatp->id
[Xen-devel] [PATCH v3 6/8] 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 aa4e774..47cb383 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, start_gfn, start_gfn + 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(E
[Xen-devel] [PATCH v3 0/8] 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. I pushed a branch with this series applied on xenbits: git://xenbits.xen.org/people/julieng/xen-unstable.git branch typesafe-v3 Yours sincerely, 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 Julien Grall (8): xen/arm: Rename gmfn_to_mfn to gfn_to_mfn and use gfn/mfn typesafe xen/mm: Introduce a bunch of helpers for the typesafes mfn and gfn 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/arm: Rework the interface of p2m_lookup and use typesafe gfn and mfn xen/arm: p2m_cache_flush: Use the correct terminology and typesafe gfn xen/arch/arm/domain.c | 4 +- xen/arch/arm/domain_build.c| 6 +-- xen/arch/arm/domctl.c | 2 +- xen/arch/arm/gic-v2.c | 4 +- xen/arch/arm/mm.c | 18 +++ xen/arch/arm/p2m.c | 91 +++- 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/domain.c | 5 +- xen/arch/x86/domain_build.c| 6 +-- xen/arch/x86/hvm/ioreq.c | 8 ++-- xen/arch/x86/mm.c | 21 + xen/arch/x86/mm/p2m.c | 96 +- xen/common/domctl.c| 4 +- xen/common/grant_table.c | 11 +++-- xen/common/memory.c| 40 xen/drivers/passthrough/arm/smmu.c | 4 +- xen/include/asm-arm/domain.h | 2 +- xen/include/asm-arm/grant_table.h | 2 +- xen/include/asm-arm/p2m.h | 23 + xen/include/asm-x86/p2m.h | 11 ++--- xen/include/xen/mm.h | 45 +- xen/include/xen/p2m-common.h | 8 ++-- 25 files changed, 258 insertions(+), 202 deletions(-) -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 7/8] 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
---
xen/arch/arm/p2m.c| 37 -
xen/arch/arm/traps.c | 21 +++--
xen/include/asm-arm/p2m.h | 7 +++
3 files changed, 34 insertions(+), 31 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 47cb383..f3330dd 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -140,14 +140,15 @@ void flush_tlb_domain(struct domain *d)
}
/*
- * Lookup the MFN corresponding to a domain's PFN.
+ * Lookup the MFN corresponding to a domain's GFN.
*
* There are no processor functions to do a stage 2 only lookup therefore we
* do a a software walk.
*/
-static paddr_t __p2m_lookup(struct domain *d, paddr_t paddr, p2m_type_t *t)
+static mfn_t __p2m_lookup(struct domain *d, gfn_t gfn, p2m_type_t *t)
{
struct p2m_domain *p2m = &d->arch.p2m;
+const paddr_t paddr = pfn_to_paddr(gfn_x(gfn));
const unsigned int offsets[4] = {
zeroeth_table_offset(paddr),
first_table_offset(paddr),
@@ -158,7 +159,7 @@ static paddr_t __p2m_lookup(struct domain *d, paddr_t
paddr, p2m_type_t *t)
ZEROETH_MASK, FIRST_MASK, SECOND_MASK, THIRD_MASK
};
lpae_t pte, *map;
-paddr_t maddr = INVALID_PADDR;
+mfn_t mfn = _mfn(INVALID_MFN);
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_x(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,7 +1500,7 @@ p2m_mem_access_check_and_get_page(vaddr_t gva, unsigned
long flag)
{
long rc;
paddr_t ipa;
-unsigned long maddr;
+gfn_t gfn;
unsigned long mfn;
xenmem_access_t xma;
p2m_type_t t;
@@ -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,11 +1565,10 @@ p2m_mem_access_check_and_get_page(vaddr_t gva, unsigned
long flag)
* We had a mem_access permission limiting the access, but the page type
* could also be limiting, so we need to check that as well.
*/
-maddr = __p2m_lookup(current->domain, ipa, &t);
-if ( maddr == INVALID_PADDR )
+mfn = mfn_x(__p2m_lookup(current->domain, gfn, &t));
+if ( mfn == INVALID_MFN )
goto err;
-mfn = maddr >> PAGE_SHIFT;
if ( !mfn_valid(mfn) )
goto err;
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 44926ca..02fe117 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2319,14 +2319,16 @@ void dump_guest_s1_walk(struct domain *d, vaddr_t addr)
{
register_t ttbcr = READ_SYSREG(TCR_EL1);
uint64_t ttbr0 = READ_SYSREG64(TTBR0_EL1);
-padd
[Xen-devel] [PATCH v3 8/8] xen/arm: p2m_cache_flush: Use the correct terminology and 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.
Note that the type of the parameters 'start' and 'end' 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 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| 10 +-
xen/include/asm-arm/p2m.h | 2 +-
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c
index 30453d8..b94e97c 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), _gfn(e));
}
case XEN_DOMCTL_bind_pt_irq:
{
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index f3330dd..9149981 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1469,16 +1469,16 @@ 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, gfn_t end)
{
struct p2m_domain *p2m = &d->arch.p2m;
-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(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..976e51e 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, gfn_t end);
/* 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 v3 3/8] 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
---
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 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 ab0cb41..aa4e774 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 v3 2/8] xen/mm: Introduce a bunch of helpers for the typesafes mfn and gfn
Those helpers will be useful to do common operations without having to
unbox/box manually the GFNs/MFNs.
Signed-off-by: Julien Grall
---
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 inline functions rather than macros
Changes in v2:
- Rename min_gfn/max_gfn to gfn_min/gfn_max
- Add more helpers for gfn and mfn
---
xen/include/xen/mm.h | 41 +
1 file changed, 41 insertions(+)
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index 3cf646a..13f706e 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -50,6 +50,7 @@
#include
#include
#include
+#include
#include
TYPE_SAFE(unsigned long, mfn);
@@ -61,6 +62,26 @@ TYPE_SAFE(unsigned long, mfn);
#undef mfn_t
#endif
+static inline mfn_t mfn_add(mfn_t mfn, unsigned long i)
+{
+return _mfn(mfn_x(mfn) + i);
+}
+
+static inline mfn_t mfn_max(mfn_t x, mfn_t y)
+{
+return _mfn(max(mfn_x(x), mfn_x(y)));
+}
+
+static inline mfn_t mfn_min(mfn_t x, mfn_t y)
+{
+return _mfn(min(mfn_x(x), mfn_x(y)));
+}
+
+static inline bool_t mfn_eq(mfn_t x, mfn_t y)
+{
+return mfn_x(x) == mfn_x(y);
+}
+
TYPE_SAFE(unsigned long, gfn);
#define PRI_gfn "05lx"
#define INVALID_GFN (~0UL)
@@ -70,6 +91,26 @@ TYPE_SAFE(unsigned long, gfn);
#undef gfn_t
#endif
+static inline gfn_t gfn_add(gfn_t gfn, unsigned long i)
+{
+return _gfn(gfn_x(gfn) + i);
+}
+
+static inline gfn_t gfn_max(gfn_t x, gfn_t y)
+{
+return _gfn(max(gfn_x(x), gfn_x(y)));
+}
+
+static inline gfn_t gfn_min(gfn_t x, gfn_t y)
+{
+return _gfn(min(gfn_x(x), gfn_x(y)));
+}
+
+static inline bool_t gfn_eq(gfn_t x, gfn_t y)
+{
+return gfn_x(x) == gfn_x(y);
+}
+
TYPE_SAFE(unsigned long, pfn);
#define PRI_pfn "05lx"
#define INVALID_PFN (~0UL)
--
1.9.1
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 1/8] xen/arm: Rename gmfn_to_mfn to gfn_to_mfn and use gfn/mfn typesafe
The correct acronym for a guest physical frame is gfn. Also use
the recently introduced typesafe gfn/mfn to avoid mixing the two
different kind of 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 v2:
- Add Jan's acked-by
- CCed the relevant maintainers
---
xen/arch/arm/p2m.c| 6 +++---
xen/common/grant_table.c | 4 ++--
xen/common/memory.c | 4 ++--
xen/include/asm-arm/p2m.h | 2 +-
4 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 65d8f1a..ab0cb41 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1481,10 +1481,10 @@ int p2m_cache_flush(struct domain *d, xen_pfn_t
start_mfn, xen_pfn_t end_mfn)
d->arch.p2m.default_access);
}
-unsigned long gmfn_to_mfn(struct domain *d, unsigned long gpfn)
+mfn_t gfn_to_mfn(struct domain *d, gfn_t gfn)
{
-paddr_t p = p2m_lookup(d, pfn_to_paddr(gpfn), NULL);
-return p >> PAGE_SHIFT;
+paddr_t p = p2m_lookup(d, pfn_to_paddr(gfn_x(gfn)), NULL);
+return _mfn(p >> PAGE_SHIFT);
}
/*
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 8b22299..3c304f4 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -256,7 +256,7 @@ static int __get_paged_frame(unsigned long gfn, unsigned
long *frame, struct pag
}
*frame = page_to_mfn(*page);
#else
-*frame = gmfn_to_mfn(rd, gfn);
+*frame = mfn_x(gfn_to_mfn(rd, _gfn(gfn)));
*page = mfn_valid(*frame) ? mfn_to_page(*frame) : NULL;
if ( (!(*page)) || (!get_page(*page, rd)) )
{
@@ -1788,7 +1788,7 @@ gnttab_transfer(
mfn = INVALID_MFN;
}
#else
-mfn = gmfn_to_mfn(d, gop.mfn);
+mfn = mfn_x(gfn_to_mfn(d, _gfn(gop.mfn)));
#endif
/* Check the passed page frame for basic validity. */
diff --git a/xen/common/memory.c b/xen/common/memory.c
index 46b1d9f..b54b076 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -264,7 +264,7 @@ int guest_remove_page(struct domain *d, unsigned long gmfn)
return 1;
}
#else
-mfn = gmfn_to_mfn(d, gmfn);
+mfn = mfn_x(gfn_to_mfn(d, _gfn(gmfn)));
#endif
if ( unlikely(!mfn_valid(mfn)) )
{
@@ -488,7 +488,7 @@ static long
memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg)
goto fail;
}
#else /* !CONFIG_X86 */
-mfn = gmfn_to_mfn(d, gmfn + k);
+mfn = mfn_x(gfn_to_mfn(d, _gfn(gmfn + k)));
#endif
if ( unlikely(!mfn_valid(mfn)) )
{
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index d240d1e..75c65a8 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -178,7 +178,7 @@ void guest_physmap_remove_page(struct domain *d,
unsigned long gpfn,
unsigned long mfn, unsigned int page_order);
-unsigned long gmfn_to_mfn(struct domain *d, unsigned long gpfn);
+mfn_t gfn_to_mfn(struct domain *d, gfn_t gfn);
/*
* Populate-on-demand
--
1.9.1
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 5/8] 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
---
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
Re: [Xen-devel] [PATCH 1/3] xen/arm: drivers: scif: Remove dead code
On Tue, Jun 21, 2016 at 4:01 PM, Dirk Behme wrote:
> Hello Oleksandr,
>
>
> On 21.06.2016 14:54, Oleksandr Tyshchenko wrote:
>>
>> On Tue, Jun 21, 2016 at 3:15 PM, Julien Grall
>> wrote:
>>>
>>>
>>>
>>> On 21/06/16 11:16, Oleksandr Tyshchenko wrote:
Hi, Dirk.
>>>
>>>
>>>
>>> Hello Oleksandr,
>>
>>
>> Hello Julien.
>>>
>>>
>>>
On Tue, Jun 21, 2016 at 12:15 PM, Dirk Behme
wrote:
>
>
> In scif_uart_init() uart->baud is set to BAUD_AUTO. So its a basic
> error
> if this is different later. Detect this by an ASSERT, but remove the
> dead code normally never reached.
>
> Signed-off-by: Dirk Behme
> ---
> xen/drivers/char/scif-uart.c | 23 ++-
> 1 file changed, 6 insertions(+), 17 deletions(-)
>
> diff --git a/xen/drivers/char/scif-uart.c
> b/xen/drivers/char/scif-uart.c
> index 51a2233..ca88c0f 100644
> --- a/xen/drivers/char/scif-uart.c
> +++ b/xen/drivers/char/scif-uart.c
> @@ -143,23 +143,12 @@ static void __init scif_uart_init_preirq(struct
> serial_port *port)
> scif_writew(uart, SCIF_SCSMR, val);
>
> ASSERT( uart->clock_hz > 0 );
> -if ( uart->baud != BAUD_AUTO )
> -{
> -/* Setup desired Baud rate */
> -divisor = uart->clock_hz / (uart->baud << 4);
> -ASSERT( divisor >= 1 && divisor <= (uint16_t)UINT_MAX );
> -scif_writew(uart, SCIF_DL, (uint16_t)divisor);
> -/* Selects the frequency divided clock (SC_CLK external input)
> */
> -scif_writew(uart, SCIF_CKS, 0);
> -udelay(100 / uart->baud + 1);
This part of code might be used for people who are not satisfied with
default baudrate which has been set in U-Boot.
>>>
>>>
>>>
>>> Can you elaborate? If the baud rate is different, it will not be possible
>>> to
>>> get output from U-boot.
>>>
If we remove this we will take away the opportunity to just change
uart->baud from BAUD_AUTO to desired one.
>>>
>>>
>>>
>>> I have some doubt that the current code is valid. The clock frequency is
>>> hardcoded (see SCIF_CLK_FREQ), so are you saying that the frequency is
>>> always the same across all the platforms?
>>
>>
>> No.
>>
>> This driver has been initially written for Renesas Lager board based
>> on R-Car H2 SoC which
>> has SCIF compatible UART. And the current code is valid for it. I have
>> tested both auto and
>> variable (38400, 115200) baudrates.
>
>
>
> Could you help me a little to understand this?
>
> The driver has
>
> scif_uart_init()
> {
> ...
> struct scif_uart *uart;
> ...
> uart->baud = BAUD_AUTO;
> ...
> }
>
> I checked the code for struct scif_uart but it isn't used anywhere outside
> this driver. So uart->baud is a driver local variable, which looks to me
> isn't used anywhere useful.
>
> What have I missed?
Unfortunately, I don't have recent Xen sources right now to be 100%
sure. But, it seems you are right: uart->baud is a driver local
variable.
>
> Best regards
>
> Dirk
>
>
>
>> But, I am afraid that current code won't be suitable for
>> all of the boards which have the same UART IP. It depends at least
>> from clock source
>> (external/internal) and clock frequency.
>>
>>>
>>> I would rather avoid to keep dead code (or not accessible without hacking
>>> Xen). For what is worth, we recently removed a similar code from the
>>> PL011
>>> driver as this should be correctly configured by the firmware.
>>>
> -}
> -else
> -{
> -/* Read current Baud rate */
> -divisor = scif_readw(uart, SCIF_DL);
> -ASSERT( divisor >= 1 && divisor <= (uint16_t)UINT_MAX );
> -uart->baud = uart->clock_hz / (divisor << 4);
> -}
> +ASSERT( uart->baud == BAUD_AUTO );
> +
> +/* Read current Baud rate */
> +divisor = scif_readw(uart, SCIF_DL);
> +ASSERT( divisor >= 1 && divisor <= (uint16_t)UINT_MAX );
> +uart->baud = uart->clock_hz / (divisor << 4);
>
> /* Setup trigger level for TX/RX FIFOs */
> scif_writew(uart, SCIF_SCFCR, SCFCR_RTRG11 | SCFCR_TTRG11);
> --
> 2.8.0
>
>>>
>>> Regards,
--
Oleksandr Tyshchenko | Embedded Dev
GlobalLogic
www.globallogic.com
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v9 1/3] vt-d: fix the IOMMU flush issue
>>> On 17.06.16 at 05:37, wrote:
> @@ -546,17 +550,37 @@ static int __must_check iommu_flush_all(void)
> struct acpi_drhd_unit *drhd;
> struct iommu *iommu;
> int flush_dev_iotlb;
> +int rc = 0;
>
> flush_all_cache();
> for_each_drhd_unit ( drhd )
> {
> +int context_rc, iotlb_rc;
> +
> iommu = drhd->iommu;
> -iommu_flush_context_global(iommu, 0);
> +context_rc = iommu_flush_context_global(iommu, 0);
> flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
> -iommu_flush_iotlb_global(iommu, 0, flush_dev_iotlb);
> +iotlb_rc = iommu_flush_iotlb_global(iommu, 0, flush_dev_iotlb);
> +
> +/*
> + * The current logic for returns:
> + * - positive invoke iommu_flush_write_buffer to flush cache.
> + * - zero on success.
> + * - negative on failure. Continue to flush IOMMU IOTLB on a
> + * best effort basis.
> + */
> +if ( context_rc > 0 || iotlb_rc > 0 )
> +iommu_flush_write_buffer(iommu);
> +if ( context_rc >= 0 )
Wasn't this meant to be just "rc"? (I can't, btw, imagine Kevin's ack
to be rightfully retained with a change like this.)
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [libvirt test] 96038: tolerable FAIL - PUSHED
flight 96038 libvirt real [real]
http://logs.test-lab.xenproject.org/osstest/logs/96038/
Failures :-/ but no regressions.
Tests which did not succeed, but are not blocking:
test-armhf-armhf-libvirt 14 guest-saverestorefail never pass
test-armhf-armhf-libvirt 12 migrate-support-checkfail never pass
test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail never pass
test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail never pass
test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass
test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass
test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail never pass
test-amd64-i386-libvirt-xsm 12 migrate-support-checkfail never pass
test-amd64-i386-libvirt 12 migrate-support-checkfail never pass
test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check
fail never pass
test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check
fail never pass
test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail never pass
test-armhf-armhf-libvirt-raw 13 guest-saverestorefail never pass
test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail never pass
version targeted for testing:
libvirt a1e1679c8aaa8256332954c20ec24dd938ef8a58
baseline version:
libvirt eac167e2610d3e59b32f7ec7ba78cbc8c420a425
Last test of basis95913 2016-06-19 04:21:33 Z2 days
Testing same since96038 2016-06-21 04:21:00 Z0 days1 attempts
People who touched revisions under test:
Andrea Bolognani
Chen Hanxiao
Jaroslav Suchanek
Ján Tomko
Peter Krempa
Tomasz Flendrich
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
Pushing revision :
+ branch=libvirt
+ revision=a1e1679c8aaa8256332954c20ec24dd938ef8a58
+ . ./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 libvirt
a1e1679c8aaa8
[Xen-devel] [xen-unstable-smoke test] 96055: tolerable all pass - PUSHED
flight 96055 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/96055/
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 57a57465daaf8fb66d192ff98b8477524091e82c
baseline version:
xen 0630892fafe87ff5e3b65422d38158de46db3ed0
Last test of basis96011 2016-06-20 14:02:02 Z0 days
Testing same since96055 2016-06-21 11:02:43 Z0 days1 attempts
People who touched revisions under test:
Dario Faggioli
George Dunlap
Jan Beulich
Juergen Gross
Julien Grall
Kevin Tian
Razvan Cojocaru
Tamas K Lengyel
Wei Liu
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=57a57465daaf8fb66d192ff98b8477524091e82c
+ . ./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
57a57465daaf8fb66d192ff98b8477524091e82c
+ branch=xen-unstable-smoke
+ revision=57a57465daaf8fb66d192ff98b8477524091e82c
+ . ./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
+ '[' x57a57465daaf8fb66d192ff98b8477524091e82c = 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 di
Re: [Xen-devel] [PATCH 0/8] Add support for parsing per CPU Redistributor entry
On 06/21/2016 04:28 AM, Julien Grall wrote: Hello Shanker, On 19/06/16 00:45, Shanker Donthineni wrote: The current driver doesn't support parsing Redistributor entries that are described in the MADT GICC table. Not all the GIC implementors places the Redistributor regions in the always-on power domain. On systems, the UEFI firmware should describe Redistributor base address in the associated GIC CPU Interface (GICC) instead of GIC Redistributor (GICR) table. The maximum number of mmio handlers and struct vgic_rdist_region that holds Redistributor addresses are allocated through a static array with hardcoded size. I don't think this is the right approach and is not scalable for implementing features like this. I have decided to convert static to dynamic allocation based on comments from the below link. https://patchwork.kernel.org/patch/9163435/ You addressed only one part of my comment. This series increases the number of I/O handlers but the lookup is still linear (see handle_mmio in arch/arm/io.c). I agree with you, we need to bring binary search algorithm similar to Linux KVM code. I want to keep it this change outside of this patchset. After this series, the maximum number of I/O handlers is 160. So in the worst case, we have to do 160 iterations before finding an handler or concluding the I/O cannot be emulated. 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] [PATCH 0/8] Add support for parsing per CPU Redistributor entry
On 21/06/16 14:37, Shanker Donthineni wrote: On 06/21/2016 04:28 AM, Julien Grall wrote: On 19/06/16 00:45, Shanker Donthineni wrote: The current driver doesn't support parsing Redistributor entries that are described in the MADT GICC table. Not all the GIC implementors places the Redistributor regions in the always-on power domain. On systems, the UEFI firmware should describe Redistributor base address in the associated GIC CPU Interface (GICC) instead of GIC Redistributor (GICR) table. The maximum number of mmio handlers and struct vgic_rdist_region that holds Redistributor addresses are allocated through a static array with hardcoded size. I don't think this is the right approach and is not scalable for implementing features like this. I have decided to convert static to dynamic allocation based on comments from the below link. https://patchwork.kernel.org/patch/9163435/ You addressed only one part of my comment. This series increases the number of I/O handlers but the lookup is still linear (see handle_mmio in arch/arm/io.c). I agree with you, we need to bring binary search algorithm similar to Linux KVM code. I want to keep it this change outside of this patchset. This should be a prerequisite of this series then, not a follow-up. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 3/8] arm/gic-v3: Parse per-cpu redistributor entry in GICC subtable
On 06/21/2016 05:16 AM, Julien Grall wrote:
Hello Shanker,
On 19/06/16 00:45, Shanker Donthineni wrote:
The redistributor address can be specified either as part of GICC or
GICR subtable depending on the power domain. The current driver
doesn't support parsing redistributor entry that is defined in GICC
subtable. The GIC CPU subtable entry holds the associated Redistributor
base address if it is not on always-on power domain.
This patch adds necessary code to handle both types of Redistributors
base addresses.
Signed-off-by: Shanker Donthineni
---
xen/arch/arm/gic-v3.c | 97
---
xen/include/asm-arm/gic.h | 2 +
xen/include/asm-arm/gic_v3_defs.h | 1 +
3 files changed, 83 insertions(+), 17 deletions(-)
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index af12ebc..42cf848 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -659,6 +659,10 @@ static int __init gicv3_populate_rdist(void)
smp_processor_id(), i, ptr);
return 0;
}
+
+if ( gicv3.rdist_regions[i].single_rdist )
+break;
+
if ( gicv3.rdist_stride )
ptr += gicv3.rdist_stride;
else
@@ -1282,6 +1286,11 @@ static int gicv3_iomem_deny_access(const struct
domain *d)
}
#ifdef CONFIG_ACPI
+static bool gic_dist_supports_dvis(void)
static inline and please use bool_t here.
Still learning XEN coding style, I'll fix it.
+{
+return !!(readl_relaxed(GICD + GICD_TYPER) & GICD_TYPER_DVIS);
+}
+
static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset)
{
struct acpi_subtable_header *header;
@@ -1393,18 +1402,39 @@ gic_acpi_parse_madt_redistributor(struct
acpi_subtable_header *header,
const unsigned long end)
{
struct acpi_madt_generic_redistributor *rdist;
+struct acpi_madt_generic_interrupt *processor;
struct rdist_region *region;
region = gicv3.rdist_regions + gicv3.rdist_count;
-rdist = (struct acpi_madt_generic_redistributor *)header;
-if ( BAD_MADT_ENTRY(rdist, end) )
-return -EINVAL;
+if ( header->type == ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR )
+{
+rdist = (struct acpi_madt_generic_redistributor *)header;
+if ( BAD_MADT_ENTRY(rdist, end) )
+return -EINVAL;
-if ( !rdist->base_address || !rdist->length )
-return -EINVAL;
+if ( !rdist->base_address || !rdist->length )
+return -EINVAL;
+
+region->base = rdist->base_address;
+region->size = rdist->length;
+}
+else if ( header->type == ACPI_MADT_TYPE_GENERIC_INTERRUPT )
+{
Parsing the GICC and the redistributor is quite different. I would
much prefer a function for parsing each table and an helper to add a
new redistributor.
I'll do.
+processor = (struct acpi_madt_generic_interrupt *)header;
+if ( BAD_MADT_ENTRY(processor, end) )
+return -EINVAL;
+
+if ( !(processor->flags & ACPI_MADT_ENABLED) )
+return 0;
+
+if ( !processor->gicr_base_address )
+return -EINVAL;
+
+region->base = processor->gicr_base_address;
+region->size = gic_dist_supports_dvis() ? SZ_256K : SZ_128K;
Please explain in the commit message how you find the size. I would
also prefer if you use (4 x SZ_64K) and (2 * SZ_64K) as we do in
populate_rdist.
+region->single_rdist = true;
The indentation looks wrong.
+ }
-region->base = rdist->base_address;
-region->size = rdist->length;
region->map_base = ioremap_nocache(region->base, region->size);
if ( !region->map_base )
@@ -1412,6 +1442,7 @@ gic_acpi_parse_madt_redistributor(struct
acpi_subtable_header *header,
printk("Unable to map GICR registers\n");
return -ENOMEM;
}
+
Spurious change.
gicv3.rdist_count++;
return 0;
@@ -1421,9 +1452,22 @@ static int __init
gic_acpi_get_madt_redistributor_num(struct acpi_subtable_header
*header,
const unsigned long end)
{
-/* Nothing to do here since it only wants to get the number of GIC
- * redistributors.
- */
+struct acpi_madt_generic_redistributor *rdist;
+struct acpi_madt_generic_interrupt *cpuif;
+
+if ( header->type == ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR )
+{
+ rdist = (struct acpi_madt_generic_redistributor *)header;
+ if ( BAD_MADT_ENTRY(rdist, end) || !rdist->base_address )
+ return -EINVAL;
+}
+else if ( header->type == ACPI_MADT_TYPE_GENERIC_INTERRUPT )
+{
+ cpuif = (struct acpi_madt_generic_interrupt *)header;
+ if ( BAD_MADT_ENTRY(cpuif, end) || !cpuif->gicr_base_address )
+ return -EINVAL;
+}
+
Ditto for the parsing.
return 0;
}
[...]
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 44b9ef6..7f9ad86 100644
--- a/xen/incl
Re: [Xen-devel] [PATCH 1/8] x86/time: improve cross-CPU clock monotonicity (and more)
On 06/21/2016 01:28 PM, Jan Beulich wrote:
On 21.06.16 at 14:05, wrote:
>
>>
>> On 06/17/2016 08:32 AM, Jan Beulich wrote:
>> On 16.06.16 at 22:27, wrote:
> I.e. my plan was, once the backwards moves are small enough, to maybe
> indeed compensate them by maintaining a global variable tracking
> the most recently returned value. There are issues with such an
> approach too, though: HT effects can result in one hyperthread
> making it just past that check of the global, then hardware
> switching to the other hyperthread, NOW() producing a slightly
> larger value there, and hardware switching back to the first
> hyperthread only after the second one consumed the result of
> NOW(). Dario's use would be unaffected by this aiui, as his NOW()
> invocations are globally serialized through a spinlock, but arbitrary
> NOW() invocations on two hyperthreads can't be made such that
> the invoking party can be guaranteed to see strictly montonic
> values.
>
> And btw., similar considerations apply for two fully independent
> CPUs, if one runs at a much higher P-state than the other (i.e.
> the faster one could overtake the slower one between the
> montonicity check in NOW() and the callers consuming the returned
> values). So in the end I'm not sure it's worth the performance hit
> such a global montonicity check would incur, and therefore I didn't
> make a respective patch part of this series.
>
Hm, guests pvclock should have faced similar issues too as their
local stamps for each vcpu diverge. Linux commit 489fb49 ("x86, paravirt:
Add a
global synchronization point for pvclock") depicts a fix to similar
situations to the
scenarios you just described - which lead to have a global variable to
keep
track of
most recent timestamp. One important chunk of that commit is pasted below
for
convenience:
--
/*
* Assumption here is that last_value, a global accumulator, always goes
* forward. If we are less than that, we should not be much smaller.
* We assume there is an error marging we're inside, and then the
correction
* does not sacrifice accuracy.
*
* For reads: global may have changed between test and return,
* but this means someone else updated poked the clock at a later time.
* We just need to make sure we are not seeing a backwards event.
*
* For updates: last_value = ret is not enough, since two vcpus could be
* updating at the same time, and one of them could be slightly behind,
* making the assumption that last_value always go forward fail to hold.
*/
last = atomic64_read(&last_value);
do {
if (ret < last)
return last;
last = atomic64_cmpxchg(&last_value, last, ret);
} while (unlikely(last != ret));
--
>>>
>>> Meaning they decided it's worth the overhead. But (having read
>>> through the entire description) they don't even discuss whether this
>>> indeed eliminates _all_ apparent backward moves due to effects
>>> like the ones named above.
>>>
>>> Plus, the contention they're facing is limited to a single VM, i.e. likely
>>> much more narrow than that on an entire physical system. So for
>>> us to do the same in the hypervisor, quite a bit more of win would
>>> be needed to outweigh the cost.
>>>
>> Experimental details look very unclear too - likely running the time
>> warp test for 5 days would get some of these cases cleared out. But
>> as you say it should be much more narrow that of an entire system.
>>
>> BTW It was implicit in the discussion but my apologies for not
>> formally/explicitly stating. So FWIW:
>>
>> Tested-by: Joao Martins
>
> Thanks, but this ...
>
>> This series is certainly a way forward into improving cross-CPU monotonicity,
>> and I am seeing indeed less occurrences of time going backwards on my
>> systems.
>
> ... leaves me guessing whether the above was meant for just this
> patch, or the entire series.
>
Ah sorry, a little ambiguous on my end - It is meant for the entire series.
Joao
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 2/8] arm/gic-v3: Fold GICR subtable parsing into a new function
On 06/21/2016 05:17 AM, Julien Grall wrote:
Hello Shanker,
On 19/06/16 00:45, Shanker Donthineni wrote:
Add a new function for parsing GICR subtable and move the code
that is specific to GICR table to new function without changing
the function gicv3_acpi_init() behavior.
Signed-off-by: Shanker Donthineni
---
xen/arch/arm/gic-v3.c | 64
+--
1 file changed, 42 insertions(+), 22 deletions(-)
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index ab1f380..af12ebc 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1387,6 +1387,36 @@ 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;
+struct rdist_region *region;
+
+region = gicv3.rdist_regions + gicv3.rdist_count;
+rdist = (struct acpi_madt_generic_redistributor *)header;
+if ( BAD_MADT_ENTRY(rdist, end) )
+return -EINVAL;
+
+if ( !rdist->base_address || !rdist->length )
+return -EINVAL;
+
+region->base = rdist->base_address;
+region->size = rdist->length;
+
+region->map_base = ioremap_nocache(region->base, region->size);
In the commit message you said there is no functional change, however
the remapping is not part of gicv3_acpi_init. So why did you add this
line here?
Thanks for catching coding bug, it was my mistake and this code should
not be here.
+if ( !region->map_base )
+{
+printk("Unable to map GICR registers\n");
+return -ENOMEM;
+}
+gicv3.rdist_count++;
+
+return 0;
+}
+
[...]
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] [PATCH 0/8] Add support for parsing per CPU Redistributor entry
On 06/21/2016 08:50 AM, Julien Grall wrote: On 21/06/16 14:37, Shanker Donthineni wrote: On 06/21/2016 04:28 AM, Julien Grall wrote: On 19/06/16 00:45, Shanker Donthineni wrote: The current driver doesn't support parsing Redistributor entries that are described in the MADT GICC table. Not all the GIC implementors places the Redistributor regions in the always-on power domain. On systems, the UEFI firmware should describe Redistributor base address in the associated GIC CPU Interface (GICC) instead of GIC Redistributor (GICR) table. The maximum number of mmio handlers and struct vgic_rdist_region that holds Redistributor addresses are allocated through a static array with hardcoded size. I don't think this is the right approach and is not scalable for implementing features like this. I have decided to convert static to dynamic allocation based on comments from the below link. https://patchwork.kernel.org/patch/9163435/ You addressed only one part of my comment. This series increases the number of I/O handlers but the lookup is still linear (see handle_mmio in arch/arm/io.c). I agree with you, we need to bring binary search algorithm similar to Linux KVM code. I want to keep it this change outside of this patchset. This should be a prerequisite of this series then, not a follow-up. For the functionality and correctness purpose we don't need this change immediately. We are not able to boot XEN on Qualcomm Technologies because of not supporting GICC table parsing for GICR address. I am okay to wait for my patchset if someone adding bseach look ups for mmio handlers. 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
[Xen-devel] [PATCH] xen/PMU: Log VPMU initialization error at lower level
This will match how PMU errors are reported at check_hw_exists()'s
msr_fail label, which is reached when VPMU initialzation fails.
Signed-off-by: Boris Ostrovsky
---
arch/x86/xen/pmu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/xen/pmu.c b/arch/x86/xen/pmu.c
index 9466354..32bdc2c 100644
--- a/arch/x86/xen/pmu.c
+++ b/arch/x86/xen/pmu.c
@@ -547,7 +547,7 @@ void xen_pmu_init(int cpu)
return;
fail:
- pr_warn_once("Could not initialize VPMU for cpu %d, error %d\n",
+ pr_info_once("Could not initialize VPMU for cpu %d, error %d\n",
cpu, err);
free_pages((unsigned long)xenpmu_data, 0);
}
--
1.8.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen/PMU: Log VPMU initialization error at lower level
On Tue, Jun 21, 2016 at 10:17:33AM -0400, Boris Ostrovsky wrote:
> This will match how PMU errors are reported at check_hw_exists()'s
> msr_fail label, which is reached when VPMU initialzation fails.
>
> Signed-off-by: Boris Ostrovsky
Reviewed-by: Konrad Rzeszutek Wilk
> ---
> arch/x86/xen/pmu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/xen/pmu.c b/arch/x86/xen/pmu.c
> index 9466354..32bdc2c 100644
> --- a/arch/x86/xen/pmu.c
> +++ b/arch/x86/xen/pmu.c
> @@ -547,7 +547,7 @@ void xen_pmu_init(int cpu)
> return;
>
> fail:
> - pr_warn_once("Could not initialize VPMU for cpu %d, error %d\n",
> + pr_info_once("Could not initialize VPMU for cpu %d, error %d\n",
> cpu, err);
> free_pages((unsigned long)xenpmu_data, 0);
> }
> --
> 1.8.1.4
>
>
> ___
> 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 3/3] libxl: refactor domcreate_attach_dtdev() to use device type framework
Signed-off-by: Juergen Gross
---
tools/libxl/libxl_create.c | 72 ++
1 file changed, 35 insertions(+), 37 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index c4e85f0..e93b880 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -742,9 +742,6 @@ static void domcreate_bootloader_done(libxl__egc *egc,
static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *aodevs,
int ret);
-static void domcreate_attach_dtdev(libxl__egc *egc, libxl__multidev *multidev,
- int ret);
-
static void domcreate_console_available(libxl__egc *egc,
libxl__domain_create_state *dcs);
@@ -1399,12 +1396,43 @@ static void domcreate_launch_dm(libxl__egc *egc,
libxl__multidev *multidev,
domcreate_complete(egc, dcs, ret);
}
+static void libxl__add_dtdevs(libxl__egc *egc, libxl__ao *ao, uint32_t domid,
+ libxl_domain_config *d_config,
+ libxl__multidev *multidev)
+{
+AO_GC;
+libxl__ao_device *aodev = libxl__multidev_prepare(multidev);
+int i, rc = 0;
+
+for (i = 0; i < d_config->num_dtdevs; i++) {
+const libxl_device_dtdev *dtdev = &d_config->dtdevs[i];
+
+LOG(DEBUG, "Assign device \"%s\" to dom%u", dtdev->path, domid);
+rc = xc_assign_dt_device(CTX->xch, domid, dtdev->path);
+if (rc < 0) {
+LOG(ERROR, "xc_assign_dtdevice failed: %d", rc);
+goto out;
+}
+}
+
+out:
+aodev->rc = rc;
+aodev->callback(egc, aodev);
+}
+
+static struct libxl_device_type libxl__dtdev_devtype = {
+.type = "dtdev",
+.num_offset = offsetof(libxl_domain_config, num_dtdevs),
+.add= libxl__add_dtdevs,
+};
+
static struct libxl_device_type *device_type_tbl[] = {
&libxl__nic_devtype,
&libxl__vtpm_devtype,
&libxl__usbctrl_devtype,
&libxl__usbdev_devtype,
&libxl__pci_devtype,
+&libxl__dtdev_devtype,
};
static void domcreate_attach_devices(libxl__egc *egc,
@@ -1439,7 +1467,10 @@ static void domcreate_attach_devices(libxl__egc *egc,
return;
}
-domcreate_attach_dtdev(egc, multidev, 0);
+domcreate_console_available(egc, dcs);
+
+domcreate_complete(egc, dcs, 0);
+
return;
error_out:
@@ -1479,39 +1510,6 @@ error_out:
domcreate_complete(egc, dcs, ret);
}
-static void domcreate_attach_dtdev(libxl__egc *egc,
- libxl__multidev *multidev,
- int ret)
-{
-libxl__domain_create_state *dcs = CONTAINER_OF(multidev, *dcs, multidev);
-STATE_AO_GC(dcs->ao);
-int i;
-int domid = dcs->guest_domid;
-
-/* convenience aliases */
-libxl_domain_config *const d_config = dcs->guest_config;
-
-for (i = 0; i < d_config->num_dtdevs; i++) {
-const libxl_device_dtdev *dtdev = &d_config->dtdevs[i];
-
-LOG(DEBUG, "Assign device \"%s\" to dom%u", dtdev->path, domid);
-ret = xc_assign_dt_device(CTX->xch, domid, dtdev->path);
-if (ret < 0) {
-LOG(ERROR, "xc_assign_dtdevice failed: %d", ret);
-goto error_out;
-}
-}
-
-domcreate_console_available(egc, dcs);
-
-domcreate_complete(egc, dcs, 0);
-return;
-
-error_out:
-assert(ret);
-domcreate_complete(egc, dcs, ret);
-}
-
static void domcreate_complete(libxl__egc *egc,
libxl__domain_create_state *dcs,
int rc)
--
2.6.6
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH 2/3] libxl: refactor domcreate_attach_pci() to use device type framework
Signed-off-by: Juergen Gross
---
tools/libxl/libxl_create.c | 54 ++--
tools/libxl/libxl_internal.h | 1 +
tools/libxl/libxl_pci.c | 36 +
3 files changed, 44 insertions(+), 47 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index bb0f535..c4e85f0 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -742,10 +742,8 @@ static void domcreate_bootloader_done(libxl__egc *egc,
static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *aodevs,
int ret);
-static void domcreate_attach_pci(libxl__egc *egc, libxl__multidev *aodevs,
- int ret);
-static void domcreate_attach_dtdev(libxl__egc *egc,
- libxl__domain_create_state *dcs);
+static void domcreate_attach_dtdev(libxl__egc *egc, libxl__multidev *multidev,
+ int ret);
static void domcreate_console_available(libxl__egc *egc,
libxl__domain_create_state *dcs);
@@ -1406,6 +1404,7 @@ static struct libxl_device_type *device_type_tbl[] = {
&libxl__vtpm_devtype,
&libxl__usbctrl_devtype,
&libxl__usbdev_devtype,
+&libxl__pci_devtype,
};
static void domcreate_attach_devices(libxl__egc *egc,
@@ -1440,7 +1439,7 @@ static void domcreate_attach_devices(libxl__egc *egc,
return;
}
-domcreate_attach_pci(egc, multidev, 0);
+domcreate_attach_dtdev(egc, multidev, 0);
return;
error_out:
@@ -1480,52 +1479,13 @@ error_out:
domcreate_complete(egc, dcs, ret);
}
-static void domcreate_attach_pci(libxl__egc *egc, libxl__multidev *multidev,
- int ret)
-{
-libxl__domain_create_state *dcs = CONTAINER_OF(multidev, *dcs, multidev);
-STATE_AO_GC(dcs->ao);
-int i;
-int domid = dcs->guest_domid;
-
-/* convenience aliases */
-libxl_domain_config *const d_config = dcs->guest_config;
-
-if (ret) {
-goto error_out;
-}
-
-for (i = 0; i < d_config->num_pcidevs; i++) {
-ret = libxl__device_pci_add(gc, domid, &d_config->pcidevs[i], 1);
-if (ret < 0) {
-LOG(ERROR, "libxl_device_pci_add failed: %d", ret);
-goto error_out;
-}
-}
-
-if (d_config->num_pcidevs > 0) {
-ret = libxl__create_pci_backend(gc, domid, d_config->pcidevs,
-d_config->num_pcidevs);
-if (ret < 0) {
-LOG(ERROR, "libxl_create_pci_backend failed: %d", ret);
-goto error_out;
-}
-}
-
-domcreate_attach_dtdev(egc, dcs);
-return;
-
-error_out:
-assert(ret);
-domcreate_complete(egc, dcs, ret);
-}
-
static void domcreate_attach_dtdev(libxl__egc *egc,
- libxl__domain_create_state *dcs)
+ libxl__multidev *multidev,
+ int ret)
{
+libxl__domain_create_state *dcs = CONTAINER_OF(multidev, *dcs, multidev);
STATE_AO_GC(dcs->ao);
int i;
-int ret;
int domid = dcs->guest_domid;
/* convenience aliases */
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 2603b33..547a78b 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3401,6 +3401,7 @@ extern struct libxl_device_type libxl__nic_devtype;
extern struct libxl_device_type libxl__vtpm_devtype;
extern struct libxl_device_type libxl__usbctrl_devtype;
extern struct libxl_device_type libxl__usbdev_devtype;
+extern struct libxl_device_type libxl__pci_devtype;
/*- Domain destruction -*/
/* Domain destruction has been split into two functions:
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 236bdd0..fd245ea 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -1291,6 +1291,36 @@ out:
return rc;
}
+static void libxl__add_pcis(libxl__egc *egc, libxl__ao *ao, uint32_t domid,
+libxl_domain_config *d_config,
+libxl__multidev *multidev)
+{
+AO_GC;
+libxl__ao_device *aodev = libxl__multidev_prepare(multidev);
+int i, rc = 0;
+
+for (i = 0; i < d_config->num_pcidevs; i++) {
+rc = libxl__device_pci_add(gc, domid, &d_config->pcidevs[i], 1);
+if (rc < 0) {
+LOG(ERROR, "libxl_device_pci_add failed: %d", rc);
+goto out;
+}
+}
+
+if (d_config->num_pcidevs > 0) {
+rc = libxl__create_pci_backend(gc, domid, d_config->pcidevs,
+d_config->num_pcidevs);
+if (rc < 0) {
+LOG(ERROR, "libxl_create_pci_backend failed: %d", rc);
+goto out;
+}
+}
+
+out:
+aodev->rc = rc;
+aodev->callback(egc, aodev);
+}
+
static int qemu_pci_remove_xenstore(libxl__gc *gc, uint32_t domid,
libxl_devi
[Xen-devel] [PATCH 1/3] libxl: add framework for device types
Instead of duplicate coding for each device type (vtpms, usbctrls, ...)
especially on domain creation introduce a framework for that purpose.
Signed-off-by: Juergen Gross
---
tools/libxl/libxl.c | 12
tools/libxl/libxl_create.c | 163 +--
tools/libxl/libxl_internal.h | 13
tools/libxl/libxl_pvusb.c| 13
4 files changed, 87 insertions(+), 114 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 1c81239..df94f2e 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -7434,6 +7434,18 @@ out:
return rc;
}
+struct libxl_device_type libxl__nic_devtype = {
+.type = "nic",
+.num_offset = offsetof(libxl_domain_config, num_nics),
+.add= libxl__add_nics,
+};
+
+struct libxl_device_type libxl__vtpm_devtype = {
+.type = "vtpm",
+.num_offset = offsetof(libxl_domain_config, num_vtpms),
+.add= libxl__add_vtpms,
+};
+
/*
* Local variables:
* mode: C
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 1b99472..bb0f535 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -742,12 +742,6 @@ static void domcreate_bootloader_done(libxl__egc *egc,
static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *aodevs,
int ret);
-static void domcreate_attach_vtpms(libxl__egc *egc, libxl__multidev *multidev,
- int ret);
-static void domcreate_attach_usbctrls(libxl__egc *egc,
- libxl__multidev *multidev, int ret);
-static void domcreate_attach_usbdevs(libxl__egc *egc, libxl__multidev
*multidev,
- int ret);
static void domcreate_attach_pci(libxl__egc *egc, libxl__multidev *aodevs,
int ret);
static void domcreate_attach_dtdev(libxl__egc *egc,
@@ -1407,6 +1401,53 @@ static void domcreate_launch_dm(libxl__egc *egc,
libxl__multidev *multidev,
domcreate_complete(egc, dcs, ret);
}
+static struct libxl_device_type *device_type_tbl[] = {
+&libxl__nic_devtype,
+&libxl__vtpm_devtype,
+&libxl__usbctrl_devtype,
+&libxl__usbdev_devtype,
+};
+
+static void domcreate_attach_devices(libxl__egc *egc,
+ libxl__multidev *multidev,
+ int ret)
+{
+libxl__domain_create_state *dcs = CONTAINER_OF(multidev, *dcs, multidev);
+STATE_AO_GC(dcs->ao);
+int domid = dcs->guest_domid;
+libxl_domain_config *const d_config = dcs->guest_config;
+struct libxl_device_type *dt;
+
+if (ret) {
+LOG(ERROR, "unable to add %s devices",
+device_type_tbl[dcs->device_type_idx]->type);
+goto error_out;
+}
+
+dcs->device_type_idx++;
+if (dcs->device_type_idx < ARRAY_SIZE(device_type_tbl)) {
+dt = device_type_tbl[dcs->device_type_idx];
+if (*(int *)((void *)d_config + dt->num_offset) > 0) {
+/* Attach devices */
+libxl__multidev_begin(ao, &dcs->multidev);
+dcs->multidev.callback = domcreate_attach_devices;
+dt->add(egc, ao, domid, d_config, &dcs->multidev);
+libxl__multidev_prepared(egc, &dcs->multidev, 0);
+return;
+}
+
+domcreate_attach_devices(egc, &dcs->multidev, 0);
+return;
+}
+
+domcreate_attach_pci(egc, multidev, 0);
+return;
+
+error_out:
+assert(ret);
+domcreate_complete(egc, dcs, ret);
+}
+
static void domcreate_devmodel_started(libxl__egc *egc,
libxl__dm_spawn_state *dmss,
int ret)
@@ -1430,113 +1471,8 @@ static void domcreate_devmodel_started(libxl__egc *egc,
}
}
-/* Plug nic interfaces */
-if (d_config->num_nics > 0) {
-/* Attach nics */
-libxl__multidev_begin(ao, &dcs->multidev);
-dcs->multidev.callback = domcreate_attach_vtpms;
-libxl__add_nics(egc, ao, domid, d_config, &dcs->multidev);
-libxl__multidev_prepared(egc, &dcs->multidev, 0);
-return;
-}
-
-domcreate_attach_vtpms(egc, &dcs->multidev, 0);
-return;
-
-error_out:
-assert(ret);
-domcreate_complete(egc, dcs, ret);
-}
-
-static void domcreate_attach_vtpms(libxl__egc *egc,
- libxl__multidev *multidev,
- int ret)
-{
- libxl__domain_create_state *dcs = CONTAINER_OF(multidev, *dcs, multidev);
- STATE_AO_GC(dcs->ao);
- int domid = dcs->guest_domid;
-
- libxl_domain_config* const d_config = dcs->guest_config;
-
- if(ret) {
- LOG(ERROR, "unable to add nic devices");
- goto error_out;
- }
-
-/* Plug vtpm devices */
- if (d_config->num_vtpms > 0) {
- /* Attach vtpms */
- libxl__multidev_begin(ao, &dcs->multidev);
- dcs->multidev.callback = domcreate_attach_usb
[Xen-devel] [PATCH 0/3] libxl: add framework for device types
Instead of duplicate coding for each device type (vtpms, usbctrls, ...) especially on domain creation introduce a framework for that purpose. I especially found it annoying that e.g. the vtpm callback issued the error message for a failed attach of nic devices. Juergen Gross (3): libxl: add framework for device types libxl: refactor domcreate_attach_pci() to use device type framework libxl: refactor domcreate_attach_dtdev() to use device type framework tools/libxl/libxl.c | 12 ++ tools/libxl/libxl_create.c | 275 +-- tools/libxl/libxl_internal.h | 14 +++ tools/libxl/libxl_pci.c | 36 ++ tools/libxl/libxl_pvusb.c| 13 ++ 5 files changed, 159 insertions(+), 191 deletions(-) -- 2.6.6 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen/PMU: Log VPMU initialization error at lower level
On 21/06/16 16:17, Boris Ostrovsky wrote:
> This will match how PMU errors are reported at check_hw_exists()'s
> msr_fail label, which is reached when VPMU initialzation fails.
>
> Signed-off-by: Boris Ostrovsky
Acked-by: Juergen Gross
> ---
> arch/x86/xen/pmu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/xen/pmu.c b/arch/x86/xen/pmu.c
> index 9466354..32bdc2c 100644
> --- a/arch/x86/xen/pmu.c
> +++ b/arch/x86/xen/pmu.c
> @@ -547,7 +547,7 @@ void xen_pmu_init(int cpu)
> return;
>
> fail:
> - pr_warn_once("Could not initialize VPMU for cpu %d, error %d\n",
> + pr_info_once("Could not initialize VPMU for cpu %d, error %d\n",
> cpu, err);
> free_pages((unsigned long)xenpmu_data, 0);
> }
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 6/8] arm: vgic: Split vgic_domain_init() functionality into two functions
On 06/21/2016 05:49 AM, Julien Grall wrote:
Hello Shanker,
On 19/06/16 00:45, Shanker Donthineni wrote:
Split code that installs mmio handlers for GICD and Re-distributor
regions to a new function. The intension of this separation is to defer
steps that registers vgic_v2/v3 mmio handlers.
Looking at this patch and the follow-up ones, I don't think this is
the right way to go. You differ the registration of the IO handlers
just because you are unable to find the size of the handlers array.
Is there any better approach?
I am wondering if the array for the handlers is the best solution
here. On another side, it would be possible to find the maximum of
handlers before hand.
The purpose of this change is to limit size of 'struct domain' less than
PAGE_SIZE. I can think of second approach split vgic_init() into two
stages, one for vgic registration and the second one for vgic_init().
This also requires a few lines of code changes to vgic_v2/v3_init() and
vgic_init().
int arch_domain_create(struct domain *d, unsigned int domcr_flags,
struct xen_arch_domainconfig *config)
...
domain_vgic_register(d));
domain_io_init(d, mmio_count);
domain_vgic_init(d, config->nr_spis));
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 5df5f01..5b39e0d 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -151,9 +151,12 @@ int domain_vgic_init(struct domain *d, unsigned int
nr_spis)
for ( i = 0; i < NR_GIC_SGI; i++ )
set_bit(i, d->arch.vgic.allocated_irqs);
+d->arch.vgic.handler->domain_register_mmio(d);
+
return 0;
}
+
Spurious change.
void register_vgic_ops(struct domain *d, const struct vgic_ops *ops)
{
d->arch.vgic.handler = ops;
diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
index fbb763a..8fe65b4 100644
--- a/xen/include/asm-arm/vgic.h
+++ b/xen/include/asm-arm/vgic.h
@@ -132,6 +132,8 @@ struct vgic_ops {
void (*domain_free)(struct domain *d);
/* vGIC sysreg emulation */
int (*emulate_sysreg)(struct cpu_user_regs *regs, union hsr hsr);
+/* Register mmio handlers */
+void (*domain_register_mmio)(struct domain *d);
/* Maximum number of vCPU supported */
const unsigned int max_vcpus;
};
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
[Xen-devel] [PATCH] xen/pciback: Fix conf_space read/write overlap check.
Current overlap check is evaluating to false a case where a filter field
is fully contained (proper subset) of a r/w request.
This change applies classical overlap check instead to include
all the scenarios.
Related to https://www.mail-archive.com/xen-devel@lists.xen.org/msg72174.html
Cc: Jan Beulich
Cc: Boris Ostrovsky
Cc: sta...@vger.kernel.org
Signed-off-by: Andrey Grodzovsky
---
drivers/xen/xen-pciback/conf_space.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/xen/xen-pciback/conf_space.c
b/drivers/xen/xen-pciback/conf_space.c
index 8e67336..6a25533 100644
--- a/drivers/xen/xen-pciback/conf_space.c
+++ b/drivers/xen/xen-pciback/conf_space.c
@@ -183,8 +183,7 @@ int xen_pcibk_config_read(struct pci_dev *dev, int offset,
int size,
field_start = OFFSET(cfg_entry);
field_end = OFFSET(cfg_entry) + field->size;
- if ((req_start >= field_start && req_start < field_end)
- || (req_end > field_start && req_end <= field_end)) {
+if (req_end > field_start && field_end > req_start) {
err = conf_space_read(dev, cfg_entry, field_start,
&tmp_val);
if (err)
@@ -230,8 +229,7 @@ int xen_pcibk_config_write(struct pci_dev *dev, int offset,
int size, u32 value)
field_start = OFFSET(cfg_entry);
field_end = OFFSET(cfg_entry) + field->size;
- if ((req_start >= field_start && req_start < field_end)
- || (req_end > field_start && req_end <= field_end)) {
+if (req_end > field_start && field_end > req_start) {
tmp_val = 0;
err = xen_pcibk_config_read(dev, field_start,
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 3/3] x86/ioreq server: Add HVMOP to map guest ram with p2m_ioreq_server to an ioreq server.
On 21/06/16 10:47, Jan Beulich wrote: > And then - didn't we mean to disable that part of XenGT during > migration, i.e. temporarily accept the higher performance > overhead without the p2m_ioreq_server entries? In which case > flipping everything back to p2m_ram_rw after (completed or > canceled) migration would be exactly what we want. The (new > or previous) ioreq server should attach only afterwards, and > can then freely re-establish any p2m_ioreq_server entries it > deems necessary. > Well, I agree this part of XenGT should be disabled during migration. But in such case I think it's device model's job to trigger the p2m type flipping(i.e. by calling HVMOP_set_mem_type). >>> I agree - this would seem to be the simpler model here, despite (as >>> George validly says) the more consistent model would be for the >>> hypervisor to do the cleanup. Such cleanup would imo be reasonable >>> only if there was an easy way for the hypervisor to enumerate all >>> p2m_ioreq_server pages. >> >> Well, for me, the "easy way" means we should avoid traversing the whole ept >> paging structure all at once, right? > > Yes. Does calling p2m_change_entry_type_global() not satisfy this requirement? >> I have not figured out any clean >> solution >> in hypervisor side, that's one reason I'd like to left this job to >> device model >> side(another reason is that I do think device model should take this >> responsibility). > > Let's see if we can get George to agree. Well I had in principle already agreed to letting this be the interface on the previous round of patches; we're having this discussion because you (Jan) asked about what happens if an ioreq server is de-registered while there are still outstanding p2m types. :-) I do think having Xen change the type makes the most sense, but if you're happy to leave that up to the ioreq server, I'm OK with things being done that way as well. I think we can probably change it later if we want. -George ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 0/8] Add support for parsing per CPU Redistributor entry
On 21/06/16 15:16, Shanker Donthineni wrote: On 06/21/2016 08:50 AM, Julien Grall wrote: On 21/06/16 14:37, Shanker Donthineni wrote: On 06/21/2016 04:28 AM, Julien Grall wrote: On 19/06/16 00:45, Shanker Donthineni wrote: The current driver doesn't support parsing Redistributor entries that are described in the MADT GICC table. Not all the GIC implementors places the Redistributor regions in the always-on power domain. On systems, the UEFI firmware should describe Redistributor base address in the associated GIC CPU Interface (GICC) instead of GIC Redistributor (GICR) table. The maximum number of mmio handlers and struct vgic_rdist_region that holds Redistributor addresses are allocated through a static array with hardcoded size. I don't think this is the right approach and is not scalable for implementing features like this. I have decided to convert static to dynamic allocation based on comments from the below link. https://patchwork.kernel.org/patch/9163435/ You addressed only one part of my comment. This series increases the number of I/O handlers but the lookup is still linear (see handle_mmio in arch/arm/io.c). I agree with you, we need to bring binary search algorithm similar to Linux KVM code. I want to keep it this change outside of this patchset. This should be a prerequisite of this series then, not a follow-up. For the functionality and correctness purpose we don't need this change immediately. We are not able to boot XEN on Qualcomm Technologies because of not supporting GICC table parsing for GICR address. I am okay to wait for my patchset if someone adding bseach look ups for mmio handlers. I am not aware of anyone planning to add bsearch. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] XSA-180 follow-up: repurpose xenconsoled for logging
Here is what we have gathered so far: 1. Virtlogd is not the right answer to XSA-180 because it is inherently designed to work within libvirt. 2. Syslog is not suitable because it doesn't provide a fd for QEMU to write to. 3. Logrotate is not suitable because it only rotates periodically. 4. Syslog + logrotate combo is not suitable because (see above). We can, however, just make recommendation that system administrators can easily set up and call it a day. There are suggestions that we can recommend conserver or sympathy, but I haven't seen a concrete viable proposal yet. What I hope is that we can have a document in tree in the end. Another way is to invent our own "virtlogd" -- it could be a new daemon, it could be xenconsoled. The major concern is that we're adding a critical component to the system and it may not scale well. We can make a compromise by using non-blocking fd to make the new component less critical and doesn't hinder scalability. Another way is to alter libxl API and ask the application to pass in a fd for logging. The major concern is that this is not suitable in the context of a security issue. My ultimate goal is to remove the custom patch we have in QEMU tree so that we don't create a problem for distro maintainers. So I'm fine with any solution really. Wei. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 6/8] arm: vgic: Split vgic_domain_init() functionality into two functions
On 21/06/16 15:36, Shanker Donthineni wrote:
On 06/21/2016 05:49 AM, Julien Grall wrote:
Hello Shanker,
On 19/06/16 00:45, Shanker Donthineni wrote:
Split code that installs mmio handlers for GICD and Re-distributor
regions to a new function. The intension of this separation is to defer
steps that registers vgic_v2/v3 mmio handlers.
Looking at this patch and the follow-up ones, I don't think this is
the right way to go. You differ the registration of the IO handlers
just because you are unable to find the size of the handlers array.
Is there any better approach?
Possibly using a different data structure.
I am wondering if the array for the handlers is the best solution
here. On another side, it would be possible to find the maximum of
handlers before hand.
The purpose of this change is to limit size of 'struct domain' less than
PAGE_SIZE. I can think of second approach split vgic_init() into two
stages, one for vgic registration and the second one for vgic_init().
This also requires a few lines of code changes to vgic_v2/v3_init() and
vgic_init().
I am fine as long as vgic_register_ does not do more than counting the
number of IO handlers. You could re-use vgic_init_v{2,3} for this purpose.
Regards,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4] xen: add steal_clock support on x86
On 20/05/16 08:26, Juergen Gross wrote: > The pv_time_ops structure contains a function pointer for the > "steal_clock" functionality used only by KVM and Xen on ARM. Xen on x86 > uses its own mechanism to account for the "stolen" time a thread wasn't > able to run due to hypervisor scheduling. > > Add support in Xen arch independent time handling for this feature by > moving it out of the arm arch into drivers/xen and remove the x86 Xen > hack. Applied for-linus-4.8, thanks. David ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen/pciback: Update data filter intersection logic.
On Tue, Jun 21, 2016 at 5:23 AM, Jan Beulich wrote: > >>> On 21.06.16 at 11:04, wrote: > On 20.06.16 at 18:23, wrote: > >> Here is printouts with applying the new logic > >> > >> [ 382.13] xen-pciback: :06:00.0: write request 4 bytes at 0xc = > > 4000 > >> [ 382.14] xen-pciback: :06:00.0: read 1 bytes at 0xc > >> [ 382.28] xen-pciback: :06:00.0: read 1 bytes at 0xc = 0 > >> [ 382.38] xen-pciback: :06:00.0: read 1 bytes at 0xd > >> [ 382.81] xen-pciback: :06:00.0: read 1 bytes at 0xd = 0 > >> [ 382.222313] xen-pciback: :06:00.0: read 1 bytes at 0xf > >> [ 382.222341] xen-pciback: :06:00.0: read 1 bytes at 0xf = 0 > >> > >> So from prints the logic is not equivalent. 0xd is included in this > case. > >> > >> In original logic field 0xd is excluded because > >> Not if ((req_start >= field_start && req_start < field_end) > >> Nor (req_end > field_start && req_end <= field_end)) evaluate to true. > >> Where request would be 4b segment starting with 0xc > > > > Oh, I see - the current expression is screwed in two ways: For one > > it has req_* and field_* the wrong way round (or should I better > > say their uses are not symmetric, which for any kind of overlap > > check they of course should be), and then it uses || instead of && > > (i.e. kind of, but not really checking that req is contained in field, > > whereas for the purpose here we'd need the other direction). So > > yes, your change is not just a simplification, but a correction. > > > > But then on top of the previously mentioned change to your patch > > you should also fix the read path in a similar manner. And the > > description should of course accurately reflect the change (i.e. > > no talk of quirks and overlapping filters, and a proper explanation > > of what's wrong with the current expression). > > Sent updated patch for review. > Yet then what I don't understand: How does this change help you? > There's still not going to be any actual write to the Latency Timer > register, as conf_space_write() - even if now getting called - won't > do anything for it. > > > Jan > > Not sure, seems to me sometime the reset sequence in the net driver doesn't actually erase the conf space so it masks the issue ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [xen-4.6-testing test] 96031: tolerable FAIL - PUSHED
flight 96031 xen-4.6-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/96031/ Failures :-/ but no regressions. Regressions which are regarded as allowable (not blocking): test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 95849 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 95849 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 95849 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail like 95849 test-armhf-armhf-xl-rtds 15 guest-start/debian.repeatfail like 95849 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-pvh-intel 11 guest-start fail never pass test-amd64-amd64-xl-pvh-amd 11 guest-start fail never pass test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-libvirt 14 guest-saverestorefail never pass test-armhf-armhf-libvirt 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-xsm 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-xsm 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 11 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-i386-libvirt-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass test-armhf-armhf-xl 12 migrate-support-checkfail never pass test-armhf-armhf-xl 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail never pass test-armhf-armhf-xl-vhd 11 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 12 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 guest-saverestorefail never pass test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 12 migrate-support-checkfail never pass version targeted for testing: xen 285248d91b20bc8245f9241e21d3e7b23f67b550 baseline version: xen c68f2364d54fb7c3707aa91882b54c9529a1d445 Last test of basis95849 2016-06-17 07:40:53 Z4 days Testing same since96006 2016-06-20 12:42:11 Z1 days2 attempts People who touched revisions under test: Ian Jackson Jan Beulich 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-prev pass build-i386-prev pass build-amd64-pvopspass build-armhf-pvopspass build-i386-pvops pass build-amd64-rumpuserxen pass build-i386-rumpuserxen pass test-amd64-amd64-xl
