Page MenuHomeFreeBSD
Differential D50116

bhyve: when accessing non-backed gpa, emulate hw
AcceptedPublic
Actions

Authored by kib on Fri, May 2, 3:23 PM.

Details

Reviewers
markj
andrew
Group Reviewers
bhyve
Summary
which reads all bits one, and ignores writes.

PR:     286467

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

kib created this revision.Fri, May 2, 3:23 PM
Herald added subscribers: bcran, rgrimes, imp. · View Herald TranscriptFri, May 2, 3:23 PM
kib requested review of this revision.Fri, May 2, 3:23 PM
markj accepted this revision as: markj.Sat, May 3, 10:01 PM
markj added a subscriber: markj.
markj added inline comments.
usr.sbin/bhyve/mem.c
223

Maybe make this a global variable?

226
This revision is now accepted and ready to land.Sat, May 3, 10:01 PM
kib updated this revision to Diff 154777.Sun, May 4, 8:51 AM
kib marked 2 inline comments as done.

Make fb_entry static. Reword the message.

This revision now requires review to proceed.Sun, May 4, 8:51 AM
kib updated this revision to Diff 154782.Sun, May 4, 9:43 AM

Drop now unused string.h

markj added a subscriber: andrew.Sun, May 4, 3:12 PM

One issue with this is that the code is MI, but the implemented behaviour is x86-specific -- I'm not sure what happens on arm64 when a non-existent GPA is accessed, maybe an serror is raised. Maybe @andrew could comment.

kib added inline comments.Sun, May 4, 4:11 PM
usr.sbin/bhyve/mem.c
226

I reworded as you suggested. But note that the address always exists, it is the backing memory that could not.

kib updated this revision to Diff 154795.Sun, May 4, 4:11 PM

Restore original behavior on !x86.

kib updated this revision to Diff 154796.Sun, May 4, 4:57 PM

Eliminate arch ifdefs, move code into mem_x86.c, mem_md.c.

Herald added a reviewer: andrew. · View Herald TranscriptSun, May 4, 4:57 PM
Herald added a subscriber: emaste. · View Herald Transcript
kib updated this revision to Diff 154797.Sun, May 4, 5:04 PM

Stray ifdef.

kib updated this revision to Diff 154798.Sun, May 4, 5:06 PM

Another stray change, sorry.

andrew added a comment.Mon, May 5, 10:17 AM
In D50116#1143835, @markj wrote:

One issue with this is that the code is MI, but the implemented behaviour is x86-specific -- I'm not sure what happens on arm64 when a non-existent GPA is accessed, maybe an serror is raised. Maybe @andrew could comment.

On arm64 I would expect we implement it as a synchronous external abort. We should have all the information needed & it lets the guest recover, or at least report the precise location of the fault.

2khramtsov_gmail.com added a subscriber: 2khramtsov_gmail.com.Thu, May 8, 2:16 PM
kib updated this revision to Diff 155174.Fri, May 9, 9:11 AM

On armv8, generate simple-minded SError.

andrew added inline comments.Fri, May 9, 10:00 AM
usr.sbin/bhyve/aarch64/mem_aarch64.c
48 ↗(On Diff #155174)

I don't think this is correct as it looks like this should only be raised for an SError exception. The kernel can currently only raise synchronous exceptions. I think the logic should be:

spsr = get(VM_REG_GUEST_CPSR);
if ((spsr & PSR_M_MASK) == PSR_M_EL0t)
    esr = EXCP_DATA_ABORT_L << ESR_ELx_EC_SHIFT;
else
    esr = EXCP_DATA_ABORT << ESR_ELx_EC_SHIFT;
esr |= ESR_ELx_IL | ISS_DATA_DFSC_EXT;
kib updated this revision to Diff 155178.Fri, May 9, 10:03 AM

Add needed headers for mem_aarch64.c

kib updated this revision to Diff 155180.Fri, May 9, 10:22 AM

Implement andrew' suggestion.

andrew accepted this revision.Fri, May 9, 2:29 PM

I'm ok with the arm64 parts

This revision is now accepted and ready to land.Fri, May 9, 2:29 PM

Revision Contents
Changeset List

PathSize
usr.sbin/
bhyve/
38 lines

Diff 154777

View Options

usr.sbin/bhyve/mem.c

Loading...