* [PATCH v5 01/22] KVM: arm/arm64: Add ITS save/restore API documentation
From: Auger Eric @ 2017-04-27 17:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170427163852.GO50776@lvm>
Hi Christoffer,
On 27/04/2017 18:38, Christoffer Dall wrote:
> On Thu, Apr 27, 2017 at 05:29:35PM +0200, Auger Eric wrote:
>>
>>
>> On 27/04/2017 16:45, Christoffer Dall wrote:
>>> Hi Eric,
>>>
>>> On Thu, Apr 27, 2017 at 02:51:00PM +0200, Auger Eric wrote:
>>>> On 27/04/2017 13:02, Christoffer Dall wrote:
>>>>> On Thu, Apr 27, 2017 at 11:33:39AM +0200, Auger Eric wrote:
>>>>>> On 27/04/2017 10:57, Christoffer Dall wrote:
>>>>>>> On Wed, Apr 26, 2017 at 05:48:32PM +0200, Auger Eric wrote:
>>>>>>>> On 26/04/2017 14:31, Christoffer Dall wrote:
>>>>>>>>> On Fri, Apr 14, 2017 at 12:15:13PM +0200, Eric Auger wrote:
>>>>>>>>>> Add description for how to access ITS registers and how to save/restore
>>>>>>>>>> ITS tables into/from memory.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>>>>>>>>
>>>>>>>>>> ---
>>>>>>>>>> v4 -> v5:
>>>>>>>>>> - take into account Christoffer's comments
>>>>>>>>>> - pending table save on GICV3 side now
>>>>>>>>>>
>>>>>>>>>> v3 -> v4:
>>>>>>>>>> - take into account Peter's comments:
>>>>>>>>>> - typos
>>>>>>>>>> - KVM_DEV_ARM_VGIC_GRP_ITS_TABLES kvm_device_attr = 0
>>>>>>>>>> - add a validity bit in DTE
>>>>>>>>>> - document all fields in CTE and ITE
>>>>>>>>>> - document ABI revision
>>>>>>>>>> - take into account Andre's comments:
>>>>>>>>>> - document restrictions about GITS_CREADR writing and GITS_IIDR
>>>>>>>>>> - document -EBUSY error if one or more VCPUS are runnning
>>>>>>>>>> - document 64b registers only can be accessed with 64b access
>>>>>>>>>> - itt_addr field matches bits [51:8] of the itt_addr
>>>>>>>>>>
>>>>>>>>>> v1 -> v2:
>>>>>>>>>> - DTE and ITE now are 8 bytes
>>>>>>>>>> - DTE and ITE now indexed by deviceid/eventid
>>>>>>>>>> - use ITE name instead of ITTE
>>>>>>>>>> - mentions ITT_addr matches bits [51:8] of the actual address
>>>>>>>>>> - mentions LE layout
>>>>>>>>>> ---
>>>>>>>>>> Documentation/virtual/kvm/devices/arm-vgic-its.txt | 99 ++++++++++++++++++++++
>>>>>>>>>> 1 file changed, 99 insertions(+)
>>>>>>>>>>
>>>>>>>>>> diff --git a/Documentation/virtual/kvm/devices/arm-vgic-its.txt b/Documentation/virtual/kvm/devices/arm-vgic-its.txt
>>>>>>>>>> index 6081a5b..b5f010d 100644
>>>>>>>>>> --- a/Documentation/virtual/kvm/devices/arm-vgic-its.txt
>>>>>>>>>> +++ b/Documentation/virtual/kvm/devices/arm-vgic-its.txt
>>>>>>>>>> @@ -32,7 +32,106 @@ Groups:
>>>>>>>>>> KVM_DEV_ARM_VGIC_CTRL_INIT
>>>>>>>>>> request the initialization of the ITS, no additional parameter in
>>>>>>>>>> kvm_device_attr.addr.
>>>>>>>>>> +
>>>>>>>>>> + KVM_DEV_ARM_ITS_SAVE_TABLES
>>>>>>>>>> + save the ITS table data into guest RAM, at the location provisioned
>>>>>>>>>> + by the guest in corresponding registers/table entries.
>>>>>>>>>> +
>>>>>>>>>> + The layout of the tables in guest memory defines an ABI. The entries
>>>>>>>>>> + are laid out in little endian format as described in the last paragraph.
>>>>>>>>>> +
>>>>>>>>>> + KVM_DEV_ARM_ITS_RESTORE_TABLES
>>>>>>>>>> + restore the ITS tables from guest RAM to ITS internal structures.
>>>>>>>>>> +
>>>>>>>>>> + The GICV3 must be restored before the ITS and all ITS registers but
>>>>>>>>>> + the GITS_CTLR must be restored before restoring the ITS tables.
>>>>>>>>>> +
>>>>>>>>>> + The GITS_IIDR read-only register must also be restored before
>>>>>>>>>> + the table restore as the IIDR revision field encodes the ABI revision.
>>>>>>>>>> +
>>>>>>>>>
>>>>>>>>> what is the expected sequence of operations. For example, to restore
>>>>>>>>> the ITS, do I call KVM_DEV_ARM_VGIC_CTRL_INIT first, then restore all
>>>>>>>>> the memory and registers, and finally call KVM_DEV_ARM_ITS_RESTORE_TABLES?
>>>>>>>> Yes KVM_DEV_ARM_VGIC_CTRL_INIT comes first, then restore all registers
>>>>>>>> except GITS_CTLR, then table restore, then GITS_CTLR
>>>>>>>>>
>>>>>>>>> Is there any interaction between when you call KVM_DEV_ARM_ITS_RESTORE_TABLES
>>>>>>>>> and restore GITS_CTLR (which enables the ITS)?
>>>>>>>>
>>>>>>>> Yep, when GITS_CTLR is set, LPIs may be enabled and this on that event
>>>>>>>> that the pending table is read. But the whole pending table is not read
>>>>>>>> as we only iterate on registered LPIs. So the ITT must have been
>>>>>>>> restored previously.
>>>>>>>>
>>>>>>>> I became aware that the pending table sync is done twice, once in the
>>>>>>>> pending table restore, and once in the GITS_CTLR restore. So if we
>>>>>>>> leave this order specification, I should be able to remove the sync on
>>>>>>>> table restore. This was the original reason why GITS_CTLR restore has
>>>>>>>> been done at the very end.
>>>>>>>
>>>>>>> I'm sorry, I'm a bit confused. Do we not need
>>>>>>> KVM_DEV_ARM_ITS_RESTORE_TABLES at all then?
>>>>>>
>>>>>> Yes you do. I was talking about the RDIST pending table sync. The save
>>>>>> is explicit using GICV3 device KVM_DEV_ARM_VGIC_SAVE_PENDING_TABLES.
>>>>>> However the sync is implicit on GITS_CTLR restore if LPIs are enabled.
>>>>>> and today I do it also on ITS device KVM_DEV_ARM_ITS_RESTORE_TABLES
>>>>>> which is not requested I think since GITS_CTLR restore does it already.
>>>>>
>>>>> Shouldn't restoring the pending tables happen when restoring some
>>>>> redeistributor state and not anything related to the ITS?
>>>>
>>>> Marc wrote:
>>>> "
>>>> I don't think you necessarily need a coarse map. When restoring the ITS
>>>> tables, you can always read the pending bit when creating the LPI
>>>> structure (it has been written to RAM at save time). Note that we
>>>> already do something like this in vgic_enable_lpis().
>>>> "
>>>>
>>>> This is currently what is implemented I think. the pending tables are
>>>> currently sync'ed on GITS_CTLR set (if LPI are enabled) + erroneously
>>>> also on on ITS table restore
>>>>
>>>> The problematic is: Either you know in advance which LPI INTIDare used
>>>> or you need to parse the whole pending table (possibly using the 1st kB
>>>> as coarse mapping).
>>>>
>>>> If you don't know the LPI INTIDs in advance it is only possible to
>>>> restore the pending bit of pending LPIs. At that time you would
>>>> re-allocate those pending LPI (vgic_add_lpi) and when you restore the
>>>> ITS ITT you would do the same for those which were not pending. Looks
>>>> really heavy to me: coarse mapping + dual vgic_add_lpi path.
>>>>
>>>> Otherwise we would need to add another dependency between RDIST pending
>>>> table restore and ITS table restore but this looks even more weird, no?
>>>>
>>>>
>>> So I just sat down with Andre and Marc and we tried to work through this
>>> and came up with the best scheme. I apologize in advance for the
>>> one-way nature of this e-mail, and I am of course open to discussing the
>>> following proposal again if you do not agree.
>>>
>>> What I think this document should say, is that the following ordering
>>> must be followed when restoring the GIC and the ITS:
>>>
>>> First, restore all guest memory
>>>
>>> Second, restore ALL redistributors
>>>
>>> Third, restore the ITS, in the following order:
>>> 1. Initialize the ITS (KVM_DEV_ARM_VGIC_CTRL_INIT)
>>> 2. Restore GITS_CBASER
>>> 3. Restore all other GITS_ registers, except GITS_CTLR!
>>> 4. Load the ITS table data (KVM_DEV_ARM_ITS_RESTORE_TABLES)
>>> 5. Restore GITS_CTLR
>>>
>>> The rationale is that we really want the redistributor and the ITS
>>> restore to be independent and follow the architecture. This means that
>>> our ABI for the redistributor should still work without restoring an ITS
>>> (if we ever decide to support LPIs for KVM without the ITS).
>>
>> OK. Note I already mentioned that GICv3 must be restored before the ITS.
>> To me this comprised the RDIST.
>
> Possibly, but I think it's good to write out the whole thing so we
> clearly understand the flow. That could better be achieved by
> correcting my proposed text above to say something like "Second, restore
> ALL redistributors to ensure the pending and configuration tables can be
> read."
>
>>
>> I understand the above description of the ordering comes in addition to
>> the existing text, right?
>
> Yes
>
>> in other words I keep the GITS_READR,
>> GITS_IIDR specific text as well as KVM_DEV_ARM_ITS_SAVE/RESTORE_TABLES
>> section.
>>
>
> Yes. But you don't need to do any reading of the pending table on any
> of the restore operations.
well you told me to do it on vgic_add_lpi(). This is obviously called on
ITS table restore. /me confused. Obviously this is implicit and should
not be documented. Is that what you meant? btw this is not documented
atm I think.
>
> Btw., I think it's GITS_CREADR, not GITS_READR.
right.
Thanks
Eric
>
>>>
>>> In terms of our current implementation this means that vgic_add_lpi()
>>> should ask the redistributor what the state of the LPI is (priority,
>>> enabled, pending).
>> this practically means I move update_lpi_config call from
>> vgic_its_restore_ite to vgic_add_lpi(). OK
>
> Pretty much, yes.
>
> Thanks,
> -Christoffer
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* [PATCH v5 01/22] KVM: arm/arm64: Add ITS save/restore API documentation
From: Christoffer Dall @ 2017-04-27 17:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <6227482e-1ef6-bb5f-156c-2a8e522f1a20@redhat.com>
On Thu, Apr 27, 2017 at 07:14:29PM +0200, Auger Eric wrote:
> Hi Marc, Christoffer,
>
> On 27/04/2017 18:23, Marc Zyngier wrote:
> > On 27/04/17 16:29, Auger Eric wrote:
> >>
> >>
> >> On 27/04/2017 16:45, Christoffer Dall wrote:
> >>> Hi Eric,
> >>>
> >>> On Thu, Apr 27, 2017 at 02:51:00PM +0200, Auger Eric wrote:
> >>>> On 27/04/2017 13:02, Christoffer Dall wrote:
> >>>>> On Thu, Apr 27, 2017 at 11:33:39AM +0200, Auger Eric wrote:
> >>>>>> On 27/04/2017 10:57, Christoffer Dall wrote:
> >>>>>>> On Wed, Apr 26, 2017 at 05:48:32PM +0200, Auger Eric wrote:
> >>>>>>>> On 26/04/2017 14:31, Christoffer Dall wrote:
> >>>>>>>>> On Fri, Apr 14, 2017 at 12:15:13PM +0200, Eric Auger wrote:
> >>>>>>>>>> Add description for how to access ITS registers and how to save/restore
> >>>>>>>>>> ITS tables into/from memory.
> >>>>>>>>>>
> >>>>>>>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> >>>>>>>>>>
> >>>>>>>>>> ---
> >>>>>>>>>> v4 -> v5:
> >>>>>>>>>> - take into account Christoffer's comments
> >>>>>>>>>> - pending table save on GICV3 side now
> >>>>>>>>>>
> >>>>>>>>>> v3 -> v4:
> >>>>>>>>>> - take into account Peter's comments:
> >>>>>>>>>> - typos
> >>>>>>>>>> - KVM_DEV_ARM_VGIC_GRP_ITS_TABLES kvm_device_attr = 0
> >>>>>>>>>> - add a validity bit in DTE
> >>>>>>>>>> - document all fields in CTE and ITE
> >>>>>>>>>> - document ABI revision
> >>>>>>>>>> - take into account Andre's comments:
> >>>>>>>>>> - document restrictions about GITS_CREADR writing and GITS_IIDR
> >>>>>>>>>> - document -EBUSY error if one or more VCPUS are runnning
> >>>>>>>>>> - document 64b registers only can be accessed with 64b access
> >>>>>>>>>> - itt_addr field matches bits [51:8] of the itt_addr
> >>>>>>>>>>
> >>>>>>>>>> v1 -> v2:
> >>>>>>>>>> - DTE and ITE now are 8 bytes
> >>>>>>>>>> - DTE and ITE now indexed by deviceid/eventid
> >>>>>>>>>> - use ITE name instead of ITTE
> >>>>>>>>>> - mentions ITT_addr matches bits [51:8] of the actual address
> >>>>>>>>>> - mentions LE layout
> >>>>>>>>>> ---
> >>>>>>>>>> Documentation/virtual/kvm/devices/arm-vgic-its.txt | 99 ++++++++++++++++++++++
> >>>>>>>>>> 1 file changed, 99 insertions(+)
> >>>>>>>>>>
> >>>>>>>>>> diff --git a/Documentation/virtual/kvm/devices/arm-vgic-its.txt b/Documentation/virtual/kvm/devices/arm-vgic-its.txt
> >>>>>>>>>> index 6081a5b..b5f010d 100644
> >>>>>>>>>> --- a/Documentation/virtual/kvm/devices/arm-vgic-its.txt
> >>>>>>>>>> +++ b/Documentation/virtual/kvm/devices/arm-vgic-its.txt
> >>>>>>>>>> @@ -32,7 +32,106 @@ Groups:
> >>>>>>>>>> KVM_DEV_ARM_VGIC_CTRL_INIT
> >>>>>>>>>> request the initialization of the ITS, no additional parameter in
> >>>>>>>>>> kvm_device_attr.addr.
> >>>>>>>>>> +
> >>>>>>>>>> + KVM_DEV_ARM_ITS_SAVE_TABLES
> >>>>>>>>>> + save the ITS table data into guest RAM, at the location provisioned
> >>>>>>>>>> + by the guest in corresponding registers/table entries.
> >>>>>>>>>> +
> >>>>>>>>>> + The layout of the tables in guest memory defines an ABI. The entries
> >>>>>>>>>> + are laid out in little endian format as described in the last paragraph.
> >>>>>>>>>> +
> >>>>>>>>>> + KVM_DEV_ARM_ITS_RESTORE_TABLES
> >>>>>>>>>> + restore the ITS tables from guest RAM to ITS internal structures.
> >>>>>>>>>> +
> >>>>>>>>>> + The GICV3 must be restored before the ITS and all ITS registers but
> >>>>>>>>>> + the GITS_CTLR must be restored before restoring the ITS tables.
> >>>>>>>>>> +
> >>>>>>>>>> + The GITS_IIDR read-only register must also be restored before
> >>>>>>>>>> + the table restore as the IIDR revision field encodes the ABI revision.
> >>>>>>>>>> +
> >>>>>>>>>
> >>>>>>>>> what is the expected sequence of operations. For example, to restore
> >>>>>>>>> the ITS, do I call KVM_DEV_ARM_VGIC_CTRL_INIT first, then restore all
> >>>>>>>>> the memory and registers, and finally call KVM_DEV_ARM_ITS_RESTORE_TABLES?
> >>>>>>>> Yes KVM_DEV_ARM_VGIC_CTRL_INIT comes first, then restore all registers
> >>>>>>>> except GITS_CTLR, then table restore, then GITS_CTLR
> >>>>>>>>>
> >>>>>>>>> Is there any interaction between when you call KVM_DEV_ARM_ITS_RESTORE_TABLES
> >>>>>>>>> and restore GITS_CTLR (which enables the ITS)?
> >>>>>>>>
> >>>>>>>> Yep, when GITS_CTLR is set, LPIs may be enabled and this on that event
> >>>>>>>> that the pending table is read. But the whole pending table is not read
> >>>>>>>> as we only iterate on registered LPIs. So the ITT must have been
> >>>>>>>> restored previously.
> >>>>>>>>
> >>>>>>>> I became aware that the pending table sync is done twice, once in the
> >>>>>>>> pending table restore, and once in the GITS_CTLR restore. So if we
> >>>>>>>> leave this order specification, I should be able to remove the sync on
> >>>>>>>> table restore. This was the original reason why GITS_CTLR restore has
> >>>>>>>> been done at the very end.
> >>>>>>>
> >>>>>>> I'm sorry, I'm a bit confused. Do we not need
> >>>>>>> KVM_DEV_ARM_ITS_RESTORE_TABLES at all then?
> >>>>>>
> >>>>>> Yes you do. I was talking about the RDIST pending table sync. The save
> >>>>>> is explicit using GICV3 device KVM_DEV_ARM_VGIC_SAVE_PENDING_TABLES.
> >>>>>> However the sync is implicit on GITS_CTLR restore if LPIs are enabled.
> >>>>>> and today I do it also on ITS device KVM_DEV_ARM_ITS_RESTORE_TABLES
> >>>>>> which is not requested I think since GITS_CTLR restore does it already.
> >>>>>
> >>>>> Shouldn't restoring the pending tables happen when restoring some
> >>>>> redeistributor state and not anything related to the ITS?
> >>>>
> >>>> Marc wrote:
> >>>> "
> >>>> I don't think you necessarily need a coarse map. When restoring the ITS
> >>>> tables, you can always read the pending bit when creating the LPI
> >>>> structure (it has been written to RAM at save time). Note that we
> >>>> already do something like this in vgic_enable_lpis().
> >>>> "
> >>>>
> >>>> This is currently what is implemented I think. the pending tables are
> >>>> currently sync'ed on GITS_CTLR set (if LPI are enabled) + erroneously
> >>>> also on on ITS table restore
> >>>>
> >>>> The problematic is: Either you know in advance which LPI INTIDare used
> >>>> or you need to parse the whole pending table (possibly using the 1st kB
> >>>> as coarse mapping).
> >>>>
> >>>> If you don't know the LPI INTIDs in advance it is only possible to
> >>>> restore the pending bit of pending LPIs. At that time you would
> >>>> re-allocate those pending LPI (vgic_add_lpi) and when you restore the
> >>>> ITS ITT you would do the same for those which were not pending. Looks
> >>>> really heavy to me: coarse mapping + dual vgic_add_lpi path.
> >>>>
> >>>> Otherwise we would need to add another dependency between RDIST pending
> >>>> table restore and ITS table restore but this looks even more weird, no?
> >>>>
> >>>>
> >>> So I just sat down with Andre and Marc and we tried to work through this
> >>> and came up with the best scheme. I apologize in advance for the
> >>> one-way nature of this e-mail, and I am of course open to discussing the
> >>> following proposal again if you do not agree.
> >>>
> >>> What I think this document should say, is that the following ordering
> >>> must be followed when restoring the GIC and the ITS:
> >>>
> >>> First, restore all guest memory
> >>>
> >>> Second, restore ALL redistributors
> >>>
> >>> Third, restore the ITS, in the following order:
> >>> 1. Initialize the ITS (KVM_DEV_ARM_VGIC_CTRL_INIT)
> >>> 2. Restore GITS_CBASER
> >>> 3. Restore all other GITS_ registers, except GITS_CTLR!
> >>> 4. Load the ITS table data (KVM_DEV_ARM_ITS_RESTORE_TABLES)
> >>> 5. Restore GITS_CTLR
> >>>
> >>> The rationale is that we really want the redistributor and the ITS
> >>> restore to be independent and follow the architecture. This means that
> >>> our ABI for the redistributor should still work without restoring an ITS
> >>> (if we ever decide to support LPIs for KVM without the ITS).
> >>
> >> OK. Note I already mentioned that GICv3 must be restored before the ITS.
> >> To me this comprised the RDIST.
> >>
> >> I understand the above description of the ordering comes in addition to
> >> the existing text, right? in other words I keep the GITS_READR,
> >> GITS_IIDR specific text as well as KVM_DEV_ARM_ITS_SAVE/RESTORE_TABLES
> >> section.
> >>
> >>>
> >>> In terms of our current implementation this means that vgic_add_lpi()
> >>> should ask the redistributor what the state of the LPI is (priority,
> >>> enabled, pending).
> >> this practically means I move update_lpi_config call from
> >> vgic_its_restore_ite to vgic_add_lpi(). OK
> >>
> >> However for getting the LPI pending state I must know which RDIST the
> >> LPI is attached to. This is not known at LPI allocation time. Do I
> >> misunderstand something?
> >
> > Once you have rebuilt the ITS data structures and allocated the IRQ
> > structures, you should have a target_cpu field pointing to the right
> > vcpu. From there, you can surely find the corresponding redistributor
> > and the pending table.
> Yes that's understood but Christoffer said "vgic_add_lpi() should ask
> the redistributor what the state of the LPI is (priority,enabled,
> pending)." Fetching the properties is fine.
>
> vgic_add_lpi() is called before update_affinity_ite() which uses
> ite->irq and sets the target_vcpu.
>
> Well at least this requires some function reshape. I will investigate
> though.
Thanks. If it looks impossible, let me know, and I can help having a
look at the code.
-Christoffer
^ permalink raw reply
* [PATCH] perf evsel: Fix to perf-stat malloc corruption on arm64 platforms
From: Ganapatrao Kulkarni @ 2017-04-27 17:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170427155221.GB6646@leverpostej>
On Thu, Apr 27, 2017 at 9:22 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Thu, Apr 27, 2017 at 09:16:41PM +0530, Ganapatrao Kulkarni wrote:
>> > Could you please give my diff a go?
>>
>> i tried your diff, and testing looks ok.
>
> Can I take that as a Tested-by when I post this as a proper patch?
sure.
>
>> below is the cleanly merged diff on top of latest commit
>> f832460 Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/sparc
>
> Thanks for the rebase.
>
> Mark.
>
>> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
>> index 13b5499..4be2980 100644
>> --- a/tools/perf/builtin-stat.c
>> +++ b/tools/perf/builtin-stat.c
>> @@ -346,6 +346,28 @@ static void read_counters(void)
>> }
>> }
>>
>> +/*
>> + * Close all evnt FDs we open in __run_perf_stat() and
>> + * create_perf_stat_counter(), taking care to match the number of
>> threads and CPUs.
>> + *
>> + * Note that perf_evlist__close(evsel_list) is not equivalent, as it doesn't
>> + * take the target into account.
>> + */
>> +static void close_counters(void)
>> +{
>> + bool per_cpu = target__has_cpu(&target);
>> + struct perf_evsel *evsel;
>> +
>> + evlist__for_each_entry(evsel_list, evsel) {
>> + if (per_cpu)
>> + perf_evsel__close_per_cpu(evsel,
>> + perf_evsel__cpus(evsel));
>> + else
>> + perf_evsel__close_per_thread(evsel,
>> + evsel_list->threads);
>> + }
>> +}
>> +
>> static void process_interval(void)
>> {
>> struct timespec ts, rs;
>> @@ -686,7 +708,7 @@ static int __run_perf_stat(int argc, const char **argv)
>> * group leaders.
>> */
>> read_counters();
>> - perf_evlist__close(evsel_list);
>> + close_counters();
>>
>> return WEXITSTATUS(status);
>> }
>> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
>> index ac59710..ecd9778 100644
>> --- a/tools/perf/util/evsel.c
>> +++ b/tools/perf/util/evsel.c
>> @@ -1691,6 +1691,20 @@ int perf_evsel__open_per_thread(struct perf_evsel *evsel,
>> return perf_evsel__open(evsel, NULL, threads);
>> }
>>
>> +void perf_evsel__close_per_cpu(struct perf_evsel *evsel,
>> + struct cpu_map *cpus)
>> + {
>> + int ncpus = cpus ? cpus->nr : 1;
>> + perf_evsel__close(evsel, ncpus, 1);
>> + }
>> +
>> +void perf_evsel__close_per_thread(struct perf_evsel *evsel,
>> + struct thread_map *threads)
>> + {
>> + int nthreads = threads ? threads->nr : 1;
>> + perf_evsel__close(evsel, 1, nthreads);
>> + }
>> +
>> static int perf_evsel__parse_id_sample(const struct perf_evsel *evsel,
>> const union perf_event *event,
>> struct perf_sample *sample)
>> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
>> index 06ef6f2..6779bd2 100644
>> --- a/tools/perf/util/evsel.h
>> +++ b/tools/perf/util/evsel.h
>> @@ -250,6 +250,10 @@ int perf_evsel__open_per_cpu(struct perf_evsel *evsel,
>> struct cpu_map *cpus);
>> int perf_evsel__open_per_thread(struct perf_evsel *evsel,
>> struct thread_map *threads);
>> +void perf_evsel__close_per_cpu(struct perf_evsel *evsel,
>> + struct cpu_map *cpus);
>> +void perf_evsel__close_per_thread(struct perf_evsel *evsel,
>> + struct thread_map *threads);
>> int perf_evsel__open(struct perf_evsel *evsel, struct cpu_map *cpus,
>> struct thread_map *threads);
>> void perf_evsel__close(struct perf_evsel *evsel, int ncpus, int nthreads);
^ permalink raw reply
* [PATCH v5 13/22] KVM: arm64: vgic-its: Check the device id matches TYPER DEVBITS range
From: Auger Eric @ 2017-04-27 17:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170427164837.GQ50776@lvm>
Hi,
On 27/04/2017 18:48, Christoffer Dall wrote:
> On Fri, Apr 14, 2017 at 12:15:25PM +0200, Eric Auger wrote:
>> On MAPD we currently check the device id can be stored in the device table.
>> Let's first check it can be encoded within the range defined by TYPER
>> DEVBITS.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>
>> ---
>>
>> v4 -> v5:
>> - use GIC_ENCODE_SZ macro
>>
>> v3 -> v4:
>> - VITS_TYPER_DEVBITS set to 16 for homogeneity
>> - use BIT_ULL
>> ---
>> virt/kvm/arm/vgic/vgic-its.c | 10 +++++++---
>> 1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
>> index 757598d..de1ed6d 100644
>> --- a/virt/kvm/arm/vgic/vgic-its.c
>> +++ b/virt/kvm/arm/vgic/vgic-its.c
>> @@ -194,6 +194,7 @@ static struct its_ite *find_ite(struct vgic_its *its, u32 device_id,
>> #define GIC_LPI_OFFSET 8192
>>
>> #define VITS_TYPER_IDBITS 16
>> +#define VITS_TYPER_DEVBITS 16
>>
>> /*
>> * Finds and returns a collection in the ITS collection table.
>> @@ -394,7 +395,7 @@ static unsigned long vgic_mmio_read_its_typer(struct kvm *kvm,
>> * To avoid memory waste in the guest, we keep the number of IDBits and
>> * DevBits low - as least for the time being.
>> */
>> - reg |= 0x0f << GITS_TYPER_DEVBITS_SHIFT;
>> + reg |= GIC_ENCODE_SZ(VITS_TYPER_DEVBITS, 5) << GITS_TYPER_DEVBITS_SHIFT;
>> reg |= GIC_ENCODE_SZ(VITS_TYPER_IDBITS, 5) << GITS_TYPER_IDBITS_SHIFT;
>> reg |= GIC_ENCODE_SZ(abi->ite_esz, 4) << GITS_TYPER_ITT_ENTRY_SIZE_SHIFT;
>>
>> @@ -639,10 +640,10 @@ static int vgic_its_cmd_handle_movi(struct kvm *kvm, struct vgic_its *its,
>> * Check whether an ID can be stored into the corresponding guest table.
>> * For a direct table this is pretty easy, but gets a bit nasty for
>> * indirect tables. We check whether the resulting guest physical address
>> - * is actually valid (covered by a memslot and guest accessbible).
>> + * is actually valid (covered by a memslot and guest accessible).
>> * For this we have to read the respective first level entry.
>> */
>> -static bool vgic_its_check_id(struct vgic_its *its, u64 baser, int id)
>> +static bool vgic_its_check_id(struct vgic_its *its, u64 baser, u32 id)
>> {
>> int l1_tbl_size = GITS_BASER_NR_PAGES(baser) * SZ_64K;
>> int index;
>> @@ -650,6 +651,9 @@ static bool vgic_its_check_id(struct vgic_its *its, u64 baser, int id)
>> gfn_t gfn;
>> int esz = GITS_BASER_ENTRY_SIZE(baser);
>>
>> + if (id >= BIT_ULL(VITS_TYPER_DEVBITS))
>> + return false;
>> +
>
> Isn't vgic_its_check_id called with both a device id and a collection
> id? How can this then be a valid check?
Hum yes that's correct. In practice the test is correct for collection
ID too since our virtual implementation supports collections in memory
(GITS_TYPER.CIL ==0, spec 8.19.8) and a 16-bit collection ID is
supported. But this is by chance and this really deserves some proper
differentiation. Thank you for spotting that one too!
Thanks
Eric
>
>> if (!(baser & GITS_BASER_INDIRECT)) {
>> phys_addr_t addr;
>>
>> --
>> 2.5.5
>>
>
> Thanks,
> -Christoffer
>
^ permalink raw reply
* [PATCH v5 14/22] KVM: arm64: vgic-its: KVM_DEV_ARM_ITS_SAVE/RESTORE_TABLES
From: Christoffer Dall @ 2017-04-27 17:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1492164934-988-15-git-send-email-eric.auger@redhat.com>
On Fri, Apr 14, 2017 at 12:15:26PM +0200, Eric Auger wrote:
> Introduce new attributes in KVM_DEV_ARM_VGIC_GRP_CTRL group:
> - KVM_DEV_ARM_ITS_SAVE_TABLES: saves the ITS tables into guest RAM
> - KVM_DEV_ARM_ITS_RESTORE_TABLES: restores them into VGIC internal
> structures.
>
> We hold the vcpus lock during the save and restore to make
> sure no vcpu is running.
>
> At this stage the functionality is not yet implemented. Only
> the skeleton is put in place.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>
> ---
>
> v4 -> v5:
> - use KVM_DEV_ARM_ITS_SAVE_TABLES and KVM_DEV_ARM_ITS_RESTORE_TABLES
> - rename *flush* into *save*
> - call its_sync_lpi_pending_table at the end of restore
> - use abi framework
>
> v3 -> v4:
> - pass kvm struct handle to vgic_its_flush/restore_pending_tables
> - take the kvm lock and vcpu locks
> - ABI revision check
> - check attr->attr is null
>
> v1 -> v2:
> - remove useless kvm parameter
> ---
> arch/arm/include/uapi/asm/kvm.h | 4 +-
> arch/arm64/include/uapi/asm/kvm.h | 4 +-
> virt/kvm/arm/vgic/vgic-its.c | 108 ++++++++++++++++++++++++++++++++++++--
> 3 files changed, 110 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
> index 4beb83b..8e6563c 100644
> --- a/arch/arm/include/uapi/asm/kvm.h
> +++ b/arch/arm/include/uapi/asm/kvm.h
> @@ -199,7 +199,9 @@ struct kvm_arch_memory_slot {
> #define KVM_DEV_ARM_VGIC_LINE_LEVEL_INTID_MASK 0x3ff
> #define VGIC_LEVEL_INFO_LINE_LEVEL 0
>
> -#define KVM_DEV_ARM_VGIC_CTRL_INIT 0
> +#define KVM_DEV_ARM_VGIC_CTRL_INIT 0
> +#define KVM_DEV_ARM_ITS_SAVE_TABLES 1
> +#define KVM_DEV_ARM_ITS_RESTORE_TABLES 2
>
> /* KVM_IRQ_LINE irq field index values */
> #define KVM_ARM_IRQ_TYPE_SHIFT 24
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index 7e8dd69..1e35115 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -219,7 +219,9 @@ struct kvm_arch_memory_slot {
> #define KVM_DEV_ARM_VGIC_LINE_LEVEL_INTID_MASK 0x3ff
> #define VGIC_LEVEL_INFO_LINE_LEVEL 0
>
> -#define KVM_DEV_ARM_VGIC_CTRL_INIT 0
> +#define KVM_DEV_ARM_VGIC_CTRL_INIT 0
> +#define KVM_DEV_ARM_ITS_SAVE_TABLES 1
> +#define KVM_DEV_ARM_ITS_RESTORE_TABLES 2
>
> /* Device Control API on vcpu fd */
> #define KVM_ARM_VCPU_PMU_V3_CTRL 0
> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
> index de1ed6d..55267ab 100644
> --- a/virt/kvm/arm/vgic/vgic-its.c
> +++ b/virt/kvm/arm/vgic/vgic-its.c
> @@ -1648,12 +1648,68 @@ int vgic_its_attr_regs_access(struct kvm_device *dev,
> }
>
> /**
> + * vgic_its_save_device_tables - Save the device table and all ITT
> + * into guest RAM
> + */
> +static int vgic_its_save_device_tables(struct vgic_its *its)
> +{
> + return -ENXIO;
> +}
> +
> +/**
> + * vgic_its_restore_device_tables - Restore the device table and all ITT
> + * from guest RAM to internal data structs
> + */
> +static int vgic_its_restore_device_tables(struct vgic_its *its)
> +{
> + return -ENXIO;
> +}
> +
> +/**
> + * vgic_its_save_collection_table - Save the collection table into
> + * guest RAM
> + */
> +static int vgic_its_save_collection_table(struct vgic_its *its)
> +{
> + return -ENXIO;
> +}
> +
> +/**
> + * vgic_its_restore_collection_table - reads the collection table
> + * in guest memory and restores the ITS internal state. Requires the
> + * BASER registers to be restored before.
> + */
> +static int vgic_its_restore_collection_table(struct vgic_its *its)
> +{
> + return -ENXIO;
> +}
> +
> +/**
> * vgic_its_save_tables_v0 - Save the ITS tables into guest ARM
> * according to v0 ABI
> */
> static int vgic_its_save_tables_v0(struct vgic_its *its)
> {
> - return -ENXIO;
> + struct kvm *kvm = its->dev->kvm;
> + int ret;
> +
> + mutex_lock(&kvm->lock);
> +
> + if (!lock_all_vcpus(kvm)) {
> + mutex_unlock(&kvm->lock);
> + return -EBUSY;
> + }
> +
> + ret = vgic_its_save_device_tables(its);
> + if (ret)
> + goto out;
> +
> + ret = vgic_its_save_collection_table(its);
> +
> +out:
> + unlock_all_vcpus(kvm);
> + mutex_unlock(&kvm->lock);
> + return ret;
> }
>
> /**
> @@ -1663,7 +1719,41 @@ static int vgic_its_save_tables_v0(struct vgic_its *its)
> */
> static int vgic_its_restore_tables_v0(struct vgic_its *its)
> {
> - return -ENXIO;
> + struct kvm *kvm = its->dev->kvm;
> + struct kvm_vcpu *vcpu;
> + int ret, c;
> +
> + mutex_lock(&kvm->lock);
> +
> + if (!lock_all_vcpus(kvm)) {
> + mutex_unlock(&kvm->lock);
> + return -EBUSY;
> + }
> +
> + ret = vgic_its_restore_collection_table(its);
> + if (ret)
> + goto out;
> +
> + ret = vgic_its_restore_device_tables(its);
> +
> + kvm_for_each_vcpu(c, vcpu, kvm) {
> + ret = its_sync_lpi_pending_table(vcpu);
> + if (ret)
> + break;
> + }
> +
> +out:
> + unlock_all_vcpus(kvm);
> + mutex_unlock(&kvm->lock);
> +
> + if (ret)
> + return ret;
> +
> + /*
> + * On restore path, MSI injections can happen before the
> + * first VCPU run so let's complete the GIC init here.
> + */
> + return kvm_vgic_map_resources(its->dev->kvm);
wait, this is supposed to run when everything else is done and just
before running a VCPU, but now we run this way earlier? Is that safe.
It feels incredibly dangerous.
Otherwise I'll wait with looking at this patch in more detail until you
respin based on not doing the its_sync_lpi_pending_table() here anymore
as a result of the ABI discussion.
Thanks,
-Christoffer
> }
>
> static int vgic_its_commit_v0(struct vgic_its *its)
> @@ -1718,6 +1808,10 @@ static int vgic_its_has_attr(struct kvm_device *dev,
> switch (attr->attr) {
> case KVM_DEV_ARM_VGIC_CTRL_INIT:
> return 0;
> + case KVM_DEV_ARM_ITS_SAVE_TABLES:
> + return 0;
> + case KVM_DEV_ARM_ITS_RESTORE_TABLES:
> + return 0;
> }
> break;
> case KVM_DEV_ARM_VGIC_GRP_ITS_REGS:
> @@ -1753,14 +1847,20 @@ static int vgic_its_set_attr(struct kvm_device *dev,
>
> return 0;
> }
> - case KVM_DEV_ARM_VGIC_GRP_CTRL:
> + case KVM_DEV_ARM_VGIC_GRP_CTRL: {
> + const struct vgic_its_abi *abi = vgic_its_get_abi(its);
> +
> switch (attr->attr) {
> case KVM_DEV_ARM_VGIC_CTRL_INIT:
> its->initialized = true;
>
> return 0;
> + case KVM_DEV_ARM_ITS_SAVE_TABLES:
> + return abi->save_tables(its);
> + case KVM_DEV_ARM_ITS_RESTORE_TABLES:
> + return abi->restore_tables(its);
> }
> - break;
> + }
> case KVM_DEV_ARM_VGIC_GRP_ITS_REGS: {
> u64 __user *uaddr = (u64 __user *)(long)attr->addr;
> u64 reg;
> --
> 2.5.5
>
^ permalink raw reply
* [PATCH net-next 1/4] ixgbe: sparc: rename the ARCH_WANT_RELAX_ORDER to IXGBE_ALLOW_RELAXED_ORDER
From: Bjorn Helgaas @ 2017-04-27 17:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKgT0Uc4L=GgYbpO-Fm9OfN+_fLypbDP1c+X4T_ta90ecQiyGQ@mail.gmail.com>
[+cc Casey]
On Wed, Apr 26, 2017 at 09:18:33AM -0700, Alexander Duyck wrote:
> On Wed, Apr 26, 2017 at 2:26 AM, Ding Tianhong <dingtianhong@huawei.com> wrote:
> > Hi Amir:
> >
> > It is really glad to hear that the mlx5 will support RO mode this year, if so, do you agree that enable it dynamic by ethtool -s xxx,
> > we have try it several month ago but there was only one drivers would use it at that time so the maintainer against it, it mlx5 would support RO,
> > we could try to restart this solution, what do you think about it. :)
> >
> > Thanks
> > Ding
>
> Hi Ding,
>
> Enabing relaxed ordering really doesn't have any place in ethtool. It
> is a PCIe attribute that you are essentially wanting to enable.
>
> It might be worth while to take a look at updating the PCIe code path
> to handle this. Really what we should probably do is guarantee that
> the architectures that need relaxed ordering are setting it in the
> PCIe Device Control register and that the ones that don't are clearing
> the bit. It's possible that this is already occurring, but I don't
> know the state of handling those bits is in the kernel. Once we can
> guarantee that we could use that to have the drivers determine their
> behavior in regards to relaxed ordering. For example in the case of
> igb/ixgbe we could probably change the behavior so that it will bey
> default try to use relaxed ordering but if it is not enabled in PCIe
> Device Control register the hardware should not request to use it. It
> would simplify things in the drivers and allow for each architecture
> to control things as needed in their PCIe code.
I thought Relaxed Ordering was an optimization. Are there cases where
it is actually required for correct behavior?
The PCI core doesn't currently do anything with Relaxed Ordering.
Some drivers enable/disable it directly. I think it would probably be
better if the core provided an interface for this. One reason is
because I think Casey has identified some systems where Relaxed
Ordering doesn't work correctly, and I'd rather deal with them once in
the core than in every driver.
Are you hinting that the PCI core or arch code could actually *enable*
Relaxed Ordering without the driver doing anything? Is it safe to do
that? Is there such a thing as a device that is capable of using RO,
but where the driver must be aware of it being enabled, so it programs
the device appropriately?
Bjorn
^ permalink raw reply
* [PATCH] arm64: cpufeature: use static_branch_enable_cpuslocked()
From: Mark Rutland @ 2017-04-27 17:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2e321402-7042-9d5b-aa85-52b0e6f1e20c@arm.com>
On Thu, Apr 27, 2017 at 06:03:35PM +0100, Suzuki K Poulose wrote:
> On 27/04/17 17:35, Suzuki K Poulose wrote:
> >@@ -1092,7 +1093,9 @@ void check_local_cpu_capabilities(void)
> >
> > static void __init setup_feature_capabilities(void)
> > {
> >- update_cpu_capabilities(arm64_features, "detected feature:");
> >+ get_online_cpus();
> >+ update_cpu_capabilities(arm6_features, "detected feature:");
>
> s/arm6_features/arm64_features
>
> And we need the following hunk:
>
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 62bdab4..19c359a 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -1086,7 +1086,7 @@ void check_local_cpu_capabilities(void)
> * advertised capabilities.
> */
> if (!sys_caps_initialised)
> - update_cpu_errata_workarounds();
> + update_secondary_cpu_errata_workarounds();
> else
> verify_local_cpu_capabilities();
> }
>
> Sorry about that.
No worries; thanks for the fixups.
With those this is working for me, so I'll send this and Sebastian's
patch (with Ccs) as a new series.
>
> Suzuki
^ permalink raw reply
* [PATCH v5 11/22] KVM: arm64: vgic-its: Interpret MAPD Size field and check related errors
From: Auger Eric @ 2017-04-27 17:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170427162528.GN50776@lvm>
Hi,
On 27/04/2017 18:25, Christoffer Dall wrote:
> On Fri, Apr 14, 2017 at 12:15:23PM +0200, Eric Auger wrote:
>> Up to now the MAPD's ITT size field has been ignored. It encodes
>> the number of eventid bit minus 1. It should be used to check
>> the eventid when a MAPTI command is issued on a device. Let's
>> store the number of eventid bits in the its_device and do the
>> check on MAPTI. Also make sure the ITT size field does
>> not exceed the GITS_TYPER IDBITS field.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>
>> ---
>> v4 -> v5:
>> - its_cmd_get_size macro now returns the actual number of eventid bits
>> - use GIC_ENCODE_SZ macro to encode ID_bits
>>
>> v3 -> v4:
>> - VITS_TYPER_IDBITS set to 16 to be homogeneous with VITS_ESZ definition
>> and correct users
>> - nb_eventid_bits correspond to the actual number of eventid bits
>> ---
>> include/linux/irqchip/arm-gic-v3.h | 2 ++
>> virt/kvm/arm/vgic/vgic-its.c | 15 ++++++++++++++-
>> 2 files changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
>> index 54c20bd..0c6798c 100644
>> --- a/include/linux/irqchip/arm-gic-v3.h
>> +++ b/include/linux/irqchip/arm-gic-v3.h
>> @@ -345,9 +345,11 @@
>> #define E_ITS_INT_UNMAPPED_INTERRUPT 0x010307
>> #define E_ITS_CLEAR_UNMAPPED_INTERRUPT 0x010507
>> #define E_ITS_MAPD_DEVICE_OOR 0x010801
>> +#define E_ITS_MAPD_ITTSIZE_OOR 0x010802
>> #define E_ITS_MAPC_PROCNUM_OOR 0x010902
>> #define E_ITS_MAPC_COLLECTION_OOR 0x010903
>> #define E_ITS_MAPTI_UNMAPPED_DEVICE 0x010a04
>> +#define E_ITS_MAPTI_ID_OOR 0x010a05
>> #define E_ITS_MAPTI_PHYSICALID_OOR 0x010a06
>> #define E_ITS_INV_UNMAPPED_INTERRUPT 0x010c07
>> #define E_ITS_INVALL_UNMAPPED_COLLECTION 0x010d09
>> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
>> index 1b5797e..0f3c8f3 100644
>> --- a/virt/kvm/arm/vgic/vgic-its.c
>> +++ b/virt/kvm/arm/vgic/vgic-its.c
>> @@ -105,6 +105,7 @@ struct its_device {
>>
>> /* the head for the list of ITTEs */
>> struct list_head itt_head;
>> + u32 nb_eventid_bits;
>
> what does nb_ mean?
number ;-)
Eric
>
>> u32 device_id;
>> };
>>
>> @@ -191,6 +192,8 @@ static struct its_ite *find_ite(struct vgic_its *its, u32 device_id,
>>
>> #define GIC_LPI_OFFSET 8192
>>
>> +#define VITS_TYPER_IDBITS 16
>> +
>> /*
>> * Finds and returns a collection in the ITS collection table.
>> * Must be called with the its_lock mutex held.
>> @@ -391,7 +394,7 @@ static unsigned long vgic_mmio_read_its_typer(struct kvm *kvm,
>> * DevBits low - as least for the time being.
>> */
>> reg |= 0x0f << GITS_TYPER_DEVBITS_SHIFT;
>> - reg |= 0x0f << GITS_TYPER_IDBITS_SHIFT;
>> + reg |= GIC_ENCODE_SZ(VITS_TYPER_IDBITS, 5) << GITS_TYPER_IDBITS_SHIFT;
>> reg |= GIC_ENCODE_SZ(abi->ite_esz, 4) << GITS_TYPER_ITT_ENTRY_SIZE_SHIFT;
>>
>> return extract_bytes(reg, addr & 7, len);
>> @@ -562,6 +565,7 @@ static u64 its_cmd_mask_field(u64 *its_cmd, int word, int shift, int size)
>>
>> #define its_cmd_get_command(cmd) its_cmd_mask_field(cmd, 0, 0, 8)
>> #define its_cmd_get_deviceid(cmd) its_cmd_mask_field(cmd, 0, 32, 32)
>> +#define its_cmd_get_size(cmd) (its_cmd_mask_field(cmd, 1, 0, 5) + 1)
>> #define its_cmd_get_id(cmd) its_cmd_mask_field(cmd, 1, 0, 32)
>> #define its_cmd_get_physical_id(cmd) its_cmd_mask_field(cmd, 1, 32, 32)
>> #define its_cmd_get_collection(cmd) its_cmd_mask_field(cmd, 2, 0, 16)
>> @@ -752,6 +756,9 @@ static int vgic_its_cmd_handle_mapi(struct kvm *kvm, struct vgic_its *its,
>> if (!device)
>> return E_ITS_MAPTI_UNMAPPED_DEVICE;
>>
>> + if (event_id >= BIT_ULL(device->nb_eventid_bits))
>> + return E_ITS_MAPTI_ID_OOR;
>> +
>> if (its_cmd_get_command(its_cmd) == GITS_CMD_MAPTI)
>> lpi_nr = its_cmd_get_physical_id(its_cmd);
>> else
>> @@ -832,11 +839,15 @@ static int vgic_its_cmd_handle_mapd(struct kvm *kvm, struct vgic_its *its,
>> {
>> u32 device_id = its_cmd_get_deviceid(its_cmd);
>> bool valid = its_cmd_get_validbit(its_cmd);
>> + u8 nb_eventid_bits = its_cmd_get_size(its_cmd);
>> struct its_device *device;
>>
>> if (!vgic_its_check_id(its, its->baser_device_table, device_id))
>> return E_ITS_MAPD_DEVICE_OOR;
>>
>> + if (valid && nb_eventid_bits > VITS_TYPER_IDBITS)
>> + return E_ITS_MAPD_ITTSIZE_OOR;
>> +
>> device = find_its_device(its, device_id);
>>
>> /*
>> @@ -859,6 +870,8 @@ static int vgic_its_cmd_handle_mapd(struct kvm *kvm, struct vgic_its *its,
>> return -ENOMEM;
>>
>> device->device_id = device_id;
>> + device->nb_eventid_bits = nb_eventid_bits;
>> +
>> INIT_LIST_HEAD(&device->itt_head);
>>
>> list_add_tail(&device->dev_list, &its->device_list);
>> --
>> 2.5.5
>>
>
> Otherwise:
>
> Reviewed-by: Christoffer Dall <cdall@linaro.org>
>
^ permalink raw reply
* [PATCH v5 01/22] KVM: arm/arm64: Add ITS save/restore API documentation
From: Auger Eric @ 2017-04-27 17:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <0551f729-2499-bf2b-7aac-6e56991de373@arm.com>
Hi Marc, Christoffer,
On 27/04/2017 18:23, Marc Zyngier wrote:
> On 27/04/17 16:29, Auger Eric wrote:
>>
>>
>> On 27/04/2017 16:45, Christoffer Dall wrote:
>>> Hi Eric,
>>>
>>> On Thu, Apr 27, 2017 at 02:51:00PM +0200, Auger Eric wrote:
>>>> On 27/04/2017 13:02, Christoffer Dall wrote:
>>>>> On Thu, Apr 27, 2017 at 11:33:39AM +0200, Auger Eric wrote:
>>>>>> On 27/04/2017 10:57, Christoffer Dall wrote:
>>>>>>> On Wed, Apr 26, 2017 at 05:48:32PM +0200, Auger Eric wrote:
>>>>>>>> On 26/04/2017 14:31, Christoffer Dall wrote:
>>>>>>>>> On Fri, Apr 14, 2017 at 12:15:13PM +0200, Eric Auger wrote:
>>>>>>>>>> Add description for how to access ITS registers and how to save/restore
>>>>>>>>>> ITS tables into/from memory.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>>>>>>>>
>>>>>>>>>> ---
>>>>>>>>>> v4 -> v5:
>>>>>>>>>> - take into account Christoffer's comments
>>>>>>>>>> - pending table save on GICV3 side now
>>>>>>>>>>
>>>>>>>>>> v3 -> v4:
>>>>>>>>>> - take into account Peter's comments:
>>>>>>>>>> - typos
>>>>>>>>>> - KVM_DEV_ARM_VGIC_GRP_ITS_TABLES kvm_device_attr = 0
>>>>>>>>>> - add a validity bit in DTE
>>>>>>>>>> - document all fields in CTE and ITE
>>>>>>>>>> - document ABI revision
>>>>>>>>>> - take into account Andre's comments:
>>>>>>>>>> - document restrictions about GITS_CREADR writing and GITS_IIDR
>>>>>>>>>> - document -EBUSY error if one or more VCPUS are runnning
>>>>>>>>>> - document 64b registers only can be accessed with 64b access
>>>>>>>>>> - itt_addr field matches bits [51:8] of the itt_addr
>>>>>>>>>>
>>>>>>>>>> v1 -> v2:
>>>>>>>>>> - DTE and ITE now are 8 bytes
>>>>>>>>>> - DTE and ITE now indexed by deviceid/eventid
>>>>>>>>>> - use ITE name instead of ITTE
>>>>>>>>>> - mentions ITT_addr matches bits [51:8] of the actual address
>>>>>>>>>> - mentions LE layout
>>>>>>>>>> ---
>>>>>>>>>> Documentation/virtual/kvm/devices/arm-vgic-its.txt | 99 ++++++++++++++++++++++
>>>>>>>>>> 1 file changed, 99 insertions(+)
>>>>>>>>>>
>>>>>>>>>> diff --git a/Documentation/virtual/kvm/devices/arm-vgic-its.txt b/Documentation/virtual/kvm/devices/arm-vgic-its.txt
>>>>>>>>>> index 6081a5b..b5f010d 100644
>>>>>>>>>> --- a/Documentation/virtual/kvm/devices/arm-vgic-its.txt
>>>>>>>>>> +++ b/Documentation/virtual/kvm/devices/arm-vgic-its.txt
>>>>>>>>>> @@ -32,7 +32,106 @@ Groups:
>>>>>>>>>> KVM_DEV_ARM_VGIC_CTRL_INIT
>>>>>>>>>> request the initialization of the ITS, no additional parameter in
>>>>>>>>>> kvm_device_attr.addr.
>>>>>>>>>> +
>>>>>>>>>> + KVM_DEV_ARM_ITS_SAVE_TABLES
>>>>>>>>>> + save the ITS table data into guest RAM, at the location provisioned
>>>>>>>>>> + by the guest in corresponding registers/table entries.
>>>>>>>>>> +
>>>>>>>>>> + The layout of the tables in guest memory defines an ABI. The entries
>>>>>>>>>> + are laid out in little endian format as described in the last paragraph.
>>>>>>>>>> +
>>>>>>>>>> + KVM_DEV_ARM_ITS_RESTORE_TABLES
>>>>>>>>>> + restore the ITS tables from guest RAM to ITS internal structures.
>>>>>>>>>> +
>>>>>>>>>> + The GICV3 must be restored before the ITS and all ITS registers but
>>>>>>>>>> + the GITS_CTLR must be restored before restoring the ITS tables.
>>>>>>>>>> +
>>>>>>>>>> + The GITS_IIDR read-only register must also be restored before
>>>>>>>>>> + the table restore as the IIDR revision field encodes the ABI revision.
>>>>>>>>>> +
>>>>>>>>>
>>>>>>>>> what is the expected sequence of operations. For example, to restore
>>>>>>>>> the ITS, do I call KVM_DEV_ARM_VGIC_CTRL_INIT first, then restore all
>>>>>>>>> the memory and registers, and finally call KVM_DEV_ARM_ITS_RESTORE_TABLES?
>>>>>>>> Yes KVM_DEV_ARM_VGIC_CTRL_INIT comes first, then restore all registers
>>>>>>>> except GITS_CTLR, then table restore, then GITS_CTLR
>>>>>>>>>
>>>>>>>>> Is there any interaction between when you call KVM_DEV_ARM_ITS_RESTORE_TABLES
>>>>>>>>> and restore GITS_CTLR (which enables the ITS)?
>>>>>>>>
>>>>>>>> Yep, when GITS_CTLR is set, LPIs may be enabled and this on that event
>>>>>>>> that the pending table is read. But the whole pending table is not read
>>>>>>>> as we only iterate on registered LPIs. So the ITT must have been
>>>>>>>> restored previously.
>>>>>>>>
>>>>>>>> I became aware that the pending table sync is done twice, once in the
>>>>>>>> pending table restore, and once in the GITS_CTLR restore. So if we
>>>>>>>> leave this order specification, I should be able to remove the sync on
>>>>>>>> table restore. This was the original reason why GITS_CTLR restore has
>>>>>>>> been done at the very end.
>>>>>>>
>>>>>>> I'm sorry, I'm a bit confused. Do we not need
>>>>>>> KVM_DEV_ARM_ITS_RESTORE_TABLES at all then?
>>>>>>
>>>>>> Yes you do. I was talking about the RDIST pending table sync. The save
>>>>>> is explicit using GICV3 device KVM_DEV_ARM_VGIC_SAVE_PENDING_TABLES.
>>>>>> However the sync is implicit on GITS_CTLR restore if LPIs are enabled.
>>>>>> and today I do it also on ITS device KVM_DEV_ARM_ITS_RESTORE_TABLES
>>>>>> which is not requested I think since GITS_CTLR restore does it already.
>>>>>
>>>>> Shouldn't restoring the pending tables happen when restoring some
>>>>> redeistributor state and not anything related to the ITS?
>>>>
>>>> Marc wrote:
>>>> "
>>>> I don't think you necessarily need a coarse map. When restoring the ITS
>>>> tables, you can always read the pending bit when creating the LPI
>>>> structure (it has been written to RAM at save time). Note that we
>>>> already do something like this in vgic_enable_lpis().
>>>> "
>>>>
>>>> This is currently what is implemented I think. the pending tables are
>>>> currently sync'ed on GITS_CTLR set (if LPI are enabled) + erroneously
>>>> also on on ITS table restore
>>>>
>>>> The problematic is: Either you know in advance which LPI INTIDare used
>>>> or you need to parse the whole pending table (possibly using the 1st kB
>>>> as coarse mapping).
>>>>
>>>> If you don't know the LPI INTIDs in advance it is only possible to
>>>> restore the pending bit of pending LPIs. At that time you would
>>>> re-allocate those pending LPI (vgic_add_lpi) and when you restore the
>>>> ITS ITT you would do the same for those which were not pending. Looks
>>>> really heavy to me: coarse mapping + dual vgic_add_lpi path.
>>>>
>>>> Otherwise we would need to add another dependency between RDIST pending
>>>> table restore and ITS table restore but this looks even more weird, no?
>>>>
>>>>
>>> So I just sat down with Andre and Marc and we tried to work through this
>>> and came up with the best scheme. I apologize in advance for the
>>> one-way nature of this e-mail, and I am of course open to discussing the
>>> following proposal again if you do not agree.
>>>
>>> What I think this document should say, is that the following ordering
>>> must be followed when restoring the GIC and the ITS:
>>>
>>> First, restore all guest memory
>>>
>>> Second, restore ALL redistributors
>>>
>>> Third, restore the ITS, in the following order:
>>> 1. Initialize the ITS (KVM_DEV_ARM_VGIC_CTRL_INIT)
>>> 2. Restore GITS_CBASER
>>> 3. Restore all other GITS_ registers, except GITS_CTLR!
>>> 4. Load the ITS table data (KVM_DEV_ARM_ITS_RESTORE_TABLES)
>>> 5. Restore GITS_CTLR
>>>
>>> The rationale is that we really want the redistributor and the ITS
>>> restore to be independent and follow the architecture. This means that
>>> our ABI for the redistributor should still work without restoring an ITS
>>> (if we ever decide to support LPIs for KVM without the ITS).
>>
>> OK. Note I already mentioned that GICv3 must be restored before the ITS.
>> To me this comprised the RDIST.
>>
>> I understand the above description of the ordering comes in addition to
>> the existing text, right? in other words I keep the GITS_READR,
>> GITS_IIDR specific text as well as KVM_DEV_ARM_ITS_SAVE/RESTORE_TABLES
>> section.
>>
>>>
>>> In terms of our current implementation this means that vgic_add_lpi()
>>> should ask the redistributor what the state of the LPI is (priority,
>>> enabled, pending).
>> this practically means I move update_lpi_config call from
>> vgic_its_restore_ite to vgic_add_lpi(). OK
>>
>> However for getting the LPI pending state I must know which RDIST the
>> LPI is attached to. This is not known at LPI allocation time. Do I
>> misunderstand something?
>
> Once you have rebuilt the ITS data structures and allocated the IRQ
> structures, you should have a target_cpu field pointing to the right
> vcpu. From there, you can surely find the corresponding redistributor
> and the pending table.
Yes that's understood but Christoffer said "vgic_add_lpi() should ask
the redistributor what the state of the LPI is (priority,enabled,
pending)." Fetching the properties is fine.
vgic_add_lpi() is called before update_affinity_ite() which uses
ite->irq and sets the target_vcpu.
Well at least this requires some function reshape. I will investigate
though.
Thanks
Eric
>
> BTW, we should document the fact that vcpus must have been created
> before reloading the ITS (that's not completely obvious).
>
> Thanks,
>
> M.
>
^ permalink raw reply
* [PATCH] arm64: cpufeature: use static_branch_enable_cpuslocked()
From: Suzuki K Poulose @ 2017-04-27 17:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170427163546.GA30972@e107814-lin.cambridge.arm.com>
On 27/04/17 17:35, Suzuki K Poulose wrote:
> rom f3b0809224e4915197d3ae4a38ebe7f210e74abf Mon Sep 17 00:00:00 2001
> From: Mark Rutland <mark.rutland@arm.com>
> Date: Thu, 27 Apr 2017 16:48:06 +0100
> Subject: [PATCH] arm64: cpufeature: use static_branch_enable_cpuslocked()
>
Build break alert. There are some issues with patch below.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Reported-by: Catalin Marinas <catalin.marinas@arm.com>
> Suggested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Suggested-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Suzuki Poulose <suzuki,poulose@arm.com>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
> arch/arm64/include/asm/cpufeature.h | 5 +++--
> arch/arm64/kernel/cpu_errata.c | 13 ++++++++++++-
> arch/arm64/kernel/cpufeature.c | 5 ++++-
> arch/arm64/kernel/smp.c | 7 +++----
> 4 files changed, 22 insertions(+), 8 deletions(-)
>
> void __init enable_errata_workarounds(void)
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 94b8f7f..62bdab4 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -966,6 +966,7 @@ static void __init setup_elf_hwcaps(const struct arm64_cpu_capabilities *hwcaps)
> cap_set_elf_hwcap(hwcaps);
> }
>
> +/* Should be called with CPU hotplug lock held */
> void update_cpu_capabilities(const struct arm64_cpu_capabilities *caps,
> const char *info)
> {
> @@ -1092,7 +1093,9 @@ void check_local_cpu_capabilities(void)
>
> static void __init setup_feature_capabilities(void)
> {
> - update_cpu_capabilities(arm64_features, "detected feature:");
> + get_online_cpus();
> + update_cpu_capabilities(arm6_features, "detected feature:");
s/arm6_features/arm64_features
And we need the following hunk:
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 62bdab4..19c359a 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1086,7 +1086,7 @@ void check_local_cpu_capabilities(void)
* advertised capabilities.
*/
if (!sys_caps_initialised)
- update_cpu_errata_workarounds();
+ update_secondary_cpu_errata_workarounds();
else
verify_local_cpu_capabilities();
}
Sorry about that.
Suzuki
^ permalink raw reply related
* [PATCH 2/3] iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #74
From: Will Deacon @ 2017-04-27 17:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170427164237.GA7114@leverpostej>
On Thu, Apr 27, 2017 at 05:42:37PM +0100, Mark Rutland wrote:
> On Thu, Apr 27, 2017 at 05:16:23PM +0530, Geetha sowjanya wrote:
> > + /*
> > + * Override the size, for Cavium CN99xx implementations
> > + * which doesn't support the page 1 SMMU register space.
> > + */
> > + cpu_model = read_cpuid_id() & MIDR_CPU_MODEL_MASK;
> > + if (cpu_model == MIDR_THUNDERX_99XX ||
> > + cpu_model == MIDR_BRCM_VULCAN)
> > + size = SZ_64K;
>
> If you're trying to identify an SMMU erratum, identify the SMMU, not the
> CPU it happens to be paired with this time.
>
> There are ID registers in the SMMU you can use to do so.
>
> NAK to using the CPU ID here.
Agreed. I had some off-list discussion with Geetha where we agreed to use
the "silicon ID", which I assumed was the SMMU IIDR register.
Will
^ permalink raw reply
* [PATCH v5 13/22] KVM: arm64: vgic-its: Check the device id matches TYPER DEVBITS range
From: Christoffer Dall @ 2017-04-27 16:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1492164934-988-14-git-send-email-eric.auger@redhat.com>
On Fri, Apr 14, 2017 at 12:15:25PM +0200, Eric Auger wrote:
> On MAPD we currently check the device id can be stored in the device table.
> Let's first check it can be encoded within the range defined by TYPER
> DEVBITS.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>
> ---
>
> v4 -> v5:
> - use GIC_ENCODE_SZ macro
>
> v3 -> v4:
> - VITS_TYPER_DEVBITS set to 16 for homogeneity
> - use BIT_ULL
> ---
> virt/kvm/arm/vgic/vgic-its.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
> index 757598d..de1ed6d 100644
> --- a/virt/kvm/arm/vgic/vgic-its.c
> +++ b/virt/kvm/arm/vgic/vgic-its.c
> @@ -194,6 +194,7 @@ static struct its_ite *find_ite(struct vgic_its *its, u32 device_id,
> #define GIC_LPI_OFFSET 8192
>
> #define VITS_TYPER_IDBITS 16
> +#define VITS_TYPER_DEVBITS 16
>
> /*
> * Finds and returns a collection in the ITS collection table.
> @@ -394,7 +395,7 @@ static unsigned long vgic_mmio_read_its_typer(struct kvm *kvm,
> * To avoid memory waste in the guest, we keep the number of IDBits and
> * DevBits low - as least for the time being.
> */
> - reg |= 0x0f << GITS_TYPER_DEVBITS_SHIFT;
> + reg |= GIC_ENCODE_SZ(VITS_TYPER_DEVBITS, 5) << GITS_TYPER_DEVBITS_SHIFT;
> reg |= GIC_ENCODE_SZ(VITS_TYPER_IDBITS, 5) << GITS_TYPER_IDBITS_SHIFT;
> reg |= GIC_ENCODE_SZ(abi->ite_esz, 4) << GITS_TYPER_ITT_ENTRY_SIZE_SHIFT;
>
> @@ -639,10 +640,10 @@ static int vgic_its_cmd_handle_movi(struct kvm *kvm, struct vgic_its *its,
> * Check whether an ID can be stored into the corresponding guest table.
> * For a direct table this is pretty easy, but gets a bit nasty for
> * indirect tables. We check whether the resulting guest physical address
> - * is actually valid (covered by a memslot and guest accessbible).
> + * is actually valid (covered by a memslot and guest accessible).
> * For this we have to read the respective first level entry.
> */
> -static bool vgic_its_check_id(struct vgic_its *its, u64 baser, int id)
> +static bool vgic_its_check_id(struct vgic_its *its, u64 baser, u32 id)
> {
> int l1_tbl_size = GITS_BASER_NR_PAGES(baser) * SZ_64K;
> int index;
> @@ -650,6 +651,9 @@ static bool vgic_its_check_id(struct vgic_its *its, u64 baser, int id)
> gfn_t gfn;
> int esz = GITS_BASER_ENTRY_SIZE(baser);
>
> + if (id >= BIT_ULL(VITS_TYPER_DEVBITS))
> + return false;
> +
Isn't vgic_its_check_id called with both a device id and a collection
id? How can this then be a valid check?
> if (!(baser & GITS_BASER_INDIRECT)) {
> phys_addr_t addr;
>
> --
> 2.5.5
>
Thanks,
-Christoffer
^ permalink raw reply
* [PATCH] arm64: dts: freescale: update the copyright claims
From: Li Yang @ 2017-04-27 16:47 UTC (permalink / raw)
To: linux-arm-kernel
Update the copyright claims to comply with company policy.
Signed-off-by: Li Yang <leoyang.li@nxp.com>
---
arch/arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts | 2 +-
arch/arm64/boot/dts/freescale/fsl-ls1012a-qds.dts | 2 +-
arch/arm64/boot/dts/freescale/fsl-ls1012a-rdb.dts | 2 +-
arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi | 2 +-
arch/arm64/boot/dts/freescale/fsl-ls1043a-qds.dts | 2 +-
arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts | 2 +-
arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 2 +-
arch/arm64/boot/dts/freescale/fsl-ls1046a-qds.dts | 2 +-
arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts | 2 +-
arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 2 +-
arch/arm64/boot/dts/freescale/fsl-ls2080a-qds.dts | 3 ++-
arch/arm64/boot/dts/freescale/fsl-ls2080a-rdb.dts | 3 ++-
arch/arm64/boot/dts/freescale/fsl-ls2080a-simu.dts | 2 +-
arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi | 2 +-
arch/arm64/boot/dts/freescale/fsl-ls2088a-qds.dts | 3 ++-
arch/arm64/boot/dts/freescale/fsl-ls2088a-rdb.dts | 3 ++-
arch/arm64/boot/dts/freescale/fsl-ls2088a.dtsi | 3 ++-
arch/arm64/boot/dts/freescale/fsl-ls208xa-qds.dtsi | 3 ++-
arch/arm64/boot/dts/freescale/fsl-ls208xa-rdb.dtsi | 3 ++-
arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 3 ++-
20 files changed, 28 insertions(+), 20 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts b/arch/arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts
index 17fae81..7286b1e 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts
@@ -1,7 +1,7 @@
/*
* Device Tree file for Freescale LS1012A Freedom Board.
*
- * Copyright 2016, Freescale Semiconductor
+ * Copyright 2016 Freescale Semiconductor, Inc.
*
* This file is dual-licensed: you can use it either under the terms
* of the GPLv2 or the X11 license, at your option. Note that this dual
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1012a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-ls1012a-qds.dts
index e2a93d5..40ef468 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1012a-qds.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1012a-qds.dts
@@ -1,7 +1,7 @@
/*
* Device Tree file for Freescale LS1012A QDS Board.
*
- * Copyright 2016, Freescale Semiconductor
+ * Copyright 2016 Freescale Semiconductor, Inc.
*
* This file is dual-licensed: you can use it either under the terms
* of the GPLv2 or the X11 license, at your option. Note that this dual
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1012a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls1012a-rdb.dts
index ed77f6b..65d4313 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1012a-rdb.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1012a-rdb.dts
@@ -1,7 +1,7 @@
/*
* Device Tree file for Freescale LS1012A RDB Board.
*
- * Copyright 2016, Freescale Semiconductor
+ * Copyright 2016 Freescale Semiconductor, Inc.
*
* This file is dual-licensed: you can use it either under the terms
* of the GPLv2 or the X11 license, at your option. Note that this dual
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
index b497ac1..ae47156 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
@@ -1,7 +1,7 @@
/*
* Device Tree Include file for Freescale Layerscape-1012A family SoC.
*
- * Copyright 2016, Freescale Semiconductor
+ * Copyright 2016 Freescale Semiconductor, Inc.
*
* This file is dual-licensed: you can use it either under the terms
* of the GPLv2 or the X11 license, at your option. Note that this dual
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-ls1043a-qds.dts
index 0989d63..a5f1306 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1043a-qds.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a-qds.dts
@@ -1,7 +1,7 @@
/*
* Device Tree Include file for Freescale Layerscape-1043A family SoC.
*
- * Copyright 2014-2015, Freescale Semiconductor
+ * Copyright 2014-2015 Freescale Semiconductor, Inc.
*
* Mingkai Hu <Mingkai.hu@freescale.com>
*
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts
index c37110b..d64370e 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a-rdb.dts
@@ -1,7 +1,7 @@
/*
* Device Tree Include file for Freescale Layerscape-1043A family SoC.
*
- * Copyright 2014-2015, Freescale Semiconductor
+ * Copyright 2014-2015 Freescale Semiconductor, Inc.
*
* Mingkai Hu <Mingkai.hu@freescale.com>
*
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
index 45cface..595da1a 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
@@ -1,7 +1,7 @@
/*
* Device Tree Include file for Freescale Layerscape-1043A family SoC.
*
- * Copyright 2014-2015, Freescale Semiconductor
+ * Copyright 2014-2015 Freescale Semiconductor, Inc.
*
* Mingkai Hu <Mingkai.hu@freescale.com>
*
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-ls1046a-qds.dts
index 290e5b0..0effea0 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1046a-qds.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a-qds.dts
@@ -1,7 +1,7 @@
/*
* Device Tree Include file for Freescale Layerscape-1046A family SoC.
*
- * Copyright 2016, Freescale Semiconductor, Inc.
+ * Copyright 2016 Freescale Semiconductor, Inc.
*
* Shaohui Xie <Shaohui.Xie@nxp.com>
*
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts
index d1ccc00..c5361e9 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts
@@ -1,7 +1,7 @@
/*
* Device Tree Include file for Freescale Layerscape-1046A family SoC.
*
- * Copyright 2016, Freescale Semiconductor, Inc.
+ * Copyright 2016 Freescale Semiconductor, Inc.
*
* Mingkai Hu <mingkai.hu@nxp.com>
*
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
index f4b8b7e..d67557f 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
@@ -1,7 +1,7 @@
/*
* Device Tree Include file for Freescale Layerscape-1046A family SoC.
*
- * Copyright 2016, Freescale Semiconductor, Inc.
+ * Copyright 2016 Freescale Semiconductor, Inc.
*
* Mingkai Hu <mingkai.hu@nxp.com>
*
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls2080a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-ls2080a-qds.dts
index c1e76df..ed209cd 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls2080a-qds.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls2080a-qds.dts
@@ -1,7 +1,8 @@
/*
* Device Tree file for Freescale LS2080a QDS Board.
*
- * Copyright (C) 2015-17, Freescale Semiconductor
+ * Copyright 2015-2016 Freescale Semiconductor, Inc.
+ * Copyright 2017 NXP
*
* Abhimanyu Saini <abhimanyu.saini@nxp.com>
* Bhupesh Sharma <bhupesh.sharma@freescale.com>
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls2080a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls2080a-rdb.dts
index 18ad195..67ec3f9 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls2080a-rdb.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls2080a-rdb.dts
@@ -1,7 +1,8 @@
/*
* Device Tree file for Freescale LS2080a RDB Board.
*
- * Copyright (C) 2016-17, Freescale Semiconductor
+ * Copyright 2016 Freescale Semiconductor, Inc.
+ * Copyright 2017 NXP
*
* Abhimanyu Saini <abhimanyu.saini@nxp.com>
* Bhupesh Sharma <bhupesh.sharma@freescale.com>
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls2080a-simu.dts b/arch/arm64/boot/dts/freescale/fsl-ls2080a-simu.dts
index 290604b..3ee718f 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls2080a-simu.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls2080a-simu.dts
@@ -1,7 +1,7 @@
/*
* Device Tree file for Freescale LS2080a software Simulator model
*
- * Copyright (C) 2014-2015, Freescale Semiconductor
+ * Copyright 2014-2015 Freescale Semiconductor, Inc.
*
* Bhupesh Sharma <bhupesh.sharma@freescale.com>
*
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
index 46a26c0..d789c68 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
@@ -1,7 +1,7 @@
/*
* Device Tree Include file for Freescale Layerscape-2080A family SoC.
*
- * Copyright (C) 2014-2016, Freescale Semiconductor
+ * Copyright 2014-2016 Freescale Semiconductor, Inc.
*
* Abhimanyu Saini <abhimanyu.saini@nxp.com>
* Bhupesh Sharma <bhupesh.sharma@freescale.com>
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls2088a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-ls2088a-qds.dts
index ebcd6ee..4a1df5c 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls2088a-qds.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls2088a-qds.dts
@@ -1,7 +1,8 @@
/*
* Device Tree file for Freescale LS2088A QDS Board.
*
- * Copyright (C) 2016-17, Freescale Semiconductor
+ * Copyright 2016 Freescale Semiconductor, Inc.
+ * Copyright 2017 NXP
*
* Abhimanyu Saini <abhimanyu.saini@nxp.com>
*
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls2088a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls2088a-rdb.dts
index 5992dc1..a76d4b4 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls2088a-rdb.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls2088a-rdb.dts
@@ -1,7 +1,8 @@
/*
* Device Tree file for Freescale LS2088A RDB Board.
*
- * Copyright (C) 2016-17, Freescale Semiconductor
+ * Copyright 2016 Freescale Semiconductor, Inc.
+ * Copyright 2017 NXP
*
* Abhimanyu Saini <abhimanyu.saini@nxp.com>
*
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls2088a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls2088a.dtsi
index 33ce404..5c695c6 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls2088a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls2088a.dtsi
@@ -1,7 +1,8 @@
/*
* Device Tree Include file for Freescale Layerscape-2088A family SoC.
*
- * Copyright (C) 2016-17, Freescale Semiconductor
+ * Copyright 2016 Freescale Semiconductor, Inc.
+ * Copyright 2017 NXP
*
* Abhimanyu Saini <abhimanyu.saini@nxp.com>
*
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls208xa-qds.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls208xa-qds.dtsi
index 8b62048..c4282d9 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls208xa-qds.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls208xa-qds.dtsi
@@ -1,7 +1,8 @@
/*
* Device Tree file for Freescale LS2080A QDS Board.
*
- * Copyright (C) 2016-17, Freescale Semiconductor
+ * Copyright 2016 Freescale Semiconductor, Inc.
+ * Copyright 2017 NXP
*
* Abhimanyu Saini <abhimanyu.saini@nxp.com>
*
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls208xa-rdb.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls208xa-rdb.dtsi
index 3737587..9a1d0d2 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls208xa-rdb.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls208xa-rdb.dtsi
@@ -1,7 +1,8 @@
/*
* Device Tree file for Freescale LS2080A RDB Board.
*
- * Copyright (C) 2016-17, Freescale Semiconductor
+ * Copyright 2016 Freescale Semiconductor, Inc.
+ * Copyright 2017 NXP
*
* Abhimanyu Saini <abhimanyu.saini@nxp.com>
*
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
index abb2fff..6e531b9 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
@@ -1,7 +1,8 @@
/*
* Device Tree Include file for Freescale Layerscape-2080A family SoC.
*
- * Copyright (C) 2016-2017, Freescale Semiconductor
+ * Copyright 2016 Freescale Semiconductor, Inc.
+ * Copyright 2017 NXP
*
* Abhimanyu Saini <abhimanyu.saini@nxp.com>
*
--
1.9.0
^ permalink raw reply related
* [PATCH v5 12/22] KVM: arm64: vgic-its: Interpret MAPD ITT_addr field
From: Christoffer Dall @ 2017-04-27 16:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1492164934-988-13-git-send-email-eric.auger@redhat.com>
On Fri, Apr 14, 2017 at 12:15:24PM +0200, Eric Auger wrote:
> Up to now the MAPD ITT_addr had been ignored. We will need it
> for save/restore. Let's record it in the its_device struct.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>
> ---
> v4 -> v5:
> - its_cmd_get_ittaddr macro now returns the actual ITT GPA
>
> v3 -> v4:
> - in vgic_its_cmd_handle_mapd, itt_addr directly is shifted
> - correct ittaddr bitmask to support 48bit GPA
> ---
> virt/kvm/arm/vgic/vgic-its.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
> index 0f3c8f3..757598d 100644
> --- a/virt/kvm/arm/vgic/vgic-its.c
> +++ b/virt/kvm/arm/vgic/vgic-its.c
> @@ -106,6 +106,7 @@ struct its_device {
> /* the head for the list of ITTEs */
> struct list_head itt_head;
> u32 nb_eventid_bits;
> + gpa_t itt_addr;
> u32 device_id;
> };
>
> @@ -569,6 +570,7 @@ static u64 its_cmd_mask_field(u64 *its_cmd, int word, int shift, int size)
> #define its_cmd_get_id(cmd) its_cmd_mask_field(cmd, 1, 0, 32)
> #define its_cmd_get_physical_id(cmd) its_cmd_mask_field(cmd, 1, 32, 32)
> #define its_cmd_get_collection(cmd) its_cmd_mask_field(cmd, 2, 0, 16)
> +#define its_cmd_get_ittaddr(cmd) (its_cmd_mask_field(cmd, 2, 8, 47) << 8)
Why 47? The ITT_addr field is bits [51:8] unless I'm looking at
something wrong?
> #define its_cmd_get_target_addr(cmd) its_cmd_mask_field(cmd, 2, 16, 32)
> #define its_cmd_get_validbit(cmd) its_cmd_mask_field(cmd, 2, 63, 1)
>
> @@ -840,6 +842,7 @@ static int vgic_its_cmd_handle_mapd(struct kvm *kvm, struct vgic_its *its,
> u32 device_id = its_cmd_get_deviceid(its_cmd);
> bool valid = its_cmd_get_validbit(its_cmd);
> u8 nb_eventid_bits = its_cmd_get_size(its_cmd);
> + gpa_t itt_addr = its_cmd_get_ittaddr(its_cmd);
> struct its_device *device;
>
> if (!vgic_its_check_id(its, its->baser_device_table, device_id))
> @@ -871,6 +874,7 @@ static int vgic_its_cmd_handle_mapd(struct kvm *kvm, struct vgic_its *its,
>
> device->device_id = device_id;
> device->nb_eventid_bits = nb_eventid_bits;
> + device->itt_addr = itt_addr;
>
> INIT_LIST_HEAD(&device->itt_head);
>
> --
> 2.5.5
>
Thanks,
-Christoffer
^ permalink raw reply
* [PATCH 2/3] iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #74
From: Mark Rutland @ 2017-04-27 16:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1493293584-20287-3-git-send-email-gakula@caviumnetworks.com>
On Thu, Apr 27, 2017 at 05:16:23PM +0530, Geetha sowjanya wrote:
> + /*
> + * Override the size, for Cavium CN99xx implementations
> + * which doesn't support the page 1 SMMU register space.
> + */
> + cpu_model = read_cpuid_id() & MIDR_CPU_MODEL_MASK;
> + if (cpu_model == MIDR_THUNDERX_99XX ||
> + cpu_model == MIDR_BRCM_VULCAN)
> + size = SZ_64K;
If you're trying to identify an SMMU erratum, identify the SMMU, not the
CPU it happens to be paired with this time.
There are ID registers in the SMMU you can use to do so.
NAK to using the CPU ID here.
Mark.
^ permalink raw reply
* [PATCH v4 00/21] PCI: fix config space memory mappings
From: Khuong Dinh @ 2017-04-27 16:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <745824fc-016a-38b2-01a3-398399d4c80a@huawei.com>
Hi,
They're same before and after applying the patch.
It was tested with X-Gene 1 and X-Gene 2 with DT (Device Tree) and ACPI boot.
X-Gene 1 - DT :
[root@(none) ~]# lspci -s 01:00.0 -v
01:00.0 Ethernet controller: Intel Corporation 82574L Gigabit Network Connection
Subsystem: Intel Corporation Gigabit CT Desktop Adapter
Flags: bus master, fast devsel, latency 0, IRQ 68
Memory at e1800c0000 (32-bit, non-prefetchable) [size=128K]
Memory at e180000000 (32-bit, non-prefetchable) [size=512K]
I/O ports at 1000 [disabled] [size=32]
Memory at e1800e0000 (32-bit, non-prefetchable) [size=16K]
Expansion ROM at e180080000 [disabled] [size=256K]
Capabilities: [c8] Power Management version 2
Capabilities: [d0] MSI: Enable- Count=1/1 Maskable- 64bit+
Capabilities: [e0] Express Endpoint, MSI 00
Capabilities: [a0] MSI-X: Enable+ Count=5 Masked-
Capabilities: [100] Advanced Error Reporting
Capabilities: [140] Device Serial Number 00-1b-21-ff-ff-4f-68-3d
Kernel driver in use: e1000e
X-Gene 1 - ACPI :
[root@(none) ~]# lspci -s 01:00.0 -v
01:00.0 Ethernet controller: Intel Corporation 82574L Gigabit Network Connection
Subsystem: Intel Corporation Gigabit CT Desktop Adapter
Flags: bus master, fast devsel, latency 0, IRQ 117
Memory at e0400c0000 (32-bit, non-prefetchable) [size=128K]
Memory at e040000000 (32-bit, non-prefetchable) [size=512K]
I/O ports at 1000 [disabled] [size=32]
Memory at e0400e0000 (32-bit, non-prefetchable) [size=16K]
Expansion ROM at e040080000 [disabled] [size=256K]
Capabilities: [c8] Power Management version 2
Capabilities: [d0] MSI: Enable- Count=1/1 Maskable- 64bit+
Capabilities: [e0] Express Endpoint, MSI 00
Capabilities: [a0] MSI-X: Enable- Count=5 Masked-
Capabilities: [100] Advanced Error Reporting
Capabilities: [140] Device Serial Number 00-1b-21-ff-ff-4f-68-3d
Kernel driver in use: e1000e
X-Gene 2 - DT :
[root@(none) ~]# lspci -s 0000:01:00.0 -v
0000:01:00.0 Ethernet controller: Intel Corporation 82572EI Gigabit
Ethernet Controller (Copper) (rev 06)
Subsystem: Intel Corporation PRO/1000 PT Desktop Adapter
Flags: bus master, fast devsel, latency 0, IRQ 49
Memory at c120000000 (32-bit, non-prefetchable) [size=128K]
Memory at c120020000 (32-bit, non-prefetchable) [size=128K]
I/O ports at 1000 [disabled] [size=32]
Expansion ROM at c120040000 [disabled] [size=128K]
Capabilities: [c8] Power Management version 2
Capabilities: [d0] MSI: Enable+ Count=1/1 Maskable- 64bit+
Capabilities: [e0] Express Endpoint, MSI 00
Capabilities: [100] Advanced Error Reporting
Capabilities: [140] Device Serial Number 00-1b-21-ff-ff-1e-84-4e
Kernel driver in use: e1000e
X-Gene 2 - ACPI :
[root@(none) ~]# lspci -s 0000:01:00.0 -v
0000:01:00.0 Ethernet controller: Intel Corporation 82572EI Gigabit
Ethernet Controller (Copper) (rev 06)
Subsystem: Intel Corporation PRO/1000 PT Desktop Adapter
Flags: bus master, fast devsel, latency 0, IRQ 93
Memory at c040000000 (32-bit, non-prefetchable) [size=128K]
Memory at c040020000 (32-bit, non-prefetchable) [size=128K]
I/O ports at 1000 [disabled] [size=32]
Expansion ROM at c040040000 [disabled] [size=128K]
Capabilities: [c8] Power Management version 2
Capabilities: [d0] MSI: Enable+ Count=1/1 Maskable- 64bit+
Capabilities: [e0] Express Endpoint, MSI 00
Capabilities: [100] Advanced Error Reporting
Capabilities: [140] Device Serial Number 00-1b-21-ff-ff-1e-84-4e
Kernel driver in use: e1000e
Best regards,
Khuong Dinh
On Wed, Apr 26, 2017 at 6:46 PM, Dongdong Liu <liudongdong3@huawei.com> wrote:
>
>
> ? 2017/4/27 1:24, Jingoo Han ??:
>>
>> On Wednesday, April 26, 2017 6:54 AM, Dongdong Liu wrote;
>>>
>>>
>>> Tested-by: Dongdong Liu <liudongdong3@huawei.com>
>>>
>>> I tested the patchset on HiSilicon ARM64 D05 board.It works ok with 82599
>>> netcard.
>>
>>
>> Thank you for testing these patches. HiSilicon PCIe may use
>> Designware-based
>> PCIe controller. In my opinion, other Designware-based PCIe controller
>> will
>> work properly.
>>
>> To Dongdong Liu, Khuong Dinh, and other people,
>> If possible, can you check the output of 'lspci -v'?
>> If you find something different, please share it with us.
>> Good luck.
>
>
> root@(none)$ ./lspci -v
> 0002:80:00.0 Class 0604: Device 19e5:1610 (rev 01)
> Flags: bus master, fast devsel, latency 0
> Memory at a9e00000 (32-bit, non-prefetchable) [size=64K]
> Bus: primary=80, secondary=81, subordinate=82, sec-latency=0
> I/O behind bridge: 00000000-00001fff
> Memory behind bridge: a8800000-a8ffffff
> Prefetchable memory behind bridge: 00000000a9000000-00000000a9dfffff
> Capabilities: [40] Power Management version 3
> Capabilities: [50] MSI: Enable- Count=1/32 Maskable+ 64bit+
> Capabilities: [70] Express Root Port (Slot-), MSI 00
> Capabilities: [100] Advanced Error Reporting
> Capabilities: [158] #19
> Capabilities: [178] #17
> Kernel driver in use: pcieport
>
> 0002:81:00.0 Class 0200: Device 8086:10fb (rev 01)
> Flags: bus master, fast devsel, latency 0, IRQ 255
> Memory at a9000000 (64-bit, prefetchable) [size=4M]
> I/O ports at 1000 [disabled] [size=32]
> Memory at a9800000 (64-bit, prefetchable) [size=16K]
> Expansion ROM at a8800000 [disabled] [size=4M]
> Capabilities: [40] Power Management version 3
> Capabilities: [50] MSI: Enable- Count=1/1 Maskable+ 64bit+
> Capabilities: [70] MSI-X: Enable+ Count=64 Masked-
> Capabilities: [a0] Express Endpoint, MSI 00
> Capabilities: [e0] Vital Product Data
> Capabilities: [100] Advanced Error Reporting
> Capabilities: [140] Device Serial Number 9c-37-f4-ff-ff-7b-5b-a0
> Capabilities: [150] Alternative Routing-ID Interpretation (ARI)
> Capabilities: [160] Single Root I/O Virtualization (SR-IOV)
> Kernel driver in use: ixgbe
>
> 0002:81:00.1 Class 0200: Device 8086:10fb (rev 01)
> Flags: bus master, fast devsel, latency 0, IRQ 255
> Memory at a9400000 (64-bit, prefetchable) [size=4M]
> I/O ports at 1020 [disabled] [size=32]
> Memory at a9a04000 (64-bit, prefetchable) [size=16K]
> Expansion ROM at a8c00000 [disabled] [size=4M]
> Capabilities: [40] Power Management version 3
> Capabilities: [50] MSI: Enable- Count=1/1 Maskable+ 64bit+
> Capabilities: [70] MSI-X: Enable+ Count=64 Masked-
> Capabilities: [a0] Express Endpoint, MSI 00
> Capabilities: [e0] Vital Product Data
> Capabilities: [100] Advanced Error Reporting
> Capabilities: [140] Device Serial Number 9c-37-f4-ff-ff-7b-5b-a0
> Capabilities: [150] Alternative Routing-ID Interpretation (ARI)
> Capabilities: [160] Single Root I/O Virtualization (SR-IOV)
> Kernel driver in use: ixgbe
>
> 0004:88:00.0 Class 0604: Device 19e5:1610 (rev 01)
> Flags: bus master, fast devsel, latency 0
> Memory at 8a9000000 (32-bit, non-prefetchable) [size=64K]
> Bus: primary=88, secondary=89, subordinate=89, sec-latency=0
> Capabilities: [40] Power Management version 3
> Capabilities: [50] MSI: Enable- Count=1/32 Maskable+ 64bit+
> Capabilities: [70] Express Root Port (Slot-), MSI 00
> Capabilities: [100] Advanced Error Reporting
> Capabilities: [158] #19
> Capabilities: [178] #17
> Kernel driver in use: pcieport
>
> Thanks,
> Dongdong
>>
>>
>> Best regards,
>> Jingoo Han
>>
>>>
>>> Thanks,
>>> Dongdong
>>> ? 2017/4/25 14:40, Jon Masters ??:
>>>>
>>>> On 04/19/2017 12:48 PM, Lorenzo Pieralisi wrote:
>>>>
>>>>> On some platforms (ie ARM/ARM64) ioremap fails to comply with the PCI
>>>>> configuration non-posted write transactions requirement, because it
>>>>> provides a memory mapping that issues "bufferable" or, in PCI terms
>>>>> "posted" write transactions. Likewise, the current pci_remap_iospace()
>>>>> implementation maps the physical address range that the PCI translates
>>>>> to I/O space cycles to virtual address space through pgprot_device()
>>>>> attributes that on eg ARM64 provides a memory mapping issuing
>>>>> posted writes transactions, which is not PCI specifications compliant.
>>>>
>>>>
>>>> Side note that I've pinged all of the ARM server vendors and asked them
>>>> to verify this patch series on their platforms.
>>>>
>>>> Jon.
>>>>
>>>> .
>>>>
>>
>> .
>>
>
--
CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, is
for the sole use of the intended recipient(s) and contains information that
is confidential and proprietary to Applied Micro Circuits Corporation or
its subsidiaries. It is to be used solely for the purpose of furthering the
parties' business relationship. All unauthorized review, use, disclosure or
distribution is prohibited. If you are not the intended recipient, please
contact the sender by reply e-mail and destroy all copies of the original
message.
^ permalink raw reply
* [PATCH v5 01/22] KVM: arm/arm64: Add ITS save/restore API documentation
From: Christoffer Dall @ 2017-04-27 16:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c3dd2ad2-b068-f158-10d7-53e27bd2b1bf@redhat.com>
On Thu, Apr 27, 2017 at 05:29:35PM +0200, Auger Eric wrote:
>
>
> On 27/04/2017 16:45, Christoffer Dall wrote:
> > Hi Eric,
> >
> > On Thu, Apr 27, 2017 at 02:51:00PM +0200, Auger Eric wrote:
> >> On 27/04/2017 13:02, Christoffer Dall wrote:
> >>> On Thu, Apr 27, 2017 at 11:33:39AM +0200, Auger Eric wrote:
> >>>> On 27/04/2017 10:57, Christoffer Dall wrote:
> >>>>> On Wed, Apr 26, 2017 at 05:48:32PM +0200, Auger Eric wrote:
> >>>>>> On 26/04/2017 14:31, Christoffer Dall wrote:
> >>>>>>> On Fri, Apr 14, 2017 at 12:15:13PM +0200, Eric Auger wrote:
> >>>>>>>> Add description for how to access ITS registers and how to save/restore
> >>>>>>>> ITS tables into/from memory.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> >>>>>>>>
> >>>>>>>> ---
> >>>>>>>> v4 -> v5:
> >>>>>>>> - take into account Christoffer's comments
> >>>>>>>> - pending table save on GICV3 side now
> >>>>>>>>
> >>>>>>>> v3 -> v4:
> >>>>>>>> - take into account Peter's comments:
> >>>>>>>> - typos
> >>>>>>>> - KVM_DEV_ARM_VGIC_GRP_ITS_TABLES kvm_device_attr = 0
> >>>>>>>> - add a validity bit in DTE
> >>>>>>>> - document all fields in CTE and ITE
> >>>>>>>> - document ABI revision
> >>>>>>>> - take into account Andre's comments:
> >>>>>>>> - document restrictions about GITS_CREADR writing and GITS_IIDR
> >>>>>>>> - document -EBUSY error if one or more VCPUS are runnning
> >>>>>>>> - document 64b registers only can be accessed with 64b access
> >>>>>>>> - itt_addr field matches bits [51:8] of the itt_addr
> >>>>>>>>
> >>>>>>>> v1 -> v2:
> >>>>>>>> - DTE and ITE now are 8 bytes
> >>>>>>>> - DTE and ITE now indexed by deviceid/eventid
> >>>>>>>> - use ITE name instead of ITTE
> >>>>>>>> - mentions ITT_addr matches bits [51:8] of the actual address
> >>>>>>>> - mentions LE layout
> >>>>>>>> ---
> >>>>>>>> Documentation/virtual/kvm/devices/arm-vgic-its.txt | 99 ++++++++++++++++++++++
> >>>>>>>> 1 file changed, 99 insertions(+)
> >>>>>>>>
> >>>>>>>> diff --git a/Documentation/virtual/kvm/devices/arm-vgic-its.txt b/Documentation/virtual/kvm/devices/arm-vgic-its.txt
> >>>>>>>> index 6081a5b..b5f010d 100644
> >>>>>>>> --- a/Documentation/virtual/kvm/devices/arm-vgic-its.txt
> >>>>>>>> +++ b/Documentation/virtual/kvm/devices/arm-vgic-its.txt
> >>>>>>>> @@ -32,7 +32,106 @@ Groups:
> >>>>>>>> KVM_DEV_ARM_VGIC_CTRL_INIT
> >>>>>>>> request the initialization of the ITS, no additional parameter in
> >>>>>>>> kvm_device_attr.addr.
> >>>>>>>> +
> >>>>>>>> + KVM_DEV_ARM_ITS_SAVE_TABLES
> >>>>>>>> + save the ITS table data into guest RAM, at the location provisioned
> >>>>>>>> + by the guest in corresponding registers/table entries.
> >>>>>>>> +
> >>>>>>>> + The layout of the tables in guest memory defines an ABI. The entries
> >>>>>>>> + are laid out in little endian format as described in the last paragraph.
> >>>>>>>> +
> >>>>>>>> + KVM_DEV_ARM_ITS_RESTORE_TABLES
> >>>>>>>> + restore the ITS tables from guest RAM to ITS internal structures.
> >>>>>>>> +
> >>>>>>>> + The GICV3 must be restored before the ITS and all ITS registers but
> >>>>>>>> + the GITS_CTLR must be restored before restoring the ITS tables.
> >>>>>>>> +
> >>>>>>>> + The GITS_IIDR read-only register must also be restored before
> >>>>>>>> + the table restore as the IIDR revision field encodes the ABI revision.
> >>>>>>>> +
> >>>>>>>
> >>>>>>> what is the expected sequence of operations. For example, to restore
> >>>>>>> the ITS, do I call KVM_DEV_ARM_VGIC_CTRL_INIT first, then restore all
> >>>>>>> the memory and registers, and finally call KVM_DEV_ARM_ITS_RESTORE_TABLES?
> >>>>>> Yes KVM_DEV_ARM_VGIC_CTRL_INIT comes first, then restore all registers
> >>>>>> except GITS_CTLR, then table restore, then GITS_CTLR
> >>>>>>>
> >>>>>>> Is there any interaction between when you call KVM_DEV_ARM_ITS_RESTORE_TABLES
> >>>>>>> and restore GITS_CTLR (which enables the ITS)?
> >>>>>>
> >>>>>> Yep, when GITS_CTLR is set, LPIs may be enabled and this on that event
> >>>>>> that the pending table is read. But the whole pending table is not read
> >>>>>> as we only iterate on registered LPIs. So the ITT must have been
> >>>>>> restored previously.
> >>>>>>
> >>>>>> I became aware that the pending table sync is done twice, once in the
> >>>>>> pending table restore, and once in the GITS_CTLR restore. So if we
> >>>>>> leave this order specification, I should be able to remove the sync on
> >>>>>> table restore. This was the original reason why GITS_CTLR restore has
> >>>>>> been done at the very end.
> >>>>>
> >>>>> I'm sorry, I'm a bit confused. Do we not need
> >>>>> KVM_DEV_ARM_ITS_RESTORE_TABLES at all then?
> >>>>
> >>>> Yes you do. I was talking about the RDIST pending table sync. The save
> >>>> is explicit using GICV3 device KVM_DEV_ARM_VGIC_SAVE_PENDING_TABLES.
> >>>> However the sync is implicit on GITS_CTLR restore if LPIs are enabled.
> >>>> and today I do it also on ITS device KVM_DEV_ARM_ITS_RESTORE_TABLES
> >>>> which is not requested I think since GITS_CTLR restore does it already.
> >>>
> >>> Shouldn't restoring the pending tables happen when restoring some
> >>> redeistributor state and not anything related to the ITS?
> >>
> >> Marc wrote:
> >> "
> >> I don't think you necessarily need a coarse map. When restoring the ITS
> >> tables, you can always read the pending bit when creating the LPI
> >> structure (it has been written to RAM at save time). Note that we
> >> already do something like this in vgic_enable_lpis().
> >> "
> >>
> >> This is currently what is implemented I think. the pending tables are
> >> currently sync'ed on GITS_CTLR set (if LPI are enabled) + erroneously
> >> also on on ITS table restore
> >>
> >> The problematic is: Either you know in advance which LPI INTIDare used
> >> or you need to parse the whole pending table (possibly using the 1st kB
> >> as coarse mapping).
> >>
> >> If you don't know the LPI INTIDs in advance it is only possible to
> >> restore the pending bit of pending LPIs. At that time you would
> >> re-allocate those pending LPI (vgic_add_lpi) and when you restore the
> >> ITS ITT you would do the same for those which were not pending. Looks
> >> really heavy to me: coarse mapping + dual vgic_add_lpi path.
> >>
> >> Otherwise we would need to add another dependency between RDIST pending
> >> table restore and ITS table restore but this looks even more weird, no?
> >>
> >>
> > So I just sat down with Andre and Marc and we tried to work through this
> > and came up with the best scheme. I apologize in advance for the
> > one-way nature of this e-mail, and I am of course open to discussing the
> > following proposal again if you do not agree.
> >
> > What I think this document should say, is that the following ordering
> > must be followed when restoring the GIC and the ITS:
> >
> > First, restore all guest memory
> >
> > Second, restore ALL redistributors
> >
> > Third, restore the ITS, in the following order:
> > 1. Initialize the ITS (KVM_DEV_ARM_VGIC_CTRL_INIT)
> > 2. Restore GITS_CBASER
> > 3. Restore all other GITS_ registers, except GITS_CTLR!
> > 4. Load the ITS table data (KVM_DEV_ARM_ITS_RESTORE_TABLES)
> > 5. Restore GITS_CTLR
> >
> > The rationale is that we really want the redistributor and the ITS
> > restore to be independent and follow the architecture. This means that
> > our ABI for the redistributor should still work without restoring an ITS
> > (if we ever decide to support LPIs for KVM without the ITS).
>
> OK. Note I already mentioned that GICv3 must be restored before the ITS.
> To me this comprised the RDIST.
Possibly, but I think it's good to write out the whole thing so we
clearly understand the flow. That could better be achieved by
correcting my proposed text above to say something like "Second, restore
ALL redistributors to ensure the pending and configuration tables can be
read."
>
> I understand the above description of the ordering comes in addition to
> the existing text, right?
Yes
> in other words I keep the GITS_READR,
> GITS_IIDR specific text as well as KVM_DEV_ARM_ITS_SAVE/RESTORE_TABLES
> section.
>
Yes. But you don't need to do any reading of the pending table on any
of the restore operations.
Btw., I think it's GITS_CREADR, not GITS_READR.
> >
> > In terms of our current implementation this means that vgic_add_lpi()
> > should ask the redistributor what the state of the LPI is (priority,
> > enabled, pending).
> this practically means I move update_lpi_config call from
> vgic_its_restore_ite to vgic_add_lpi(). OK
Pretty much, yes.
Thanks,
-Christoffer
^ permalink raw reply
* [PATCH 2/2] ARM: dts: NSP: Add Thermal Support
From: Eduardo Valentin @ 2017-04-27 16:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1493153351-12698-3-git-send-email-jon.mason@broadcom.com>
On Tue, Apr 25, 2017 at 04:49:11PM -0400, Jon Mason wrote:
> Add thermal support via the ns-thermal driver and create a single
> thermal zone for the entire SoC.
>
> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
Acked-by: Eduardo Valentin <edubezval@gmail.com>
> ---
> arch/arm/boot/dts/bcm-nsp.dtsi | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/arch/arm/boot/dts/bcm-nsp.dtsi b/arch/arm/boot/dts/bcm-nsp.dtsi
> index 832795b..be6fcfb 100644
> --- a/arch/arm/boot/dts/bcm-nsp.dtsi
> +++ b/arch/arm/boot/dts/bcm-nsp.dtsi
> @@ -383,6 +383,12 @@
> <0x3f408 0x04>;
> };
>
> + thermal: thermal at 3f2c0 {
> + compatible = "brcm,ns-thermal";
> + reg = <0x3f2c0 0x10>;
> + #thermal-sensor-cells = <0>;
> + };
> +
> sata_phy: sata_phy at 40100 {
> compatible = "brcm,iproc-nsp-sata-phy";
> reg = <0x40100 0x340>;
> @@ -533,4 +539,24 @@
> brcm,pcie-msi-inten;
> };
> };
> +
> + thermal-zones {
> + cpu-thermal {
> + polling-delay-passive = <0>;
> + polling-delay = <1000>;
> + coefficients = <(-556) 418000>;
> + thermal-sensors = <&thermal>;
> +
> + trips {
> + cpu-crit {
> + temperature = <125000>;
> + hysteresis = <0>;
> + type = "critical";
> + };
> + };
> +
> + cooling-maps {
> + };
> + };
> + };
> };
> --
> 2.7.4
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170427/0f0f5b1a/attachment.sig>
^ permalink raw reply
* [PATCH 0/3] Cavium ThunderX2 SMMUv3 errata workarounds
From: Sunil Kovvuri @ 2017-04-27 16:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170427133910.GB31368@rric.localdomain>
On Thu, Apr 27, 2017 at 7:09 PM, Robert Richter
<robert.richter@cavium.com> wrote:
> On 27.04.17 17:16:21, Geetha sowjanya wrote:
>> From: Geetha <gakula@cavium.com>
>>
>> Cavium CN99xx SMMUv3 implementation has two Silicon Erratas.
>> 1. Errata ID #74
>> SMMU register alias Page 1 is not implemented
>> 2. Errata ID #126
>> SMMU doesnt support unique IRQ lines for gerror, eventq and cmdq-sync
>>
>> The following patchset does software workaround for these two erratas.
>>
>> This series is based on RFC patch.
>> https://www.spinics.net/lists/arm-kernel/msg575739.html
>>
>> As suggested by Will Deacon, code is modified to use silicon id to
>> enable errata#74 workaround.
>
> Can we go with the previous series [1] and:
>
> * drop the iort model numbering part,
>
> * add an enablement function that enables flags (smmu->options)
> depending on midr values (which replaces the macro code)?
I don't see how it is efficient and consistent, if we take data from DT
for non-ACPI mode and read CPU ID from MIDR for ACPI mode.
Thanks,
Sunil.
>
> E.g.:
>
> static void acpi_smmu_enable_cavium(struct arm_smmu_device *smmu)
> {
> u32 cpu_model;
>
> if (!IS_ENABLED(CONFIG_ARM64))
> return;
>
> cpu_model = read_cpuid_id() & MIDR_CPU_MODEL_MASK;
> switch (cpu_model) {
> case ...:
> case ...:
> break;
> default:
> /* No Cavium CN99xx SMMU v3 */
> return;
> }
>
> smmu->options |= (ARM_SMMU_OPT_PAGE0_REGS_ONLY |
> ARM_SMMU_OPT_USE_SHARED_IRQS);
> }
>
> -Robert
>
>
> [1] [RFC PATCH 0/7] Cavium CN99xx SMMUv3 Errata workarounds
> https://marc.info/?l=linux-acpi&m=149192179623708&w=2
>
>>
>> Linu Cherian (1):
>> iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #74
>>
>> Geetha (2):
>> arm64: Add MIDR values for Cavium cn99xx SoCs
>> iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #126
>>
>> Documentation/arm64/silicon-errata.txt | 2 ++
>> arch/arm64/include/asm/cputype.h | 3 ++
>> drivers/acpi/arm64/iort.c | 14 +++++++-
>> drivers/iommu/arm-smmu-v3.c | 64 +++++++++++++++++++++++++++++-----
>> 4 files changed, 73 insertions(+), 10 deletions(-)
>>
>> --
>> 1.9.1
>>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 1/2] thermal: broadcom: Allow for NSP to use ns-thermal driver
From: Eduardo Valentin @ 2017-04-27 16:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1493153351-12698-2-git-send-email-jon.mason@broadcom.com>
Hey Jason,
On Tue, Apr 25, 2017 at 04:49:10PM -0400, Jon Mason wrote:
> Change the iProc Kconfig to select THERMAL and THERMAL_OF, which allows
> the ns-thermal driver to be selected via menuconfig. Also, change the
> ns-thermal driver to work on any iProc based SoC. Finally, tweak the
> Kconfig description to mention support for NSP and make the default on
> for iProc based platforms.
Thanks for the patch, but..
>
> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
> ---
> arch/arm/mach-bcm/Kconfig | 2 ++
> drivers/thermal/broadcom/Kconfig | 9 +++++----
> 2 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
> index a0e66d8..da2bfeb 100644
> --- a/arch/arm/mach-bcm/Kconfig
> +++ b/arch/arm/mach-bcm/Kconfig
> @@ -19,6 +19,8 @@ config ARCH_BCM_IPROC
> select GPIOLIB
> select ARM_AMBA
> select PINCTRL
> + select THERMAL
> + select THERMAL_OF
> help
> This enables support for systems based on Broadcom IPROC architected SoCs.
> The IPROC complex contains one or more ARM CPUs along with common
It would be better if this is split and sent through your arch tree, to
avoid conflicts. I could also pick it if you get an ack from one of your
maintainers. Still, first option is preferable.
> diff --git a/drivers/thermal/broadcom/Kconfig b/drivers/thermal/broadcom/Kconfig
> index f0dea8a..26d706c 100644
> --- a/drivers/thermal/broadcom/Kconfig
> +++ b/drivers/thermal/broadcom/Kconfig
> @@ -1,8 +1,9 @@
> config BCM_NS_THERMAL
> tristate "Northstar thermal driver"
> depends on ARCH_BCM_IPROC || COMPILE_TEST
> + default ARCH_BCM_IPROC
Not sure if this is really what you wanted. Based on your commit log
message, you meant the following, perhaps?
+ default y if ARCH_BCM_IPROC
> help
> - Northstar is a family of SoCs that includes e.g. BCM4708, BCM47081,
> - BCM4709 and BCM47094. It contains DMU (Device Management Unit) block
> - with a thermal sensor that allows checking CPU temperature. This
> - driver provides support for it.
> + Support for the Northstar and Northstar Plus family of SoCs (e.g.
> + BCM4708, BCM4709, BCM5301x, BCM95852X, etc). It contains DMU (Device
Did we look BCM47094 somehow on this patch?
> + Management Unit) block with a thermal sensor that allows checking CPU
> + temperature.
> --
> 2.7.4
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170427/c74ae1b8/attachment.sig>
^ permalink raw reply
* [PATCH] arm64: cpufeature: use static_branch_enable_cpuslocked() (was: Re: [patch V2 00/24] cpu/hotplug: Convert get_online_cpus() to a percpu_rwsem)
From: Suzuki K Poulose @ 2017-04-27 16:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170427154806.GA6646@leverpostej>
On Thu, Apr 27, 2017 at 04:48:06PM +0100, Mark Rutland wrote:
> Hi Catalin/Will,
>
> The below addresses a boot failure Catalin spotted in next-20170424,
> based on Sebastian's patch [1]. I've given it a spin on Juno R1, where I
> can reproduce the issue prior to applying this patch.
>
> I believe this would need to go via tip, as the issue is a result of
> change in the tip smp/hotplug branch, and the fix depends on
> infrastructure introduced there.
>
> Are you happy with the fix, and for it to go via the tip tree?
>
> Thanks,
> Mark.
>
> [1] https://lkml.kernel.org/r/20170425172838.mr3kyccsdteyjso5 at linutronix.de
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/log/?h=smp/hotplug
>
> ---->8----
> From 6cdb503b060f74743769c9f601c35f985d3c58eb Mon Sep 17 00:00:00 2001
> From: Mark Rutland <mark.rutland@arm.com>
> Date: Wed, 26 Apr 2017 09:46:47 +0100
> Subject: [PATCH] arm64: cpufeature: use static_branch_enable_cpuslocked()
>
> Recently, the hotplug locking was conveted to use a percpu rwsem. Unlike
> the existing {get,put}_online_cpus() logic, this can't nest.
> Unfortunately, in arm64's secondary boot path we can end up nesting via
> static_branch_enable() in cpus_set_cap() when we detect an erratum.
>
> This leads to a stream of messages as below, where the secondary
> attempts to schedule before it has been fully onlined. As the CPU
> orchestrating the onlining holds the rswem, this hangs the system.
>
> [ 0.250334] BUG: scheduling while atomic: swapper/1/0/0x00000002
> [ 0.250337] Modules linked in:
> [ 0.250346] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.11.0-rc7-next-20170424 #2
> [ 0.250349] Hardware name: ARM Juno development board (r1) (DT)
> [ 0.250353] Call trace:
> [ 0.250365] [<ffff000008088510>] dump_backtrace+0x0/0x238
> [ 0.250371] [<ffff00000808880c>] show_stack+0x14/0x20
> [ 0.250377] [<ffff00000839d854>] dump_stack+0x9c/0xc0
> [ 0.250384] [<ffff0000080e3540>] __schedule_bug+0x50/0x70
> [ 0.250391] [<ffff000008932ecc>] __schedule+0x52c/0x5a8
> [ 0.250395] [<ffff000008932f80>] schedule+0x38/0xa0
> [ 0.250400] [<ffff000008935e8c>] rwsem_down_read_failed+0xc4/0x108
> [ 0.250407] [<ffff0000080fe8e0>] __percpu_down_read+0x100/0x118
> [ 0.250414] [<ffff0000080c0b60>] get_online_cpus+0x70/0x78
> [ 0.250420] [<ffff0000081749e8>] static_key_enable+0x28/0x48
> [ 0.250425] [<ffff00000808de90>] update_cpu_capabilities+0x78/0xf8
> [ 0.250430] [<ffff00000808d14c>] update_cpu_errata_workarounds+0x1c/0x28
> [ 0.250435] [<ffff00000808e004>] check_local_cpu_capabilities+0xf4/0x128
> [ 0.250440] [<ffff00000808e894>] secondary_start_kernel+0x8c/0x118
> [ 0.250444] [<000000008093d1b4>] 0x8093d1b4
>
> We call cpus_set_cap() from update_cpu_capabilities(), which is called
> from the secondary boot path (where the CPU orchestrating the onlining
> holds the hotplug rwsem), and in the primary boot path, where this is
> not held.
>
> This patch makes cpus_set_cap() use static_branch_enable_cpuslocked(),
> and updates the primary CPU boot path to hold the rwsem so as to keep
> the *_cpuslocked() code happy.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Reported-by: Catalin Marinas <catalin.marinas@arm.com>
> Suggested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Suggested-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Suzuki Poulose <suzuki,poulose@arm.com>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> ---
> arch/arm64/include/asm/cpufeature.h | 2 +-
> arch/arm64/kernel/smp.c | 8 +++++---
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index f31c48d..349b5cd 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -145,7 +145,7 @@ static inline void cpus_set_cap(unsigned int num)
> num, ARM64_NCAPS);
> } else {
> __set_bit(num, cpu_hwcaps);
> - static_branch_enable(&cpu_hwcap_keys[num]);
> + static_branch_enable_cpuslocked(&cpu_hwcap_keys[num]);
> }
> }
>
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index 9b10365..c2ce9aa 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -447,11 +447,13 @@ void __init smp_prepare_boot_cpu(void)
> cpuinfo_store_boot_cpu();
> save_boot_cpu_run_el();
> /*
> - * Run the errata work around checks on the boot CPU, once we have
> - * initialised the cpu feature infrastructure from
> - * cpuinfo_store_boot_cpu() above.
> + * Run the errata work around checks on the boot CPU, now that
> + * cpuinfo_store_boot_cpu() has set things up. We hold the percpu rwsem
> + * to keep the workaround setup code happy.
> */
> + get_online_cpus();
> update_cpu_errata_workarounds();
> + put_online_cpus();
We need similar locking for updat_cpu_capabilities() called
from setup_cpu_feature_capabilities(). i.e., the following fixup is required.
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 94b8f7f..62bdab4 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -966,6 +966,7 @@ static void __init setup_elf_hwcaps(const struct arm64_cpu_capabilities *hwcaps)
cap_set_elf_hwcap(hwcaps);
}
+/* Should be called with CPU hotplug lock held */
void update_cpu_capabilities(const struct arm64_cpu_capabilities *caps,
const char *info)
{
@@ -1092,7 +1093,9 @@ void check_local_cpu_capabilities(void)
static void __init setup_feature_capabilities(void)
{
- update_cpu_capabilities(arm64_features, "detected feature:");
+ get_online_cpus();
+ update_cpu_capabilities(arm6_features, "detected feature:");
+ put_online_cpus();
enable_cpu_capabilities(arm64_features);
}
--
---
Also, I think having update_cpu_errata_workarounds() called with and without
the CPU hotplug lock makes it a bit confusing without proper explanation.
How about the following patch which makes things a bit more reader friendly ?
---8>---
>From f3b0809224e4915197d3ae4a38ebe7f210e74abf Mon Sep 17 00:00:00 2001
From: Mark Rutland <mark.rutland@arm.com>
Date: Thu, 27 Apr 2017 16:48:06 +0100
Subject: [PATCH] arm64: cpufeature: use static_branch_enable_cpuslocked()
Recently, the hotplug locking was conveted to use a percpu rwsem. Unlike
the existing {get,put}_online_cpus() logic, this can't nest.
Unfortunately, in arm64's secondary boot path we can end up nesting via
static_branch_enable() in cpus_set_cap() when we detect an erratum.
This leads to a stream of messages as below, where the secondary
attempts to schedule before it has been fully onlined. As the CPU
orchestrating the onlining holds the rswem, this hangs the system.
[ 0.250334] BUG: scheduling while atomic: swapper/1/0/0x00000002
[ 0.250337] Modules linked in:
[ 0.250346] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.11.0-rc7-next-20170424 #2
[ 0.250349] Hardware name: ARM Juno development board (r1) (DT)
[ 0.250353] Call trace:
[ 0.250365] [<ffff000008088510>] dump_backtrace+0x0/0x238
[ 0.250371] [<ffff00000808880c>] show_stack+0x14/0x20
[ 0.250377] [<ffff00000839d854>] dump_stack+0x9c/0xc0
[ 0.250384] [<ffff0000080e3540>] __schedule_bug+0x50/0x70
[ 0.250391] [<ffff000008932ecc>] __schedule+0x52c/0x5a8
[ 0.250395] [<ffff000008932f80>] schedule+0x38/0xa0
[ 0.250400] [<ffff000008935e8c>] rwsem_down_read_failed+0xc4/0x108
[ 0.250407] [<ffff0000080fe8e0>] __percpu_down_read+0x100/0x118
[ 0.250414] [<ffff0000080c0b60>] get_online_cpus+0x70/0x78
[ 0.250420] [<ffff0000081749e8>] static_key_enable+0x28/0x48
[ 0.250425] [<ffff00000808de90>] update_cpu_capabilities+0x78/0xf8
[ 0.250430] [<ffff00000808d14c>] update_cpu_errata_workarounds+0x1c/0x28
[ 0.250435] [<ffff00000808e004>] check_local_cpu_capabilities+0xf4/0x128
[ 0.250440] [<ffff00000808e894>] secondary_start_kernel+0x8c/0x118
[ 0.250444] [<000000008093d1b4>] 0x8093d1b4
We call cpus_set_cap() from update_cpu_capabilities(), which is called
from the secondary boot path (where the CPU orchestrating the onlining
holds the hotplug rwsem), and in the primary boot path, where this is
not held.
This patch makes cpus_set_cap() use static_branch_enable_cpuslocked(),
and updates all the callers of update_cpu_capabilities() consistent with
the change.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Reported-by: Catalin Marinas <catalin.marinas@arm.com>
Suggested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Suzuki Poulose <suzuki,poulose@arm.com>
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
arch/arm64/include/asm/cpufeature.h | 5 +++--
arch/arm64/kernel/cpu_errata.c | 13 ++++++++++++-
arch/arm64/kernel/cpufeature.c | 5 ++++-
arch/arm64/kernel/smp.c | 7 +++----
4 files changed, 22 insertions(+), 8 deletions(-)
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index e7f84a7..2a832c6 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -145,7 +145,7 @@ static inline void cpus_set_cap(unsigned int num)
num, ARM64_NCAPS);
} else {
__set_bit(num, cpu_hwcaps);
- static_branch_enable(&cpu_hwcap_keys[num]);
+ static_branch_enable_cpuslocked(&cpu_hwcap_keys[num]);
}
}
@@ -222,7 +222,8 @@ void update_cpu_capabilities(const struct arm64_cpu_capabilities *caps,
void enable_cpu_capabilities(const struct arm64_cpu_capabilities *caps);
void check_local_cpu_capabilities(void);
-void update_cpu_errata_workarounds(void);
+void update_secondary_cpu_errata_workarounds(void);
+void update_boot_cpu_errata_workarounds(void);
void __init enable_errata_workarounds(void);
void verify_local_cpu_errata_workarounds(void);
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 2ed2a76..f2d889b 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -190,9 +190,20 @@ void verify_local_cpu_errata_workarounds(void)
}
}
-void update_cpu_errata_workarounds(void)
+/*
+ * Secondary CPUs are booted with the waker holding the
+ * CPU hotplug lock, hence we don't need to lock it here again.
+ */
+void update_secondary_cpu_errata_workarounds(void)
+{
+ update_cpu_capabilities(arm64_errata, "enabling workaround for");
+}
+
+void update_boot_cpu_errata_workarounds(void)
{
+ get_online_cpus();
update_cpu_capabilities(arm64_errata, "enabling workaround for");
+ put_online_cpus();
}
void __init enable_errata_workarounds(void)
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 94b8f7f..62bdab4 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -966,6 +966,7 @@ static void __init setup_elf_hwcaps(const struct arm64_cpu_capabilities *hwcaps)
cap_set_elf_hwcap(hwcaps);
}
+/* Should be called with CPU hotplug lock held */
void update_cpu_capabilities(const struct arm64_cpu_capabilities *caps,
const char *info)
{
@@ -1092,7 +1093,9 @@ void check_local_cpu_capabilities(void)
static void __init setup_feature_capabilities(void)
{
- update_cpu_capabilities(arm64_features, "detected feature:");
+ get_online_cpus();
+ update_cpu_capabilities(arm6_features, "detected feature:");
+ put_online_cpus();
enable_cpu_capabilities(arm64_features);
}
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 6e0e16a..51ba91f 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -450,11 +450,10 @@ void __init smp_prepare_boot_cpu(void)
cpuinfo_store_boot_cpu();
save_boot_cpu_run_el();
/*
- * Run the errata work around checks on the boot CPU, once we have
- * initialised the cpu feature infrastructure from
- * cpuinfo_store_boot_cpu() above.
+ * Run the errata work around checks on the boot CPU, now that
+ * cpuinfo_store_boot_cpu() has set things up.
*/
- update_cpu_errata_workarounds();
+ update_boot_cpu_errata_workarounds();
}
static u64 __init of_get_cpu_mpidr(struct device_node *dn)
--
2.7.4
^ permalink raw reply related
* [PATCH 3/4] arm64: entry: improve data abort handling of tagged pointers
From: Kristina Martsenko @ 2017-04-27 16:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170421182450.GR3750@e103592.cambridge.arm.com>
On 21/04/17 19:24, Dave Martin wrote:
> On Thu, Apr 20, 2017 at 07:17:13PM +0100, Kristina Martsenko wrote:
>> When handling a data abort from EL0, we currently zero the top byte of
>> the faulting address, as we assume the address is a TTBR0 address, which
>> may contain a non-zero address tag. However, the address may be a TTBR1
>> address, in which case we should not zero the top byte. This patch fixes
>> that. The effect is that the full TTBR1 address is passed to the task's
>> signal handler (or printed out in the kernel log).
>>
>> When handling a data abort from EL1, we leave the faulting address
>> intact, as we assume it's either a TTBR1 address or a TTBR0 address with
>> tag 0x00. This is true as far as I'm aware, we don't seem to access a
>> tagged TTBR0 address anywhere in the kernel. Regardless, it's easy to
>> forget about address tags, and code added in the future may not always
>> remember to remove tags from addresses before accessing them. So add tag
>> handling to the EL1 data abort handler as well. This also makes it
>> consistent with the EL0 data abort handler.
>
> Possibly it doesn't matter whether the tag bits are cleared for an EL0
> fault on a TTBR1 address, since userspace can't have a valid pointer in
> this range to (mis)match the fault address against ... or did I miss
> something?
I don't think you've missed anything. But I don't see why userspace
can't match against an invalid (TTBR1) address, I think that would be a
valid thing to do (even if unlikely).
> Factoring out the tag handling makes the intent of the code clearer
> though, either way.
I assume this means you're fine with the patch as is.
Thanks,
Kristina
^ permalink raw reply
* [PATCH 1/4] arm64: traps: fix userspace cache maintenance emulation on a tagged pointer
From: Kristina Martsenko @ 2017-04-27 16:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <f5ef088a-7d95-a43a-843e-064f8e13d2a7@arm.com>
Hi Andre,
On 21/04/17 11:59, Andre Przywara wrote:
> On 20/04/17 19:17, Kristina Martsenko wrote:
>> When we emulate userspace cache maintenance in the kernel, we can
>> currently send the task a SIGSEGV even though the maintenance was done
>> on a valid address. This happens if the address has a non-zero address
>> tag, and happens to not be mapped in.
>>
>> When we get the address from a user register, we don't currently remove
>> the address tag before performing cache maintenance on it. If the
>> maintenance faults, we end up in either __do_page_fault, where find_vma
>> can't find the VMA if the address has a tag, or in do_translation_fault,
>> where the tagged address will appear to be above TASK_SIZE. In both
>> cases, the address is not mapped in, and the task is sent a SIGSEGV.
>
> Right, well spotted!
> So thanks for the patch, which I think is correct. But ...
Thanks for taking a look.
>> This patch removes the tag from the address before using it. With this
>> patch, the fault is handled correctly, the address gets mapped in, and
>> the cache maintenance succeeds.
>
> Looking more closely at this code, I see that we actually don't use the
> address parameter in the force_signal_inject() function. Instead we
> always put the PC address into the siginfo structure, which is wrong in
> case this SEGV is triggered by an invalid address of a cache maintenance
> operation.
I agree this is a bug in existing code, as it means we currently put a
different address into the siginfo structure when we emulate cache
maintenance, compared to when we don't emulate it.
I think the bug is independent of this series though, and the fix should
be sent as a separate patch/series, as it is not blocking this series
and doesn't involve address tags.
> I made a simple patch to fix this (using the address argument and
> explicitly passing the PC in when we fault on an invalid instruction).
Sounds about right to me. Although I noticed that swp emulation also
goes through arm64_notify_segfault. I think the behaviour for swp
emulation should stay the same as in arch/arm/ set_segfault(), i.e.
faulting address used in find_vma, but PC passed in siginfo. Unless
that's a bug in arch/arm/ (for a non-emulated swp, they seem to pass the
faulting address in siginfo instead).
> But now we would pass the untagged address back into userland. I am not
> sure this is a real problem, since we don't promise anything in case of
> tagged pointers, if I got this correctly.
Yes, as documented in Documentation/arm64/tagged-pointers.txt, we do not
guarantee that the tag is preserved when delivering a signal.
> But also our untagged_addr() macro seems to not cover all cases
> correctly, for instance passing in 0x00ffffffffff5678 (which is an
> invalid, but untagged address) would extend to some probably valid
> kernel pointer. And although this would fail our user space address
> check, we would return a wrong address (with all the upper bits being 1)
> in siginfo.
>
> Do we care about this?
> What would be the best fix for the untagged_addr macro? Is that macro
> actually the proper place to fix this issue?
I don't know if we care, but personally I think that if
force_signal_inject is fixed, then untagged_addr should be fixed along
with it. I think the macro is the right place to fix it, by only sign
extending if bit 55 is 0. That way it will turn a tagged address into an
untagged address, and will not change a non-tagged address. (This is
also what clear_address_tag in patch #3 of this series does.)
Thanks,
Kristina
^ permalink raw reply
* [PATCH v5 11/22] KVM: arm64: vgic-its: Interpret MAPD Size field and check related errors
From: Christoffer Dall @ 2017-04-27 16:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1492164934-988-12-git-send-email-eric.auger@redhat.com>
On Fri, Apr 14, 2017 at 12:15:23PM +0200, Eric Auger wrote:
> Up to now the MAPD's ITT size field has been ignored. It encodes
> the number of eventid bit minus 1. It should be used to check
> the eventid when a MAPTI command is issued on a device. Let's
> store the number of eventid bits in the its_device and do the
> check on MAPTI. Also make sure the ITT size field does
> not exceed the GITS_TYPER IDBITS field.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>
> ---
> v4 -> v5:
> - its_cmd_get_size macro now returns the actual number of eventid bits
> - use GIC_ENCODE_SZ macro to encode ID_bits
>
> v3 -> v4:
> - VITS_TYPER_IDBITS set to 16 to be homogeneous with VITS_ESZ definition
> and correct users
> - nb_eventid_bits correspond to the actual number of eventid bits
> ---
> include/linux/irqchip/arm-gic-v3.h | 2 ++
> virt/kvm/arm/vgic/vgic-its.c | 15 ++++++++++++++-
> 2 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
> index 54c20bd..0c6798c 100644
> --- a/include/linux/irqchip/arm-gic-v3.h
> +++ b/include/linux/irqchip/arm-gic-v3.h
> @@ -345,9 +345,11 @@
> #define E_ITS_INT_UNMAPPED_INTERRUPT 0x010307
> #define E_ITS_CLEAR_UNMAPPED_INTERRUPT 0x010507
> #define E_ITS_MAPD_DEVICE_OOR 0x010801
> +#define E_ITS_MAPD_ITTSIZE_OOR 0x010802
> #define E_ITS_MAPC_PROCNUM_OOR 0x010902
> #define E_ITS_MAPC_COLLECTION_OOR 0x010903
> #define E_ITS_MAPTI_UNMAPPED_DEVICE 0x010a04
> +#define E_ITS_MAPTI_ID_OOR 0x010a05
> #define E_ITS_MAPTI_PHYSICALID_OOR 0x010a06
> #define E_ITS_INV_UNMAPPED_INTERRUPT 0x010c07
> #define E_ITS_INVALL_UNMAPPED_COLLECTION 0x010d09
> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
> index 1b5797e..0f3c8f3 100644
> --- a/virt/kvm/arm/vgic/vgic-its.c
> +++ b/virt/kvm/arm/vgic/vgic-its.c
> @@ -105,6 +105,7 @@ struct its_device {
>
> /* the head for the list of ITTEs */
> struct list_head itt_head;
> + u32 nb_eventid_bits;
what does nb_ mean?
> u32 device_id;
> };
>
> @@ -191,6 +192,8 @@ static struct its_ite *find_ite(struct vgic_its *its, u32 device_id,
>
> #define GIC_LPI_OFFSET 8192
>
> +#define VITS_TYPER_IDBITS 16
> +
> /*
> * Finds and returns a collection in the ITS collection table.
> * Must be called with the its_lock mutex held.
> @@ -391,7 +394,7 @@ static unsigned long vgic_mmio_read_its_typer(struct kvm *kvm,
> * DevBits low - as least for the time being.
> */
> reg |= 0x0f << GITS_TYPER_DEVBITS_SHIFT;
> - reg |= 0x0f << GITS_TYPER_IDBITS_SHIFT;
> + reg |= GIC_ENCODE_SZ(VITS_TYPER_IDBITS, 5) << GITS_TYPER_IDBITS_SHIFT;
> reg |= GIC_ENCODE_SZ(abi->ite_esz, 4) << GITS_TYPER_ITT_ENTRY_SIZE_SHIFT;
>
> return extract_bytes(reg, addr & 7, len);
> @@ -562,6 +565,7 @@ static u64 its_cmd_mask_field(u64 *its_cmd, int word, int shift, int size)
>
> #define its_cmd_get_command(cmd) its_cmd_mask_field(cmd, 0, 0, 8)
> #define its_cmd_get_deviceid(cmd) its_cmd_mask_field(cmd, 0, 32, 32)
> +#define its_cmd_get_size(cmd) (its_cmd_mask_field(cmd, 1, 0, 5) + 1)
> #define its_cmd_get_id(cmd) its_cmd_mask_field(cmd, 1, 0, 32)
> #define its_cmd_get_physical_id(cmd) its_cmd_mask_field(cmd, 1, 32, 32)
> #define its_cmd_get_collection(cmd) its_cmd_mask_field(cmd, 2, 0, 16)
> @@ -752,6 +756,9 @@ static int vgic_its_cmd_handle_mapi(struct kvm *kvm, struct vgic_its *its,
> if (!device)
> return E_ITS_MAPTI_UNMAPPED_DEVICE;
>
> + if (event_id >= BIT_ULL(device->nb_eventid_bits))
> + return E_ITS_MAPTI_ID_OOR;
> +
> if (its_cmd_get_command(its_cmd) == GITS_CMD_MAPTI)
> lpi_nr = its_cmd_get_physical_id(its_cmd);
> else
> @@ -832,11 +839,15 @@ static int vgic_its_cmd_handle_mapd(struct kvm *kvm, struct vgic_its *its,
> {
> u32 device_id = its_cmd_get_deviceid(its_cmd);
> bool valid = its_cmd_get_validbit(its_cmd);
> + u8 nb_eventid_bits = its_cmd_get_size(its_cmd);
> struct its_device *device;
>
> if (!vgic_its_check_id(its, its->baser_device_table, device_id))
> return E_ITS_MAPD_DEVICE_OOR;
>
> + if (valid && nb_eventid_bits > VITS_TYPER_IDBITS)
> + return E_ITS_MAPD_ITTSIZE_OOR;
> +
> device = find_its_device(its, device_id);
>
> /*
> @@ -859,6 +870,8 @@ static int vgic_its_cmd_handle_mapd(struct kvm *kvm, struct vgic_its *its,
> return -ENOMEM;
>
> device->device_id = device_id;
> + device->nb_eventid_bits = nb_eventid_bits;
> +
> INIT_LIST_HEAD(&device->itt_head);
>
> list_add_tail(&device->dev_list, &its->device_list);
> --
> 2.5.5
>
Otherwise:
Reviewed-by: Christoffer Dall <cdall@linaro.org>
^ permalink raw reply
* [PATCH v5 01/22] KVM: arm/arm64: Add ITS save/restore API documentation
From: Marc Zyngier @ 2017-04-27 16:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c3dd2ad2-b068-f158-10d7-53e27bd2b1bf@redhat.com>
On 27/04/17 16:29, Auger Eric wrote:
>
>
> On 27/04/2017 16:45, Christoffer Dall wrote:
>> Hi Eric,
>>
>> On Thu, Apr 27, 2017 at 02:51:00PM +0200, Auger Eric wrote:
>>> On 27/04/2017 13:02, Christoffer Dall wrote:
>>>> On Thu, Apr 27, 2017 at 11:33:39AM +0200, Auger Eric wrote:
>>>>> On 27/04/2017 10:57, Christoffer Dall wrote:
>>>>>> On Wed, Apr 26, 2017 at 05:48:32PM +0200, Auger Eric wrote:
>>>>>>> On 26/04/2017 14:31, Christoffer Dall wrote:
>>>>>>>> On Fri, Apr 14, 2017 at 12:15:13PM +0200, Eric Auger wrote:
>>>>>>>>> Add description for how to access ITS registers and how to save/restore
>>>>>>>>> ITS tables into/from memory.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>>>>>>>
>>>>>>>>> ---
>>>>>>>>> v4 -> v5:
>>>>>>>>> - take into account Christoffer's comments
>>>>>>>>> - pending table save on GICV3 side now
>>>>>>>>>
>>>>>>>>> v3 -> v4:
>>>>>>>>> - take into account Peter's comments:
>>>>>>>>> - typos
>>>>>>>>> - KVM_DEV_ARM_VGIC_GRP_ITS_TABLES kvm_device_attr = 0
>>>>>>>>> - add a validity bit in DTE
>>>>>>>>> - document all fields in CTE and ITE
>>>>>>>>> - document ABI revision
>>>>>>>>> - take into account Andre's comments:
>>>>>>>>> - document restrictions about GITS_CREADR writing and GITS_IIDR
>>>>>>>>> - document -EBUSY error if one or more VCPUS are runnning
>>>>>>>>> - document 64b registers only can be accessed with 64b access
>>>>>>>>> - itt_addr field matches bits [51:8] of the itt_addr
>>>>>>>>>
>>>>>>>>> v1 -> v2:
>>>>>>>>> - DTE and ITE now are 8 bytes
>>>>>>>>> - DTE and ITE now indexed by deviceid/eventid
>>>>>>>>> - use ITE name instead of ITTE
>>>>>>>>> - mentions ITT_addr matches bits [51:8] of the actual address
>>>>>>>>> - mentions LE layout
>>>>>>>>> ---
>>>>>>>>> Documentation/virtual/kvm/devices/arm-vgic-its.txt | 99 ++++++++++++++++++++++
>>>>>>>>> 1 file changed, 99 insertions(+)
>>>>>>>>>
>>>>>>>>> diff --git a/Documentation/virtual/kvm/devices/arm-vgic-its.txt b/Documentation/virtual/kvm/devices/arm-vgic-its.txt
>>>>>>>>> index 6081a5b..b5f010d 100644
>>>>>>>>> --- a/Documentation/virtual/kvm/devices/arm-vgic-its.txt
>>>>>>>>> +++ b/Documentation/virtual/kvm/devices/arm-vgic-its.txt
>>>>>>>>> @@ -32,7 +32,106 @@ Groups:
>>>>>>>>> KVM_DEV_ARM_VGIC_CTRL_INIT
>>>>>>>>> request the initialization of the ITS, no additional parameter in
>>>>>>>>> kvm_device_attr.addr.
>>>>>>>>> +
>>>>>>>>> + KVM_DEV_ARM_ITS_SAVE_TABLES
>>>>>>>>> + save the ITS table data into guest RAM, at the location provisioned
>>>>>>>>> + by the guest in corresponding registers/table entries.
>>>>>>>>> +
>>>>>>>>> + The layout of the tables in guest memory defines an ABI. The entries
>>>>>>>>> + are laid out in little endian format as described in the last paragraph.
>>>>>>>>> +
>>>>>>>>> + KVM_DEV_ARM_ITS_RESTORE_TABLES
>>>>>>>>> + restore the ITS tables from guest RAM to ITS internal structures.
>>>>>>>>> +
>>>>>>>>> + The GICV3 must be restored before the ITS and all ITS registers but
>>>>>>>>> + the GITS_CTLR must be restored before restoring the ITS tables.
>>>>>>>>> +
>>>>>>>>> + The GITS_IIDR read-only register must also be restored before
>>>>>>>>> + the table restore as the IIDR revision field encodes the ABI revision.
>>>>>>>>> +
>>>>>>>>
>>>>>>>> what is the expected sequence of operations. For example, to restore
>>>>>>>> the ITS, do I call KVM_DEV_ARM_VGIC_CTRL_INIT first, then restore all
>>>>>>>> the memory and registers, and finally call KVM_DEV_ARM_ITS_RESTORE_TABLES?
>>>>>>> Yes KVM_DEV_ARM_VGIC_CTRL_INIT comes first, then restore all registers
>>>>>>> except GITS_CTLR, then table restore, then GITS_CTLR
>>>>>>>>
>>>>>>>> Is there any interaction between when you call KVM_DEV_ARM_ITS_RESTORE_TABLES
>>>>>>>> and restore GITS_CTLR (which enables the ITS)?
>>>>>>>
>>>>>>> Yep, when GITS_CTLR is set, LPIs may be enabled and this on that event
>>>>>>> that the pending table is read. But the whole pending table is not read
>>>>>>> as we only iterate on registered LPIs. So the ITT must have been
>>>>>>> restored previously.
>>>>>>>
>>>>>>> I became aware that the pending table sync is done twice, once in the
>>>>>>> pending table restore, and once in the GITS_CTLR restore. So if we
>>>>>>> leave this order specification, I should be able to remove the sync on
>>>>>>> table restore. This was the original reason why GITS_CTLR restore has
>>>>>>> been done at the very end.
>>>>>>
>>>>>> I'm sorry, I'm a bit confused. Do we not need
>>>>>> KVM_DEV_ARM_ITS_RESTORE_TABLES at all then?
>>>>>
>>>>> Yes you do. I was talking about the RDIST pending table sync. The save
>>>>> is explicit using GICV3 device KVM_DEV_ARM_VGIC_SAVE_PENDING_TABLES.
>>>>> However the sync is implicit on GITS_CTLR restore if LPIs are enabled.
>>>>> and today I do it also on ITS device KVM_DEV_ARM_ITS_RESTORE_TABLES
>>>>> which is not requested I think since GITS_CTLR restore does it already.
>>>>
>>>> Shouldn't restoring the pending tables happen when restoring some
>>>> redeistributor state and not anything related to the ITS?
>>>
>>> Marc wrote:
>>> "
>>> I don't think you necessarily need a coarse map. When restoring the ITS
>>> tables, you can always read the pending bit when creating the LPI
>>> structure (it has been written to RAM at save time). Note that we
>>> already do something like this in vgic_enable_lpis().
>>> "
>>>
>>> This is currently what is implemented I think. the pending tables are
>>> currently sync'ed on GITS_CTLR set (if LPI are enabled) + erroneously
>>> also on on ITS table restore
>>>
>>> The problematic is: Either you know in advance which LPI INTIDare used
>>> or you need to parse the whole pending table (possibly using the 1st kB
>>> as coarse mapping).
>>>
>>> If you don't know the LPI INTIDs in advance it is only possible to
>>> restore the pending bit of pending LPIs. At that time you would
>>> re-allocate those pending LPI (vgic_add_lpi) and when you restore the
>>> ITS ITT you would do the same for those which were not pending. Looks
>>> really heavy to me: coarse mapping + dual vgic_add_lpi path.
>>>
>>> Otherwise we would need to add another dependency between RDIST pending
>>> table restore and ITS table restore but this looks even more weird, no?
>>>
>>>
>> So I just sat down with Andre and Marc and we tried to work through this
>> and came up with the best scheme. I apologize in advance for the
>> one-way nature of this e-mail, and I am of course open to discussing the
>> following proposal again if you do not agree.
>>
>> What I think this document should say, is that the following ordering
>> must be followed when restoring the GIC and the ITS:
>>
>> First, restore all guest memory
>>
>> Second, restore ALL redistributors
>>
>> Third, restore the ITS, in the following order:
>> 1. Initialize the ITS (KVM_DEV_ARM_VGIC_CTRL_INIT)
>> 2. Restore GITS_CBASER
>> 3. Restore all other GITS_ registers, except GITS_CTLR!
>> 4. Load the ITS table data (KVM_DEV_ARM_ITS_RESTORE_TABLES)
>> 5. Restore GITS_CTLR
>>
>> The rationale is that we really want the redistributor and the ITS
>> restore to be independent and follow the architecture. This means that
>> our ABI for the redistributor should still work without restoring an ITS
>> (if we ever decide to support LPIs for KVM without the ITS).
>
> OK. Note I already mentioned that GICv3 must be restored before the ITS.
> To me this comprised the RDIST.
>
> I understand the above description of the ordering comes in addition to
> the existing text, right? in other words I keep the GITS_READR,
> GITS_IIDR specific text as well as KVM_DEV_ARM_ITS_SAVE/RESTORE_TABLES
> section.
>
>>
>> In terms of our current implementation this means that vgic_add_lpi()
>> should ask the redistributor what the state of the LPI is (priority,
>> enabled, pending).
> this practically means I move update_lpi_config call from
> vgic_its_restore_ite to vgic_add_lpi(). OK
>
> However for getting the LPI pending state I must know which RDIST the
> LPI is attached to. This is not known at LPI allocation time. Do I
> misunderstand something?
Once you have rebuilt the ITS data structures and allocated the IRQ
structures, you should have a target_cpu field pointing to the right
vcpu. From there, you can surely find the corresponding redistributor
and the pending table.
BTW, we should document the fact that vcpus must have been created
before reloading the ITS (that's not completely obvious).
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox