Page MenuHomeFreeBSD
Differential D34750

riscv: support Allwinner D1 interrupt controllers
Needs ReviewPublic
Actions

Authored by julien.cassette_gmail.com on Apr 2 2022, 11:39 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Apr 19, 1:52 AM
Unknown Object (File)
Thu, Apr 17, 9:58 AM
Unknown Object (File)
Thu, Apr 17, 3:23 AM
Unknown Object (File)
Mon, Apr 14, 6:38 PM
Unknown Object (File)
Sun, Apr 13, 1:14 PM
Unknown Object (File)
Tue, Apr 8, 3:49 AM
Unknown Object (File)
Mon, Mar 31, 10:25 PM
Unknown Object (File)
Mar 4 2025, 7:11 AM
View All 97 Files
Subscribers
gehaowu_bitmoe.com
imp
jrtc27
mhorne
Contributor Reviews (src)

Details

Reviewers
None
Group Reviewers
riscv
Summary

The main interrupt controller of the D1 is the PLIC, but it does not
provide any interface for configuring IRQ trigger type or wakeup.

These features are supported through registers in a separate memory
region (RISCV_CFG), and the device tree defines another interrupt
controller (INTC) as a wrapper around the PLIC and those registers.

Since most peripherals use INTC as their interrupt parent, and do not
use IRQ trigger type or wakeup features, do the following:

  • add the D1 compatible data to the PLIC driver,
  • add a stub driver for INTC to route things back to the PLIC.

Signed-off-by: Julien Cassette <julien.cassette@gmail.com>

Test Plan

A driver can allocate IRQ resources successfully.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 44977
Build 41865: arc lint + arc unit

Event Timeline

julien.cassette_gmail.com created this revision.Apr 2 2022, 11:39 AM
Herald added subscribers: Contributor Reviews (src), imp. · View Herald TranscriptApr 2 2022, 11:39 AM
julien.cassette_gmail.com requested review of this revision.Apr 2 2022, 11:39 AM
Harbormaster completed remote builds in B44964: Diff 104498.Apr 2 2022, 11:39 AM
julien.cassette_gmail.com added a parent revision: D34749: aw_wdog: support Allwinner D1 watchdog.Apr 2 2022, 11:39 AM
julien.cassette_gmail.com edited the test plan for this revision. (Show Details)Apr 2 2022, 11:46 AM
julien.cassette_gmail.com added a reviewer: riscv.
jrtc27 added a subscriber: jrtc27.Apr 2 2022, 11:55 AM

Don't see why the PLIC changes are needed, but if they are they should be separated out from the addition of the D1 interrupt controller.

sys/riscv/riscv/plic.c
381

I don't see why this is needed? Anything that references the PLIC should just work with bus_setup_intr etc.

julien.cassette_gmail.com added a comment.Apr 2 2022, 1:15 PM
In D34750#787496, @jrtc27 wrote:

Don't see why the PLIC changes are needed, but if they are they should be separated out from the addition of the D1 interrupt controller.

Hello Jessica.
The first change is because the PLIC is named "thead,c900-plic" in the device tree. The second change is so that INTC can find the PLIC device.
I guessed it made sense to have both changes in the same revision, but I can create an extra revision for only 1 line if you want.
Regards, Julien.

sys/riscv/riscv/plic.c
381

This is so that aw_d1_intc can find its parent device using OF_device_from_xref.

jrtc27 added inline comments.Apr 2 2022, 1:22 PM
sys/riscv/riscv/plic.c
381

Why does it care? It just needs to register an interrupt via the bus APIs like any other device?

jrtc27 added inline comments.Apr 2 2022, 1:24 PM
sys/riscv/allwinner/aw_d1_intc.c
160

These don't make any sense. If this allwinner,sun20i-d1-intc device is just a wrapper around the PLIC that forwards every request directly to it, what on earth is the hardware doing and why does it exist?

julien.cassette_gmail.com added inline comments.Apr 3 2022, 4:51 PM
sys/riscv/allwinner/aw_d1_intc.c
160

It's not really nice or helpful to say that "this does not make any sense" just because you don't understand.
The hardware is clearly described in the device tree and in the D1 user manual: the registers associated to INTC are at 0x0601000 and allow configuration of IRQ wakeup enable bits.
This is similar to aw_r_intc for ARM devices.

sys/riscv/riscv/plic.c
381

It's done the same in aw_r_intc. If you have a suggestion of a modification please do.

jrtc27 added inline comments.Apr 3 2022, 4:55 PM
sys/riscv/allwinner/aw_d1_intc.c
160

Except you don't do any of that. You don't even map a memory resource (though you do have an unused res field in the softc which I guess is for that). This kind of information should be in the commit message, because as it stands this driver literally does nothing other than act as a transparent wrapper around the PLIC.

julien.cassette_gmail.com updated this revision to Diff 104536.Apr 3 2022, 6:39 PM

Update the commit message, and remove unused 'res'

