On Thu, Aug 20, 2020 at 2:49 PM Alexander Graf <graf@xxxxxxxxxx> wrote:
> The only real downside I can see is that we just wasted ~8kb of RAM.
> Nothing I would really get hung up on though.
I also suspect that the MSR permission bitmap modifications are going
to be a bit more expensive with 4kb (6kb on AMD) of pertinent
allow-bitmaps than they would be with a few bytes of pertinent
deny-bitmaps.
> If you really desperately believe a deny list is a better fit for your
> use case, we could redesign the interface differently:
>
> struct msr_set_accesslist {
> #define MSR_ACCESSLIST_DEFAULT_ALLOW 0
> #define MSR_ACCESSLIST_DEFAULT_DENY 1
> u32 flags;
> struct {
> u32 flags;
> u32 nmsrs; /* MSRs in bitmap */
> u32 base; /* first MSR address to bitmap */
> void *bitmap; /* pointer to bitmap, 1 means allow, 0 deny */
> } lists[10];
> };
>
> which means in your use case, you can do
>
> u64 deny = 0;
> struct msr_set_accesslist access = {
> .flags = MSR_ACCESSLIST_DEFAULT_ALLOW,
> .lists = {
> {
> .nmsrs = 1,
> .base = IA32_ARCH_CAPABILITIES,
> .bitmap = &deny,
> }, {
> {
> .nmsrs = 1,
> .base = HV_X64_MSR_REFERENCE_TSC,
> .bitmap = &deny,
> }, {
> {
> .nmsrs = 1,
> /* can probably be combined with the ones below? */
> .base = MSR_GOOGLE_TRUE_TIME,
> .bitmap = &deny,
> }, {
> {
> .nmsrs = 1,
> .base = MSR_GOOGLE_FDR_TRACE,
> .bitmap = &deny,
> }, {
> {
> .nmsrs = 1,
> .base = MSR_GOOGLE_HBI,
> .bitmap = &deny,
> },
> }
> };
>
> msr_set_accesslist(kvm_fd, &access);
>
> while I can do the same dance as before, but with a single call rather
> than multiple ones.
>
> What do you think?
I like it. I think this suits our use case well.
