[09/15] ps3: Limit the number of regions per storage device

There can be only 8 regions, add a sanity check

Signed-off-by: Andre Heider <a.heider@gmail.com>
---
 arch/powerpc/include/asm/ps3stor.h       |    1 +
 arch/powerpc/platforms/ps3/device-init.c |    8 ++++++++
 2 files changed, 9 insertions(+), 0 deletions(-)

On Mon, Aug 1, 2011 at 22:03, Andre Heider <a.heider@gmail.com> wrote:
> There can be only 8 regions, add a sanity check

Why can there be only 8 regions?

> Signed-off-by: Andre Heider <a.heider@gmail.com>
> ---
>  arch/powerpc/include/asm/ps3stor.h       |    1 +
>  arch/powerpc/platforms/ps3/device-init.c |    8 ++++++++
>  2 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/ps3stor.h b/arch/powerpc/include/asm/ps3stor.h
> index 6fcaf71..d51e53c 100644
> --- a/arch/powerpc/include/asm/ps3stor.h
> +++ b/arch/powerpc/include/asm/ps3stor.h
> @@ -25,6 +25,7 @@
>
>  #include <asm/ps3.h>
>
> +#define PS3_STORAGE_MAX_REGIONS                (8)
>
>  struct ps3_storage_region {
>        unsigned int id;
> diff --git a/arch/powerpc/platforms/ps3/device-init.c b/arch/powerpc/platforms/ps3/device-init.c
> index 6c4b583..830d741 100644
> --- a/arch/powerpc/platforms/ps3/device-init.c
> +++ b/arch/powerpc/platforms/ps3/device-init.c
> @@ -349,6 +349,14 @@ static int ps3_setup_storage_dev(const struct ps3_repository_device *repo,
>                return -ENODEV;
>        }
>
> +       if (num_regions > PS3_STORAGE_MAX_REGIONS) {
> +               pr_warning("%s:%u: device %u:%u reported %u regions, "
> +                          "limiting to %u\n", __func__, __LINE__,
> +                          num_regions, repo->bus_index, repo->dev_index,
> +                          PS3_STORAGE_MAX_REGIONS);
> +               num_regions = PS3_STORAGE_MAX_REGIONS;
> +       }
> +
>        pr_debug("%s:%u: (%u:%u:%u): port %llu blk_size %llu num_blocks %llu "
>                 "num_regions %u\n", __func__, __LINE__, repo->bus_index,
>                 repo->dev_index, repo->dev_type, port, blk_size, num_blocks,

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

On Mon, Aug 1, 2011 at 10:30 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Mon, Aug 1, 2011 at 22:03, Andre Heider <a.heider@gmail.com> wrote:
>> There can be only 8 regions, add a sanity check
>
> Why can there be only 8 regions?

I believe lv1 limits it to 8? I might be mistaken here, it mostly is a
check for the patches after this one

On Mon, Aug 1, 2011 at 10:58 PM, Andre Heider <a.heider@gmail.com> wrote:
> On Mon, Aug 1, 2011 at 10:30 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Mon, Aug 1, 2011 at 22:03, Andre Heider <a.heider@gmail.com> wrote:
>>> There can be only 8 regions, add a sanity check
>>
>> Why can there be only 8 regions?
>
> I believe lv1 limits it to 8? I might be mistaken here, it mostly is a
> check for the patches after this one

Small follow-up:
While the repository contains ("bus", "dev", "n_regs") to describe the
actual number of regions, it also contains ("bus", "dev", "region", [
"id" | "start" | "size" ]) for always exactly 8 regions (with a value
of 0xdeadbeef for invalid regions).

I added this check for the storage drivers, which contain:
  for (region_idx = 0; region_idx < dev->num_regions; region_idx++) {
    ...
    gendisk->first_minor = devidx * PS3DISK_MINORS + region_idx;

But that limit might be raised in future hypervisor versions.
Maybe a
  BUG_ON(dev->num_regions <= PS3DISK_MINORS);
is more appropriate?

On Sat, Aug 6, 2011 at 2:28 PM, Andre Heider <a.heider@gmail.com> wrote:
> On Mon, Aug 1, 2011 at 10:58 PM, Andre Heider <a.heider@gmail.com> wrote:
>> On Mon, Aug 1, 2011 at 10:30 PM, Geert Uytterhoeven
>> <geert@linux-m68k.org> wrote:
>>> On Mon, Aug 1, 2011 at 22:03, Andre Heider <a.heider@gmail.com> wrote:
>>>> There can be only 8 regions, add a sanity check
>>>
>>> Why can there be only 8 regions?
>>
>> I believe lv1 limits it to 8? I might be mistaken here, it mostly is a
>> check for the patches after this one
>
> Small follow-up:
> While the repository contains ("bus", "dev", "n_regs") to describe the
> actual number of regions, it also contains ("bus", "dev", "region", [
> "id" | "start" | "size" ]) for always exactly 8 regions (with a value
> of 0xdeadbeef for invalid regions).
>
> I added this check for the storage drivers, which contain:
>  for (region_idx = 0; region_idx < dev->num_regions; region_idx++) {
>    ...
>    gendisk->first_minor = devidx * PS3DISK_MINORS + region_idx;
>
> But that limit might be raised in future hypervisor versions.
> Maybe a
>  BUG_ON(dev->num_regions <= PS3DISK_MINORS);
> is more appropriate?

Of course I meant the exact opposite, heh:
  BUG_ON(dev->num_regions > PS3DISK_MINORS);