Re: [PATCH v1] efi/apple-properties: Use memremap() instead of ioremap()

2018-03-07 Thread Ard Biesheuvel 
On 7 March 2018 at 17:44, Andy Shevchenko
 wrote:
> On Tue, 2018-02-27 at 17:31 +, Ard Biesheuvel wrote:
>> On 27 February 2018 at 17:27, Andy Shevchenko
>>  wrote:
>> > The memory we are accessing through virtual address has no IO side
>> > effects. Moreover, for IO memory we have to use special accessors,
>> > which we don't use.
>> >
>>
>> Not 100% relevant in this context, but perhaps interesting
>> nonetheless: using device mappings to access things like firmware
>> tables or other structured data will break ARM and other non-x86
>> architectures that do not tolerate unaligned accessed to device
>> memory.
>>
>> > Due to above, convert the driver to use memremap() instead of
>> > ioremap().
>> >
>> > Signed-off-by: Andy Shevchenko 
>>
>> Acked-by: Ard Biesheuvel 
>
> Thanks!
>
> It's supposed to go through EFI tree.
>

Yes, I have queued it already. I expect to send out the PR to -tip by
the end of the next week.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1] efi/apple-properties: Use memremap() instead of ioremap()

2018-03-07 Thread Andy Shevchenko 
On Tue, 2018-02-27 at 17:31 +, Ard Biesheuvel wrote:
> On 27 February 2018 at 17:27, Andy Shevchenko
>  wrote:
> > The memory we are accessing through virtual address has no IO side
> > effects. Moreover, for IO memory we have to use special accessors,
> > which we don't use.
> > 
> 
> Not 100% relevant in this context, but perhaps interesting
> nonetheless: using device mappings to access things like firmware
> tables or other structured data will break ARM and other non-x86
> architectures that do not tolerate unaligned accessed to device
> memory.
> 
> > Due to above, convert the driver to use memremap() instead of
> > ioremap().
> > 
> > Signed-off-by: Andy Shevchenko 
> 
> Acked-by: Ard Biesheuvel 

Thanks!

It's supposed to go through EFI tree.

