Re: Unaligned access in kernel on ARMv6+ (Re: CVS commit: src/sys/dev/usb)

2019-01-06 Thread Patrick Klos

On 1/6/2019 5:50 PM, Jason Thorpe wrote:

On Jan 6, 2019, at 1:46 PM, matthew green  wrote:

... how do we convey the structure alignment needed for
softc, or do we fix it by moving it into its own separate
aligned allocation?

The latter, I think.


I agree.  When a driver has a strict alignment requirement, it's up to 
the driver to implement that alignment.  The driver can't know for sure 
that malloc() is going to return memory with any specific alignment 
considerations, so the driver has to enure the alignment on their own.


Patrick Klos



Re: Unaligned access in kernel on ARMv6+ (Re: CVS commit: src/sys/dev/usb)

2019-01-06 Thread Jason Thorpe



> On Jan 6, 2019, at 2:26 PM, Christos Zoulas  wrote:
> 
> Shouldn't the compiler know how to do this right by padding around the
> structure that needs alignment and assuming the default alignment for
> the enclosing structure?

No, because the compiler can't be assured of what alignment the allocator will 
return.

-- thorpej



Re: Unaligned access in kernel on ARMv6+ (Re: CVS commit: src/sys/dev/usb)

2019-01-06 Thread Jason Thorpe



> On Jan 6, 2019, at 1:46 PM, matthew green  wrote:
> 
> ... how do we convey the structure alignment needed for
> softc, or do we fix it by moving it into its own separate
> aligned allocation?

The latter, I think.

-- thorpej



re: Unaligned access in kernel on ARMv6+ (Re: CVS commit: src/sys/dev/usb)

2019-01-06 Thread matthew green
> | for xhci, all of these seem to be the same issue and it
> | appears to be a "high level allocator doesn't know what
> | it is allocating and does not align properly".  the
> | problem is likely:
> | 
> | #define XHCI_TRB_ALIGN 16
> | struct xhci_trb {
> | ...
> | } __packed __aligned(XHCI_TRB_ALIGN);   
> | 
> | which is included in:
> | 
> | struct xhci_softc {
> | ...
> | struct xhci_trb sc_result_trb;
> | 
> | 
> | ... how do we convey the structure alignment needed for
> | softc, or do we fix it by moving it into its own separate
> | aligned allocation?
> 
> Shouldn't the compiler know how to do this right by padding around the
> structure that needs alignment and assuming the default alignment for
> the enclosing structure?

how can it?  it has to be 16 byte aligned.  the alignment
provided is only 8.  so sometimes the padding will work and
sometimes the padding will fault, depending on what bit 3
is of the returned address.  it may already pad before hand
to get 16 byte alignment from the start of the structure,
as well as force the whole structure alignment to 16, looking
above this member, it's hard to easily tell, it could be
needing 4, 8 or 12 bytes of padding (4/12 only 32 bit only.)

this is why the compiler assumes it must be 16 byte aligned
already -- because that's the only case that is valid.


.mrg.


re: Unaligned access in kernel on ARMv6+ (Re: CVS commit: src/sys/dev/usb)

2019-01-06 Thread Christos Zoulas
On Jan 7,  8:46am, m...@eterna.com.au (matthew green) wrote:
-- Subject: re: Unaligned access in kernel on ARMv6+ (Re: CVS commit: src/sys

| > http://netbsd.org/~kamil/kubsan/0007-boot-real-hardware.txt
| 
| for xhci, all of these seem to be the same issue and it
| appears to be a "high level allocator doesn't know what
| it is allocating and does not align properly".  the
| problem is likely:
| 
| #define XHCI_TRB_ALIGN 16
| struct xhci_trb {
| ...
| } __packed __aligned(XHCI_TRB_ALIGN);   
| 
| which is included in:
| 
| struct xhci_softc {
| ...
| struct xhci_trb sc_result_trb;
| 
| 
| ... how do we convey the structure alignment needed for
| softc, or do we fix it by moving it into its own separate
| aligned allocation?

Shouldn't the compiler know how to do this right by padding around the
structure that needs alignment and assuming the default alignment for
the enclosing structure?

christos


re: Unaligned access in kernel on ARMv6+ (Re: CVS commit: src/sys/dev/usb)

2019-01-06 Thread matthew green
> http://netbsd.org/~kamil/kubsan/0007-boot-real-hardware.txt

for xhci, all of these seem to be the same issue and it
appears to be a "high level allocator doesn't know what
it is allocating and does not align properly".  the
problem is likely:

#define XHCI_TRB_ALIGN 16
struct xhci_trb {
...
} __packed __aligned(XHCI_TRB_ALIGN);   

which is included in:

struct xhci_softc {
...
struct xhci_trb sc_result_trb;


... how do we convey the structure alignment needed for
softc, or do we fix it by moving it into its own separate
aligned allocation?


.mrg.