* [PATCH v2] arm64: KVM: pmu: Reset PMSELR_EL0.SEL to a sane value before entering the guest
From: Marc Zyngier @ 2016-12-06 17:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161206162918.GE4816@cbox>
On 06/12/16 16:29, Christoffer Dall wrote:
> On Tue, Dec 06, 2016 at 02:56:50PM +0000, Marc Zyngier wrote:
>> The ARMv8 architecture allows the cycle counter to be configured
>> by setting PMSELR_EL0.SEL==0x1f and then accessing PMXEVTYPER_EL0,
>> hence accessing PMCCFILTR_EL0. But it disallows the use of
>> PMSELR_EL0.SEL==0x1f to access the cycle counter itself through
>> PMXEVCNTR_EL0.
>>
>> Linux itself doesn't violate this rule, but we may end up with
>> PMSELR_EL0.SEL being set to 0x1f when we enter a guest. If that
>> guest accesses PMXEVCNTR_EL0, the access may UNDEF at EL1,
>> despite the guest not having done anything wrong.
>>
>> In order to avoid this unfortunate course of events (haha!), let's
>
> I actually did find this funny.
>
>> sanitize PMSELR_EL0 on guest entry. This ensures that the guest
>> won't explode unexpectedly.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>> This is another approach to fix this issue, this time nuking PMSELR_EL0
>> on guest entry instead of relying on perf not to clobber the register.
>>
>> Tested on v4.9-rc8 with a Rev A3 X-Gene.
>>
>> arch/arm64/kvm/hyp/switch.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
>> index 83037cd..3b7cfbd 100644
>> --- a/arch/arm64/kvm/hyp/switch.c
>> +++ b/arch/arm64/kvm/hyp/switch.c
>> @@ -85,7 +85,13 @@ static void __hyp_text __activate_traps(struct kvm_vcpu *vcpu)
>> write_sysreg(val, hcr_el2);
>> /* Trap on AArch32 cp15 c15 accesses (EL1 or EL0) */
>> write_sysreg(1 << 15, hstr_el2);
>> - /* Make sure we trap PMU access from EL0 to EL2 */
>> + /*
>> + * Make sure we trap PMU access from EL0 to EL2. Also sanitize
>> + * PMSELR_EL0 to make sure it never contains the cycle
>> + * counter, which could make a PMXEVCNTR_EL0 access UNDEF.
>> + */
>> + if (vcpu->arch.mdcr_el2 & MDCR_EL2_HPMN_MASK)
>> + write_sysreg(0, pmselr_el0);
>
> I'm a bit confused about how we use the HPMN field. This value is
> always set to the full number of counters available on the system and
> never modified by the guest, right? So this is in essence a check that
> says 'do you have any performance counters, then make sure accesses
> don't undef to el1 instead of trapping to el2', but then my question is,
> why not just set pmselr_el0 to zero unconditionally, because in the case
> where (vcpu->arch.mdcr_el2 & MDCR_EL2_HPMN_MASK) == 0, it means we have
> no counters, which we'll have exposed to the guest anyhow, and we should
> undef at el1 in the guest, or did I get this completely wrong (like
> everything else today)?
Yeah, that's probably the best course of action. If the guest does
something silly, tough. I'll drop the test and repost the thing.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCHv4 08/10] mm/kasan: Switch to using __pa_symbol and lm_alias
From: Mark Rutland @ 2016-12-06 17:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2f3ac043-c4cc-5c5a-8ac7-1396b6bb193f@virtuozzo.com>
On Thu, Dec 01, 2016 at 02:36:05PM +0300, Andrey Ryabinin wrote:
> On 11/29/2016 09:55 PM, Laura Abbott wrote:
> > __pa_symbol is the correct API to find the physical address of symbols.
> > Switch to it to allow for debugging APIs to work correctly.
>
> But __pa() is correct for symbols. I see how __pa_symbol() might be a little
> faster than __pa(), but there is nothing wrong in using __pa() on symbols.
While it's true today that __pa() works on symbols, this is for
pragmatic reasons (allowing existing code to work until it is all
cleaned up), and __pa_symbol() is the correct API to use.
Relying on this means that __pa() can't be optimised for the (vastly
common) case of translating linear map addresses.
Consistent use of __pa_symbol() will allow for subsequent optimisation
of __pa() in the common case, in adition to being necessary for arm64's
DEBUG_VIRTUAL.
> > Other functions such as p*d_populate may call __pa internally.
> > Ensure that the address passed is in the linear region by calling
> > lm_alias.
>
> Why it should be linear mapping address? __pa() translates kernel
> image address just fine.
As above, while that's true today, but is something that we wish to
change.
> Generated code is probably worse too.
Even if that is the case, given this is code run once at boot on a debug
build, I think it's outweighed by the gain from DEBUG_VIRTUAL, and as a
step towards optimising __pa().
Thanks,
Mark.
^ permalink raw reply
* [RFC v3 03/10] iommu: Add new reserved IOMMU attributes
From: Robin Murphy @ 2016-12-06 17:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479215363-2898-4-git-send-email-eric.auger@redhat.com>
On 15/11/16 13:09, Eric Auger wrote:
> IOMMU_RESV_NOMAP is used to tag reserved IOVAs that are not
> supposed to be IOMMU mapped. IOMMU_RESV_MSI tags IOVAs
> corresponding to MSIs that need to be IOMMU mapped.
>
> IOMMU_RESV_MASK allows to check if the IOVA is reserved.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
> include/linux/iommu.h | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 7f6ebd0..02cf565 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -32,6 +32,10 @@
> #define IOMMU_NOEXEC (1 << 3)
> #define IOMMU_MMIO (1 << 4) /* e.g. things like MSI doorbells */
>
> +#define IOMMU_RESV_MASK 0x300 /* Reserved IOVA mask */
> +#define IOMMU_RESV_NOMAP (1 << 8) /* IOVA that cannot be mapped */
> +#define IOMMU_RESV_MSI (1 << 9) /* MSI region transparently mapped */
It feels a bit grotty encoding these in prot - NOMAP sort of fits, but
MSI really is something else entirely. On reflection I think a separate
iommu_resv_region::type field would be better, particularly as it's
something we might potentially want to expose via the sysfs entry.
Robin.
> +
> struct iommu_ops;
> struct iommu_group;
> struct bus_type;
>
^ permalink raw reply
* [PATCH 2/5] ARM: BCM5301X: Specify USB controllers in DT
From: Ray Jui @ 2016-12-06 17:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161206171714.22738-2-zajec5@gmail.com>
On 12/6/2016 9:17 AM, Rafa? Mi?ecki wrote:
> From: Rafa? Mi?ecki <rafal@milecki.pl>
>
> There are 3 separated controllers, one per USB /standard/. With PHY
> drivers in place they can be simply supported with generic drivers.
>
> Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
> ---
> arch/arm/boot/dts/bcm5301x.dtsi | 33 ++++++++++++++++++++++++++++++++-
> 1 file changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi
> index f09a2bb..bfc98d19 100644
> --- a/arch/arm/boot/dts/bcm5301x.dtsi
> +++ b/arch/arm/boot/dts/bcm5301x.dtsi
> @@ -248,8 +248,26 @@
>
> #address-cells = <1>;
> #size-cells = <1>;
> + ranges;
>
> - phys = <&usb2_phy>;
> + interrupt-parent = <&gic>;
> +
> + ohci: ohci at 21000 {
> + #usb-cells = <0>;
> +
> + compatible = "generic-ohci";
> + reg = <0x00022000 0x1000>;
Your label ohci at 21000 does not match the 'reg' at 0x22000.
> + interrupts = <GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH>;
> + };
> +
> + ehci: ehci at 22000 {
> + #usb-cells = <0>;
> +
> + compatible = "generic-ehci";
> + reg = <0x00021000 0x1000>;
Looks like you got the label of ohci and ehci reversed?
> + interrupts = <GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH>;
> + phys = <&usb2_phy>;
> + };
> };
>
> usb3: usb3 at 23000 {
> @@ -257,6 +275,19 @@
>
> #address-cells = <1>;
> #size-cells = <1>;
> + ranges;
> +
> + interrupt-parent = <&gic>;
> +
> + xhci: xhci at 23000 {
> + #usb-cells = <0>;
> +
> + compatible = "generic-xhci";
> + reg = <0x00023000 0x1000>;
> + interrupts = <GIC_SPI 80 IRQ_TYPE_LEVEL_HIGH>;
> + phys = <&usb3_phy>;
> + phy-names = "usb";
> + };
> };
>
> spi at 29000 {
>
^ permalink raw reply
* [PATCH RFC] drm/sun4i: rgb: Add 5% tolerance to dot clock frequency check
From: Maxime Ripard @ 2016-12-06 17:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161124112231.4297-1-wens@csie.org>
On Thu, Nov 24, 2016 at 07:22:31PM +0800, Chen-Yu Tsai wrote:
> The panels shipped with Allwinner devices are very "generic", i.e.
> they do not have model numbers or reliable sources of information
> for the timings (that we know of) other than the fex files shipped
> on them. The dot clock frequency provided in the fex files have all
> been rounded to the nearest MHz, as that is the unit used in them.
>
> We were using the simple panel "urt,umsh-8596md-t" as a substitute
> for the A13 Q8 tablets in the absence of a specific model for what
> may be many different but otherwise timing compatible panels. This
> was usable without any visual artifacts or side effects, until the
> dot clock rate check was added in commit bb43d40d7c83 ("drm/sun4i:
> rgb: Validate the clock rate").
>
> The reason this check fails is because the dotclock frequency for
> this model is 33.26 MHz, which is not achievable with our dot clock
> hardware, and the rate returned by clk_round_rate deviates slightly,
> causing the driver to reject the display mode.
>
> The LCD panels have some tolerance on the dot clock frequency, even
> if it's not specified in their datasheets.
>
> This patch adds a 5% tolerence to the dot clock check.
As we discussed already, I really believe this is just as arbitrary as
the current behaviour.
Some panels require an exact frequency, some have a minimal frequency
but no maximum, some have a maximum frequency but no minimal, and I
guess most of them deviates by how much exactly they can take (and
possibly can take more easily a higher frequency, but are less
tolerant if you take a frequency lower than the nominal.
And we cannot remove that check entirely, since some bridges will
report out of range frequencies for higher modes that we know we
cannot reach.
We could just try to see if the screen pixel clock frequency is out of
the pixel clock range we can generate, but then we will loop back on
how much out of range is it exactly, and is it within the screen
tolerancy.
We have an API to deal with the panel tolerancies in the DRM panel
framework, we can (and should) use it.
I'm not sure how others usually deal with this though. I think I
remember Eric telling me that for the RPi they just adjusted the
timings a bit, but they only really had a single panel to deal with.
Daniel, Eric, Laurent, Sean? Any ideas?
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161206/ca91b559/attachment.sig>
^ permalink raw reply
* [RFC v3 02/10] iommu: Rename iommu_dm_regions into iommu_resv_regions
From: Robin Murphy @ 2016-12-06 17:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479215363-2898-3-git-send-email-eric.auger@redhat.com>
On 15/11/16 13:09, Eric Auger wrote:
> We want to extend the callbacks used for dm regions and
> use them for reserved regions. Reserved regions can be
> - directly mapped regions
> - regions that cannot be iommu mapped (PCI host bridge windows, ...)
> - MSI regions (because they belong to another address space or because
> they are not translated by the IOMMU and need special handling)
>
> So let's rename the struct and also the callbacks.
Acked-by: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
> drivers/iommu/amd_iommu.c | 20 ++++++++++----------
> drivers/iommu/iommu.c | 22 +++++++++++-----------
> include/linux/iommu.h | 29 +++++++++++++++--------------
> 3 files changed, 36 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> index 754595e..a6c351d 100644
> --- a/drivers/iommu/amd_iommu.c
> +++ b/drivers/iommu/amd_iommu.c
> @@ -3159,8 +3159,8 @@ static bool amd_iommu_capable(enum iommu_cap cap)
> return false;
> }
>
> -static void amd_iommu_get_dm_regions(struct device *dev,
> - struct list_head *head)
> +static void amd_iommu_get_resv_regions(struct device *dev,
> + struct list_head *head)
> {
> struct unity_map_entry *entry;
> int devid;
> @@ -3170,7 +3170,7 @@ static void amd_iommu_get_dm_regions(struct device *dev,
> return;
>
> list_for_each_entry(entry, &amd_iommu_unity_map, list) {
> - struct iommu_dm_region *region;
> + struct iommu_resv_region *region;
>
> if (devid < entry->devid_start || devid > entry->devid_end)
> continue;
> @@ -3193,18 +3193,18 @@ static void amd_iommu_get_dm_regions(struct device *dev,
> }
> }
>
> -static void amd_iommu_put_dm_regions(struct device *dev,
> +static void amd_iommu_put_resv_regions(struct device *dev,
> struct list_head *head)
> {
> - struct iommu_dm_region *entry, *next;
> + struct iommu_resv_region *entry, *next;
>
> list_for_each_entry_safe(entry, next, head, list)
> kfree(entry);
> }
>
> -static void amd_iommu_apply_dm_region(struct device *dev,
> +static void amd_iommu_apply_resv_region(struct device *dev,
> struct iommu_domain *domain,
> - struct iommu_dm_region *region)
> + struct iommu_resv_region *region)
> {
> struct dma_ops_domain *dma_dom = to_dma_ops_domain(to_pdomain(domain));
> unsigned long start, end;
> @@ -3228,9 +3228,9 @@ static void amd_iommu_apply_dm_region(struct device *dev,
> .add_device = amd_iommu_add_device,
> .remove_device = amd_iommu_remove_device,
> .device_group = amd_iommu_device_group,
> - .get_dm_regions = amd_iommu_get_dm_regions,
> - .put_dm_regions = amd_iommu_put_dm_regions,
> - .apply_dm_region = amd_iommu_apply_dm_region,
> + .get_resv_regions = amd_iommu_get_resv_regions,
> + .put_resv_regions = amd_iommu_put_resv_regions,
> + .apply_resv_region = amd_iommu_apply_resv_region,
> .pgsize_bitmap = AMD_IOMMU_PGSIZES,
> };
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 9a2f196..c7ed334 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -318,7 +318,7 @@ static int iommu_group_create_direct_mappings(struct iommu_group *group,
> struct device *dev)
> {
> struct iommu_domain *domain = group->default_domain;
> - struct iommu_dm_region *entry;
> + struct iommu_resv_region *entry;
> struct list_head mappings;
> unsigned long pg_size;
> int ret = 0;
> @@ -331,14 +331,14 @@ static int iommu_group_create_direct_mappings(struct iommu_group *group,
> pg_size = 1UL << __ffs(domain->pgsize_bitmap);
> INIT_LIST_HEAD(&mappings);
>
> - iommu_get_dm_regions(dev, &mappings);
> + iommu_get_resv_regions(dev, &mappings);
>
> /* We need to consider overlapping regions for different devices */
> list_for_each_entry(entry, &mappings, list) {
> dma_addr_t start, end, addr;
>
> - if (domain->ops->apply_dm_region)
> - domain->ops->apply_dm_region(dev, domain, entry);
> + if (domain->ops->apply_resv_region)
> + domain->ops->apply_resv_region(dev, domain, entry);
>
> start = ALIGN(entry->start, pg_size);
> end = ALIGN(entry->start + entry->length, pg_size);
> @@ -358,7 +358,7 @@ static int iommu_group_create_direct_mappings(struct iommu_group *group,
> }
>
> out:
> - iommu_put_dm_regions(dev, &mappings);
> + iommu_put_resv_regions(dev, &mappings);
>
> return ret;
> }
> @@ -1546,20 +1546,20 @@ int iommu_domain_set_attr(struct iommu_domain *domain,
> }
> EXPORT_SYMBOL_GPL(iommu_domain_set_attr);
>
> -void iommu_get_dm_regions(struct device *dev, struct list_head *list)
> +void iommu_get_resv_regions(struct device *dev, struct list_head *list)
> {
> const struct iommu_ops *ops = dev->bus->iommu_ops;
>
> - if (ops && ops->get_dm_regions)
> - ops->get_dm_regions(dev, list);
> + if (ops && ops->get_resv_regions)
> + ops->get_resv_regions(dev, list);
> }
>
> -void iommu_put_dm_regions(struct device *dev, struct list_head *list)
> +void iommu_put_resv_regions(struct device *dev, struct list_head *list)
> {
> const struct iommu_ops *ops = dev->bus->iommu_ops;
>
> - if (ops && ops->put_dm_regions)
> - ops->put_dm_regions(dev, list);
> + if (ops && ops->put_resv_regions)
> + ops->put_resv_regions(dev, list);
> }
>
> /* Request that a device is direct mapped by the IOMMU */
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 436dc21..7f6ebd0 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -118,13 +118,13 @@ enum iommu_attr {
> };
>
> /**
> - * struct iommu_dm_region - descriptor for a direct mapped memory region
> + * struct iommu_resv_region - descriptor for a reserved memory region
> * @list: Linked list pointers
> * @start: System physical start address of the region
> * @length: Length of the region in bytes
> * @prot: IOMMU Protection flags (READ/WRITE/...)
> */
> -struct iommu_dm_region {
> +struct iommu_resv_region {
> struct list_head list;
> phys_addr_t start;
> size_t length;
> @@ -150,9 +150,9 @@ struct iommu_dm_region {
> * @device_group: find iommu group for a particular device
> * @domain_get_attr: Query domain attributes
> * @domain_set_attr: Change domain attributes
> - * @get_dm_regions: Request list of direct mapping requirements for a device
> - * @put_dm_regions: Free list of direct mapping requirements for a device
> - * @apply_dm_region: Temporary helper call-back for iova reserved ranges
> + * @get_resv_regions: Request list of reserved regions for a device
> + * @put_resv_regions: Free list of reserved regions for a device
> + * @apply_resv_region: Temporary helper call-back for iova reserved ranges
> * @domain_window_enable: Configure and enable a particular window for a domain
> * @domain_window_disable: Disable a particular window for a domain
> * @domain_set_windows: Set the number of windows for a domain
> @@ -184,11 +184,12 @@ struct iommu_ops {
> int (*domain_set_attr)(struct iommu_domain *domain,
> enum iommu_attr attr, void *data);
>
> - /* Request/Free a list of direct mapping requirements for a device */
> - void (*get_dm_regions)(struct device *dev, struct list_head *list);
> - void (*put_dm_regions)(struct device *dev, struct list_head *list);
> - void (*apply_dm_region)(struct device *dev, struct iommu_domain *domain,
> - struct iommu_dm_region *region);
> + /* Request/Free a list of reserved regions for a device */
> + void (*get_resv_regions)(struct device *dev, struct list_head *list);
> + void (*put_resv_regions)(struct device *dev, struct list_head *list);
> + void (*apply_resv_region)(struct device *dev,
> + struct iommu_domain *domain,
> + struct iommu_resv_region *region);
>
> /* Window handling functions */
> int (*domain_window_enable)(struct iommu_domain *domain, u32 wnd_nr,
> @@ -233,8 +234,8 @@ extern size_t default_iommu_map_sg(struct iommu_domain *domain, unsigned long io
> extern void iommu_set_fault_handler(struct iommu_domain *domain,
> iommu_fault_handler_t handler, void *token);
>
> -extern void iommu_get_dm_regions(struct device *dev, struct list_head *list);
> -extern void iommu_put_dm_regions(struct device *dev, struct list_head *list);
> +extern void iommu_get_resv_regions(struct device *dev, struct list_head *list);
> +extern void iommu_put_resv_regions(struct device *dev, struct list_head *list);
> extern int iommu_request_dm_for_dev(struct device *dev);
>
> extern int iommu_attach_group(struct iommu_domain *domain,
> @@ -439,12 +440,12 @@ static inline void iommu_set_fault_handler(struct iommu_domain *domain,
> {
> }
>
> -static inline void iommu_get_dm_regions(struct device *dev,
> +static inline void iommu_get_resv_regions(struct device *dev,
> struct list_head *list)
> {
> }
>
> -static inline void iommu_put_dm_regions(struct device *dev,
> +static inline void iommu_put_resv_regions(struct device *dev,
> struct list_head *list)
> {
> }
>
^ permalink raw reply
* [RFC v3 04/10] iommu: iommu_alloc_resv_region
From: Robin Murphy @ 2016-12-06 17:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479215363-2898-5-git-send-email-eric.auger@redhat.com>
On 15/11/16 13:09, Eric Auger wrote:
> Introduce a new helper serving the purpose to allocate a reserved
> region. This will be used in iommu driver implementing reserved
> region callbacks.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
> drivers/iommu/iommu.c | 16 ++++++++++++++++
> include/linux/iommu.h | 8 ++++++++
> 2 files changed, 24 insertions(+)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index c7ed334..6ee529f 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -1562,6 +1562,22 @@ void iommu_put_resv_regions(struct device *dev, struct list_head *list)
> ops->put_resv_regions(dev, list);
> }
>
> +struct iommu_resv_region *iommu_alloc_resv_region(phys_addr_t start,
> + size_t length,
> + unsigned int prot)
> +{
> + struct iommu_resv_region *region;
> +
> + region = kzalloc(sizeof(*region), GFP_KERNEL);
> + if (!region)
> + return NULL;
> +
> + region->start = start;
> + region->length = length;
> + region->prot = prot;
> + return region;
> +}
I think you need an INIT_LIST_HEAD() in here as well, or
CONFIG_DEBUG_LIST might get unhappy about using an uninitialised head later.
Robin.
> +
> /* Request that a device is direct mapped by the IOMMU */
> int iommu_request_dm_for_dev(struct device *dev)
> {
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 02cf565..0aea877 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -241,6 +241,8 @@ extern void iommu_set_fault_handler(struct iommu_domain *domain,
> extern void iommu_get_resv_regions(struct device *dev, struct list_head *list);
> extern void iommu_put_resv_regions(struct device *dev, struct list_head *list);
> extern int iommu_request_dm_for_dev(struct device *dev);
> +extern struct iommu_resv_region *
> +iommu_alloc_resv_region(phys_addr_t start, size_t length, unsigned int prot);
>
> extern int iommu_attach_group(struct iommu_domain *domain,
> struct iommu_group *group);
> @@ -454,6 +456,12 @@ static inline void iommu_put_resv_regions(struct device *dev,
> {
> }
>
> +static inline struct iommu_resv_region *
> +iommu_alloc_resv_region(phys_addr_t start, size_t length, unsigned int prot)
> +{
> + return NULL;
> +}
> +
> static inline int iommu_request_dm_for_dev(struct device *dev)
> {
> return -ENODEV;
>
^ permalink raw reply
* [PATCH V6 6/6] iommu/arm-smmu: Set privileged attribute to 'default' instead of 'unprivileged'
From: Will Deacon @ 2016-12-06 17:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480690509-13490-7-git-send-email-sricharan@codeaurora.org>
On Fri, Dec 02, 2016 at 08:25:09PM +0530, Sricharan R wrote:
> Currently the driver sets all the device transactions privileges
> to UNPRIVILEGED, but there are cases where the iommu masters wants
> to isolate privileged supervisor and unprivileged user.
> So don't override the privileged setting to unprivileged, instead
> set it to default as incoming and let it be controlled by the pagetable
> settings.
>
> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
Acked-by: Will Deacon <will.deacon@arm.com>
I'll let you and Robin sort out what you want to do with this series :)
Will
^ permalink raw reply
* [PATCH 2/5] ARM: BCM5301X: Specify USB controllers in DT
From: Rafał Miłecki @ 2016-12-06 17:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c721c71c-117f-185d-0544-954981dbcf04@broadcom.com>
On 6 December 2016 at 18:28, Ray Jui <ray.jui@broadcom.com> wrote:
> On 12/6/2016 9:17 AM, Rafa? Mi?ecki wrote:
>> From: Rafa? Mi?ecki <rafal@milecki.pl>
>>
>> There are 3 separated controllers, one per USB /standard/. With PHY
>> drivers in place they can be simply supported with generic drivers.
>>
>> Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
>> ---
>> arch/arm/boot/dts/bcm5301x.dtsi | 33 ++++++++++++++++++++++++++++++++-
>> 1 file changed, 32 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi
>> index f09a2bb..bfc98d19 100644
>> --- a/arch/arm/boot/dts/bcm5301x.dtsi
>> +++ b/arch/arm/boot/dts/bcm5301x.dtsi
>> @@ -248,8 +248,26 @@
>>
>> #address-cells = <1>;
>> #size-cells = <1>;
>> + ranges;
>>
>> - phys = <&usb2_phy>;
>> + interrupt-parent = <&gic>;
>> +
>> + ohci: ohci at 21000 {
>> + #usb-cells = <0>;
>> +
>> + compatible = "generic-ohci";
>> + reg = <0x00022000 0x1000>;
>
> Your label ohci at 21000 does not match the 'reg' at 0x22000.
>
>> + interrupts = <GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH>;
>> + };
>> +
>> + ehci: ehci at 22000 {
>> + #usb-cells = <0>;
>> +
>> + compatible = "generic-ehci";
>> + reg = <0x00021000 0x1000>;
>
> Looks like you got the label of ohci and ehci reversed?
Nice catch, thanks! I'll fix it in V2 (just let me wait a day to see
if there will be other comments).
--
Rafa?
^ permalink raw reply
* [RFC v3 05/10] iommu: Do not map reserved regions
From: Robin Murphy @ 2016-12-06 17:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479215363-2898-6-git-send-email-eric.auger@redhat.com>
On 15/11/16 13:09, Eric Auger wrote:
> As we introduced IOMMU_RESV_NOMAP and IOMMU_RESV_MSI regions,
> let's prevent those new regions from being mapped.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
> drivers/iommu/iommu.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 6ee529f..a4530ad 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -343,6 +343,9 @@ static int iommu_group_create_direct_mappings(struct iommu_group *group,
> start = ALIGN(entry->start, pg_size);
> end = ALIGN(entry->start + entry->length, pg_size);
>
> + if (entry->prot & IOMMU_RESV_MASK)
This seems to be the only place that this mask is used, and frankly I
think it's less clear than simply "(IOMMU_RESV_NOMAP | IOMMU_RESV_MSI)"
would be, at which point we may as well drop the mask and special value
trickery altogether. Plus, per my previous comment, if it were to be "if
(entry->type != <direct mapped type>)" instead, that's about as obvious
as it can get.
Robin.
> + continue;
> +
> for (addr = start; addr < end; addr += pg_size) {
> phys_addr_t phys_addr;
>
>
^ permalink raw reply
* [PATCH 2/5] ARM: BCM5301X: Specify USB controllers in DT
From: Ray Jui @ 2016-12-06 17:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACna6rw4iY_xhEUyWkGrchs0SOn3nLBKYMuA7qa0UHAbxpGN0A@mail.gmail.com>
On 12/6/2016 9:31 AM, Rafa? Mi?ecki wrote:
> On 6 December 2016 at 18:28, Ray Jui <ray.jui@broadcom.com> wrote:
>> On 12/6/2016 9:17 AM, Rafa? Mi?ecki wrote:
>>> From: Rafa? Mi?ecki <rafal@milecki.pl>
>>>
>>> There are 3 separated controllers, one per USB /standard/. With PHY
>>> drivers in place they can be simply supported with generic drivers.
>>>
>>> Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
>>> ---
>>> arch/arm/boot/dts/bcm5301x.dtsi | 33 ++++++++++++++++++++++++++++++++-
>>> 1 file changed, 32 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi
>>> index f09a2bb..bfc98d19 100644
>>> --- a/arch/arm/boot/dts/bcm5301x.dtsi
>>> +++ b/arch/arm/boot/dts/bcm5301x.dtsi
>>> @@ -248,8 +248,26 @@
>>>
>>> #address-cells = <1>;
>>> #size-cells = <1>;
>>> + ranges;
>>>
>>> - phys = <&usb2_phy>;
>>> + interrupt-parent = <&gic>;
>>> +
>>> + ohci: ohci at 21000 {
>>> + #usb-cells = <0>;
>>> +
>>> + compatible = "generic-ohci";
>>> + reg = <0x00022000 0x1000>;
>>
>> Your label ohci at 21000 does not match the 'reg' at 0x22000.
>>
>>> + interrupts = <GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH>;
>>> + };
>>> +
>>> + ehci: ehci at 22000 {
>>> + #usb-cells = <0>;
>>> +
>>> + compatible = "generic-ehci";
>>> + reg = <0x00021000 0x1000>;
>>
>> Looks like you got the label of ohci and ehci reversed?
>
> Nice catch, thanks! I'll fix it in V2 (just let me wait a day to see
> if there will be other comments).
>
In V2, please remember to put the nodes in ascending order based on the
base address of the registers, i.e., ehci at 21000, and then followed by
ohci at 22000.
Thanks,
Ray
^ permalink raw reply
* [PATCH 0/2] Hibernate fixes for 'Fix memmap to be initialized for the entire section'
From: Will Deacon @ 2016-12-06 17:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKv+Gu9AVVPp+UM5pCeYCt4HE-Azijp0iUf0QZv6YKFgAuckYw@mail.gmail.com>
On Mon, Dec 05, 2016 at 03:42:14PM +0000, Ard Biesheuvel wrote:
> On 2 December 2016 at 14:49, James Morse <james.morse@arm.com> wrote:
> > Patch "arm64: mm: Fix memmap to be initialized for the entire section"
> > changes pfn_valid() in a way that breaks hibernate. These patches fix
> > hibernate, and provided struct page's are allocated for nomap pages,
> > can be applied before [0].
> >
> > Hibernate core code belives 'valid' to mean "I can access this". It
> > uses pfn_valid() to test the page if the page is 'valid'.
> >
> > pfn_valid() needs to be changed so that all struct pages in a numa
> > node have the same node-id. Currently 'nomap' pages are skipped, and
> > retain their pre-numa node-ids, which leads to a later BUG_ON().
> >
> > These patches make hibernate's savable_page() take its escape route
> > via 'if (PageReserved(page) && pfn_is_nosave(pfn))'.
> >
>
> This makes me feel slightly uneasy. Robert makes a convincing point,
> but I wonder if we can expect more fallout from the ambiguity of
> pfn_valid(). Now we are not only forced to assign non-existing (as far
> as the OS is concerned) pages to the correct NUMA node, we also need
> to set certain page flags.
Yes, I really don't know how to proceed here. Playing whack-a-mole with
pfn_valid() users doesn't sounds like an improvement on the current
situation to me.
Robert -- if we leave pfn_valid() as it is, would a point-hack to
memmap_init_zone help, or do you anticipate other problems?
Will
^ permalink raw reply
* [PATCH v2 2/2] arm64: Work around broken .inst when defective gas is detected
From: Dave Martin @ 2016-12-06 17:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161206162826.GW1574@e103592.cambridge.arm.com>
On Tue, Dec 06, 2016 at 04:28:26PM +0000, Dave Martin wrote:
> On Tue, Dec 06, 2016 at 03:27:45PM +0000, Marc Zyngier wrote:
> > .inst being largely broken with older binutils, it'd be better not
> > to emit it altogether when detecting such configuration (as it
> > leads to all kind of horrors when using alternatives).
>
> GAS seems to provide few guarantees about the constantness of anything
> other than an expression composed purely of literal constants. So,
> this may rather be lack of a useful feature rather than a bug per se.
>
> > Generalize the __emit_inst macro and use it extensively in
> > asm/sysreg.h, and make it generate a .long when a broken gas is
> > detected. The disassembly will be crap, but at least we can write
> > semi-sane code.
>
> Don't know how much this helps, but if we want decent disassembly then
> one option is to use macros instead of symbols, so that the argument to
> .inst involves only literals after macro expansion.
[...]
It seems that this doesn't solve your problem though ... oh well.
Cheers
---Dave
^ permalink raw reply
* [PATCHv4 08/10] mm/kasan: Switch to using __pa_symbol and lm_alias
From: Mark Rutland @ 2016-12-06 17:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480445729-27130-9-git-send-email-labbott@redhat.com>
On Tue, Nov 29, 2016 at 10:55:27AM -0800, Laura Abbott wrote:
> __pa_symbol is the correct API to find the physical address of symbols.
> Switch to it to allow for debugging APIs to work correctly. Other
> functions such as p*d_populate may call __pa internally. Ensure that the
> address passed is in the linear region by calling lm_alias.
I've given this a go on Juno with CONFIG_KASAN_INLINE enabled, and
everything seems happy.
We'll need an include of <linux/mm.h> as that appears to be missing. I
guess we're getting lucky with transitive includes. Otherwise this looks
good to me.
With that fixed up:
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Tested-by: Mark Rutland <mark.rutland@arm.com>
Thanks,
Mark.
> Signed-off-by: Laura Abbott <labbott@redhat.com>
> ---
> Pointed out during review/testing of v3.
> ---
> mm/kasan/kasan_init.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/mm/kasan/kasan_init.c b/mm/kasan/kasan_init.c
> index 3f9a41c..ff04721 100644
> --- a/mm/kasan/kasan_init.c
> +++ b/mm/kasan/kasan_init.c
> @@ -49,7 +49,7 @@ static void __init zero_pte_populate(pmd_t *pmd, unsigned long addr,
> pte_t *pte = pte_offset_kernel(pmd, addr);
> pte_t zero_pte;
>
> - zero_pte = pfn_pte(PFN_DOWN(__pa(kasan_zero_page)), PAGE_KERNEL);
> + zero_pte = pfn_pte(PFN_DOWN(__pa_symbol(kasan_zero_page)), PAGE_KERNEL);
> zero_pte = pte_wrprotect(zero_pte);
>
> while (addr + PAGE_SIZE <= end) {
> @@ -69,7 +69,7 @@ static void __init zero_pmd_populate(pud_t *pud, unsigned long addr,
> next = pmd_addr_end(addr, end);
>
> if (IS_ALIGNED(addr, PMD_SIZE) && end - addr >= PMD_SIZE) {
> - pmd_populate_kernel(&init_mm, pmd, kasan_zero_pte);
> + pmd_populate_kernel(&init_mm, pmd, lm_alias(kasan_zero_pte));
> continue;
> }
>
> @@ -94,7 +94,7 @@ static void __init zero_pud_populate(pgd_t *pgd, unsigned long addr,
>
> pud_populate(&init_mm, pud, kasan_zero_pmd);
> pmd = pmd_offset(pud, addr);
> - pmd_populate_kernel(&init_mm, pmd, kasan_zero_pte);
> + pmd_populate_kernel(&init_mm, pmd, lm_alias(kasan_zero_pte));
> continue;
> }
>
> @@ -135,11 +135,11 @@ void __init kasan_populate_zero_shadow(const void *shadow_start,
> * puds,pmds, so pgd_populate(), pud_populate()
> * is noops.
> */
> - pgd_populate(&init_mm, pgd, kasan_zero_pud);
> + pgd_populate(&init_mm, pgd, lm_alias(kasan_zero_pud));
> pud = pud_offset(pgd, addr);
> - pud_populate(&init_mm, pud, kasan_zero_pmd);
> + pud_populate(&init_mm, pud, lm_alias(kasan_zero_pmd));
> pmd = pmd_offset(pud, addr);
> - pmd_populate_kernel(&init_mm, pmd, kasan_zero_pte);
> + pmd_populate_kernel(&init_mm, pmd, lm_alias(kasan_zero_pte));
> continue;
> }
>
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH v11 7/7] arm: pmu: Add PMU definitions for cores not initially online
From: Jeremy Linton @ 2016-12-06 17:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161206152118.GK2498@arm.com>
Hi,
On 12/06/2016 09:21 AM, Will Deacon wrote:
> On Fri, Dec 02, 2016 at 12:56:01PM -0600, Jeremy Linton wrote:
>> ACPI CPUs aren't associated with a PMU until they have been put
>> online. This means that we potentially have to update a PMU
>> definition the first time a CPU is hot added to the machine.
>>
>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>> ---
>> drivers/perf/arm_pmu.c | 38 ++++++++++++++++++++++++++++++++++++--
>> include/linux/perf/arm_pmu.h | 4 ++++
>> 2 files changed, 40 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
>> index fa40294..4abb2fe 100644
>> --- a/drivers/perf/arm_pmu.c
>> +++ b/drivers/perf/arm_pmu.c
>> @@ -711,6 +711,30 @@ static int cpu_pmu_request_irq(struct arm_pmu *cpu_pmu, irq_handler_t handler)
>> return 0;
>> }
>>
>> +static DEFINE_SPINLOCK(arm_pmu_resource_lock);
>
> Why do you need this spinlock? The hotplug notifiers are serialised afaik,
> and you don't take it anywhere else.
Well, I assumed they were serialized, but then I went looking for a
guarantee and couldn't find one specific to the notifiers, even though
the previous lock was removed. Admittedly, I didn't spend too long
looking, but there is a piece missing...
Which is the sync between the hotplug notification and perf start/stop.
By itself that extends this lock into the consumers of the resource
structure. Which might not be the right choice because even without
these ACPI specific bits, simply running a few cpus online/offline while
simultaneously doing something like `perf stat -e cache-misses ls &` in
a loop causes deadlocks/crashes.
That problem doesn't appear to be specific to the ACPI/PMU so I've
stayed away from it in this patch set, although potentially a larger fix
might cover this as well.
^ permalink raw reply
* [PATCH 1/2] arm: kernel: Add SMC structure parameter
From: Andy Gross @ 2016-12-06 18:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161206115507.GD2498@arm.com>
On Tue, Dec 06, 2016 at 11:55:08AM +0000, Will Deacon wrote:
> Hi Andy,
>
> On Tue, Nov 29, 2016 at 01:44:22AM -0600, Andy Gross wrote:
> > This patch adds a quirk parameter to the arm_smccc_smc call. The quirk
> > structure allows for specialized SMC operations due to SoC specific
> > requirements.
> >
> > This patch also fixes up all the current users of the arm_smccc_smc API.
> >
> > This patch and partial implementation was suggested by Will Deacon.
> >
> > Signed-off-by: Andy Gross <andy.gross@linaro.org>
> > ---
> > arch/arm/kernel/smccc-call.S | 3 ++-
> > arch/arm/mach-artpec/board-artpec6.c | 2 +-
> > arch/arm64/kernel/asm-offsets.c | 7 +++++--
> > arch/arm64/kernel/smccc-call.S | 3 ++-
> > drivers/clk/rockchip/clk-ddr.c | 6 +++---
> > drivers/devfreq/rk3399_dmc.c | 6 +++---
> > drivers/firmware/meson/meson_sm.c | 2 +-
> > drivers/firmware/psci.c | 2 +-
> > drivers/firmware/qcom_scm-64.c | 4 ++--
> > drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +-
> > include/linux/arm-smccc.h | 18 ++++++++++++++++--
> > 11 files changed, 37 insertions(+), 18 deletions(-)
>
> Thanks for respinning this; I'd forgotten about it!
>
> > diff --git a/arch/arm/kernel/smccc-call.S b/arch/arm/kernel/smccc-call.S
> > index 37669e7..e77950a 100644
> > --- a/arch/arm/kernel/smccc-call.S
> > +++ b/arch/arm/kernel/smccc-call.S
> > @@ -47,7 +47,8 @@ UNWIND( .fnend)
> > /*
> > * void smccc_smc(unsigned long a0, unsigned long a1, unsigned long a2,
> > * unsigned long a3, unsigned long a4, unsigned long a5,
> > - * unsigned long a6, unsigned long a7, struct arm_smccc_res *res)
> > + * unsigned long a6, unsigned long a7, struct arm_smccc_res *res,
> > + * struct arm_smccc_quirk *quirk)
>
> I'd not thought of doing it like this -- I envisaged embedding the quirk
> structure into arm_smccc_res, but this works too. I wonder if we could avoid
> having to pass NULL everywhere if we renamed arm_smccc_{hvc.smc} and added
> a default wrapper around them?
>
> For example, rename arm_smccc_smc to __arm_smccc_smc, add a macro called
> arm_smccc_smc that passes a NULL argument for the quirk, then finally add
> a macro arm_smccc_smc_quirk that takes the additional parameter?
This would work pretty well. I'll send another version with this
implementation.
Regards,
Andy
^ permalink raw reply
* next-20161205 build: 3 failures 4 warnings (next-20161205)
From: Bjorn Andersson @ 2016-12-06 18:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <7b3daf13-dbab-5acf-cd5f-2414141500b3@arm.com>
On Mon 05 Dec 07:44 PST 2016, Marc Zyngier wrote:
> On 05/12/16 11:20, Mark Brown wrote:
> > On Mon, Dec 05, 2016 at 07:56:06AM +0000, Build bot for Mark Brown wrote:
> >
> > Today's -next fails to build an arm64 allnodconfig and allmodconfig
> > with:
> >
> >> arm64-allnoconfig
> >> ../arch/arm64/lib/clear_user.S:33: Error: bad or irreducible absolute expression
> >> ../arch/arm64/lib/clear_user.S:53: Error: bad or irreducible absolute expression
> >> ../arch/arm64/lib/clear_user.S:33: Error: attempt to move .org backwards
> >> ../arch/arm64/lib/clear_user.S:53: Error: attempt to move .org backwards
> >> ../arch/arm64/lib/copy_from_user.S:67: Error: bad or irreducible absolute expression
> >> ../arch/arm64/lib/copy_from_user.S:70: Error: bad or irreducible absolute expression
> >> ../arch/arm64/lib/copy_from_user.S:67: Error: attempt to move .org backwards
> >> ../arch/arm64/lib/copy_from_user.S:70: Error: attempt to move .org backwards
> >> ../arch/arm64/lib/copy_in_user.S:68: Error: bad or irreducible absolute expression
> >> ../arch/arm64/lib/copy_in_user.S:71: Error: bad or irreducible absolute expression
> >> ../arch/arm64/lib/copy_in_user.S:68: Error: attempt to move .org backwards
> >> ../arch/arm64/lib/copy_in_user.S:71: Error: attempt to move .org backwards
> >> ../arch/arm64/lib/copy_to_user.S:66: Error: bad or irreducible absolute expression
> >> ../arch/arm64/lib/copy_to_user.S:69: Error: bad or irreducible absolute expression
> >> ../arch/arm64/lib/copy_to_user.S:66: Error: attempt to move .org backwards
> >> ../arch/arm64/lib/copy_to_user.S:69: Error: attempt to move .org backwards
> >
> > This was triggered somehow by bca8f17f57bd7 (arm64: Get rid of
> > asm/opcodes.h) though I didn't figure out how.
>
> Old and broken gas. I have a workaround stashed there:
>
> http://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/commit/?h=arm64/standalone.h&id=559f97365362ed9e96f594200020379df46630d8
>
> At least binutils 2.24 and 2.25 are affected, while 2.27 is not.
>
Made me realize that the Ubuntu 15.10 release I'm on is deprecated.
If I read the release notes for Ubuntu correctly the 14.04 LTS release
is supported until April 2019, with binutils 2.24. So I would be
surprised if this won't bite quite a bunch of people down the road.
Regards,
Bjorn
^ permalink raw reply
* [RFC v3 06/10] iommu: iommu_get_group_resv_regions
From: Robin Murphy @ 2016-12-06 18:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479215363-2898-7-git-send-email-eric.auger@redhat.com>
On 15/11/16 13:09, Eric Auger wrote:
> Introduce iommu_get_group_resv_regions whose role consists in
> enumerating all devices from the group and collecting their
> reserved regions. It checks duplicates.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>
> ---
>
> - we do not move list elements from device to group list since
> the iommu_put_resv_regions() could not be called.
> - at the moment I did not introduce any iommu_put_group_resv_regions
> since it simply consists in voiding/freeing the list
> ---
> drivers/iommu/iommu.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/iommu.h | 8 ++++++++
> 2 files changed, 61 insertions(+)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index a4530ad..e0fbcc5 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -133,6 +133,59 @@ static ssize_t iommu_group_show_name(struct iommu_group *group, char *buf)
> return sprintf(buf, "%s\n", group->name);
> }
>
> +static bool iommu_resv_region_present(struct iommu_resv_region *region,
> + struct list_head *head)
> +{
> + struct iommu_resv_region *entry;
> +
> + list_for_each_entry(entry, head, list) {
> + if ((region->start == entry->start) &&
> + (region->length == entry->length) &&
> + (region->prot == entry->prot))
> + return true;
> + }
> + return false;
> +}
> +
> +static int
> +iommu_insert_device_resv_regions(struct list_head *dev_resv_regions,
> + struct list_head *group_resv_regions)
> +{
> + struct iommu_resv_region *entry, *region;
> +
> + list_for_each_entry(entry, dev_resv_regions, list) {
> + if (iommu_resv_region_present(entry, group_resv_regions))
> + continue;
In the case of overlapping regions which _aren't_ an exact match, would
it be better to expand the existing one rather than leave the caller to
sort it out? It seems a bit inconsistent to handle only the one case here.
> + region = iommu_alloc_resv_region(entry->start, entry->length,
> + entry->prot);
> + if (!region)
> + return -ENOMEM;
> +
> + list_add_tail(®ion->list, group_resv_regions);
> + }
> + return 0;
> +}
> +
> +int iommu_get_group_resv_regions(struct iommu_group *group,
> + struct list_head *head)
> +{
> + struct iommu_device *device;
> + int ret = 0;
> +
> + list_for_each_entry(device, &group->devices, list) {
Should we not be taking the group mutex around this?
Robin.
> + struct list_head dev_resv_regions;
> +
> + INIT_LIST_HEAD(&dev_resv_regions);
> + iommu_get_resv_regions(device->dev, &dev_resv_regions);
> + ret = iommu_insert_device_resv_regions(&dev_resv_regions, head);
> + iommu_put_resv_regions(device->dev, &dev_resv_regions);
> + if (ret)
> + break;
> + }
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(iommu_get_group_resv_regions);
> +
> static IOMMU_GROUP_ATTR(name, S_IRUGO, iommu_group_show_name, NULL);
>
> static void iommu_group_release(struct kobject *kobj)
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 0aea877..0f7ae2c 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -243,6 +243,8 @@ extern void iommu_set_fault_handler(struct iommu_domain *domain,
> extern int iommu_request_dm_for_dev(struct device *dev);
> extern struct iommu_resv_region *
> iommu_alloc_resv_region(phys_addr_t start, size_t length, unsigned int prot);
> +extern int iommu_get_group_resv_regions(struct iommu_group *group,
> + struct list_head *head);
>
> extern int iommu_attach_group(struct iommu_domain *domain,
> struct iommu_group *group);
> @@ -462,6 +464,12 @@ static inline void iommu_put_resv_regions(struct device *dev,
> return NULL;
> }
>
> +static inline int iommu_get_group_resv_regions(struct iommu_group *group,
> + struct list_head *head)
> +{
> + return -ENODEV;
> +}
> +
> static inline int iommu_request_dm_for_dev(struct device *dev)
> {
> return -ENODEV;
>
^ permalink raw reply
* [PATCHv4 09/10] mm/usercopy: Switch to using lm_alias
From: Mark Rutland @ 2016-12-06 18:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGXu5jKrBc6R9JYay1L6pd958Vm5-6p=37tiUYgg6uPeZb1HtQ@mail.gmail.com>
On Tue, Nov 29, 2016 at 11:39:44AM -0800, Kees Cook wrote:
> On Tue, Nov 29, 2016 at 10:55 AM, Laura Abbott <labbott@redhat.com> wrote:
> >
> > The usercopy checking code currently calls __va(__pa(...)) to check for
> > aliases on symbols. Switch to using lm_alias instead.
> >
> > Signed-off-by: Laura Abbott <labbott@redhat.com>
>
> Acked-by: Kees Cook <keescook@chromium.org>
>
> I should probably add a corresponding alias test to lkdtm...
>
> -Kees
Something like the below?
It uses lm_alias(), so it depends on Laura's patches. We seem to do the
right thing, anyhow:
root at ribbensteg:/home/nanook# echo USERCOPY_KERNEL_ALIAS > /sys/kernel/debug/provoke-crash/DIRECT
[ 44.493400] usercopy: kernel memory exposure attempt detected from ffff80000031a730 (<linear kernel text>) (4096 bytes)
[ 44.504263] kernel BUG at mm/usercopy.c:75!
Thanks,
Mark.
---->8----
diff --git a/drivers/misc/lkdtm.h b/drivers/misc/lkdtm.h
index fdf954c..96d8d76 100644
--- a/drivers/misc/lkdtm.h
+++ b/drivers/misc/lkdtm.h
@@ -56,5 +56,6 @@
void lkdtm_USERCOPY_STACK_FRAME_FROM(void);
void lkdtm_USERCOPY_STACK_BEYOND(void);
void lkdtm_USERCOPY_KERNEL(void);
+void lkdtm_USERCOPY_KERNEL_ALIAS(void);
#endif
diff --git a/drivers/misc/lkdtm_core.c b/drivers/misc/lkdtm_core.c
index f9154b8..f6bc6d6 100644
--- a/drivers/misc/lkdtm_core.c
+++ b/drivers/misc/lkdtm_core.c
@@ -228,6 +228,7 @@ struct crashtype crashtypes[] = {
CRASHTYPE(USERCOPY_STACK_FRAME_FROM),
CRASHTYPE(USERCOPY_STACK_BEYOND),
CRASHTYPE(USERCOPY_KERNEL),
+ CRASHTYPE(USERCOPY_KERNEL_ALIAS),
};
diff --git a/drivers/misc/lkdtm_usercopy.c b/drivers/misc/lkdtm_usercopy.c
index 1dd6114..955f2dc 100644
--- a/drivers/misc/lkdtm_usercopy.c
+++ b/drivers/misc/lkdtm_usercopy.c
@@ -279,9 +279,16 @@ void lkdtm_USERCOPY_STACK_BEYOND(void)
do_usercopy_stack(true, false);
}
-void lkdtm_USERCOPY_KERNEL(void)
+static void do_usercopy_kernel(bool use_alias)
{
unsigned long user_addr;
+ const void *rodata = test_text;
+ void *text = vm_mmap;
+
+ if (use_alias) {
+ rodata = lm_alias(rodata);
+ text = lm_alias(text);
+ }
user_addr = vm_mmap(NULL, 0, PAGE_SIZE,
PROT_READ | PROT_WRITE | PROT_EXEC,
@@ -292,14 +299,14 @@ void lkdtm_USERCOPY_KERNEL(void)
}
pr_info("attempting good copy_to_user from kernel rodata\n");
- if (copy_to_user((void __user *)user_addr, test_text,
+ if (copy_to_user((void __user *)user_addr, rodata,
unconst + sizeof(test_text))) {
pr_warn("copy_to_user failed unexpectedly?!\n");
goto free_user;
}
pr_info("attempting bad copy_to_user from kernel text\n");
- if (copy_to_user((void __user *)user_addr, vm_mmap,
+ if (copy_to_user((void __user *)user_addr, text,
unconst + PAGE_SIZE)) {
pr_warn("copy_to_user failed, but lacked Oops\n");
goto free_user;
@@ -309,6 +316,16 @@ void lkdtm_USERCOPY_KERNEL(void)
vm_munmap(user_addr, PAGE_SIZE);
}
+void lkdtm_USERCOPY_KERNEL(void)
+{
+ do_usercopy_kernel(false);
+}
+
+void lkdtm_USERCOPY_KERNEL_ALIAS(void)
+{
+ do_usercopy_kernel(true);
+}
+
void __init lkdtm_usercopy_init(void)
{
/* Prepare cache that lacks SLAB_USERCOPY flag. */
^ permalink raw reply related
* [PATCHv4 09/10] mm/usercopy: Switch to using lm_alias
From: Mark Rutland @ 2016-12-06 18:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480445729-27130-10-git-send-email-labbott@redhat.com>
On Tue, Nov 29, 2016 at 10:55:28AM -0800, Laura Abbott wrote:
>
> The usercopy checking code currently calls __va(__pa(...)) to check for
> aliases on symbols. Switch to using lm_alias instead.
>
> Signed-off-by: Laura Abbott <labbott@redhat.com>
I've given this a go on Juno, which boots happily. LKDTM triggers as
expected when copying from the kernel text and its alias.
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Tested-by: Mark Rutland <mark.rutland@arm.com>
Thanks,
Mark.
> ---
> Found when reviewing the kernel. Tested.
> ---
> mm/usercopy.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/usercopy.c b/mm/usercopy.c
> index 3c8da0a..8345299 100644
> --- a/mm/usercopy.c
> +++ b/mm/usercopy.c
> @@ -108,13 +108,13 @@ static inline const char *check_kernel_text_object(const void *ptr,
> * __pa() is not just the reverse of __va(). This can be detected
> * and checked:
> */
> - textlow_linear = (unsigned long)__va(__pa(textlow));
> + textlow_linear = (unsigned long)lm_alias(textlow);
> /* No different mapping: we're done. */
> if (textlow_linear == textlow)
> return NULL;
>
> /* Check the secondary mapping... */
> - texthigh_linear = (unsigned long)__va(__pa(texthigh));
> + texthigh_linear = (unsigned long)lm_alias(texthigh);
> if (overlaps(ptr, n, textlow_linear, texthigh_linear))
> return "<linear kernel text>";
>
> --
> 2.7.4
>
^ permalink raw reply
* next-20161205 build: 3 failures 4 warnings (next-20161205)
From: Marc Zyngier @ 2016-12-06 18:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161206181050.GF30492@tuxbot>
On 06/12/16 18:10, Bjorn Andersson wrote:
> On Mon 05 Dec 07:44 PST 2016, Marc Zyngier wrote:
>
>> On 05/12/16 11:20, Mark Brown wrote:
>>> On Mon, Dec 05, 2016 at 07:56:06AM +0000, Build bot for Mark Brown wrote:
>>>
>>> Today's -next fails to build an arm64 allnodconfig and allmodconfig
>>> with:
>>>
>>>> arm64-allnoconfig
>>>> ../arch/arm64/lib/clear_user.S:33: Error: bad or irreducible absolute expression
>>>> ../arch/arm64/lib/clear_user.S:53: Error: bad or irreducible absolute expression
>>>> ../arch/arm64/lib/clear_user.S:33: Error: attempt to move .org backwards
>>>> ../arch/arm64/lib/clear_user.S:53: Error: attempt to move .org backwards
>>>> ../arch/arm64/lib/copy_from_user.S:67: Error: bad or irreducible absolute expression
>>>> ../arch/arm64/lib/copy_from_user.S:70: Error: bad or irreducible absolute expression
>>>> ../arch/arm64/lib/copy_from_user.S:67: Error: attempt to move .org backwards
>>>> ../arch/arm64/lib/copy_from_user.S:70: Error: attempt to move .org backwards
>>>> ../arch/arm64/lib/copy_in_user.S:68: Error: bad or irreducible absolute expression
>>>> ../arch/arm64/lib/copy_in_user.S:71: Error: bad or irreducible absolute expression
>>>> ../arch/arm64/lib/copy_in_user.S:68: Error: attempt to move .org backwards
>>>> ../arch/arm64/lib/copy_in_user.S:71: Error: attempt to move .org backwards
>>>> ../arch/arm64/lib/copy_to_user.S:66: Error: bad or irreducible absolute expression
>>>> ../arch/arm64/lib/copy_to_user.S:69: Error: bad or irreducible absolute expression
>>>> ../arch/arm64/lib/copy_to_user.S:66: Error: attempt to move .org backwards
>>>> ../arch/arm64/lib/copy_to_user.S:69: Error: attempt to move .org backwards
>>>
>>> This was triggered somehow by bca8f17f57bd7 (arm64: Get rid of
>>> asm/opcodes.h) though I didn't figure out how.
>>
>> Old and broken gas. I have a workaround stashed there:
>>
>> http://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/commit/?h=arm64/standalone.h&id=559f97365362ed9e96f594200020379df46630d8
>>
>> At least binutils 2.24 and 2.25 are affected, while 2.27 is not.
>>
>
> Made me realize that the Ubuntu 15.10 release I'm on is deprecated.
>
> If I read the release notes for Ubuntu correctly the 14.04 LTS release
> is supported until April 2019, with binutils 2.24. So I would be
> surprised if this won't bite quite a bunch of people down the road.
Catalin has taken a slightly different set of fixes which do address
this problem:
https://git.kernel.org/cgit/linux/kernel/git/arm64/linux.git/commit/?h=for-next/core&id=bbb56c27228d4ad64aca858c5af49d0f2f11c645
https://git.kernel.org/cgit/linux/kernel/git/arm64/linux.git/commit/?h=for-next/core&id=cd9e1927a525f6ce7c0d99c6e038f0a0b9e85176
The build system will warn you that your binutils are broken, and enable
the workaround (which will only result in slightly painful disassembly
for some instructions).
Some of the most visible distros have also other "features", such as
compilers that do not implement basic capabilities on which the kernel
relies for performance (jump labels, for example)...
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH] arm64: Add CMDLINE_EXTEND
From: Geoff Levand @ 2016-12-06 18:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161206122040.GH2498@arm.com>
Hi Will,
On 12/06/2016 04:20 AM, Will Deacon wrote:
> On Mon, Dec 05, 2016 at 09:41:06AM -0800, Geoff Levand wrote:
>> On 12/05/2016 04:08 AM, Catalin Marinas wrote:
>>> On Fri, Dec 02, 2016 at 02:17:02PM -0800, Geoff Levand wrote:
>>>> The device tree code already supports CMDLINE_EXTEND,
>>>> so add the config option to make it available on arm64.
>>>
>>> What's your use-case for this patch? Note that both CMDLINE_FORCE and
>>> CMDLINE_EXTEND (if we introduce it) are ignored by the EFI stub.
>>> However, we don't seem to have stated this anywhere.
>>
>> I use this in CoreOS, where we need to set "acpi=force" for
>> arm64. CoreOS uses a proper UEFI + grub.
>
> So why can't you just set that in grub if you want to boot with ACPI?
That is how I originally did it, but it was suggested I
do it in the kernel config. See:
https://github.com/coreos/scripts/pull/610
Is there any reason why we don't want arm64 to have
CMDLINE_EXTEND?
-Geoff
^ permalink raw reply
* [Question] New mmap64 syscall?
From: Yury Norov @ 2016-12-06 18:54 UTC (permalink / raw)
To: linux-arm-kernel
Hi all,
(Sorry if there is similar discussion, and I missed it. I didn't
find something in LKML in last half a year.)
In aarch64/ilp32 discussion Catalin wondered why we don't pass offset
in mmap() as 64-bit value (in 2 registers if needed). Looking at kernel
code I found that there's no generic interface for it. But almost all
architectures provide their own implementations, like this:
SYSCALL_DEFINE6(mips_mmap, unsigned long, addr, unsigned long, len,
unsigned long, prot, unsigned long, flags, unsigned long,
fd, off_t, offset)
{
unsigned long result;
result = -EINVAL;
if (offset & ~PAGE_MASK)
goto out;
result = sys_mmap_pgoff(addr, len, prot, flags, fd, offset >> PAGE_SHIFT);
out:
return result;
}
On glibc side things are even worse. There's no mmap() implementation
that allows to pass 64-bit offset in 32-bit architecture. mmap64() which
is supposed to do this is simply broken:
void *
__mmap64 (void *addr, size_t len, int prot, int flags, int fd, off64_t
offset)
{
[...]
void *result;
result = (void *) INLINE_SYSCALL (mmap2, 6, addr,
len, prot, flags, fd,
(off_t) (offset >> page_shift));
return result;
}
It explicitly declares offset as 64-bit value, but casts it to 32-bit
before passing to the kernel, which is wrong for me. Even if arch has
64-bit off_t, like aarch64/ilp32, the cast will take place because
offset is passed in a single register, which is 32-bit.
I see 3 solutions for my problem:
1. Reuse aarch64/lp64 mmap code for ilp32 in glibc, but wrap offset with
SYSCALL_LL64() macro - which converts offset to the pair for 32-bit
ports. This is simple but local solution. And most probably it's enough.
2. Add new flag to mmap, like MAP_OFFSET_IN_PAIR. This will also work.
The problem here is that there are too much arches that implement
their custom sys_mmap2(). And, of course, this type of flags is
looking ugly.
3. Introduce new mmap64() syscall like this:
sys_mmap64(void *addr, size_t len, int prot, int flags, int fd, struct off_pair *off);
(The pointer here because otherwise we have 7 args, if simply pass off_hi and
off_lo in registers.)
With new 64-bit interface we can deprecate mmap2(), and generalize all
implementations in kernel.
I think we can discuss it because 64-bit is the default size for off_t
in all new 32-bit architectures. So generic solution may take place.
The last question here is how important to support offsets bigger than
2^44 on 32-bit machines in practice? It may be a case for ARM64 servers,
which are looking like main aarch64/ilp32 users. If no, we can leave
things as is, and just do nothing.
Yury
On Mon, Dec 05, 2016 at 05:12:43PM +0000, Catalin Marinas wrote:
> On Fri, Oct 21, 2016 at 11:33:10PM +0300, Yury Norov wrote:
> > off_t is passed in register pair just like in aarch32.
> > In this patch corresponding aarch32 handlers are shared to
> > ilp32 code.
> [...]
> > +/*
> > + * Note: off_4k (w5) is always in units of 4K. If we can't do the
> > + * requested offset because it is not page-aligned, we return -EINVAL.
> > + */
> > +ENTRY(compat_sys_mmap2_wrapper)
> > +#if PAGE_SHIFT > 12
> > + tst w5, #~PAGE_MASK >> 12
> > + b.ne 1f
> > + lsr w5, w5, #PAGE_SHIFT - 12
> > +#endif
> > + b sys_mmap_pgoff
> > +1: mov x0, #-EINVAL
> > + ret
> > +ENDPROC(compat_sys_mmap2_wrapper)
>
> For compat sys_mmap2, the pgoff argument is in multiples of 4K. This was
> traditionally used for architectures where off_t is 32-bit to allow
> mapping files to 2^44.
>
> Since off_t is 64-bit with AArch64/ILP32, should we just pass the off_t
> as a 64-bit value in two different registers (w5 and w6)?
^ permalink raw reply
* [RFC v3 09/10] iommu/arm-smmu: Implement reserved region get/put callbacks
From: Robin Murphy @ 2016-12-06 18:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479215363-2898-10-git-send-email-eric.auger@redhat.com>
On 15/11/16 13:09, Eric Auger wrote:
> The get() populates the list with the PCI host bridge windows
> and the MSI IOVA range.
>
> At the moment an arbitray MSI IOVA window is set at 0x8000000
> of size 1MB. This will allow to report those info in iommu-group
> sysfs?
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>
> ---
>
> RFC v2 -> v3:
> - use existing get/put_resv_regions
>
> RFC v1 -> v2:
> - use defines for MSI IOVA base and length
> ---
> drivers/iommu/arm-smmu.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 52 insertions(+)
>
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 8f72814..81f1a83 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -278,6 +278,9 @@ enum arm_smmu_s2cr_privcfg {
>
> #define FSYNR0_WNR (1 << 4)
>
> +#define MSI_IOVA_BASE 0x8000000
> +#define MSI_IOVA_LENGTH 0x100000
> +
> static int force_stage;
> module_param(force_stage, int, S_IRUGO);
> MODULE_PARM_DESC(force_stage,
> @@ -1545,6 +1548,53 @@ static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)
> return iommu_fwspec_add_ids(dev, &fwid, 1);
> }
>
> +static void arm_smmu_get_resv_regions(struct device *dev,
> + struct list_head *head)
> +{
> + struct iommu_resv_region *region;
> + struct pci_host_bridge *bridge;
> + struct resource_entry *window;
> +
> + /* MSI region */
> + region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH,
> + IOMMU_RESV_MSI);
> + if (!region)
> + return;
> +
> + list_add_tail(®ion->list, head);
> +
> + if (!dev_is_pci(dev))
> + return;
> +
> + bridge = pci_find_host_bridge(to_pci_dev(dev)->bus);
> +
> + resource_list_for_each_entry(window, &bridge->windows) {
> + phys_addr_t start;
> + size_t length;
> +
> + if (resource_type(window->res) != IORESOURCE_MEM &&
> + resource_type(window->res) != IORESOURCE_IO)
As Joerg commented elsewhere, considering anything other than memory
resources isn't right (I appreciate you've merely copied my own mistake
here). We need some other way to handle root complexes where the CPU
MMIO views of PCI windows appear in PCI memory space - using the I/O
address of I/O resources only works by chance on Juno, and it still
doesn't account for config space. I suggest we just leave that out for
the time being to make life easier (does it even apply to anything other
than Juno?) and figure it out later.
> + continue;
> +
> + start = window->res->start - window->offset;
> + length = window->res->end - window->res->start + 1;
> + region = iommu_alloc_resv_region(start, length,
> + IOMMU_RESV_NOMAP);
> + if (!region)
> + return;
> + list_add_tail(®ion->list, head);
> + }
> +}
Either way, there's nothing SMMU-specific about PCI windows. The fact
that we'd have to copy-paste all of this into the SMMUv3 driver
unchanged suggests it should go somewhere common (although I would be
inclined to leave the insertion of the fake MSI region to driver-private
wrappers). As I said before, the current iova_reserve_pci_windows()
simply wants splitting into appropriate public callbacks for
get_resv_regions and apply_resv_regions.
Robin.
> +static void arm_smmu_put_resv_regions(struct device *dev,
> + struct list_head *head)
> +{
> + struct iommu_resv_region *entry, *next;
> +
> + list_for_each_entry_safe(entry, next, head, list)
> + kfree(entry);
> +}
> +
> static struct iommu_ops arm_smmu_ops = {
> .capable = arm_smmu_capable,
> .domain_alloc = arm_smmu_domain_alloc,
> @@ -1560,6 +1610,8 @@ static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)
> .domain_get_attr = arm_smmu_domain_get_attr,
> .domain_set_attr = arm_smmu_domain_set_attr,
> .of_xlate = arm_smmu_of_xlate,
> + .get_resv_regions = arm_smmu_get_resv_regions,
> + .put_resv_regions = arm_smmu_put_resv_regions,
> .pgsize_bitmap = -1UL, /* Restricted during device attach */
> };
>
>
^ permalink raw reply
* [PATCH v5] ARM: davinci: da8xx: Fix sleeping function called from invalid context
From: David Lechner @ 2016-12-06 18:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c4e15855-b785-8bc7-5ef2-af4ec24354e6@ti.com>
On 12/06/2016 03:57 AM, Sekhar Nori wrote:
> On Tuesday 06 December 2016 03:21 PM, Alexandre Bailon wrote:
>> On 12/06/2016 10:33 AM, Sekhar Nori wrote:
>>> On Monday 05 December 2016 07:43 PM, Alexandre Bailon wrote:
>>>> Everytime the usb20 phy is enabled, there is a
>>>> "sleeping function called from invalid context" BUG.
>>>>
>>>> clk_enable() from arch/arm/mach-davinci/clock.c uses spin_lock_irqsave()
>>>> before to invoke the callback usb20_phy_clk_enable().
>>>> usb20_phy_clk_enable() uses clk_get() and clk_enable_prepapre()
>>>> which may sleep.
>>>> Move clk_get() to da8xx_register_usb20_phy_clk() and
>>>> replace clk_prepare_enable() by clk_enable().
>>>>
>>>> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
>>>
>>> This will still cause the recursive locking problem reported by David.
>>> Not sure what the point of sending this version was.
>>>
>>> Thanks,
>>> Sekhar
>>>
>
>> What am I supposed to do ?
>
> That needs to be resolved between you and David. Perhaps convert the fix
> sent by David into a proper patch and base this patch on that. Or wait
> for David to send it himself and let him also make the modifications
> needed in this patch.
>
> David ?
>
> Thanks,
> Sekhar
>
Alexandre, I was hoping that you would just squash my patch with your
patch and take Sekhar's suggestion about a separate patch to make the
private __clk_enable() public as davinci_clk_enable() when you re-submit.
You can add "Suggested-By: David Lechner <david@lechnology.com>" to the
commit message if you would like to give me some credit for my ideas.
^ 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