Harbormaster completed remote builds in B44977: Diff 104536.Apr 3 2022, 6:39 PM
julien.cassette_gmail.com added inline comments.Apr 3 2022, 6:48 PM
sys/riscv/allwinner/aw_d1_intc.c
160

Yes, this is only a pass-through to the PLIC at the moment so that we have working interrupts.
I have updated the commit message so that it is clearer and I have removed the unused res.

mhorne added a subscriber: mhorne.Apr 12 2022, 10:56 PM

I guess this commit gives a little more context. So because the device tree declares &intc to be the interrupt parent for several peripherals, we need this stub driver in order to properly route things back to the PLIC, is that right?

sys/riscv/allwinner/aw_d1_intc.c
296

This is on the same pass as the PLIC driver, what if the PLIC doesn't attach first? Maybe BUS_PASS_ORDER_LATE is needed.

sys/riscv/riscv/plic.c
234–236

This could be converted to a ofw_compat_data table instead.

julien.cassette_gmail.com updated this revision to Diff 105011.Apr 14 2022, 6:38 PM
julien.cassette_gmail.com marked an inline comment as not done.

Correct pass level, use ofw_compat_data table

Harbormaster completed remote builds in B45172: Diff 105011.Apr 14 2022, 6:39 PM
julien.cassette_gmail.com added a comment.Apr 14 2022, 6:56 PM
In D34750#790841, @mhorne wrote:

I guess this commit gives a little more context. So because the device tree declares &intc to be the interrupt parent for several peripherals, we need this stub driver in order to properly route things back to the PLIC, is that right?

Yes that's also what I understood from that commit and aw_r_intc.

sys/riscv/allwinner/aw_d1_intc.c
296

Thanks, I saw that it needed 2 passes to attach, but then I forgot. With BUSS_PASS_ORDER_LATE it attaches properly now.

sys/riscv/riscv/plic.c
234–236

Sure, it's done

julien.cassette_gmail.com updated this revision to Diff 105078.Apr 16 2022, 5:36 PM

Use an option instead

Harbormaster completed remote builds in B45195: Diff 105078.Apr 16 2022, 5:36 PM
julien.cassette_gmail.com updated this revision to Diff 107376.Jun 25 2022, 12:03 PM

Rebase, clarify commit message

Harbormaster completed remote builds in B46110: Diff 107376.Jun 25 2022, 12:03 PM
julien.cassette_gmail.com edited the summary of this revision. (Show Details)Jun 25 2022, 12:06 PM
julien.cassette_gmail.com added a child revision: D35593: riscv: support Allwinner D1 GPIO controller.Jun 25 2022, 12:16 PM
julien.cassette_gmail.com added a child revision: D35594: riscv: support Allwinner D1 MMC controller.Jun 25 2022, 12:55 PM
gehaowu_bitmoe.com added a subscriber: gehaowu_bitmoe.com.Nov 23 2022, 12:44 PM

Revision Contents
Changeset List

PathSize
sys/
riscv/
allwinner/
296 lines
1 line
conf/
1 line
riscv/
13 lines

Diff 104536

View Options

sys/riscv/allwinner/aw_d1_intc.c

Show All 24 Lines
* *
* $FreeBSD$ * $FreeBSD$
*/ */
#include <sys/cdefs.h> #include <sys/cdefs.h>
__FBSDID("$FreeBSD$"); __FBSDID("$FreeBSD$");
#include <sys/param.h> #include <sys/param.h>
#ifndef WITHOUT_CAPSICUM
#include <sys/capsicum.h>
#endif
#include <sys/queue.h> #include <sys/queue.h>
#include <sys/errno.h> #include <sys/errno.h>
#include <sys/stat.h> #include <sys/stat.h>
#include <sys/ioctl.h> #include <sys/ioctl.h>
#include <sys/disk.h> #include <sys/disk.h>
#include <assert.h> #include <assert.h>
#include <err.h> #include <err.h>
#include <fcntl.h> #include <fcntl.h>
#include <stdio.h> #include <stdio.h>
#include <stdlib.h> #include <stdlib.h>
#include <string.h> #include <string.h>
#include <pthread.h> #include <pthread.h>
#include <pthread_np.h> #include <pthread_np.h>
#include <signal.h> #include <signal.h>
#include <sysexits.h>
#include <unistd.h> #include <unistd.h>
#include <machine/atomic.h> #include <machine/atomic.h>
#include "bhyverun.h" #include "bhyverun.h"
#include "mevent.h" #include "mevent.h"
#include "block_if.h" #include "block_if.h"
▲ Show 20 Lines • Show All 339 Lines • ▼ Show 20 Linesblockif_open(const char *optstr, const char *ident)
char name[MAXPATHLEN]; char name[MAXPATHLEN];
char *nopt, *xopts, *cp; char *nopt, *xopts, *cp;
struct blockif_ctxt *bc; struct blockif_ctxt *bc;
struct stat sbuf; struct stat sbuf;
struct diocgattr_arg arg; struct diocgattr_arg arg;
off_t size, psectsz, psectoff; off_t size, psectsz, psectoff;
int extra, fd, i, sectsz; int extra, fd, i, sectsz;
int nocache, sync, ro, candelete, geom, ssopt, pssopt; int nocache, sync, ro, candelete, geom, ssopt, pssopt;
#ifndef WITHOUT_CAPSICUM
cap_rights_t rights;
u_long cmds[] = {DIOCGFLUSH, DIOCGDELETE};
#endif
pthread_once(&blockif_once, blockif_init); pthread_once(&blockif_once, blockif_init);
fd = -1; fd = -1;
ssopt = 0; ssopt = 0;
nocache = 0; nocache = 0;
sync = 0; sync = 0;
ro = 0; ro = 0;
▲ Show 20 Lines • Show All 41 Lines • ▼ Show 20 Lines if (fd < 0) {
goto err; goto err;
} }
if (fstat(fd, &sbuf) < 0) { if (fstat(fd, &sbuf) < 0) {
warn("Could not stat backing file %s", nopt); warn("Could not stat backing file %s", nopt);
goto err; goto err;
} }
#ifndef WITHOUT_CAPSICUM
grehanUnsubmitted
Not Done
Inline Actions

