Skip to content

[xenmgr] Hook up passthrough-mmio/io for xl#105

Merged
eric-ch merged 1 commit intoOpenXT:masterfrom
crogers1:oxt-1220
Oct 4, 2017
Merged

[xenmgr] Hook up passthrough-mmio/io for xl#105
eric-ch merged 1 commit intoOpenXT:masterfrom
crogers1:oxt-1220

Conversation

@crogers1
Copy link
Copy Markdown
Contributor

passthrough-mmio and passthrough-io map to iomem and ioports
in xl. Hook up the logic in Config.hs to dump them into the config

OXT-1220

Signed-off-by: Chris rogersc@ainfosec.com

@crogers1
Copy link
Copy Markdown
Contributor Author

#106 -> stable-7

Built ok, needs testing (not currently near my test box). Use xec-vm to set these fields, use the xl syntax, comma separated per iomem/ioport entry. Xenmgr will take care of wrapping them in quotes and brackets when it generates the xl domain config in /tmp on vm start.

@jean-edouard
Copy link
Copy Markdown
Member

@dpsmith, please review

Copy link
Copy Markdown
Contributor

@jandryuk jandryuk left a comment

Choose a reason for hiding this comment

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

xenmgr/xenvm passthrough-mmio was previously defined as "start-end" range (or ranges, I'm not sure). I was thinking this code would maintain backwards compatibility by converting from a start-end byte address to a start,count page format for XL.

However, the XL format supports an additional @ GFN syntax to specify a mapping location inside the guest which cannot be represented with the old syntax.

Personally, I don't care that this breaks backwards compatibility, but does the project as a whole care? I think VMs using the old syntax would fail to boot when XL can't parse the iomem field.

Comment thread xenmgr/Vm/Config.hs
_ -> case name of
"viridian" -> name ++ "=" ++ (wrapBrackets $ wrapQuotes v)
"serial" -> name ++ "=" ++ (wrapBrackets $ wrapQuotes v)
"iomem" -> name ++ "=" ++ (wrapBrackets $ wrapQuotes v)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This naively only supports a single value when iomem can handle multiple ranges:
iomem=[ "IOMEM_START,NUM_PAGES[@gfn]", "IOMEM_START,NUM_PAGES[@gfn]", ... ]

You can jam multiple values in by writing something like "0xfed40,4','0xfff60,1" so the inner single quotes are passed through and interpreted by xl.

@stacktrust
Copy link
Copy Markdown

I don't think anyone is currently using this feature in production, so we can drop backwards compatibility in order to simplify support for XL syntax. This feature is useful for demo purposes.

@eric-ch
Copy link
Copy Markdown
Contributor

eric-ch commented Sep 26, 2017

Building 1164

@jandryuk
Copy link
Copy Markdown
Contributor

I recommend re-working this to accept multiple range definitions without resorting to cramming in partially quoted strings.

@jandryuk
Copy link
Copy Markdown
Contributor

Works.

Created an HVM CentOS 7 guest.

Passthrough the TPM mmio ranges:
db-write /vm/$( xec-vm CentOS7 get uuid )/config/passthrough-mmio "fed40,5"
modify the CentOS7 VM /boot/grub2/grub.cfg appending "tpm_tis.force=1" to the linux command line.
Boot/reboot.

cat /sys/class/tpm/tpm0/device/pcrs in the guest and see the PCRs from the TPM.

  passthrough-mmio and passthrough-io map to iomem and ioports
  in xl.  Hook up the logic in Config.hs to dump them into the config

  OXT-1220

Signed-off-by: Chris <rogersc@ainfosec.com>
@crogers1
Copy link
Copy Markdown
Contributor Author

PR updated with in-code comment block with ticket id and info regarding future implementation where ranges are added/removed at a finer granularity.

Copy link
Copy Markdown
Member

@dpsmith dpsmith left a comment

Choose a reason for hiding this comment

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

@eric-ch eric-ch merged commit c0bffbd into OpenXT:master Oct 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants