Michael S. Tsirkin wrote:
> On Tue, Dec 22, 2009 at 05:00:52PM +0100, Alexander Graf wrote:
>
>> Avi Kivity wrote:
>>
>>> On 12/22/2009 05:41 PM, Alexander Graf wrote:
>>>
>>>>> We could certainly extend emulate.c to fetch instruction bytes from
>>>>> userspace. It uses ->read_std() now, so we'd need to switch to
>>>>> ->read_emulated() and add appropriate buffering.
>>>>>
>>>>>
>>>> I thought the policy on emulate.c was to not have a full instruction
>>>> emulator but only emulate instructions that do PT modifications or MMIO
>>>> access?
>>>>
>>>>
>>> It's not a policy, just laziness. With emulate_invalid_guest_state=1
>>> we need many more instructions. Of course I don't want to add
>>> instructions just for the sake of it, since they will be untested.
>>>
>>> I'd much prefer not to run from mmio if possible - just pointing out
>>> it's doable.
>>>
>> Right...
>>
>>
>>>> emulator is _really_ small. It only does a few MMU specific
>>>> instructions, a couple of privileged ones and MMIO accessing ones.
>>>>
>>>>
>>> Btw, we're in the same situation with PowerPC here. The instruction
>>>
>>> Plus, you have a fixed length instruction length, likely more regular
>>> too. I imagine powerpc is load/store, so you don't have to emulate a
>>> zillion ALU instructions?
>>>
>> Well, it's certainly doable (and easier than on x86). But I'm on the
>> same position as you on the x86 side. Why increase the emulator size at
>> least 10 times if we don't have to?
>>
>> Either way, people will report bugs when / if they actually start
>> executing code off MMIO. So let's not care too much about it for now.
>> Just make sure the read-only check is in.
>>
>> Alex
>>
>
> So I think all we need is this on top?
>
> diff --git a/hw/device-assignment.c b/hw/device-assignment.c
> index 066fdb6..0c3c8f4 100644
> --- a/hw/device-assignment.c
> +++ b/hw/device-assignment.c
> @@ -233,7 +233,8 @@ static void assigned_dev_iomem_map_slow(PCIDevice *pci_dev, int region_num,
> int m;
>
> DEBUG("slow map\n");
> - m = cpu_register_io_memory(slow_bar_read, slow_bar_write, region);
> + m = cpu_register_io_memory(slow_bar_read, region_num == PCI_ROM_SLOT ?
> + NULL : slow_bar_write, region);
> cpu_register_physical_memory(e_phys, e_size, m);
>
> /* MSI-X MMIO page */
>
I guess so, yes. I'd prefer a written out if statement though, but
that's probably personal preference.
Alex
--
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
