[PATCH v3] lkdtm: fix memory copy size for WRITE_KERN
From: Candle Sun
Though do_overwritten() follows do_nothing() in source code, the final
memory address order is determined by the compiler. We can't always
assume address of do_overwritten() is bigger than do_nothing(). At least
the Clang we are using places do_overwritten() before do_nothing() in the
object. This causes the copy size in lkdtm_WRITE_KERN() is *really* big
and WRITE_KERN test on ARM32 arch will fail.
Get absolute value of the address subtraction for memcpy() size.
Signed-off-by: Candle Sun
---
Changes in v3:
- Clean some typo.
- Remove one comment line.
Changes in v2:
- Use abs() in place of address comparison.
---
drivers/misc/lkdtm/perms.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
index 2dede2ef658f..c9227e08f97f 100644
--- a/drivers/misc/lkdtm/perms.c
+++ b/drivers/misc/lkdtm/perms.c
@@ -31,13 +31,12 @@ static unsigned long ro_after_init __ro_after_init =
0x55AA5500;
* This just returns to the caller. It is designed to be copied into
* non-executable memory regions.
*/
-static void do_nothing(void)
+static noinline void do_nothing(void)
{
return;
}
-/* Must immediately follow do_nothing for size calculuations to work out. */
-static void do_overwritten(void)
+static noinline void do_overwritten(void)
{
pr_info("do_overwritten wasn't overwritten!\n");
return;
@@ -113,7 +112,7 @@ void lkdtm_WRITE_KERN(void)
size_t size;
volatile unsigned char *ptr;
- size = (unsigned long)do_overwritten - (unsigned long)do_nothing;
+ size = (size_t)abs((uintptr_t)do_overwritten - (uintptr_t)do_nothing);
ptr = (unsigned char *)do_overwritten;
pr_info("attempting bad %zu byte write at %px\n", size, ptr);
--
2.17.0
Re: [PATCH v2] lkdtm: fix memory copy size for WRITE_KERN
On Thu, Jan 28, 2021 at 1:40 AM Nick Desaulniers
wrote:
>
> On Wed, Jan 27, 2021 at 3:05 AM Candle Sun wrote:
> >
> > From: Candle Sun
> >
> > Though do_overwritten() follows do_nothing() in source code, the final
> > memory address order is determined by compiler. We can't always assume
>
> ^ "by the compiler."
>
> > address of do_overwritten() is bigger than do_nothing(). At least the
> > Clang we are using places do_overwritten() before do_nothing() in the
> > object. This causes the copy size in lkdtm_WRITE_KERN() is *really*
>
> Hopefully nothing else gets placed in between the two, otherwise we're
> going to overwrite that, too.
>
> > big and WRITE_KERN test on ARM32 arch will fail.
> >
> > Get absolute value of the address substraction for memcpy() size.
>
> ^ typo: subtraction
>
> >
> > Signed-off-by: Candle Sun
> > ---
> > Changes in v2:
> > - Use abs() in place of address comparison.
> > ---
> > drivers/misc/lkdtm/perms.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
> > index 2dede2ef658f..fbb7f4554054 100644
> > --- a/drivers/misc/lkdtm/perms.c
> > +++ b/drivers/misc/lkdtm/perms.c
> > @@ -31,13 +31,13 @@ static unsigned long ro_after_init __ro_after_init =
> > 0x55AA5500;
> > * This just returns to the caller. It is designed to be copied into
> > * non-executable memory regions.
> > */
> > -static void do_nothing(void)
> > +static noinline void do_nothing(void)
> > {
> > return;
> > }
> >
> > /* Must immediately follow do_nothing for size calculuations to work out.
> > */
>
> ^ This comment is now obsolete and should be removed together with
> this patch. That will also fix the typo in it.
>
> > -static void do_overwritten(void)
> > +static noinline void do_overwritten(void)
> > {
> > pr_info("do_overwritten wasn't overwritten!\n");
> > return;
> > @@ -113,7 +113,7 @@ void lkdtm_WRITE_KERN(void)
> > size_t size;
> > volatile unsigned char *ptr;
> >
> > - size = (unsigned long)do_overwritten - (unsigned long)do_nothing;
> > + size = (size_t)abs((uintptr_t)do_overwritten -
> > (uintptr_t)do_nothing);
> > ptr = (unsigned char *)do_overwritten;
> >
> > pr_info("attempting bad %zu byte write at %px\n", size, ptr);
> > --
> > 2.17.0
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers
Thanks Nick. I will clean the typo and the comment line in the next patch.
Regards,
Candle
Re: [PATCH] lkdtm: fix memory copy size for WRITE_KERN
Thanks David and Nick. For simpleness. I just use abs() to get the copy size. Patch version 2 is mailed. Please help review it. Regards, Candle On Wed, Jan 27, 2021 at 6:53 AM David Laight wrote: > > From: Nick Desaulniers > > Sent: 26 January 2021 18:40 > > > > On Tue, Jan 26, 2021 at 6:13 AM Candle Sun wrote: > > > > > > On Mon, Jan 25, 2021 at 6:37 PM David Laight > > > wrote: > > > > > > > > From: Candle Sun > > > > > Sent: 25 January 2021 08:56 > > > > > > > > > > From: Candle Sun > > > > > > > > > > Though do_overwritten() follows do_nothing() in source code, the final > > > > > memory address order is determined by compiler. We can't always assume > > > > > address of do_overwritten() is bigger than do_nothing(). At least the > > > > > Clang we are using places do_overwritten() before do_nothing() in the > > > > > object. This causes the copy size in lkdtm_WRITE_KERN() is *really* > > > > > big and WRITE_KERN test on ARM32 arch will fail. > > > > > > > > > > Compare the address order before doing the subtraction. > > > > > > > > It isn't clear that helps. > > > > Compile with -ffunction-sections and/or do LTO an there > > > > is no reason at all why the functions should be together. > > > > > > > > Even without that lkdtm_WRITE_KERN() could easily be between them. > > > > > > > > You need to get the size of the 'empty function' from the > > > > symbol table. > > > > > > > > David > > > > > > Thanks David. > > > > > > I think using abs() by Nick's advice would be better. But could you > > > point out which kernel function can get function size? > > > > The Elf symbol table should contain this info, IIUC. > > > > Given a string literal of a symbol (such as a function identifier), > > kallsyms_lookup_name() can be used to return its address. > > > > From there we'd want to fetch the Elf_Sym for the address which should > > contain a st_size field which I think corresponds to the size in bytes > > of the function. (At least, from playing with `llvm-readelf -s`) > > Probably would want to validate it's an STT_FUNC symbol type, too. We > > basically want something like kexec_purgatory_find_symbol(), but that > > knows about the entire kernel image, and not the purgatory image used > > during kexec. I don't see any such function currently in the > > kernel...but it's a large codebase to search through. > > The alternative is to get the linker script to define a specific > constant to the size of the function. > You can then link against it (by using it as the address of a symbol). > > It may be easier to use an asm file for the 'return 0' code. > I'm guessing there are things in the static branch area > that could be used. > > David > > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 > 1PT, UK > Registration No: 1397386 (Wales)
[PATCH v2] lkdtm: fix memory copy size for WRITE_KERN
From: Candle Sun
Though do_overwritten() follows do_nothing() in source code, the final
memory address order is determined by compiler. We can't always assume
address of do_overwritten() is bigger than do_nothing(). At least the
Clang we are using places do_overwritten() before do_nothing() in the
object. This causes the copy size in lkdtm_WRITE_KERN() is *really*
big and WRITE_KERN test on ARM32 arch will fail.
Get absolute value of the address substraction for memcpy() size.
Signed-off-by: Candle Sun
---
Changes in v2:
- Use abs() in place of address comparison.
---
drivers/misc/lkdtm/perms.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
index 2dede2ef658f..fbb7f4554054 100644
--- a/drivers/misc/lkdtm/perms.c
+++ b/drivers/misc/lkdtm/perms.c
@@ -31,13 +31,13 @@ static unsigned long ro_after_init __ro_after_init =
0x55AA5500;
* This just returns to the caller. It is designed to be copied into
* non-executable memory regions.
*/
-static void do_nothing(void)
+static noinline void do_nothing(void)
{
return;
}
/* Must immediately follow do_nothing for size calculuations to work out. */
-static void do_overwritten(void)
+static noinline void do_overwritten(void)
{
pr_info("do_overwritten wasn't overwritten!\n");
return;
@@ -113,7 +113,7 @@ void lkdtm_WRITE_KERN(void)
size_t size;
volatile unsigned char *ptr;
- size = (unsigned long)do_overwritten - (unsigned long)do_nothing;
+ size = (size_t)abs((uintptr_t)do_overwritten - (uintptr_t)do_nothing);
ptr = (unsigned char *)do_overwritten;
pr_info("attempting bad %zu byte write at %px\n", size, ptr);
--
2.17.0
Re: [PATCH] lkdtm: fix memory copy size for WRITE_KERN
On Tue, Jan 26, 2021 at 5:16 AM Nick Desaulniers
wrote:
>
> On Mon, Jan 25, 2021 at 12:56 AM Candle Sun wrote:
> >
> > From: Candle Sun
> >
> > Though do_overwritten() follows do_nothing() in source code, the final
> > memory address order is determined by compiler. We can't always assume
> > address of do_overwritten() is bigger than do_nothing(). At least the
> > Clang we are using places do_overwritten() before do_nothing() in the
> > object. This causes the copy size in lkdtm_WRITE_KERN() is *really*
> > big and WRITE_KERN test on ARM32 arch will fail.
> >
> > Compare the address order before doing the subtraction.
> >
> > Signed-off-by: Candle Sun
> > ---
> > drivers/misc/lkdtm/perms.c | 19 +--
> > 1 file changed, 9 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
> > index 2dede2ef658f..fbfbdf89d668 100644
> > --- a/drivers/misc/lkdtm/perms.c
> > +++ b/drivers/misc/lkdtm/perms.c
> > @@ -31,13 +31,13 @@ static unsigned long ro_after_init __ro_after_init =
> > 0x55AA5500;
> > * This just returns to the caller. It is designed to be copied into
> > * non-executable memory regions.
> > */
> > -static void do_nothing(void)
> > +static noinline void do_nothing(void)
> > {
> > return;
> > }
> >
> > /* Must immediately follow do_nothing for size calculuations to work out.
> > */
> > -static void do_overwritten(void)
> > +static noinline void do_overwritten(void)
> > {
> > pr_info("do_overwritten wasn't overwritten!\n");
> > return;
> > @@ -110,15 +110,14 @@ void lkdtm_WRITE_RO_AFTER_INIT(void)
> >
> > void lkdtm_WRITE_KERN(void)
> > {
> > - size_t size;
> > - volatile unsigned char *ptr;
> > + unsigned long value_dow = (unsigned long)do_overwritten;
> > + unsigned long value_do = (unsigned long)do_nothing;
> > + size_t size = (size_t)(value_dow > value_do ?
> > + value_dow - value_do : value_do - value_dow);
> >
> > - size = (unsigned long)do_overwritten - (unsigned long)do_nothing;
>
> Thanks for the patch! Might it be simpler to do:
>
> size = abs((uintptr_t)do_overwritten - (uintptr_t)do_nothing));
>
> Then I don't think much of this function needs to be changed.
>
Thanks Nick.
abs() is better, I will check the patch.
Regards,
Candle
> > - ptr = (unsigned char *)do_overwritten;
> > -
> > - pr_info("attempting bad %zu byte write at %px\n", size, ptr);
> > - memcpy((void *)ptr, (unsigned char *)do_nothing, size);
> > - flush_icache_range((unsigned long)ptr, (unsigned long)(ptr + size));
> > + pr_info("attempting bad %zu byte write at %px\n", size,
> > do_overwritten);
> > + memcpy((void *)value_dow, (void *)value_do, size);
> > + flush_icache_range(value_dow, value_dow + (unsigned long)size);
> > pr_err("FAIL: survived bad write\n");
> >
> > do_overwritten();
> > --
> > 2.17.0
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers
Re: [PATCH] lkdtm: fix memory copy size for WRITE_KERN
On Mon, Jan 25, 2021 at 6:37 PM David Laight wrote:
>
> From: Candle Sun
> > Sent: 25 January 2021 08:56
> >
> > From: Candle Sun
> >
> > Though do_overwritten() follows do_nothing() in source code, the final
> > memory address order is determined by compiler. We can't always assume
> > address of do_overwritten() is bigger than do_nothing(). At least the
> > Clang we are using places do_overwritten() before do_nothing() in the
> > object. This causes the copy size in lkdtm_WRITE_KERN() is *really*
> > big and WRITE_KERN test on ARM32 arch will fail.
> >
> > Compare the address order before doing the subtraction.
>
> It isn't clear that helps.
> Compile with -ffunction-sections and/or do LTO an there
> is no reason at all why the functions should be together.
>
> Even without that lkdtm_WRITE_KERN() could easily be between them.
>
> You need to get the size of the 'empty function' from the
> symbol table.
>
> David
Thanks David.
I think using abs() by Nick's advice would be better. But could you
point out which kernel function can get function size?
Regards,
Candle
>
> >
> > Signed-off-by: Candle Sun
> > ---
> > drivers/misc/lkdtm/perms.c | 19 +--
> > 1 file changed, 9 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
> > index 2dede2ef658f..fbfbdf89d668 100644
> > --- a/drivers/misc/lkdtm/perms.c
> > +++ b/drivers/misc/lkdtm/perms.c
> > @@ -31,13 +31,13 @@ static unsigned long ro_after_init __ro_after_init =
> > 0x55AA5500;
> > * This just returns to the caller. It is designed to be copied into
> > * non-executable memory regions.
> > */
> > -static void do_nothing(void)
> > +static noinline void do_nothing(void)
> > {
> > return;
> > }
> >
> > /* Must immediately follow do_nothing for size calculuations to work out.
> > */
> > -static void do_overwritten(void)
> > +static noinline void do_overwritten(void)
> > {
> > pr_info("do_overwritten wasn't overwritten!\n");
> > return;
> > @@ -110,15 +110,14 @@ void lkdtm_WRITE_RO_AFTER_INIT(void)
> >
> > void lkdtm_WRITE_KERN(void)
> > {
> > - size_t size;
> > - volatile unsigned char *ptr;
> > + unsigned long value_dow = (unsigned long)do_overwritten;
> > + unsigned long value_do = (unsigned long)do_nothing;
> > + size_t size = (size_t)(value_dow > value_do ?
> > + value_dow - value_do : value_do - value_dow);
> >
> > - size = (unsigned long)do_overwritten - (unsigned long)do_nothing;
> > - ptr = (unsigned char *)do_overwritten;
> > -
> > - pr_info("attempting bad %zu byte write at %px\n", size, ptr);
> > - memcpy((void *)ptr, (unsigned char *)do_nothing, size);
> > - flush_icache_range((unsigned long)ptr, (unsigned long)(ptr + size));
> > + pr_info("attempting bad %zu byte write at %px\n", size,
> > do_overwritten);
> > + memcpy((void *)value_dow, (void *)value_do, size);
> > + flush_icache_range(value_dow, value_dow + (unsigned long)size);
> > pr_err("FAIL: survived bad write\n");
> >
> > do_overwritten();
> > --
> > 2.17.0
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1
> 1PT, UK
> Registration No: 1397386 (Wales)
>
[PATCH] lkdtm: fix memory copy size for WRITE_KERN
From: Candle Sun
Though do_overwritten() follows do_nothing() in source code, the final
memory address order is determined by compiler. We can't always assume
address of do_overwritten() is bigger than do_nothing(). At least the
Clang we are using places do_overwritten() before do_nothing() in the
object. This causes the copy size in lkdtm_WRITE_KERN() is *really*
big and WRITE_KERN test on ARM32 arch will fail.
Compare the address order before doing the subtraction.
Signed-off-by: Candle Sun
---
drivers/misc/lkdtm/perms.c | 19 +--
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
index 2dede2ef658f..fbfbdf89d668 100644
--- a/drivers/misc/lkdtm/perms.c
+++ b/drivers/misc/lkdtm/perms.c
@@ -31,13 +31,13 @@ static unsigned long ro_after_init __ro_after_init =
0x55AA5500;
* This just returns to the caller. It is designed to be copied into
* non-executable memory regions.
*/
-static void do_nothing(void)
+static noinline void do_nothing(void)
{
return;
}
/* Must immediately follow do_nothing for size calculuations to work out. */
-static void do_overwritten(void)
+static noinline void do_overwritten(void)
{
pr_info("do_overwritten wasn't overwritten!\n");
return;
@@ -110,15 +110,14 @@ void lkdtm_WRITE_RO_AFTER_INIT(void)
void lkdtm_WRITE_KERN(void)
{
- size_t size;
- volatile unsigned char *ptr;
+ unsigned long value_dow = (unsigned long)do_overwritten;
+ unsigned long value_do = (unsigned long)do_nothing;
+ size_t size = (size_t)(value_dow > value_do ?
+ value_dow - value_do : value_do - value_dow);
- size = (unsigned long)do_overwritten - (unsigned long)do_nothing;
- ptr = (unsigned char *)do_overwritten;
-
- pr_info("attempting bad %zu byte write at %px\n", size, ptr);
- memcpy((void *)ptr, (unsigned char *)do_nothing, size);
- flush_icache_range((unsigned long)ptr, (unsigned long)(ptr + size));
+ pr_info("attempting bad %zu byte write at %px\n", size, do_overwritten);
+ memcpy((void *)value_dow, (void *)value_do, size);
+ flush_icache_range(value_dow, value_dow + (unsigned long)size);
pr_err("FAIL: survived bad write\n");
do_overwritten();
--
2.17.0
[PATCH v4] HID: core: check whether Usage Page item is after Usage ID items
From: Candle Sun
Upstream commit 58e75155009c ("HID: core: move Usage Page concatenation
to Main item") adds support for Usage Page item after Usage ID items
(such as keyboards manufactured by Primax).
Usage Page concatenation in Main item works well for following report
descriptor patterns:
USAGE_PAGE (Keyboard) 05 07
USAGE_MINIMUM (Keyboard LeftControl)19 E0
USAGE_MAXIMUM (Keyboard Right GUI) 29 E7
LOGICAL_MINIMUM (0) 15 00
LOGICAL_MAXIMUM (1) 25 01
REPORT_SIZE (1) 75 01
REPORT_COUNT (8)95 08
INPUT (Data,Var,Abs)81 02
-
USAGE_MINIMUM (Keyboard LeftControl)19 E0
USAGE_MAXIMUM (Keyboard Right GUI) 29 E7
LOGICAL_MINIMUM (0) 15 00
LOGICAL_MAXIMUM (1) 25 01
REPORT_SIZE (1) 75 01
REPORT_COUNT (8)95 08
USAGE_PAGE (Keyboard) 05 07
INPUT (Data,Var,Abs)81 02
But it makes the parser act wrong for the following report
descriptor pattern(such as some Gamepads):
USAGE_PAGE (Button) 05 09
USAGE (Button 1)09 01
USAGE (Button 2)09 02
USAGE (Button 4)09 04
USAGE (Button 5)09 05
USAGE (Button 7)09 07
USAGE (Button 8)09 08
USAGE (Button 14) 09 0E
USAGE (Button 15) 09 0F
USAGE (Button 13) 09 0D
USAGE_PAGE (Consumer Devices) 05 0C
USAGE (Back)0a 24 02
USAGE (HomePage)0a 23 02
LOGICAL_MINIMUM (0) 15 00
LOGICAL_MAXIMUM (1) 25 01
REPORT_SIZE (1) 75 01
REPORT_COUNT (11) 95 0B
INPUT (Data,Var,Abs)81 02
With Usage Page concatenation in Main item, parser recognizes all the
11 Usages as consumer keys, it is not the HID device's real intention.
This patch checks whether Usage Page is really defined after Usage ID
items by comparing usage page using status.
Usage Page concatenation on currently defined Usage Page will always
do in local parsing when Usage ID items encountered.
When Main item is parsing, concatenation will do again with last
defined Usage Page if this page has not been used in the previous
usages concatenation.
Signed-off-by: Candle Sun
Signed-off-by: Nianfu Bai
---
Changes in v4:
- Fix v3 introduced BUG in hid_add_usage()
- Add checking logic to replace usage_page_last member
- Update patch description
Changes in v3:
- Rework the GET_COMPLETE_USAGE macro as static complete_usage()
function
- Add some code comments for usage_page_last
Changes in v2:
- Update patch title
- Add GET_COMPLETE_USAGE macro
- Change the logic of checking whether to concatenate usage page again
in main parsing
---
drivers/hid/hid-core.c | 51 +-
1 file changed, 45 insertions(+), 6 deletions(-)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 3eaee2c37931..c18ed7180b07 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -211,6 +211,18 @@ static unsigned hid_lookup_collection(struct hid_parser
*parser, unsigned type)
return 0; /* we know nothing about this usage type */
}
+/*
+ * Concatenate usage which defines 16 bits or less with the
+ * currently defined usage page to form a 32 bit usage
+ */
+
+static void complete_usage(struct hid_parser *parser, unsigned int index)
+{
+ parser->local.usage[index] &= 0x;
+ parser->local.usage[index] |=
+ (parser->global.usage_page & 0x) << 16;
+}
+
/*
* Add a usage to the temporary parser table.
*/
@@ -222,6 +234,14 @@ static int hid_add_usage(struct hid_parser *parser,
unsigned usage, u8 size)
return -1;
}
parser->local.usage[parser->local.usage_index] = usage;
+
+ /*
+* If Usage item only includes usage id, concatenate it with
+* currently defined usage page
+*/
+ if (size <= 2)
+ complete_usage(parser, parser->local.usage_index);
+
parser->local.usage_size[parser->local.usage_index] = size;
parser->local.collection_index[parser->local.usage_index] =
parser->collection_stack_ptr ?
@@ -543,13 +563,32 @@ static int hid_parser_local(struct hid_parser *parser,
struct hid_item *item)
* usage value."
*/
-static void hid_concatenate_usage_page(struct hid_parser *parser)
+static void hid_concatenate_last_usage_page(struct hid_parser *parser)
{
int i;
+ unsigned int us
Re: [RESEND PATCH] ARM/hw_breakpoint: add ARMv8.1/ARMv8.2 debug architecutre versions support in enable_monitor_mode()
On Tue, Oct 22, 2019 at 8:25 PM Will Deacon wrote: > > On Thu, Sep 26, 2019 at 03:38:28PM +0800, Candle Sun wrote: > > From: Candle Sun > > > > When ARMv8.1/ARMv8.2 cores are used in AArch32 mode, > > arch_hw_breakpoint_init() in arch/arm/kernel/hw_breakpoint.c will be used. > > > > From ARMv8 specification, different debug architecture versions defined: > > * 0110 ARMv8, v8 Debug architecture. > > * 0111 ARMv8.1, v8 Debug architecture, with Virtualization Host Extensions. > > * 1000 ARMv8.2, v8.2 Debug architecture. > > > > So missing ARMv8.1/ARMv8.2 cases will cause enable_monitor_mode() function > > returns -ENODEV, and arch_hw_breakpoint_init() will fail. > > > > Signed-off-by: Candle Sun > > Signed-off-by: Nianfu Bai > > --- > > arch/arm/include/asm/hw_breakpoint.h | 2 ++ > > arch/arm/kernel/hw_breakpoint.c | 2 ++ > > 2 files changed, 4 insertions(+) > > [...] > > > diff --git a/arch/arm/include/asm/hw_breakpoint.h > > b/arch/arm/include/asm/hw_breakpoint.h > > index ac54c06..9137ef6 100644 > > --- a/arch/arm/include/asm/hw_breakpoint.h > > +++ b/arch/arm/include/asm/hw_breakpoint.h > > @@ -53,6 +53,8 @@ static inline void decode_ctrl_reg(u32 reg, > > #define ARM_DEBUG_ARCH_V7_MM 4 > > #define ARM_DEBUG_ARCH_V7_1 5 > > #define ARM_DEBUG_ARCH_V86 > > +#define ARM_DEBUG_ARCH_V8_1 7 > > +#define ARM_DEBUG_ARCH_V8_2 8 > > Looks like you can also add: > > #define ARM_DEBUG_ARCH_V8_4 9 > > and treat that the same way. With that, and a fix to $SUBJECT: > > Acked-by: Will Deacon > > Please put this into the patch system [1]. > > Will > > [1] https://www.arm.linux.org.uk/developer/patches/ Thanks, Will. I will do it ASAP. Best regards, Candle
Re: [PATCH v3] HID: core: check whether usage page item is after usage id item
Hi Benjamin,
On Tue, Oct 22, 2019 at 5:48 PM Benjamin Tissoires
wrote:
>
> Hi Candle,
>
> On Mon, Oct 21, 2019 at 9:54 AM Candle Sun wrote:
> >
> > Hi,
> >
> >
> > On Mon, Oct 21, 2019 at 3:38 PM Candle Sun wrote:
> > >
> > > From: Candle Sun
> > >
> > > Upstream commit 58e75155009c ("HID: core: move Usage Page concatenation
> > > to Main item") adds support for Usage Page item after Usage ID items
> > > (such as keyboards manufactured by Primax).
> > >
> > > Usage Page concatenation in Main item works well for following report
> > > descriptor patterns:
> > >
> > > USAGE_PAGE (Keyboard) 05 07
> > > USAGE_MINIMUM (Keyboard LeftControl)19 E0
> > > USAGE_MAXIMUM (Keyboard Right GUI) 29 E7
> > > LOGICAL_MINIMUM (0) 15 00
> > > LOGICAL_MAXIMUM (1) 25 01
> > > REPORT_SIZE (1) 75 01
> > > REPORT_COUNT (8)95 08
> > > INPUT (Data,Var,Abs)81 02
> > >
> > > -
> > >
> > > USAGE_MINIMUM (Keyboard LeftControl)19 E0
> > > USAGE_MAXIMUM (Keyboard Right GUI) 29 E7
> > > LOGICAL_MINIMUM (0) 15 00
> > > LOGICAL_MAXIMUM (1) 25 01
> > > REPORT_SIZE (1) 75 01
> > > REPORT_COUNT (8)95 08
> > > USAGE_PAGE (Keyboard) 05 07
> > > INPUT (Data,Var,Abs)81 02
> > >
> > > But it makes the parser act wrong for the following report
> > > descriptor pattern(such as some Gamepads):
> > >
> > > USAGE_PAGE (Button) 05 09
> > > USAGE (Button 1)09 01
> > > USAGE (Button 2)09 02
> > > USAGE (Button 4)09 04
> > > USAGE (Button 5)09 05
> > > USAGE (Button 7)09 07
> > > USAGE (Button 8)09 08
> > > USAGE (Button 14) 09 0E
> > > USAGE (Button 15) 09 0F
> > > USAGE (Button 13) 09 0D
> > > USAGE_PAGE (Consumer Devices) 05 0C
> > > USAGE (Back)0a 24 02
> > > USAGE (HomePage)0a 23 02
> > > LOGICAL_MINIMUM (0) 15 00
> > > LOGICAL_MAXIMUM (1) 25 01
> > > REPORT_SIZE (1) 75 01
> > > REPORT_COUNT (11) 95 0B
> > > INPUT (Data,Var,Abs)81 02
> > >
> > > With Usage Page concatenation in Main item, parser recognizes all the
> > > 11 Usages as consumer keys, it is not the HID device's real intention.
> > >
> > > This patch adds usage_page_last to flag whether Usage Page is after
> > > Usage ID items. usage_page_last is false default, it is set as true
> > > once Usage Page item is encountered and is reverted by next Usage ID
> > > item.
> > >
> > > Usage Page concatenation on the currently defined Usage Page will do
> > > firstly in Local parsing when Usage ID items encountered.
> > >
> > > When Main item is parsing, concatenation will do again with last
> > > defined Usage Page if usage_page_last flag is true.
> > >
> > > Signed-off-by: Candle Sun
> > > Signed-off-by: Nianfu Bai
> > > ---
> > > Changes in v3:
> > > - Rework the GET_COMPLETE_USAGE macro as static complete_usage()
> > > function
> > > - Add some code comments for usage_page_last
> > >
> > > Changes in v2:
> > > - Update patch title
> > > - Add GET_COMPLETE_USAGE macro
> > > - Change the logic of checking whether to concatenate usage page again
> > > in main parsing
> > > ---
> > > drivers/hid/hid-core.c | 42 +-
> > > include/linux/hid.h| 1 +
> > > 2 files changed, 38 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> > > index 3eaee2c37931..779b7798dae8 100644
> > > --- a/drivers/hid/hid-core.c
> > > +++ b/drivers/hid/hid-core.c
> > > @@ -211,
Re: [PATCH v3] HID: core: check whether usage page item is after usage id item
Hi,
On Mon, Oct 21, 2019 at 3:38 PM Candle Sun wrote:
>
> From: Candle Sun
>
> Upstream commit 58e75155009c ("HID: core: move Usage Page concatenation
> to Main item") adds support for Usage Page item after Usage ID items
> (such as keyboards manufactured by Primax).
>
> Usage Page concatenation in Main item works well for following report
> descriptor patterns:
>
> USAGE_PAGE (Keyboard) 05 07
> USAGE_MINIMUM (Keyboard LeftControl)19 E0
> USAGE_MAXIMUM (Keyboard Right GUI) 29 E7
> LOGICAL_MINIMUM (0) 15 00
> LOGICAL_MAXIMUM (1) 25 01
> REPORT_SIZE (1) 75 01
> REPORT_COUNT (8)95 08
> INPUT (Data,Var,Abs)81 02
>
> -
>
> USAGE_MINIMUM (Keyboard LeftControl)19 E0
> USAGE_MAXIMUM (Keyboard Right GUI) 29 E7
> LOGICAL_MINIMUM (0) 15 00
> LOGICAL_MAXIMUM (1) 25 01
> REPORT_SIZE (1) 75 01
> REPORT_COUNT (8)95 08
> USAGE_PAGE (Keyboard) 05 07
> INPUT (Data,Var,Abs)81 02
>
> But it makes the parser act wrong for the following report
> descriptor pattern(such as some Gamepads):
>
> USAGE_PAGE (Button) 05 09
> USAGE (Button 1)09 01
> USAGE (Button 2)09 02
> USAGE (Button 4)09 04
> USAGE (Button 5)09 05
> USAGE (Button 7)09 07
> USAGE (Button 8)09 08
> USAGE (Button 14) 09 0E
> USAGE (Button 15) 09 0F
> USAGE (Button 13) 09 0D
> USAGE_PAGE (Consumer Devices) 05 0C
> USAGE (Back)0a 24 02
> USAGE (HomePage)0a 23 02
> LOGICAL_MINIMUM (0) 15 00
> LOGICAL_MAXIMUM (1) 25 01
> REPORT_SIZE (1) 75 01
> REPORT_COUNT (11) 95 0B
> INPUT (Data,Var,Abs)81 02
>
> With Usage Page concatenation in Main item, parser recognizes all the
> 11 Usages as consumer keys, it is not the HID device's real intention.
>
> This patch adds usage_page_last to flag whether Usage Page is after
> Usage ID items. usage_page_last is false default, it is set as true
> once Usage Page item is encountered and is reverted by next Usage ID
> item.
>
> Usage Page concatenation on the currently defined Usage Page will do
> firstly in Local parsing when Usage ID items encountered.
>
> When Main item is parsing, concatenation will do again with last
> defined Usage Page if usage_page_last flag is true.
>
> Signed-off-by: Candle Sun
> Signed-off-by: Nianfu Bai
> ---
> Changes in v3:
> - Rework the GET_COMPLETE_USAGE macro as static complete_usage()
> function
> - Add some code comments for usage_page_last
>
> Changes in v2:
> - Update patch title
> - Add GET_COMPLETE_USAGE macro
> - Change the logic of checking whether to concatenate usage page again
> in main parsing
> ---
> drivers/hid/hid-core.c | 42 +-
> include/linux/hid.h| 1 +
> 2 files changed, 38 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 3eaee2c37931..779b7798dae8 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -211,6 +211,18 @@ static unsigned hid_lookup_collection(struct hid_parser
> *parser, unsigned type)
> return 0; /* we know nothing about this usage type */
> }
>
> +/*
> + * Concatenate usage which defines 16 bits or less with the
> + * currently defined usage page to form a 32 bit usage
> + */
> +
> +static void complete_usage(struct hid_parser *parser, unsigned int index)
> +{
> + parser->local.usage[index] &= 0x;
> + parser->local.usage[index] |=
> + (parser->global.usage_page & 0x) << 16;
> +}
> +
> /*
> * Add a usage to the temporary parser table.
> */
> @@ -221,7 +233,18 @@ static int hid_add_usage(struct hid_parser *parser,
> unsigned usage, u8 size)
> hid_err(parser->device, "usage index exceeded\n");
> return -1;
> }
> - parser->local.usage[parser->local.usage_index] = usage;
> +
> + /*
> +* If Usage item only includes usage id, concate
[PATCH v3] HID: core: check whether usage page item is after usage id item
From: Candle Sun
Upstream commit 58e75155009c ("HID: core: move Usage Page concatenation
to Main item") adds support for Usage Page item after Usage ID items
(such as keyboards manufactured by Primax).
Usage Page concatenation in Main item works well for following report
descriptor patterns:
USAGE_PAGE (Keyboard) 05 07
USAGE_MINIMUM (Keyboard LeftControl)19 E0
USAGE_MAXIMUM (Keyboard Right GUI) 29 E7
LOGICAL_MINIMUM (0) 15 00
LOGICAL_MAXIMUM (1) 25 01
REPORT_SIZE (1) 75 01
REPORT_COUNT (8)95 08
INPUT (Data,Var,Abs)81 02
-
USAGE_MINIMUM (Keyboard LeftControl)19 E0
USAGE_MAXIMUM (Keyboard Right GUI) 29 E7
LOGICAL_MINIMUM (0) 15 00
LOGICAL_MAXIMUM (1) 25 01
REPORT_SIZE (1) 75 01
REPORT_COUNT (8)95 08
USAGE_PAGE (Keyboard) 05 07
INPUT (Data,Var,Abs)81 02
But it makes the parser act wrong for the following report
descriptor pattern(such as some Gamepads):
USAGE_PAGE (Button) 05 09
USAGE (Button 1)09 01
USAGE (Button 2)09 02
USAGE (Button 4)09 04
USAGE (Button 5)09 05
USAGE (Button 7)09 07
USAGE (Button 8)09 08
USAGE (Button 14) 09 0E
USAGE (Button 15) 09 0F
USAGE (Button 13) 09 0D
USAGE_PAGE (Consumer Devices) 05 0C
USAGE (Back)0a 24 02
USAGE (HomePage)0a 23 02
LOGICAL_MINIMUM (0) 15 00
LOGICAL_MAXIMUM (1) 25 01
REPORT_SIZE (1) 75 01
REPORT_COUNT (11) 95 0B
INPUT (Data,Var,Abs)81 02
With Usage Page concatenation in Main item, parser recognizes all the
11 Usages as consumer keys, it is not the HID device's real intention.
This patch adds usage_page_last to flag whether Usage Page is after
Usage ID items. usage_page_last is false default, it is set as true
once Usage Page item is encountered and is reverted by next Usage ID
item.
Usage Page concatenation on the currently defined Usage Page will do
firstly in Local parsing when Usage ID items encountered.
When Main item is parsing, concatenation will do again with last
defined Usage Page if usage_page_last flag is true.
Signed-off-by: Candle Sun
Signed-off-by: Nianfu Bai
---
Changes in v3:
- Rework the GET_COMPLETE_USAGE macro as static complete_usage()
function
- Add some code comments for usage_page_last
Changes in v2:
- Update patch title
- Add GET_COMPLETE_USAGE macro
- Change the logic of checking whether to concatenate usage page again
in main parsing
---
drivers/hid/hid-core.c | 42 +-
include/linux/hid.h| 1 +
2 files changed, 38 insertions(+), 5 deletions(-)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 3eaee2c37931..779b7798dae8 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -211,6 +211,18 @@ static unsigned hid_lookup_collection(struct hid_parser
*parser, unsigned type)
return 0; /* we know nothing about this usage type */
}
+/*
+ * Concatenate usage which defines 16 bits or less with the
+ * currently defined usage page to form a 32 bit usage
+ */
+
+static void complete_usage(struct hid_parser *parser, unsigned int index)
+{
+ parser->local.usage[index] &= 0x;
+ parser->local.usage[index] |=
+ (parser->global.usage_page & 0x) << 16;
+}
+
/*
* Add a usage to the temporary parser table.
*/
@@ -221,7 +233,18 @@ static int hid_add_usage(struct hid_parser *parser,
unsigned usage, u8 size)
hid_err(parser->device, "usage index exceeded\n");
return -1;
}
- parser->local.usage[parser->local.usage_index] = usage;
+
+ /*
+* If Usage item only includes usage id, concatenate it with
+* currently defined usage page and clear usage_page_last flag
+*/
+ if (size <= 2) {
+ parser->local.usage_page_last = false;
+ complete_usage(parser, parser->local.usage_index);
+ } else {
+ parser->local.usage[parser->local.usage_index] = usage;
+ }
+
parser->local.usage_size[parser->local.usage_index] = size;
parser->local.collection_index[parser->local.usage_index] =
parser->collection_stack_ptr ?
@@ -366,6 +389,8 @@ static int hid_parser_global(struct hid_par
Re: [RESEND PATCH] ARM/hw_breakpoint: add ARMv8.1/ARMv8.2 debug architecutre versions support in enable_monitor_mode()
Thanks Uwe for pointing out my typing error. Will, Is the patch ok? Do I need to send another version? Candle Candle On Fri, Oct 11, 2019 at 2:00 PM Uwe Kleine-König wrote: > > Hello, > > just noticed a typo in the subject line while going through my lakml > mailbox: > > s/architecutre/architecture/ > > Best regards > Uwe > > -- > Pengutronix e.K. | Uwe Kleine-König| > Industrial Linux Solutions | http://www.pengutronix.de/ |
Re: [RESEND PATCH] ARM/hw_breakpoint: add ARMv8.1/ARMv8.2 debug architecutre versions support in enable_monitor_mode()
Will,
Is the patch useful for you? Would you please give me some suggestions?
Thank you.
Regards,
Candle
On Tue, Oct 8, 2019 at 3:20 PM Candle Sun wrote:
>
> Hi Will,
> Sorry for not instant respond.
>
>
> On Mon, Sep 30, 2019 at 11:34 PM Will Deacon wrote:
> >
> > On Thu, Sep 26, 2019 at 03:38:28PM +0800, Candle Sun wrote:
> > > From: Candle Sun
> > >
> > > When ARMv8.1/ARMv8.2 cores are used in AArch32 mode,
> > > arch_hw_breakpoint_init() in arch/arm/kernel/hw_breakpoint.c will be used.
> > >
> > > From ARMv8 specification, different debug architecture versions defined:
> > > * 0110 ARMv8, v8 Debug architecture.
> > > * 0111 ARMv8.1, v8 Debug architecture, with Virtualization Host
> > > Extensions.
> > > * 1000 ARMv8.2, v8.2 Debug architecture.
> > >
> > > So missing ARMv8.1/ARMv8.2 cases will cause enable_monitor_mode() function
> > > returns -ENODEV, and arch_hw_breakpoint_init() will fail.
> > >
> > > Signed-off-by: Candle Sun
> > > Signed-off-by: Nianfu Bai
> > > ---
> > > arch/arm/include/asm/hw_breakpoint.h | 2 ++
> > > arch/arm/kernel/hw_breakpoint.c | 2 ++
> > > 2 files changed, 4 insertions(+)
> >
> > How did you test this?
> >
> > Will
>
> We have the SoC with A55 cores. On one Android project, for saving memory
> usage,
> we let A55 run in aarch32 mode.
> While the following failures occue on Android CtsBionicTestCases:
> --sys_ptrace#watchpoint_imprecisede
> --sys_ptrace#hardware_breakpoint
> --sys_ptrace#watchpoint_stress
>
> The code snippet for testing is:
>
> static void check_hw_feature_supported(pid_t child, HwFeature feature) {
> #if defined(__arm__)
> long capabilities;
> long result = ptrace(PTRACE_GETHBPREGS, child, 0, );
> if (result == -1) {
> EXPECT_EQ(EIO, errno);
> GTEST_SKIP() << "Hardware debug support disabled at kernel
> configuration time";
> }
> uint8_t hb_count = capabilities & 0xff;
> capabilities >>= 8;
> uint8_t wp_count = capabilities & 0xff;
> capabilities >>= 8;
> uint8_t max_wp_size = capabilities & 0xff;
> if (max_wp_size == 0) {
> GTEST_SKIP() << "Kernel reports zero maximum watchpoint size";
> } else if (feature == HwFeature::Watchpoint && wp_count == 0) {
> GTEST_SKIP() << "Kernel reports zero hardware watchpoints";
> } else if (feature == HwFeature::Breakpoint && hb_count == 0) {
> GTEST_SKIP() << "Kernel reports zero hardware breakpoints";
> }
> #elif defined(__aarch64__)
> user_hwdebug_state dreg_state;
> iovec iov;
> iov.iov_base = _state;
> iov.iov_len = sizeof(dreg_state);
>
> long result = ptrace(PTRACE_GETREGSET, child,
>feature == HwFeature::Watchpoint ?
> NT_ARM_HW_WATCH : NT_ARM_HW_BREAK, );
> if (result == -1) {
> ASSERT_EQ(EINVAL, errno);
> }
> if ((dreg_state.dbg_info & 0xff) == 0) GTEST_SKIP() << "hardware
> support missing";
> #else
> // We assume watchpoints and breakpoints are always supported on x86.
> UNUSED(child);
> UNUSED(feature);
> #endif
> }
>
> The max_wp_size field returned by __ptrace() from kernel is zero,
> which causes the test failures.
>
> After futher analysis, we found max_watchpoint_len variable is not
> right initialized in kernel
> arch_hw_breakpoint_init() function. Missing the case of ARM_DEBUG_ARCH_V8_2 in
> enable_monitor_mode() directly aborts the arch_hw_breakpoint_int().
>
> Candle
> Best regards
Re: [PATCH v2] HID: core: check whether usage page item is after usage id item
On Thu, Oct 10, 2019 at 8:24 PM Benjamin Tissoires
wrote:
>
> On Wed, Oct 9, 2019 at 2:54 PM Candle Sun wrote:
> >
> > From: Candle Sun
> >
> > Upstream commit 58e75155009c ("HID: core: move Usage Page concatenation
> > to Main item") adds support for Usage Page item after Usage ID items
> > (such as keyboards manufactured by Primax).
> >
> > Usage Page concatenation in Main item works well for following report
> > descriptor patterns:
> >
> > USAGE_PAGE (Keyboard) 05 07
> > USAGE_MINIMUM (Keyboard LeftControl)19 E0
> > USAGE_MAXIMUM (Keyboard Right GUI) 29 E7
> > LOGICAL_MINIMUM (0) 15 00
> > LOGICAL_MAXIMUM (1) 25 01
> > REPORT_SIZE (1) 75 01
> > REPORT_COUNT (8)95 08
> > INPUT (Data,Var,Abs)81 02
> >
> > -
> >
> > USAGE_MINIMUM (Keyboard LeftControl)19 E0
> > USAGE_MAXIMUM (Keyboard Right GUI) 29 E7
> > LOGICAL_MINIMUM (0) 15 00
> > LOGICAL_MAXIMUM (1) 25 01
> > REPORT_SIZE (1) 75 01
> > REPORT_COUNT (8)95 08
> > USAGE_PAGE (Keyboard) 05 07
> > INPUT (Data,Var,Abs)81 02
> >
> > But it makes the parser act wrong for the following report
> > descriptor pattern(such as some Gamepads):
> >
> > USAGE_PAGE (Button) 05 09
> > USAGE (Button 1)09 01
> > USAGE (Button 2)09 02
> > USAGE (Button 4)09 04
> > USAGE (Button 5)09 05
> > USAGE (Button 7)09 07
> > USAGE (Button 8)09 08
> > USAGE (Button 14) 09 0E
> > USAGE (Button 15) 09 0F
> > USAGE (Button 13) 09 0D
> > USAGE_PAGE (Consumer Devices) 05 0C
> > USAGE (Back)0a 24 02
> > USAGE (HomePage)0a 23 02
> > LOGICAL_MINIMUM (0) 15 00
> > LOGICAL_MAXIMUM (1) 25 01
> > REPORT_SIZE (1) 75 01
> > REPORT_COUNT (11) 95 0B
> > INPUT (Data,Var,Abs)81 02
> >
> > With Usage Page concatenation in Main item, parser recognizes all the
> > 11 Usages as consumer keys, it is not the HID device's real intention.
> >
> > This patch adds usage_page_last to flag whether Usage Page is after
> > Usage ID items. usage_page_last is false default, it is set as true
> > once Usage Page item is encountered and is reverted by next Usage ID
> > item.
> >
> > Usage Page concatenation on the currently defined Usage Page will do
> > firstly in Local parsing when Usage ID items encountered.
> >
> > When Main item is parsing, concatenation will do again with last
> > defined Usage Page if usage_page_last flag is true.
> >
> > Signed-off-by: Candle Sun
> > Signed-off-by: Nianfu Bai
> > ---
> > Changes in v2:
> > - Update patch title
> > - Add GET_COMPLETE_USAGE macro
> > - Change the logic of checking whether to concatenate usage page again
> > in main parsing
> > ---
> > drivers/hid/hid-core.c | 31 +--
> > include/linux/hid.h| 1 +
> > 2 files changed, 26 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> > index 3eaee2c..3394222 100644
> > --- a/drivers/hid/hid-core.c
> > +++ b/drivers/hid/hid-core.c
> > @@ -35,6 +35,8 @@
> >
> > #include "hid-ids.h"
> >
> > +#define GET_COMPLETE_USAGE(page, id) (((page) << 16) + ((id) & 0x))
> > +
> > /*
> > * Version Information
> > */
> > @@ -221,7 +223,15 @@ static int hid_add_usage(struct hid_parser *parser,
> > unsigned usage, u8 size)
> > hid_err(parser->device, "usage index exceeded\n");
> > return -1;
> > }
> > - parser->local.usage[parser->local.usage_index] = usage;
> > +
> > + if (size <= 2) {
> > + parser->local.usage_page_last = false;
> > + parser->local.usage[parser->local.usage_index] =
>
Re: [PATCH v2] HID: core: check whether usage page item is after usage id item
On Thu, Oct 10, 2019 at 8:17 PM Benjamin Tissoires
wrote:
>
> On Thu, Oct 10, 2019 at 5:19 AM Candle Sun wrote:
> >
> > On Thu, Oct 10, 2019 at 2:00 AM Jiri Kosina wrote:
> > >
> > > On Wed, 9 Oct 2019, Nicolas Saenz Julienne wrote:
> > >
> > > > > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> > > > > index 3eaee2c..3394222 100644
> > > > > --- a/drivers/hid/hid-core.c
> > > > > +++ b/drivers/hid/hid-core.c
> > > > > @@ -35,6 +35,8 @@
> > > > >
> > > > > #include "hid-ids.h"
> > > > >
> > > > > +#define GET_COMPLETE_USAGE(page, id) (((page) << 16) + ((id) &
> > > > > 0x))
> > > >
> > > > Not sure I like the macro. I'd rather have the explicit code. That
> > > > said, lets
> > > > see what Benjamin has to say.
> > >
> > > Not sure about Benjamin :) but I personally would ask for putting it
> > > somewhere into hid.h as static inline.
> > >
> > > And even if it's for some reason insisted on this staying macro, please at
> > > least put it as close to the place(s) it's being used as possible, in
> > > order to maintain some code sanity.
> > >
> > > Thanks,
> > >
> > > --
> > > Jiri Kosina
> > > SUSE Labs
> > >
> >
> > Thanks Nicolas and Jiri,
> > If macro is not good, I will change it to static function. But the
> > funciton is only used in hid-core.c,
> > maybe placing it into hid.h is not good?
>
> I would rather use a function too (in hid-core.c, as it's not reused
> anywhere else), and we can make it simpler from the caller point of
> view (if I am not mistaken):
> ---
> static void concatenate_usage_page(struct hid_parser *parser, int index)
> {
> parser->local.usage[index] &= 0x;
> parser->local.usage[index] |= (parser->global.usage_page & 0x) << 16;
> }
>
> // Which can then be called as:
> + parser->local.usage[parser->local.usage_index] = usage;
> + if (size <= 2)
> + concatenate_usage_page(parser, parser->local.usage_index);
> +
>
> // And
> for (i = 0; i < parser->local.usage_index; i++)
> - if (parser->local.usage_size[i] <= 2)
> - parser->local.usage[i] +=
> parser->global.usage_page << 16;
> + if (parser->local.usage_size[i] <= 2) {
> + concatenate_usage_page(parser, i);
> + }
> }
> ---
>
> And now that I have written this, the check on the size could also be
> very well integrated in concatenate_usage_page().
>
> Cheers,
> Benjamin
>
Thanks Benjamin's detailed directions. I will amend the patch.
Candle
> >
> > Regards,
> > Candle
>
Re: [PATCH v2] HID: core: check whether usage page item is after usage id item
On Thu, Oct 10, 2019 at 2:00 AM Jiri Kosina wrote: > > On Wed, 9 Oct 2019, Nicolas Saenz Julienne wrote: > > > > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c > > > index 3eaee2c..3394222 100644 > > > --- a/drivers/hid/hid-core.c > > > +++ b/drivers/hid/hid-core.c > > > @@ -35,6 +35,8 @@ > > > > > > #include "hid-ids.h" > > > > > > +#define GET_COMPLETE_USAGE(page, id) (((page) << 16) + ((id) & 0x)) > > > > Not sure I like the macro. I'd rather have the explicit code. That said, > > lets > > see what Benjamin has to say. > > Not sure about Benjamin :) but I personally would ask for putting it > somewhere into hid.h as static inline. > > And even if it's for some reason insisted on this staying macro, please at > least put it as close to the place(s) it's being used as possible, in > order to maintain some code sanity. > > Thanks, > > -- > Jiri Kosina > SUSE Labs > Thanks Nicolas and Jiri, If macro is not good, I will change it to static function. But the funciton is only used in hid-core.c, maybe placing it into hid.h is not good? Regards, Candle
Re: [PATCH v2] HID: core: check whether usage page item is after usage id item
On Thu, Oct 10, 2019 at 1:01 AM Nicolas Saenz Julienne
wrote:
>
> On Wed, 2019-10-09 at 20:53 +0800, Candle Sun wrote:
> > From: Candle Sun
> >
> > Upstream commit 58e75155009c ("HID: core: move Usage Page concatenation
> > to Main item") adds support for Usage Page item after Usage ID items
> > (such as keyboards manufactured by Primax).
> >
> > Usage Page concatenation in Main item works well for following report
> > descriptor patterns:
> >
> > USAGE_PAGE (Keyboard) 05 07
> > USAGE_MINIMUM (Keyboard LeftControl)19 E0
> > USAGE_MAXIMUM (Keyboard Right GUI) 29 E7
> > LOGICAL_MINIMUM (0) 15 00
> > LOGICAL_MAXIMUM (1) 25 01
> > REPORT_SIZE (1) 75 01
> > REPORT_COUNT (8)95 08
> > INPUT (Data,Var,Abs)81 02
> >
> > -
> >
> > USAGE_MINIMUM (Keyboard LeftControl)19 E0
> > USAGE_MAXIMUM (Keyboard Right GUI) 29 E7
> > LOGICAL_MINIMUM (0) 15 00
> > LOGICAL_MAXIMUM (1) 25 01
> > REPORT_SIZE (1) 75 01
> > REPORT_COUNT (8)95 08
> > USAGE_PAGE (Keyboard) 05 07
> > INPUT (Data,Var,Abs)81 02
> >
> > But it makes the parser act wrong for the following report
> > descriptor pattern(such as some Gamepads):
> >
> > USAGE_PAGE (Button) 05 09
> > USAGE (Button 1)09 01
> > USAGE (Button 2)09 02
> > USAGE (Button 4)09 04
> > USAGE (Button 5)09 05
> > USAGE (Button 7)09 07
> > USAGE (Button 8)09 08
> > USAGE (Button 14) 09 0E
> > USAGE (Button 15) 09 0F
> > USAGE (Button 13) 09 0D
> > USAGE_PAGE (Consumer Devices) 05 0C
> > USAGE (Back)0a 24 02
> > USAGE (HomePage)0a 23 02
> > LOGICAL_MINIMUM (0) 15 00
> > LOGICAL_MAXIMUM (1) 25 01
> > REPORT_SIZE (1) 75 01
> > REPORT_COUNT (11) 95 0B
> > INPUT (Data,Var,Abs)81 02
> >
> > With Usage Page concatenation in Main item, parser recognizes all the
> > 11 Usages as consumer keys, it is not the HID device's real intention.
> >
> > This patch adds usage_page_last to flag whether Usage Page is after
> > Usage ID items. usage_page_last is false default, it is set as true
> > once Usage Page item is encountered and is reverted by next Usage ID
> > item.
> >
> > Usage Page concatenation on the currently defined Usage Page will do
> > firstly in Local parsing when Usage ID items encountered.
> >
> > When Main item is parsing, concatenation will do again with last
> > defined Usage Page if usage_page_last flag is true.
>
> Functionally I think this is the right approach. Sadly I don't have access to
> any Primax device anymore so I can't test it. But I suggest you update
> hid-tools' parser and add a new unit test to verify we aren't missing
> anything.
>
> You can base your code on this:
>
> https://gitlab.freedesktop.org/libevdev/hid-tools/merge_requests/37/commits
>
Thanks Nicolas. I will check and try to do it.
Candle
> > Signed-off-by: Candle Sun
> > Signed-off-by: Nianfu Bai
> > ---
> > Changes in v2:
> > - Update patch title
> > - Add GET_COMPLETE_USAGE macro
> > - Change the logic of checking whether to concatenate usage page again
> > in main parsing
> > ---
> > drivers/hid/hid-core.c | 31 +--
> > include/linux/hid.h| 1 +
> > 2 files changed, 26 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> > index 3eaee2c..3394222 100644
> > --- a/drivers/hid/hid-core.c
> > +++ b/drivers/hid/hid-core.c
> > @@ -35,6 +35,8 @@
> >
> > #include "hid-ids.h"
> >
> > +#define GET_COMPLETE_USAGE(page, id) (((page) << 16) + ((id) & 0x))
>
> Not sure I like the macro. I'd rather have the explicit code. That said, lets
> see what Benjamin has to say.
>
> > +
> > /*
> > * Version Information
> &g
[PATCH v2] HID: core: check whether usage page item is after usage id item
From: Candle Sun
Upstream commit 58e75155009c ("HID: core: move Usage Page concatenation
to Main item") adds support for Usage Page item after Usage ID items
(such as keyboards manufactured by Primax).
Usage Page concatenation in Main item works well for following report
descriptor patterns:
USAGE_PAGE (Keyboard) 05 07
USAGE_MINIMUM (Keyboard LeftControl)19 E0
USAGE_MAXIMUM (Keyboard Right GUI) 29 E7
LOGICAL_MINIMUM (0) 15 00
LOGICAL_MAXIMUM (1) 25 01
REPORT_SIZE (1) 75 01
REPORT_COUNT (8)95 08
INPUT (Data,Var,Abs)81 02
-
USAGE_MINIMUM (Keyboard LeftControl)19 E0
USAGE_MAXIMUM (Keyboard Right GUI) 29 E7
LOGICAL_MINIMUM (0) 15 00
LOGICAL_MAXIMUM (1) 25 01
REPORT_SIZE (1) 75 01
REPORT_COUNT (8)95 08
USAGE_PAGE (Keyboard) 05 07
INPUT (Data,Var,Abs)81 02
But it makes the parser act wrong for the following report
descriptor pattern(such as some Gamepads):
USAGE_PAGE (Button) 05 09
USAGE (Button 1)09 01
USAGE (Button 2)09 02
USAGE (Button 4)09 04
USAGE (Button 5)09 05
USAGE (Button 7)09 07
USAGE (Button 8)09 08
USAGE (Button 14) 09 0E
USAGE (Button 15) 09 0F
USAGE (Button 13) 09 0D
USAGE_PAGE (Consumer Devices) 05 0C
USAGE (Back)0a 24 02
USAGE (HomePage)0a 23 02
LOGICAL_MINIMUM (0) 15 00
LOGICAL_MAXIMUM (1) 25 01
REPORT_SIZE (1) 75 01
REPORT_COUNT (11) 95 0B
INPUT (Data,Var,Abs)81 02
With Usage Page concatenation in Main item, parser recognizes all the
11 Usages as consumer keys, it is not the HID device's real intention.
This patch adds usage_page_last to flag whether Usage Page is after
Usage ID items. usage_page_last is false default, it is set as true
once Usage Page item is encountered and is reverted by next Usage ID
item.
Usage Page concatenation on the currently defined Usage Page will do
firstly in Local parsing when Usage ID items encountered.
When Main item is parsing, concatenation will do again with last
defined Usage Page if usage_page_last flag is true.
Signed-off-by: Candle Sun
Signed-off-by: Nianfu Bai
---
Changes in v2:
- Update patch title
- Add GET_COMPLETE_USAGE macro
- Change the logic of checking whether to concatenate usage page again
in main parsing
---
drivers/hid/hid-core.c | 31 +--
include/linux/hid.h| 1 +
2 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 3eaee2c..3394222 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -35,6 +35,8 @@
#include "hid-ids.h"
+#define GET_COMPLETE_USAGE(page, id) (((page) << 16) + ((id) & 0x))
+
/*
* Version Information
*/
@@ -221,7 +223,15 @@ static int hid_add_usage(struct hid_parser *parser,
unsigned usage, u8 size)
hid_err(parser->device, "usage index exceeded\n");
return -1;
}
- parser->local.usage[parser->local.usage_index] = usage;
+
+ if (size <= 2) {
+ parser->local.usage_page_last = false;
+ parser->local.usage[parser->local.usage_index] =
+ GET_COMPLETE_USAGE(parser->global.usage_page, usage);
+ } else {
+ parser->local.usage[parser->local.usage_index] = usage;
+ }
+
parser->local.usage_size[parser->local.usage_index] = size;
parser->local.collection_index[parser->local.usage_index] =
parser->collection_stack_ptr ?
@@ -366,6 +376,7 @@ static int hid_parser_global(struct hid_parser *parser,
struct hid_item *item)
case HID_GLOBAL_ITEM_TAG_USAGE_PAGE:
parser->global.usage_page = item_udata(item);
+ parser->local.usage_page_last = true;
return 0;
case HID_GLOBAL_ITEM_TAG_LOGICAL_MINIMUM:
@@ -543,13 +554,21 @@ static int hid_parser_local(struct hid_parser *parser,
struct hid_item *item)
* usage value."
*/
-static void hid_concatenate_usage_page(struct hid_parser *parser)
+static void hid_concatenate_last_usage_page(struct hid_parser *parser)
{
int i;
+ unsigned int usage;
+ unsigned int usage_page = parser->global.usage_page;
+
+ if (!parser-
Re: [RESEND PATCH] ARM/hw_breakpoint: add ARMv8.1/ARMv8.2 debug architecutre versions support in enable_monitor_mode()
Hi Will,
Sorry for not instant respond.
On Mon, Sep 30, 2019 at 11:34 PM Will Deacon wrote:
>
> On Thu, Sep 26, 2019 at 03:38:28PM +0800, Candle Sun wrote:
> > From: Candle Sun
> >
> > When ARMv8.1/ARMv8.2 cores are used in AArch32 mode,
> > arch_hw_breakpoint_init() in arch/arm/kernel/hw_breakpoint.c will be used.
> >
> > From ARMv8 specification, different debug architecture versions defined:
> > * 0110 ARMv8, v8 Debug architecture.
> > * 0111 ARMv8.1, v8 Debug architecture, with Virtualization Host Extensions.
> > * 1000 ARMv8.2, v8.2 Debug architecture.
> >
> > So missing ARMv8.1/ARMv8.2 cases will cause enable_monitor_mode() function
> > returns -ENODEV, and arch_hw_breakpoint_init() will fail.
> >
> > Signed-off-by: Candle Sun
> > Signed-off-by: Nianfu Bai
> > ---
> > arch/arm/include/asm/hw_breakpoint.h | 2 ++
> > arch/arm/kernel/hw_breakpoint.c | 2 ++
> > 2 files changed, 4 insertions(+)
>
> How did you test this?
>
> Will
We have the SoC with A55 cores. On one Android project, for saving memory usage,
we let A55 run in aarch32 mode.
While the following failures occue on Android CtsBionicTestCases:
--sys_ptrace#watchpoint_imprecisede
--sys_ptrace#hardware_breakpoint
--sys_ptrace#watchpoint_stress
The code snippet for testing is:
static void check_hw_feature_supported(pid_t child, HwFeature feature) {
#if defined(__arm__)
long capabilities;
long result = ptrace(PTRACE_GETHBPREGS, child, 0, );
if (result == -1) {
EXPECT_EQ(EIO, errno);
GTEST_SKIP() << "Hardware debug support disabled at kernel
configuration time";
}
uint8_t hb_count = capabilities & 0xff;
capabilities >>= 8;
uint8_t wp_count = capabilities & 0xff;
capabilities >>= 8;
uint8_t max_wp_size = capabilities & 0xff;
if (max_wp_size == 0) {
GTEST_SKIP() << "Kernel reports zero maximum watchpoint size";
} else if (feature == HwFeature::Watchpoint && wp_count == 0) {
GTEST_SKIP() << "Kernel reports zero hardware watchpoints";
} else if (feature == HwFeature::Breakpoint && hb_count == 0) {
GTEST_SKIP() << "Kernel reports zero hardware breakpoints";
}
#elif defined(__aarch64__)
user_hwdebug_state dreg_state;
iovec iov;
iov.iov_base = _state;
iov.iov_len = sizeof(dreg_state);
long result = ptrace(PTRACE_GETREGSET, child,
feature == HwFeature::Watchpoint ?
NT_ARM_HW_WATCH : NT_ARM_HW_BREAK, );
if (result == -1) {
ASSERT_EQ(EINVAL, errno);
}
if ((dreg_state.dbg_info & 0xff) == 0) GTEST_SKIP() << "hardware
support missing";
#else
// We assume watchpoints and breakpoints are always supported on x86.
UNUSED(child);
UNUSED(feature);
#endif
}
The max_wp_size field returned by __ptrace() from kernel is zero,
which causes the test failures.
After futher analysis, we found max_watchpoint_len variable is not
right initialized in kernel
arch_hw_breakpoint_init() function. Missing the case of ARM_DEBUG_ARCH_V8_2 in
enable_monitor_mode() directly aborts the arch_hw_breakpoint_int().
Candle
Best regards
Re: [PATCH] HID: core: add usage_page_preceding flag for hid_concatenate_usage_page()
Hi Benjamin,
Thank you very much for the detailed review.
On Mon, Sep 30, 2019 at 5:36 PM Benjamin Tissoires
wrote:
>
> Hi,
>
> [also addingg Nicolas, the author of 58e75155009c]
>
> On Mon, Sep 30, 2019 at 10:10 AM Candle Sun wrote:
> >
> > From: Candle Sun
> >
> > Upstream commit 58e75155009c ("HID: core: move Usage Page concatenation
> > to Main item") adds support for Usage Page item following Usage items
> > (such as keyboards manufactured by Primax).
> >
> > Usage Page concatenation in Main item works well for following report
> > descriptor patterns:
> >
> > USAGE_PAGE (Keyboard) 05 07
> > USAGE_MINIMUM (Keyboard LeftControl)19 E0
> > USAGE_MAXIMUM (Keyboard Right GUI) 29 E7
> > LOGICAL_MINIMUM (0) 15 00
> > LOGICAL_MAXIMUM (1) 25 01
> > REPORT_SIZE (1) 75 01
> > REPORT_COUNT (8)95 08
> > INPUT (Data,Var,Abs)81 02
> >
> > -
> >
> > USAGE_MINIMUM (Keyboard LeftControl)19 E0
> > USAGE_MAXIMUM (Keyboard Right GUI) 29 E7
> > LOGICAL_MINIMUM (0) 15 00
> > LOGICAL_MAXIMUM (1) 25 01
> > REPORT_SIZE (1) 75 01
> > REPORT_COUNT (8)95 08
> > USAGE_PAGE (Keyboard) 05 07
> > INPUT (Data,Var,Abs)81 02
> >
> > But it makes the parser act wrong for the following report
> > descriptor pattern(such as some Gamepads):
> >
> > USAGE_PAGE (Button) 05 09
> > USAGE (Button 1)09 01
> > USAGE (Button 2)09 02
> > USAGE (Button 4)09 04
> > USAGE (Button 5)09 05
> > USAGE (Button 7)09 07
> > USAGE (Button 8)09 08
> > USAGE (Button 14) 09 0E
> > USAGE (Button 15) 09 0F
> > USAGE (Button 13) 09 0D
> > USAGE_PAGE (Consumer Devices) 05 0C
> > USAGE (Back)0a 24 02
> > USAGE (HomePage)0a 23 02
> > LOGICAL_MINIMUM (0) 15 00
> > LOGICAL_MAXIMUM (1) 25 01
> > REPORT_SIZE (1) 75 01
> > REPORT_COUNT (11) 95 0B
> > INPUT (Data,Var,Abs)81 02
> >
> > With Usage Page concatenation in Main item, parser recognizes all the
> > 11 Usages as consumer keys, it is not the HID device's real intention.
> >
> > This patch adds usage_page_preceding flag to detect the third pattern.
> > Usage Page concatenation is done in both Local and Main parsing.
> > If usage_page_preceding equals 3(the third pattern encountered),
> > hid_concatenate_usage_page() is jumped.
>
> For anything core related (and especially the parsing), I am trying to
> enforce having regression tests.
> See https://gitlab.freedesktop.org/libevdev/hid-tools/merge_requests/37
> for the one related to 58e75155009c.
>
> So I would like to have a similar-ish MR adding the matching tests so
> I know we won't break this in the future.
>
> Few other comments in the code:
>
Thanks.
Candle
> >
> > Signed-off-by: Candle Sun
> > Signed-off-by: Nianfu Bai
> > ---
> > drivers/hid/hid-core.c | 21 +++--
> > include/linux/hid.h| 1 +
> > 2 files changed, 20 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> > index 3eaee2c..043a232 100644
> > --- a/drivers/hid/hid-core.c
> > +++ b/drivers/hid/hid-core.c
> > @@ -221,7 +221,15 @@ static int hid_add_usage(struct hid_parser *parser,
> > unsigned usage, u8 size)
> > hid_err(parser->device, "usage index exceeded\n");
> > return -1;
> > }
> > - parser->local.usage[parser->local.usage_index] = usage;
> > + if (!parser->local.usage_index && parser->global.usage_page)
>
> parser->global.usage_page is never reset, so unless I am misreading,
> it will always be set to a value except for the very first elements.
> I am just raising this in case you rely on global.usage_page being
> null in your algorithm.
>
The patch doesn't rely on the global.usage_page bei
[PATCH] HID: core: add usage_page_preceding flag for hid_concatenate_usage_page()
From: Candle Sun
Upstream commit 58e75155009c ("HID: core: move Usage Page concatenation
to Main item") adds support for Usage Page item following Usage items
(such as keyboards manufactured by Primax).
Usage Page concatenation in Main item works well for following report
descriptor patterns:
USAGE_PAGE (Keyboard) 05 07
USAGE_MINIMUM (Keyboard LeftControl)19 E0
USAGE_MAXIMUM (Keyboard Right GUI) 29 E7
LOGICAL_MINIMUM (0) 15 00
LOGICAL_MAXIMUM (1) 25 01
REPORT_SIZE (1) 75 01
REPORT_COUNT (8)95 08
INPUT (Data,Var,Abs)81 02
-
USAGE_MINIMUM (Keyboard LeftControl)19 E0
USAGE_MAXIMUM (Keyboard Right GUI) 29 E7
LOGICAL_MINIMUM (0) 15 00
LOGICAL_MAXIMUM (1) 25 01
REPORT_SIZE (1) 75 01
REPORT_COUNT (8)95 08
USAGE_PAGE (Keyboard) 05 07
INPUT (Data,Var,Abs)81 02
But it makes the parser act wrong for the following report
descriptor pattern(such as some Gamepads):
USAGE_PAGE (Button) 05 09
USAGE (Button 1)09 01
USAGE (Button 2)09 02
USAGE (Button 4)09 04
USAGE (Button 5)09 05
USAGE (Button 7)09 07
USAGE (Button 8)09 08
USAGE (Button 14) 09 0E
USAGE (Button 15) 09 0F
USAGE (Button 13) 09 0D
USAGE_PAGE (Consumer Devices) 05 0C
USAGE (Back)0a 24 02
USAGE (HomePage)0a 23 02
LOGICAL_MINIMUM (0) 15 00
LOGICAL_MAXIMUM (1) 25 01
REPORT_SIZE (1) 75 01
REPORT_COUNT (11) 95 0B
INPUT (Data,Var,Abs)81 02
With Usage Page concatenation in Main item, parser recognizes all the
11 Usages as consumer keys, it is not the HID device's real intention.
This patch adds usage_page_preceding flag to detect the third pattern.
Usage Page concatenation is done in both Local and Main parsing.
If usage_page_preceding equals 3(the third pattern encountered),
hid_concatenate_usage_page() is jumped.
Signed-off-by: Candle Sun
Signed-off-by: Nianfu Bai
---
drivers/hid/hid-core.c | 21 +++--
include/linux/hid.h| 1 +
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 3eaee2c..043a232 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -221,7 +221,15 @@ static int hid_add_usage(struct hid_parser *parser,
unsigned usage, u8 size)
hid_err(parser->device, "usage index exceeded\n");
return -1;
}
- parser->local.usage[parser->local.usage_index] = usage;
+ if (!parser->local.usage_index && parser->global.usage_page)
+ parser->local.usage_page_preceding = 1;
+ if (parser->local.usage_page_preceding == 2)
+ parser->local.usage_page_preceding = 3;
+ if (size <= 2 && parser->global.usage_page)
+ parser->local.usage[parser->local.usage_index] =
+ (usage & 0x) + (parser->global.usage_page << 16);
+ else
+ parser->local.usage[parser->local.usage_index] = usage;
parser->local.usage_size[parser->local.usage_index] = size;
parser->local.collection_index[parser->local.usage_index] =
parser->collection_stack_ptr ?
@@ -366,6 +374,8 @@ static int hid_parser_global(struct hid_parser *parser,
struct hid_item *item)
case HID_GLOBAL_ITEM_TAG_USAGE_PAGE:
parser->global.usage_page = item_udata(item);
+ if (parser->local.usage_page_preceding == 1)
+ parser->local.usage_page_preceding = 2;
return 0;
case HID_GLOBAL_ITEM_TAG_LOGICAL_MINIMUM:
@@ -547,9 +557,16 @@ static void hid_concatenate_usage_page(struct hid_parser
*parser)
{
int i;
+ if (parser->local.usage_page_preceding == 3) {
+ dbg_hid("Using preceding usage page for final usage\n");
+ return;
+ }
+
for (i = 0; i < parser->local.usage_index; i++)
if (parser->local.usage_size[i] <= 2)
- parser->local.usage[i] += parser->global.usage_page <<
16;
+ parser->local.usage[i] =
+ (parser->global.usage_page << 16)
+
[RESEND PATCH] ARM/hw_breakpoint: add ARMv8.1/ARMv8.2 debug architecutre versions support in enable_monitor_mode()
From: Candle Sun When ARMv8.1/ARMv8.2 cores are used in AArch32 mode, arch_hw_breakpoint_init() in arch/arm/kernel/hw_breakpoint.c will be used. >From ARMv8 specification, different debug architecture versions defined: * 0110 ARMv8, v8 Debug architecture. * 0111 ARMv8.1, v8 Debug architecture, with Virtualization Host Extensions. * 1000 ARMv8.2, v8.2 Debug architecture. So missing ARMv8.1/ARMv8.2 cases will cause enable_monitor_mode() function returns -ENODEV, and arch_hw_breakpoint_init() will fail. Signed-off-by: Candle Sun Signed-off-by: Nianfu Bai --- arch/arm/include/asm/hw_breakpoint.h | 2 ++ arch/arm/kernel/hw_breakpoint.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/arch/arm/include/asm/hw_breakpoint.h b/arch/arm/include/asm/hw_breakpoint.h index ac54c06..9137ef6 100644 --- a/arch/arm/include/asm/hw_breakpoint.h +++ b/arch/arm/include/asm/hw_breakpoint.h @@ -53,6 +53,8 @@ static inline void decode_ctrl_reg(u32 reg, #define ARM_DEBUG_ARCH_V7_MM 4 #define ARM_DEBUG_ARCH_V7_15 #define ARM_DEBUG_ARCH_V8 6 +#define ARM_DEBUG_ARCH_V8_17 +#define ARM_DEBUG_ARCH_V8_28 /* Breakpoint */ #define ARM_BREAKPOINT_EXECUTE 0 diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c index b0c195e..cb99612 100644 --- a/arch/arm/kernel/hw_breakpoint.c +++ b/arch/arm/kernel/hw_breakpoint.c @@ -246,6 +246,8 @@ static int enable_monitor_mode(void) case ARM_DEBUG_ARCH_V7_ECP14: case ARM_DEBUG_ARCH_V7_1: case ARM_DEBUG_ARCH_V8: + case ARM_DEBUG_ARCH_V8_1: + case ARM_DEBUG_ARCH_V8_2: ARM_DBG_WRITE(c0, c2, 2, (dscr | ARM_DSCR_MDBGEN)); isb(); break; -- 2.7.4
