Skip to content

Use memory barriers defined by xen headers #28

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ydirson wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Use memory barriers defined by xen headers #28

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 17 additions & 5 deletions varstored.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,22 @@
#include "io_port.h"
#include "option.h"

/* Get the correct defines for memory barriers - do not fall
* back to those provided by the kernel */
/* from <xen-barrier.h> in 4.20.0 */

@d3athjest3r d3athjest3r Jul 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we do:

#if __has_include(<xen-barrier.h>)
#include <xen-barrier.h>
#else
/* local fallback copy for pre-4.20 Xen headers */
...
#endif

@d3athjest3r d3athjest3r Jul 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I wouldn't even wait for including the xen-barrier.h header. Because after discussing with @andyhhp I would just add my own barrier code, which doesn't do dmb sy for performance.


#define xen_barrier() asm volatile ( "" : : : "memory")

@rosslagerwall rosslagerwall Oct 31, 2025 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this added if it is not used?

Having said that, I think it might be preferable to simply include a copy of xen-barrier.h and #include it. It would be easier to keep up-to-date and mean less tat in this file.


#if defined(__i386__)
# define smp_mb() asm volatile ( "lock addl $0, -4(%%esp)" ::: "memory" )
#define xen_mb() asm volatile ( "lock addl $0, -4(%%esp)" ::: "memory" )
#elif defined(__x86_64__)
# define smp_mb() asm volatile ( "lock addl $0, -32(%%rsp)" ::: "memory" )
#define xen_mb() asm volatile ( "lock addl $0, -32(%%rsp)" ::: "memory" )
#elif defined(__arm__)
#define xen_mb() asm volatile ("dmb" : : : "memory")
#elif defined(__aarch64__)
#define xen_mb() asm volatile ("dmb sy" : : : "memory")

@d3athjest3r d3athjest3r Jul 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a monster of a barrier. This orders against all observers: every core in every cluster, plus outer-shareable and non-shareable agents, GPU, other DMA masters, the interconnect fabric. This brings the entire system to a halt.

In the Linux kernel there a number of barriers defined for core coherency:

#define __smp_mb()   dmb(ish)
#define __smp_rmb()  dmb(ishld)
#define __smp_wmb()  dmb(ishst)

dmb ish (inner-shareable) is for ordering with respect to other cores in the same coherency domain, with no device/DMA involvement. This is what Linux's arm64 barrier.h defines. The dmb sy is only used when talking to MMIO or DMA devices.

#else
#error "Define barriers"
#endif

#define IO_PORT_ADDRESS 0x0100
Expand Down Expand Up @@ -551,15 +563,15 @@ varstored_poll_iopage(unsigned int i)
fprintf(stderr, "IO request not ready\n");
return;
}
smp_mb();
xen_mb();

ioreq->state = STATE_IOREQ_INPROCESS;

handle_ioreq(ioreq);
smp_mb();
xen_mb();

ioreq->state = STATE_IORESP_READY;
smp_mb();
xen_mb();

xenevtchn_notify(varstored_state.evth, varstored_state.ioreq_local_port[i]);
}
Expand Down