Details

Reviewers

andrew
kevans
whu

Commits

rG48a799af88c0: Hyper-V: vmbus: implementat bus_get_dma_tag in vmbus
rG85bc81352e4b: Hyper-V: vmbus: implementat bus_get_dma_tag in vmbus
rGe7a9817b8d32: Hyper-V: vmbus: implementat bus_get_dma_tag in vmbus

Summary

In ARM64 Hyper-V UFS filesystem is getting corruption and those corruptions are consistently happening just after hitting a page boundary.
It is unable to correctly read disk blocks into buffers that are not aligned to 512-byte boundaries.
It happens because storvsc needs physically contiguous memory which may not be the case when bus_dma needs to create a bounce buffer.
This can happen when the destination is not cache-line aligned.

Hyper-V VMs have VMbus synthetic devices and PCI pass-thru devices that are added
dynamically via the VMbus protocol and are not represented in the ACPI DSDT. Only
the top level VMbus node exists in the DSDT. As such, on ARM64 these devices don't
pick up coherence information and default to not hardware coherent.

Test Plan

tested on Azure x86 and arm64

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 53488
Build 50379: arc lint + arc unit

Event Timeline

schakrabarti_microsoft.com requested review of this revision.Sep 5 2023, 10:32 AM

schakrabarti_microsoft.com created this revision.

lwhsu added a subscriber: lwhsu.Sep 5 2023, 10:35 AM

emaste added a subscriber: emaste.Sep 5 2023, 12:06 PM

Can you include more context https://wiki.freebsd.org/Phabricator#Create_a_Revision_via_Web_Interface

emaste mentioned this in D41724: Ensure I/O buffers in libufs(3) are 128-byte aligned.Sep 6 2023, 11:30 PM

Updating the diff with arc

Herald added a subscriber: imp. · View Herald TranscriptSep 7 2023, 9:15 AM

Harbormaster completed remote builds in B53488: Diff 127036.Sep 7 2023, 9:15 AM

In D41728#951388, @andrew wrote:

Can you include more context https://wiki.freebsd.org/Phabricator#Create_a_Revision_via_Web_Interface

Updated using arc for more context.

andrew added inline comments.Sep 11 2023, 8:44 AM

sys/dev/hyperv/vmbus/vmbus.c
200	This should be with the other bus interface functions
227	Why only on arm64? The DMA tag exists on other architectures.
233	Missing newline
1441	No need to put `coherent` in the softc, it's only used within this function.

schakrabarti_microsoft.com added inline comments.Sep 11 2023, 9:04 AM

sys/dev/hyperv/vmbus/vmbus.c
227	In x86 gen1 we dont specify CCA in ACPI, but it is coherent by default. So keeping the get_dma_tag for arm64 only.

andrew added inline comments.Sep 11 2023, 10:13 AM

sys/dev/hyperv/vmbus/vmbus.c
227	You should be passing the parent bus to `bus_dma_tag_create`. If the bus is coherent then the child will be.

schakrabarti_microsoft.com added inline comments.Sep 11 2023, 10:37 AM

sys/dev/hyperv/vmbus/vmbus.c
1446	bus_get_dma_tag(dev) should be used here instead of using NULL ? @andrew

andrew added inline comments.Sep 11 2023, 11:59 AM

sys/dev/hyperv/vmbus/vmbus.c
1446	Yes, because of how the driver works I think `vmbus_res` is the correct device to follow.

schakrabarti_microsoft.com added inline comments.Sep 11 2023, 12:09 PM

sys/dev/hyperv/vmbus/vmbus.c
1446	Okay. It will be acpi_container, which is the parent of vmbus, we need to follow. ACPI container is the child of ACPI.

Changed the function vmbus_get_dma_tag to create the tag inside.

Harbormaster completed remote builds in B53540: Diff 127265.Sep 12 2023, 6:24 AM

schakrabarti_microsoft.com marked 6 inline comments as done.Sep 12 2023, 6:25 AM

Creating the tag in vmbus_get_dma_tag is incorrect as it will create a new tag each time a child asks for it. It was correct in vmbus_doattach as long as it is before any children are added, and you can use sc->vmbus_dev for the device.