fsync needs to be added here, or an ahci-hd file-backed disk image will fail on FreeBSD guest shutdown with

(ada0:ahcich0:0:0:0): FLUSHCACHE48. ACB: ea 00 00 00 00 40 00 00 00 00 00 00
(ada0:ahcich0:0:0:0): CAM status: ATA Status Error
(ada0:ahcich0:0:0:0): ATA status: 41 (DRDY ERR), error: 04 (ABRT )
(ada0:ahcich0:0:0:0): RES: 41 04 00 00 00 40 00 00 00 00 00
(ada0:ahcich0:0:0:0): Retrying command
(ada0:ahcich0:0:0:0): FLUSHCACHE48. ACB: ea 00 00 00 00 40 00 00 00 00 00 00
(ada0:ahcich0:0:0:0): CAM status: ATA Status Error
(ada0:ahcich0:0:0:0): ATA status: 41 (DRDY ERR), error: 04 (ABRT )
(ada0:ahcich0:0:0:0): RES: 41 04 00 00 00 40 00 00 00 00 00
(ada0:ahcich0:0:0:0): Error 5, Retries exhausted
(ada0:ahcich0:0:0:0): Synchronize cache failed

#ifndef WITHOUT_CAPSICUM

  • cap_rights_init(&rights, CAP_IOCTL, CAP_READ, CAP_SEEK, CAP_WRITE);
  • if (ro)

+ cap_rights_init(&rights, CAP_IOCTL, CAP_READ, CAP_SEEK, CAP_WRITE,
+ CAP_FSYNC);
+ if (ro) {

		cap_rights_clear(&rights, CAP_WRITE);

+ cap_rights_clear(&rights, CAP_FSYNC);
+ }

grehan: fsync needs to be added here, or an ahci-hd file-backed disk image will fail on FreeBSD guest…
cap_rights_init(&rights, CAP_IOCTL, CAP_READ, CAP_SEEK, CAP_WRITE);
if (ro)
cap_rights_clear(&rights, CAP_WRITE);
oshogboUnsubmitted
Not Done
Inline Actions

I'm not sure if here the reverse logic would be more secure.

oshogbo: I'm not sure if here the reverse logic would be more secure.
emasteUnsubmitted
Not Done
Inline Actions

That is:

cap_rights_init(&rights, CAP_IOCTL, CAP_READ, CAP_SEEK);
if (!ro)
        cap_rights_set(&rights, CAP_FSYNC, CAP_WRITE);

I agree that looks preferable.

