[PATCH] x86/iopl: Harmonize 'struct io_bitmap' and 'struct x86_io_bitmap' nomenclature

From: Ingo Molnar
Date: Tue Nov 12 2019 - 02:59:50 EST



* Ingo Molnar <mingo@xxxxxxxxxx> wrote:

> How about the patch below?
>
> It reorganizes all those fields into a new container structure, 'struct
> x86_io_bitmap', adds it as tss.io_bitmap, and uses the prefix to shorten
> and organize the names of the fields:
>
> tss.last_bitmap => tss.io_bitmap.last_bitmap
> tss.last_sequence => tss.io_bitmap.last_sequence
> tss.io_bitmap_prev_max => tss.io_bitmap.prev_max
> tss.io_bitmap_bytes => tss.io_bitmap.map_bytes
> tss.io_bitmap_all => tss.io_bitmap.map_all
>
> This makes it far more readable, and the local variable references as
> short and tidy:
>
> iobm->last_bitmap
> iobm->last_sequence
> iobm->prev_max
> iobm->map_bytes
> iobm->map_all
>
> Only build tested.

On top of this we can now do the slight reorganization of field names
below.

io_bitmap.io_bitmap_max => bytes_max
x86_io_bitmap.prev_max => bytes_max


The point is to harmonize the fields in 'struct io_bitmap' with 'struct
x86_io_bitmap':

io_bitmap.map_bytes io_bitmap.bytes_max
iobm.map_bytes iobm.bytes_max


which makes following segments of code really tidy:

memcpy(tss->io_bitmap.map_bytes, iobm->map_bytes,
max(tss->io_bitmap.bytes_max, iobm->bytes_max));

The other point is to remove the 'prev' notion from the TSS data field -
there's very little "previous" about this - we have this bitmap loaded
right now.

Also note the cleanup of the bits/bytes union:

iobm->bits => iobm->map_bits
iobm->bitmap_bytes => iobm->map_bytes

Only build tested, but 100% perfect, guaranteed.

Thanks,

Ingo

---
arch/x86/include/asm/iobitmap.h | 6 +++---
arch/x86/include/asm/processor.h | 2 +-
arch/x86/kernel/cpu/common.c | 2 +-
arch/x86/kernel/ioport.c | 10 +++++-----
arch/x86/kernel/process.c | 6 +++---
arch/x86/kernel/ptrace.c | 4 ++--
6 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/x86/include/asm/iobitmap.h b/arch/x86/include/asm/iobitmap.h
index a075f0c55fb3..faba42c7d3de 100644
--- a/arch/x86/include/asm/iobitmap.h
+++ b/arch/x86/include/asm/iobitmap.h
@@ -8,10 +8,10 @@
struct io_bitmap {
u64 sequence;
refcount_t refcnt;
- unsigned int io_bitmap_max;
+ unsigned int bytes_max;
union {
- unsigned long bits[IO_BITMAP_LONGS];
- unsigned char bitmap_bytes[IO_BITMAP_BYTES];
+ unsigned long map_bits[IO_BITMAP_LONGS];
+ unsigned char map_bytes[IO_BITMAP_BYTES];
};
};

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 4358ae63c252..d1f2c1eb14e9 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -381,7 +381,7 @@ struct x86_io_bitmap
* outside of the TSS limit. So for sane tasks there is no need to
* actually touch the io_bitmap at all.
*/
- unsigned int prev_max;
+ unsigned int bytes_max;

