On 2012-09-07 10:26, Liu Sheng wrote:
> as readonly memory is support in kvm, this patch supports this feature
> in qemu, mainly pluging the memory region with KVM_MEM_READONLY flag
> to kvm.
>
> this can be tested by a kernel module from the author of kvm readonly
> memory slot:
>
> static int rom_tester_probe(struct pci_dev *dev, const struct pci_device_id *id)
> {
> struct resource *res = &dev->resource[PCI_ROM_RESOURCE];
> char buf[6];
> size_t rom_size;
> char * __iomem map;
> int i;
>
> if (res->flags & (IORESOURCE_ROM_SHADOW | IORESOURCE_ROM_COPY |
> IORESOURCE_ROM_BIOS_COPY)) {
> dev_printk(KERN_INFO, &dev->dev, "skip ROM COPY.\n");
> return 0;
> }
>
> if (res->flags &
> (IORESOURCE_ROM_COPY | IORESOURCE_ROM_BIOS_COPY))
> dev_printk(KERN_INFO, &dev->dev, "rom tester\n");
>
> if (pci_enable_rom(dev)) {
> dev_printk(KERN_INFO, &dev->dev, "do not found Rom\n");
> goto exit;
> }
>
> map = pci_map_rom(dev, &rom_size);
> if (!map) {
> dev_printk(KERN_INFO, &dev->dev, "map rom fail.\n");
> goto disable_exit;
> }
>
> dev_printk(KERN_INFO, &dev->dev, "Rom map: %p [size: %lx], phsyc:%llx.\n",
> map, rom_size, pci_resource_start(dev, PCI_ROM_RESOURCE));
>
> if (rom_size < 6) {
> printk("map size < 6.\n");
> goto unmap_exit;
> }
>
> printk("The first 6 bytes:\n");
> for (i = 0; i < 6; i++) {
> buf[i] = map[i];
> printk("%x ", buf[i]);
> }
> printk("\n\n");
>
> memcpy(map, "KVMKVM", 6);
> if (!memcmp(map, "KVMKVM", 6)) {
> printk("Rom Test: fail.\n");
> goto unmap_exit;
> }
>
> for (i = 0; i < 6; i++)
> if (buf[i] != ((char *)map)[i]) {
> printk("The %d byte is changed: %x -> %x.\n",
> i, buf[i], map[i]);
> printk("Rom Test: fail.\n");
> goto unmap_exit;
> }
>
> printk("Rom Test: Okay.\n");
>
> unmap_exit:
> pci_unmap_rom(dev, map);
> disable_exit:
> pci_disable_rom(dev);
> exit:
> return 0;
> }
>
> static DEFINE_PCI_DEVICE_TABLE(rom_tester_tbl) = {
> { PCI_DEVICE(PCI_ANY_ID, PCI_ANY_ID)},
>
> {0,} /* 0 terminated list. */
> };
> MODULE_DEVICE_TABLE(pci, rom_tester_tbl);
>
> static struct pci_driver rom_tester = {
> .name = "pci-rom-tester",
> .id_table = rom_tester_tbl,
> .probe = rom_tester_probe,
> };
>
> static int __init pci_rom_test_init(void)
> {
> int rc;
>
> rc = pci_register_driver(&rom_tester);
> if (rc)
> return rc;
>
> return 0;
> }
>
> static void __exit pci_rom_test_exit(void)
> {
> pci_unregister_driver(&rom_tester);
> }
>
> module_init(pci_rom_test_init);
> module_exit(pci_rom_test_exit);
>
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Xiao Guangrong <xiaoguangrong@xxxxxxxxxxxxxxxxxx>");
>
> we test it with the rom of Intel 82540EM Gigabit Ethernet Controller.
> 0. start qemu:
> qemu-system-x86_64 -enable-kvm -m 1G -smp 2 -hda fedora16.qcow2 \
> -nic nic,model=e1000,vlan=1,macadrr=52:00:12:34:56 \
> -net user,vlan=1
> 1. unbind the device:
> echo "0000:00:03.0" > /sys/bus/pci/devices/0000\:00\:03.0/driver/unbind
> 2. install the test kernel module, here we name it write_rom:
> modprobe write_rom
> 3. print dmesg to verify the result is ok or fail:
> dmesg
> 4. remove the test kernel module.
> rmmod write_rom
> 5. rebind the device to its driver, test if the nic still works:
> echo "0000:00:03.0" > /sys/bus/pci/drivers/e1000/bind
> open firefox and try some web page.
>
> when I use kvm without readonly memory slot, in step 2 it reports:
> Rom Test: fail. this means we can write to the memory region of a rom,
> which is quite not safe for the guest and host.
>
> when I use kvm with readonly memory slot, in step 2 it reports:
> Rom Test: Okay.
> and the error message prints out in host console:
> Guest is writing readonly memory, phys_addr: febc0000, len:4, data[0]:K,
> skip it
> Guest is writing readonly memory, phys_addr: febc0000, len:2, data[0]:V,
> skip it
> this is what we write "KVMKVM".
> this means write operation is not successful, and return back to qemu from kvm.
> thus we can make the rom real rom.
>
> Signed-off-by: Liu Sheng <liusheng@xxxxxxxxxxxxxxxxxx>
> ---
> kvm-all.c | 47 ++++++++++++++++++++++++++++++++++++-----------
> 1 files changed, 36 insertions(+), 11 deletions(-)
>
> diff --git a/kvm-all.c b/kvm-all.c
> index badf1d8..1b66183 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -97,6 +97,9 @@ struct KVMState
> QTAILQ_HEAD(msi_hashtab, KVMMSIRoute) msi_hashtab[KVM_MSI_HASHTAB_SIZE];
> bool direct_msi;
> #endif
> +#ifdef KVM_CAP_READONLY_MEM
> + int readonly_mem;
> +#endif
See comment on patch 2: Make sure that KVM_CAP_READONLY_MEM is always
defined and remove all these #ifdefs.
> };
>
> KVMState *kvm_state;
> @@ -261,9 +264,18 @@ err:
> * dirty pages logging control
> */
>
> -static int kvm_mem_flags(KVMState *s, bool log_dirty)
> +static int kvm_mem_flags(KVMState *s, MemoryRegion *mr)
> {
> - return log_dirty ? KVM_MEM_LOG_DIRTY_PAGES : 0;
> + int flags;
> + bool log_dirty = memory_region_is_logging(mr);
> + bool readonly = mr->readonly;
I suppose you want to add some memory_region_is_readonly(). And both
bools can be inlined into the checks below.
> + flags = log_dirty ? KVM_MEM_LOG_DIRTY_PAGES : 0;
> +#ifdef KVM_CAP_READONLY_MEM
> + if (s->readonly_mem) {
> + flags |= readonly ? KVM_MEM_READONLY : 0;
> + }
> +#endif
> + return flags;
> }
>
> static int kvm_slot_dirty_pages_log_change(KVMSlot *mem, bool log_dirty)
> @@ -274,7 +286,7 @@ static int kvm_slot_dirty_pages_log_change(KVMSlot *mem, bool log_dirty)
>
> old_flags = mem->flags;
>
> - flags = (mem->flags & ~mask) | kvm_mem_flags(s, log_dirty);
> + flags = (mem->flags & ~mask) | (log_dirty ? mask : 0);
> mem->flags = flags;
>
> /* If nothing changed effectively, no need to issue ioctl */
> @@ -622,7 +634,7 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
> mem->memory_size = old.memory_size;
> mem->start_addr = old.start_addr;
> mem->ram = old.ram;
> - mem->flags = kvm_mem_flags(s, log_dirty);
> + mem->flags = kvm_mem_flags(s, mr);
>
> err = kvm_set_user_memory_region(s, mem);
> if (err) {
> @@ -643,7 +655,7 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
> mem->memory_size = start_addr - old.start_addr;
> mem->start_addr = old.start_addr;
> mem->ram = old.ram;
> - mem->flags = kvm_mem_flags(s, log_dirty);
> + mem->flags = kvm_mem_flags(s, mr);
>
> err = kvm_set_user_memory_region(s, mem);
> if (err) {
> @@ -667,7 +679,7 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
> size_delta = mem->start_addr - old.start_addr;
> mem->memory_size = old.memory_size - size_delta;
> mem->ram = old.ram + size_delta;
> - mem->flags = kvm_mem_flags(s, log_dirty);
> + mem->flags = kvm_mem_flags(s, mr);
>
> err = kvm_set_user_memory_region(s, mem);
> if (err) {
> @@ -689,7 +701,7 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
> mem->memory_size = size;
> mem->start_addr = start_addr;
> mem->ram = ram;
> - mem->flags = kvm_mem_flags(s, log_dirty);
> + mem->flags = kvm_mem_flags(s, mr);
>
> err = kvm_set_user_memory_region(s, mem);
> if (err) {
> @@ -1393,6 +1405,10 @@ int kvm_init(void)
> s->direct_msi = (kvm_check_extension(s, KVM_CAP_SIGNAL_MSI) > 0);
> #endif
>
> +#ifdef KVM_CAP_READONLY_MEM
> + s->readonly_mem = kvm_check_extension(s, KVM_CAP_READONLY_MEM);
> +#endif
> +
> s->intx_set_mask = kvm_check_extension(s, KVM_CAP_PCI_2_3);
>
> ret = kvm_arch_init(s);
> @@ -1607,10 +1623,19 @@ int kvm_cpu_exec(CPUArchState *env)
> break;
> case KVM_EXIT_MMIO:
> DPRINTF("handle_mmio\n");
> - cpu_physical_memory_rw(run->mmio.phys_addr,
> - run->mmio.data,
> - run->mmio.len,
> - run->mmio.is_write);
> + if (run->mmio.write_readonly_mem) {
> + fprintf(stderr, "Guest is writing readonly memory, "
> + "phys_addr: %llx, len:%x, data[0]:%c, skip it.\n",
> + run->mmio.phys_addr,
> + run->mmio.len,
> + run->mmio.data[0]);
This should be DPRINTF.
> + } else {
> + cpu_physical_memory_rw(run->mmio.phys_addr,
> + run->mmio.data,
> + run->mmio.len,
> + run->mmio.is_write);
> + }
> +
> ret = 0;
> break;
> case KVM_EXIT_IRQ_WINDOW_OPEN:
>
Great to see this feature for KVM finally! I'm just afraid that this
will finally break good old isapc - due to broken Seabios. KVM used to
"unbreak" it as it didn't respect write protections. ;)
Jan
--
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