> 
> > ---
> >  drivers/firmware/efi/apple-properties.c | 11 ++-
> >  1 file changed, 6 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/firmware/efi/apple-properties.c
> > b/drivers/firmware/efi/apple-properties.c
> > index b9602e0d7b50..adaa9a3714b9 100644
> > --- a/drivers/firmware/efi/apple-properties.c
> > +++ b/drivers/firmware/efi/apple-properties.c
> > @@ -19,6 +19,7 @@
> > 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -189,7 +190,7 @@ static int __init map_properties(void)
> > 
> > pa_data = boot_params.hdr.setup_data;
> > while (pa_data) {
> > -   data = ioremap(pa_data, sizeof(*data));
> > +   data = memremap(pa_data, sizeof(*data),
> > MEMREMAP_WB);
> > if (!data) {
> > pr_err("cannot map setup_data header\n");
> > return -ENOMEM;
> > @@ -197,14 +198,14 @@ static int __init map_properties(void)
> > 
> > if (data->type != SETUP_APPLE_PROPERTIES) {
> > pa_data = data->next;
> > -   iounmap(data);
> > +   memunmap(data);
> > continue;
> > }
> > 
> > data_len = data->len;
> > -   iounmap(data);
> > +   memunmap(data);
> > 
> > -   data = ioremap(pa_data, sizeof(*data) + data_len);
> > +   data = memremap(pa_data, sizeof(*data) + data_len,
> > MEMREMAP_WB);
> > if (!data) {
> > pr_err("cannot map setup_data payload\n");
> > return -ENOMEM;
> > @@ -229,7 +230,7 @@ static int __init map_properties(void)
> >  * to avoid breaking the chain of ->next pointers.
> >  */
> > data->len = 0;
> > -   iounmap(data);
> > +   memunmap(data);
> > free_bootmem_late(pa_data + sizeof(*data),
> > data_len);
> > 
> > return ret;
> > --
> > 2.16.1
> > 

-- 
Andy Shevchenko 
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1] efi/apple-properties: Use memremap() instead of ioremap()

2018-02-27 Thread Lukas Wunner 
On Tue, Feb 27, 2018 at 07:27:10PM +0200, Andy Shevchenko wrote:
> The memory we are accessing through virtual address has no IO side
> effects. Moreover, for IO memory we have to use special accessors,
> which we don't use.
> 
> Due to above, convert the driver to use memremap() instead of ioremap().
> 
> Signed-off-by: Andy Shevchenko 

Tested-by: Lukas Wunner 

Thanks!

Lukas

> ---
>  drivers/firmware/efi/apple-properties.c | 11 ++-
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/firmware/efi/apple-properties.c 
> b/drivers/firmware/efi/apple-properties.c
> index b9602e0d7b50..adaa9a3714b9 100644
> --- a/drivers/firmware/efi/apple-properties.c
> +++ b/drivers/firmware/efi/apple-properties.c
> @@ -19,6 +19,7 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -189,7 +190,7 @@ static int __init map_properties(void)
>  
>   pa_data = boot_params.hdr.setup_data;
>   while (pa_data) {
> - data = ioremap(pa_data, sizeof(*data));
> + data = memremap(pa_data, sizeof(*data), MEMREMAP_WB);
>   if (!data) {
>   pr_err("cannot map setup_data header\n");
>   return -ENOMEM;
> @@ -197,14 +198,14 @@ static int __init map_properties(void)
>  
>   if (data->type != SETUP_APPLE_PROPERTIES) {
>   pa_data = data->next;
> - iounmap(data);
> + memunmap(data);
>   continue;
>   }
>  
>   data_len = data->len;
> - iounmap(data);
> + memunmap(data);
>  
> - data = ioremap(pa_data, sizeof(*data) + data_len);
> + data = memremap(pa_data, sizeof(*data) + data_len, MEMREMAP_WB);
>   if (!data) {
>   pr_err("cannot map setup_data payload\n");
>   return -ENOMEM;
> @@ -229,7 +230,7 @@ static int __init map_properties(void)
>* to avoid breaking the chain of ->next pointers.
>*/
>   data->len = 0;
> - iounmap(data);
> + memunmap(data);
>   free_bootmem_late(pa_data + sizeof(*data), data_len);
>  
>   return ret;
> -- 
> 2.16.1
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1] efi/apple-properties: Use memremap() instead of ioremap()

2018-02-27 Thread Ard Biesheuvel 
On 27 February 2018 at 17:27, Andy Shevchenko
 wrote:
> The memory we are accessing through virtual address has no IO side
> effects. Moreover, for IO memory we have to use special accessors,
> which we don't use.
>

Not 100% relevant in this context, but perhaps interesting
nonetheless: using device mappings to access things like firmware
tables or other structured data will break ARM and other non-x86
architectures that do not tolerate unaligned accessed to device
memory.

> Due to above, convert the driver to use memremap() instead of ioremap().
>
> Signed-off-by: Andy Shevchenko 

Acked-by: Ard Biesheuvel 

> ---
>  drivers/firmware/efi/apple-properties.c | 11 ++-
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/firmware/efi/apple-properties.c 
> b/drivers/firmware/efi/apple-properties.c
> index b9602e0d7b50..adaa9a3714b9 100644
> --- a/drivers/firmware/efi/apple-properties.c
> +++ b/drivers/firmware/efi/apple-properties.c
> @@ -19,6 +19,7 @@
>
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -189,7 +190,7 @@ static int __init map_properties(void)
>
> pa_data = boot_params.hdr.setup_data;
> while (pa_data) {
> -   data = ioremap(pa_data, sizeof(*data));
> +   data = memremap(pa_data, sizeof(*data), MEMREMAP_WB);
> if (!data) {
> pr_err("cannot map setup_data header\n");
> return -ENOMEM;
> @@ -197,14 +198,14 @@ static int __init map_properties(void)
>
> if (data->type != SETUP_APPLE_PROPERTIES) {
> pa_data = data->next;
> -   iounmap(data);
> +   memunmap(data);
> continue;
> }
>
> data_len = data->len;
> -   iounmap(data);
> +   memunmap(data);
>
> -   data = ioremap(pa_data, sizeof(*data) + data_len);
> +   data = memremap(pa_data, sizeof(*data) + data_len, 
> MEMREMAP_WB);
> if (!data) {
> pr_err("cannot map setup_data payload\n");
> return -ENOMEM;
> @@ -229,7 +230,7 @@ static int __init map_properties(void)
>  * to avoid breaking the chain of ->next pointers.
>  */
> data->len = 0;
> -   iounmap(data);
> +   memunmap(data);
> free_bootmem_late(pa_data + sizeof(*data), data_len);
>
> return ret;
> --
> 2.16.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html