/*
* The extra 1 is there because the CPU will access an
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index eea0e3170de4..a35a557429e7 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1862,7 +1862,7 @@ void cpu_init(void)
tss_setup_ist(tss);
tss->x86_tss.io_bitmap_base = IO_BITMAP_OFFSET_INVALID;
tss->io_bitmap.last_bitmap = NULL;
- tss->io_bitmap.prev_max = 0;
+ tss->io_bitmap.bytes_max = 0;
memset(tss->io_bitmap.map_bytes, 0xff, sizeof(tss->io_bitmap.map_bytes));
set_tss_desc(cpu, &get_cpu_entry_area(cpu)->tss.x86_tss);

diff --git a/arch/x86/kernel/ioport.c b/arch/x86/kernel/ioport.c
index ee37a1c25ecc..203f82383bf6 100644
--- a/arch/x86/kernel/ioport.c
+++ b/arch/x86/kernel/ioport.c
@@ -86,7 +86,7 @@ long ksys_ioperm(unsigned long from, unsigned long num, int turn_on)
if (!iobm)
return -ENOMEM;

- memset(iobm->bits, 0xff, sizeof(iobm->bits));
+ memset(iobm->map_bits, 0xff, sizeof(iobm->map_bits));
refcount_set(&iobm->refcnt, 1);
}

@@ -112,9 +112,9 @@ long ksys_ioperm(unsigned long from, unsigned long num, int turn_on)
* exit to user mode. So this needs no protection.
*/
if (turn_on)
- bitmap_clear(iobm->bits, from, num);
+ bitmap_clear(iobm->map_bits, from, num);
else
- bitmap_set(iobm->bits, from, num);
+ bitmap_set(iobm->map_bits, from, num);

/*
* Search for a (possibly new) maximum. This is simple and stupid,
@@ -122,7 +122,7 @@ long ksys_ioperm(unsigned long from, unsigned long num, int turn_on)
*/
max_long = UINT_MAX;
for (i = 0; i < IO_BITMAP_LONGS; i++) {
- if (iobm->bits[i] != ~0UL)
+ if (iobm->map_bits[i] != ~0UL)
max_long = i;
}

@@ -132,7 +132,7 @@ long ksys_ioperm(unsigned long from, unsigned long num, int turn_on)
return 0;
}

- iobm->io_bitmap_max = (max_long + 1) * sizeof(unsigned long);
+ iobm->bytes_max = (max_long + 1) * sizeof(unsigned long);

/* Increment the sequence number to force a TSS update */
iobm->sequence = atomic64_add_return(1, &io_bitmap_sequence);
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 8179f3ee6a55..c4e76a540b51 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -350,14 +350,14 @@ static void tss_copy_io_bitmap(struct tss_struct *tss, struct io_bitmap *iobm)
* permitted, then the copy needs to cover those as well so they
* get turned off.
*/
- memcpy(tss->io_bitmap.map_bytes, iobm->bitmap_bytes,
- max(tss->io_bitmap.prev_max, iobm->io_bitmap_max));
+ memcpy(tss->io_bitmap.map_bytes, iobm->map_bytes,
+ max(tss->io_bitmap.bytes_max, iobm->bytes_max));

/*
* Store the new max and the sequence number of this bitmap
* and a pointer to the bitmap itself.
*/
- tss->io_bitmap.prev_max = iobm->io_bitmap_max;
+ tss->io_bitmap.bytes_max = iobm->bytes_max;
tss->io_bitmap.last_sequence = iobm->sequence;
tss->io_bitmap.last_bitmap = iobm;
}
diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index 6baa5a1a9f1c..f27c322f1c93 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -700,7 +700,7 @@ static int ioperm_active(struct task_struct *target,
{
struct io_bitmap *iobm = target->thread.io_bitmap;

- return iobm ? DIV_ROUND_UP(iobm->io_bitmap_max, regset->size) : 0;
+ return iobm ? DIV_ROUND_UP(iobm->bytes_max, regset->size) : 0;
}

static int ioperm_get(struct task_struct *target,
@@ -714,7 +714,7 @@ static int ioperm_get(struct task_struct *target,
return -ENXIO;

return user_regset_copyout(&pos, &count, &kbuf, &ubuf,
- iobm->bitmap_bytes, 0, IO_BITMAP_BYTES);
+ iobm->map_bytes, 0, IO_BITMAP_BYTES);
}

/*