emaste: That is: ``` cap_rights_init(&rights, CAP_IOCTL, CAP_READ, CAP_SEEK); if (!ro)…
if (cap_rights_limit(fd, &rights) == -1 && errno != ENOSYS)
errx(EX_OSERR, "Unable to apply rights for sandbox");
#endif
/* /*
* Deal with raw devices * Deal with raw devices
*/ */
size = sbuf.st_size; size = sbuf.st_size;
sectsz = DEV_BSIZE; sectsz = DEV_BSIZE;
psectsz = psectoff = 0; psectsz = psectoff = 0;
candelete = geom = 0; candelete = geom = 0;
if (S_ISCHR(sbuf.st_mode)) { if (S_ISCHR(sbuf.st_mode)) {
Show All 9 Lines if (S_ISCHR(sbuf.st_mode)) {
strlcpy(arg.name, "GEOM::candelete", sizeof(arg.name)); strlcpy(arg.name, "GEOM::candelete", sizeof(arg.name));
arg.len = sizeof(arg.value.i); arg.len = sizeof(arg.value.i);
if (ioctl(fd, DIOCGATTR, &arg) == 0) if (ioctl(fd, DIOCGATTR, &arg) == 0)
candelete = arg.value.i; candelete = arg.value.i;
if (ioctl(fd, DIOCGPROVIDERNAME, name) == 0) if (ioctl(fd, DIOCGPROVIDERNAME, name) == 0)
geom = 1; geom = 1;
} else } else
psectsz = sbuf.st_blksize; psectsz = sbuf.st_blksize;
#ifndef WITHOUT_CAPSICUM
if (cap_ioctls_limit(fd, cmds, nitems(cmds)) == -1 && errno != ENOSYS)
errx(EX_OSERR, "Unable to apply rights for sandbox");
#endif
if (ssopt != 0) { if (ssopt != 0) {
if (!powerof2(ssopt) || !powerof2(pssopt) || ssopt < 512 || if (!powerof2(ssopt) || !powerof2(pssopt) || ssopt < 512 ||
ssopt > pssopt) { ssopt > pssopt) {
fprintf(stderr, "Invalid sector size %d/%d\n", fprintf(stderr, "Invalid sector size %d/%d\n",
ssopt, pssopt); ssopt, pssopt);
goto err; goto err;
} }
▲ Show 20 Lines • Show All 329 Lines • Show Last 20 Lines
View Options

sys/riscv/allwinner/files.allwinner

Show All 24 Lines
* *
* $FreeBSD$ * $FreeBSD$
*/ */
#include <sys/cdefs.h> #include <sys/cdefs.h>
__FBSDID("$FreeBSD$"); __FBSDID("$FreeBSD$");
#include <sys/param.h> #include <sys/param.h>
#ifndef WITHOUT_CAPSICUM
#include <sys/capsicum.h>
#endif
#include <sys/queue.h> #include <sys/queue.h>
#include <sys/errno.h> #include <sys/errno.h>
#include <sys/stat.h> #include <sys/stat.h>
#include <sys/ioctl.h> #include <sys/ioctl.h>
#include <sys/disk.h> #include <sys/disk.h>
#include <assert.h> #include <assert.h>
#include <err.h> #include <err.h>
#include <fcntl.h> #include <fcntl.h>
#include <stdio.h> #include <stdio.h>
#include <stdlib.h> #include <stdlib.h>
#include <string.h> #include <string.h>
#include <pthread.h> #include <pthread.h>
#include <pthread_np.h> #include <pthread_np.h>
#include <signal.h> #include <signal.h>
#include <sysexits.h>
#include <unistd.h> #include <unistd.h>
#include <machine/atomic.h> #include <machine/atomic.h>
#include "bhyverun.h" #include "bhyverun.h"
#include "mevent.h" #include "mevent.h"
#include "block_if.h" #include "block_if.h"
▲ Show 20 Lines • Show All 339 Lines • ▼ Show 20 Linesblockif_open(const char *optstr, const char *ident)
char name[MAXPATHLEN]; char name[MAXPATHLEN];
char *nopt, *xopts, *cp; char *nopt, *xopts, *cp;
struct blockif_ctxt *bc; struct blockif_ctxt *bc;
struct stat sbuf; struct stat sbuf;
struct diocgattr_arg arg; struct diocgattr_arg arg;
off_t size, psectsz, psectoff; off_t size, psectsz, psectoff;
int extra, fd, i, sectsz; int extra, fd, i, sectsz;
int nocache, sync, ro, candelete, geom, ssopt, pssopt; int nocache, sync, ro, candelete, geom, ssopt, pssopt;
#ifndef WITHOUT_CAPSICUM
cap_rights_t rights;
u_long cmds[] = {DIOCGFLUSH, DIOCGDELETE};
#endif
pthread_once(&blockif_once, blockif_init); pthread_once(&blockif_once, blockif_init);
fd = -1; fd = -1;
ssopt = 0; ssopt = 0;
nocache = 0; nocache = 0;
sync = 0; sync = 0;
ro = 0; ro = 0;
▲ Show 20 Lines • Show All 41 Lines • ▼ Show 20 Lines if (fd < 0) {
goto err; goto err;
} }
if (fstat(fd, &sbuf) < 0) { if (fstat(fd, &sbuf) < 0) {
warn("Could not stat backing file %s", nopt); warn("Could not stat backing file %s", nopt);
goto err; goto err;
} }
#ifndef WITHOUT_CAPSICUM
grehanUnsubmitted
Not Done
Inline Actions

fsync needs to be added here, or an ahci-hd file-backed disk image will fail on FreeBSD guest shutdown with

(ada0:ahcich0:0:0:0): FLUSHCACHE48. ACB: ea 00 00 00 00 40 00 00 00 00 00 00
(ada0:ahcich0:0:0:0): CAM status: ATA Status Error
(ada0:ahcich0:0:0:0): ATA status: 41 (DRDY ERR), error: 04 (ABRT )
(ada0:ahcich0:0:0:0): RES: 41 04 00 00 00 40 00 00 00 00 00
(ada0:ahcich0:0:0:0): Retrying command
(ada0:ahcich0:0:0:0): FLUSHCACHE48. ACB: ea 00 00 00 00 40 00 00 00 00 00 00
(ada0:ahcich0:0:0:0): CAM status: ATA Status Error
(ada0:ahcich0:0:0:0): ATA status: 41 (DRDY ERR), error: 04 (ABRT )
(ada0:ahcich0:0:0:0): RES: 41 04 00 00 00 40 00 00 00 00 00
(ada0:ahcich0:0:0:0): Error 5, Retries exhausted
(ada0:ahcich0:0:0:0): Synchronize cache failed

#ifndef WITHOUT_CAPSICUM

  • cap_rights_init(&rights, CAP_IOCTL, CAP_READ, CAP_SEEK, CAP_WRITE);
  • if (ro)

+ cap_rights_init(&rights, CAP_IOCTL, CAP_READ, CAP_SEEK, CAP_WRITE,
+ CAP_FSYNC);
+ if (ro) {

		cap_rights_clear(&rights, CAP_WRITE);

+ cap_rights_clear(&rights, CAP_FSYNC);
+ }

grehan: fsync needs to be added here, or an ahci-hd file-backed disk image will fail on FreeBSD guest…
cap_rights_init(&rights, CAP_IOCTL, CAP_READ, CAP_SEEK, CAP_WRITE);
if (ro)
cap_rights_clear(&rights, CAP_WRITE);
oshogboUnsubmitted
Not Done
Inline Actions

I'm not sure if here the reverse logic would be more secure.

oshogbo: I'm not sure if here the reverse logic would be more secure.
emasteUnsubmitted
Not Done
Inline Actions

That is:

cap_rights_init(&rights, CAP_IOCTL, CAP_READ, CAP_SEEK);
if (!ro)
        cap_rights_set(&rights, CAP_FSYNC, CAP_WRITE);

I agree that looks preferable.

emaste: That is: ``` cap_rights_init(&rights, CAP_IOCTL, CAP_READ, CAP_SEEK); if (!ro)…
if (cap_rights_limit(fd, &rights) == -1 && errno != ENOSYS)
errx(EX_OSERR, "Unable to apply rights for sandbox");
#endif
/* /*
* Deal with raw devices * Deal with raw devices
*/ */
size = sbuf.st_size; size = sbuf.st_size;
sectsz = DEV_BSIZE; sectsz = DEV_BSIZE;
psectsz = psectoff = 0; psectsz = psectoff = 0;
candelete = geom = 0; candelete = geom = 0;
if (S_ISCHR(sbuf.st_mode)) { if (S_ISCHR(sbuf.st_mode)) {
Show All 9 Lines if (S_ISCHR(sbuf.st_mode)) {
strlcpy(arg.name, "GEOM::candelete", sizeof(arg.name)); strlcpy(arg.name, "GEOM::candelete", sizeof(arg.name));
arg.len = sizeof(arg.value.i); arg.len = sizeof(arg.value.i);
if (ioctl(fd, DIOCGATTR, &arg) == 0) if (ioctl(fd, DIOCGATTR, &arg) == 0)
candelete = arg.value.i; candelete = arg.value.i;
if (ioctl(fd, DIOCGPROVIDERNAME, name) == 0) if (ioctl(fd, DIOCGPROVIDERNAME, name) == 0)
geom = 1; geom = 1;
} else } else
psectsz = sbuf.st_blksize; psectsz = sbuf.st_blksize;
#ifndef WITHOUT_CAPSICUM
if (cap_ioctls_limit(fd, cmds, nitems(cmds)) == -1 && errno != ENOSYS)
errx(EX_OSERR, "Unable to apply rights for sandbox");
#endif
if (ssopt != 0) { if (ssopt != 0) {
if (!powerof2(ssopt) || !powerof2(pssopt) || ssopt < 512 || if (!powerof2(ssopt) || !powerof2(pssopt) || ssopt < 512 ||
ssopt > pssopt) { ssopt > pssopt) {
fprintf(stderr, "Invalid sector size %d/%d\n", fprintf(stderr, "Invalid sector size %d/%d\n",
ssopt, pssopt); ssopt, pssopt);
goto err; goto err;
} }
▲ Show 20 Lines • Show All 329 Lines • Show Last 20 Lines
View Options

sys/riscv/conf/GENERIC

Show All 24 Lines
* *
* $FreeBSD$ * $FreeBSD$
*/ */
#include <sys/cdefs.h> #include <sys/cdefs.h>
__FBSDID("$FreeBSD$"); __FBSDID("$FreeBSD$");
#include <sys/param.h> #include <sys/param.h>
#ifndef WITHOUT_CAPSICUM
#include <sys/capsicum.h>
#endif
#include <sys/queue.h> #include <sys/queue.h>
#include <sys/errno.h> #include <sys/errno.h>
#include <sys/stat.h> #include <sys/stat.h>
#include <sys/ioctl.h> #include <sys/ioctl.h>
#include <sys/disk.h> #include <sys/disk.h>
#include <assert.h> #include <assert.h>
#include <err.h> #include <err.h>
#include <fcntl.h> #include <fcntl.h>
#include <stdio.h> #include <stdio.h>
#include <stdlib.h> #include <stdlib.h>
#include <string.h> #include <string.h>
#include <pthread.h> #include <pthread.h>
#include <pthread_np.h> #include <pthread_np.h>
#include <signal.h> #include <signal.h>
#include <sysexits.h>
#include <unistd.h> #include <unistd.h>
#include <machine/atomic.h> #include <machine/atomic.h>
#include "bhyverun.h" #include "bhyverun.h"
#include "mevent.h" #include "mevent.h"
#include "block_if.h" #include "block_if.h"
▲ Show 20 Lines • Show All 339 Lines • ▼ Show 20 Linesblockif_open(const char *optstr, const char *ident)
char name[MAXPATHLEN]; char name[MAXPATHLEN];
char *nopt, *xopts, *cp; char *nopt, *xopts, *cp;
struct blockif_ctxt *bc; struct blockif_ctxt *bc;
struct stat sbuf; struct stat sbuf;
struct diocgattr_arg arg; struct diocgattr_arg arg;
off_t size, psectsz, psectoff; off_t size, psectsz, psectoff;
int extra, fd, i, sectsz; int extra, fd, i, sectsz;
int nocache, sync, ro, candelete, geom, ssopt, pssopt; int nocache, sync, ro, candelete, geom, ssopt, pssopt;
#ifndef WITHOUT_CAPSICUM
cap_rights_t rights;
u_long cmds[] = {DIOCGFLUSH, DIOCGDELETE};
#endif
pthread_once(&blockif_once, blockif_init); pthread_once(&blockif_once, blockif_init);
fd = -1; fd = -1;
ssopt = 0; ssopt = 0;
nocache = 0; nocache = 0;
sync = 0; sync = 0;
ro = 0; ro = 0;
▲ Show 20 Lines • Show All 41 Lines • ▼ Show 20 Lines if (fd < 0) {
goto err; goto err;
} }
if (fstat(fd, &sbuf) < 0) { if (fstat(fd, &sbuf) < 0) {
warn("Could not stat backing file %s", nopt); warn("Could not stat backing file %s", nopt);
goto err; goto err;
} }
#ifndef WITHOUT_CAPSICUM
grehanUnsubmitted
Not Done
Inline Actions

fsync needs to be added here, or an ahci-hd file-backed disk image will fail on FreeBSD guest shutdown with

(ada0:ahcich0:0:0:0): FLUSHCACHE48. ACB: ea 00 00 00 00 40 00 00 00 00 00 00
(ada0:ahcich0:0:0:0): CAM status: ATA Status Error
(ada0:ahcich0:0:0:0): ATA status: 41 (DRDY ERR), error: 04 (ABRT )
(ada0:ahcich0:0:0:0): RES: 41 04 00 00 00 40 00 00 00 00 00
(ada0:ahcich0:0:0:0): Retrying command
(ada0:ahcich0:0:0:0): FLUSHCACHE48. ACB: ea 00 00 00 00 40 00 00 00 00 00 00
(ada0:ahcich0:0:0:0): CAM status: ATA Status Error
(ada0:ahcich0:0:0:0): ATA status: 41 (DRDY ERR), error: 04 (ABRT )
(ada0:ahcich0:0:0:0): RES: 41 04 00 00 00 40 00 00 00 00 00
(ada0:ahcich0:0:0:0): Error 5, Retries exhausted
(ada0:ahcich0:0:0:0): Synchronize cache failed

#ifndef WITHOUT_CAPSICUM

  • cap_rights_init(&rights, CAP_IOCTL, CAP_READ, CAP_SEEK, CAP_WRITE);
  • if (ro)

+ cap_rights_init(&rights, CAP_IOCTL, CAP_READ, CAP_SEEK, CAP_WRITE,
+ CAP_FSYNC);
+ if (ro) {

		cap_rights_clear(&rights, CAP_WRITE);

+ cap_rights_clear(&rights, CAP_FSYNC);
+ }

grehan: fsync needs to be added here, or an ahci-hd file-backed disk image will fail on FreeBSD guest…
cap_rights_init(&rights, CAP_IOCTL, CAP_READ, CAP_SEEK, CAP_WRITE);
if (ro)
cap_rights_clear(&rights, CAP_WRITE);
oshogboUnsubmitted
Not Done
Inline Actions

I'm not sure if here the reverse logic would be more secure.

oshogbo: I'm not sure if here the reverse logic would be more secure.
emasteUnsubmitted
Not Done
Inline Actions

That is:

cap_rights_init(&rights, CAP_IOCTL, CAP_READ, CAP_SEEK);
if (!ro)
        cap_rights_set(&rights, CAP_FSYNC, CAP_WRITE);

I agree that looks preferable.

emaste: That is: ``` cap_rights_init(&rights, CAP_IOCTL, CAP_READ, CAP_SEEK); if (!ro)…
if (cap_rights_limit(fd, &rights) == -1 && errno != ENOSYS)
errx(EX_OSERR, "Unable to apply rights for sandbox");
#endif
/* /*
* Deal with raw devices * Deal with raw devices
*/ */
size = sbuf.st_size; size = sbuf.st_size;
sectsz = DEV_BSIZE; sectsz = DEV_BSIZE;
psectsz = psectoff = 0; psectsz = psectoff = 0;
candelete = geom = 0; candelete = geom = 0;
if (S_ISCHR(sbuf.st_mode)) { if (S_ISCHR(sbuf.st_mode)) {
Show All 9 Lines if (S_ISCHR(sbuf.st_mode)) {
strlcpy(arg.name, "GEOM::candelete", sizeof(arg.name)); strlcpy(arg.name, "GEOM::candelete", sizeof(arg.name));
arg.len = sizeof(arg.value.i); arg.len = sizeof(arg.value.i);
if (ioctl(fd, DIOCGATTR, &arg) == 0) if (ioctl(fd, DIOCGATTR, &arg) == 0)
candelete = arg.value.i; candelete = arg.value.i;
if (ioctl(fd, DIOCGPROVIDERNAME, name) == 0) if (ioctl(fd, DIOCGPROVIDERNAME, name) == 0)
geom = 1; geom = 1;
} else } else
psectsz = sbuf.st_blksize; psectsz = sbuf.st_blksize;
#ifndef WITHOUT_CAPSICUM
if (cap_ioctls_limit(fd, cmds, nitems(cmds)) == -1 && errno != ENOSYS)
errx(EX_OSERR, "Unable to apply rights for sandbox");
#endif
if (ssopt != 0) { if (ssopt != 0) {
if (!powerof2(ssopt) || !powerof2(pssopt) || ssopt < 512 || if (!powerof2(ssopt) || !powerof2(pssopt) || ssopt < 512 ||
ssopt > pssopt) { ssopt > pssopt) {
fprintf(stderr, "Invalid sector size %d/%d\n", fprintf(stderr, "Invalid sector size %d/%d\n",
ssopt, pssopt); ssopt, pssopt);
goto err; goto err;
} }
▲ Show 20 Lines • Show All 329 Lines • Show Last 20 Lines
View Options

sys/riscv/riscv/plic.c

Show All 24 Lines
* *
* $FreeBSD$ * $FreeBSD$
*/ */
#include <sys/cdefs.h> #include <sys/cdefs.h>
__FBSDID("$FreeBSD$"); __FBSDID("$FreeBSD$");
#include <sys/param.h> #include <sys/param.h>
#ifndef WITHOUT_CAPSICUM
#include <sys/capsicum.h>
#endif
#include <sys/queue.h> #include <sys/queue.h>
#include <sys/errno.h> #include <sys/errno.h>
#include <sys/stat.h> #include <sys/stat.h>
#include <sys/ioctl.h> #include <sys/ioctl.h>
#include <sys/disk.h> #include <sys/disk.h>
#include <assert.h> #include <assert.h>
#include <err.h> #include <err.h>
#include <fcntl.h> #include <fcntl.h>
#include <stdio.h> #include <stdio.h>
#include <stdlib.h> #include <stdlib.h>
#include <string.h> #include <string.h>
#include <pthread.h> #include <pthread.h>
#include <pthread_np.h> #include <pthread_np.h>
#include <signal.h> #include <signal.h>
#include <sysexits.h>
#include <unistd.h> #include <unistd.h>
#include <machine/atomic.h> #include <machine/atomic.h>
#include "bhyverun.h" #include "bhyverun.h"
#include "mevent.h" #include "mevent.h"
#include "block_if.h" #include "block_if.h"
▲ Show 20 Lines • Show All 339 Lines • ▼ Show 20 Linesblockif_open(const char *optstr, const char *ident)
char name[MAXPATHLEN]; char name[MAXPATHLEN];
char *nopt, *xopts, *cp; char *nopt, *xopts, *cp;
struct blockif_ctxt *bc; struct blockif_ctxt *bc;
struct stat sbuf; struct stat sbuf;
struct diocgattr_arg arg; struct diocgattr_arg arg;
off_t size, psectsz, psectoff; off_t size, psectsz, psectoff;
int extra, fd, i, sectsz; int extra, fd, i, sectsz;
int nocache, sync, ro, candelete, geom, ssopt, pssopt; int nocache, sync, ro, candelete, geom, ssopt, pssopt;
#ifndef WITHOUT_CAPSICUM
cap_rights_t rights;
u_long cmds[] = {DIOCGFLUSH, DIOCGDELETE};
#endif
pthread_once(&blockif_once, blockif_init); pthread_once(&blockif_once, blockif_init);
fd = -1; fd = -1;
ssopt = 0; ssopt = 0;
nocache = 0; nocache = 0;
sync = 0; sync = 0;
ro = 0; ro = 0;
▲ Show 20 Lines • Show All 41 Lines • ▼ Show 20 Lines if (fd < 0) {
goto err; goto err;
} }
if (fstat(fd, &sbuf) < 0) { if (fstat(fd, &sbuf) < 0) {
warn("Could not stat backing file %s", nopt); warn("Could not stat backing file %s", nopt);
goto err; goto err;
} }
#ifndef WITHOUT_CAPSICUM
grehanUnsubmitted
Not Done
Inline Actions

fsync needs to be added here, or an ahci-hd file-backed disk image will fail on FreeBSD guest shutdown with

(ada0:ahcich0:0:0:0): FLUSHCACHE48. ACB: ea 00 00 00 00 40 00 00 00 00 00 00
(ada0:ahcich0:0:0:0): CAM status: ATA Status Error
(ada0:ahcich0:0:0:0): ATA status: 41 (DRDY ERR), error: 04 (ABRT )
(ada0:ahcich0:0:0:0): RES: 41 04 00 00 00 40 00 00 00 00 00
(ada0:ahcich0:0:0:0): Retrying command
(ada0:ahcich0:0:0:0): FLUSHCACHE48. ACB: ea 00 00 00 00 40 00 00 00 00 00 00
(ada0:ahcich0:0:0:0): CAM status: ATA Status Error
(ada0:ahcich0:0:0:0): ATA status: 41 (DRDY ERR), error: 04 (ABRT )
(ada0:ahcich0:0:0:0): RES: 41 04 00 00 00 40 00 00 00 00 00
(ada0:ahcich0:0:0:0): Error 5, Retries exhausted
(ada0:ahcich0:0:0:0): Synchronize cache failed

#ifndef WITHOUT_CAPSICUM

  • cap_rights_init(&rights, CAP_IOCTL, CAP_READ, CAP_SEEK, CAP_WRITE);
  • if (ro)

+ cap_rights_init(&rights, CAP_IOCTL, CAP_READ, CAP_SEEK, CAP_WRITE,
+ CAP_FSYNC);
+ if (ro) {

		cap_rights_clear(&rights, CAP_WRITE);

+ cap_rights_clear(&rights, CAP_FSYNC);
+ }

grehan: fsync needs to be added here, or an ahci-hd file-backed disk image will fail on FreeBSD guest…
cap_rights_init(&rights, CAP_IOCTL, CAP_READ, CAP_SEEK, CAP_WRITE);
if (ro)
cap_rights_clear(&rights, CAP_WRITE);
oshogboUnsubmitted
Not Done
Inline Actions

I'm not sure if here the reverse logic would be more secure.

oshogbo: I'm not sure if here the reverse logic would be more secure.
emasteUnsubmitted
Not Done
Inline Actions

That is:

cap_rights_init(&rights, CAP_IOCTL, CAP_READ, CAP_SEEK);
if (!ro)
        cap_rights_set(&rights, CAP_FSYNC, CAP_WRITE);

I agree that looks preferable.

emaste: That is: ``` cap_rights_init(&rights, CAP_IOCTL, CAP_READ, CAP_SEEK); if (!ro)…
if (cap_rights_limit(fd, &rights) == -1 && errno != ENOSYS)
errx(EX_OSERR, "Unable to apply rights for sandbox");
#endif
/* /*
* Deal with raw devices * Deal with raw devices
*/ */
size = sbuf.st_size; size = sbuf.st_size;
sectsz = DEV_BSIZE; sectsz = DEV_BSIZE;
psectsz = psectoff = 0; psectsz = psectoff = 0;
candelete = geom = 0; candelete = geom = 0;
if (S_ISCHR(sbuf.st_mode)) { if (S_ISCHR(sbuf.st_mode)) {
Show All 9 Lines if (S_ISCHR(sbuf.st_mode)) {
strlcpy(arg.name, "GEOM::candelete", sizeof(arg.name)); strlcpy(arg.name, "GEOM::candelete", sizeof(arg.name));
arg.len = sizeof(arg.value.i); arg.len = sizeof(arg.value.i);
if (ioctl(fd, DIOCGATTR, &arg) == 0) if (ioctl(fd, DIOCGATTR, &arg) == 0)
candelete = arg.value.i; candelete = arg.value.i;
if (ioctl(fd, DIOCGPROVIDERNAME, name) == 0) if (ioctl(fd, DIOCGPROVIDERNAME, name) == 0)
geom = 1; geom = 1;
} else } else
psectsz = sbuf.st_blksize; psectsz = sbuf.st_blksize;
#ifndef WITHOUT_CAPSICUM
if (cap_ioctls_limit(fd, cmds, nitems(cmds)) == -1 && errno != ENOSYS)
errx(EX_OSERR, "Unable to apply rights for sandbox");
#endif
if (ssopt != 0) { if (ssopt != 0) {
if (!powerof2(ssopt) || !powerof2(pssopt) || ssopt < 512 || if (!powerof2(ssopt) || !powerof2(pssopt) || ssopt < 512 ||
ssopt > pssopt) { ssopt > pssopt) {
fprintf(stderr, "Invalid sector size %d/%d\n", fprintf(stderr, "Invalid sector size %d/%d\n",
ssopt, pssopt); ssopt, pssopt);
goto err; goto err;
} }
▲ Show 20 Lines • Show All 329 Lines • Show Last 20 Lines